Avoid accessing Preferences from background thread when loading preset palettes

src/app/CMakeLists.txt

@@ -542,6 +542,7 @@ add_library(app-lib
   file/file_data.cpp
   file/file_format.cpp
   file/file_formats_manager.cpp
+  file/file_op_config.cpp
   file/palette_file.cpp
   file/split_filename.cpp
   file_system.cpp

src/app/file/file.cpp

@@ -41,6 +41,7 @@
 #include "render/render.h"
 #include "ui/alert.h"
 #include "ui/listitem.h"
+#include "ui/system.h"
 
 #include "ask_for_color_profile.xml.h"
 #include "open_sequence.xml.h"
@@ -177,10 +178,13 @@ FileOpROI::FileOpROI(const Doc* doc,
 }
 
 // static
-FileOp* FileOp::createLoadDocumentOperation(Context* context, const std::string& filename, int flags)
+FileOp* FileOp::createLoadDocumentOperation(Context* context,
+                                            const std::string& filename,
+                                            const int flags,
+                                            const FileOpConfig* config)
 {
   std::unique_ptr<FileOp> fop(
-    new FileOp(FileOpLoad, context));
+    new FileOp(FileOpLoad, context, config));
   if (!fop)
     return nullptr;
 
@@ -329,7 +333,7 @@ FileOp* FileOp::createSaveDocumentOperation(const Context* context,
                                             const bool ignoreEmptyFrames)
 {
   std::unique_ptr<FileOp> fop(
-    new FileOp(FileOpSave, const_cast<Context*>(context)));
+    new FileOp(FileOpSave, const_cast<Context*>(context), nullptr));
 
   // Document to save
   fop->m_document = const_cast<Doc*>(roi.document());
@@ -711,7 +715,7 @@ void FileOp::operate(IFileOpProgress* progress)
       try {
         load_aseprite_data_file(m_dataFilename,
                                 m_document,
-                                m_defaultSliceColor);
+                                m_config.defaultSliceColor);
       }
       catch (const std::exception& ex) {
         setError("Error loading data file: %s\n", ex.what());
@@ -739,7 +743,7 @@ void FileOp::operate(IFileOpProgress* progress)
 
       // For each frame in the sprite.
       render::Render render;
-      render.setNewBlend(m_newBlend);
+      render.setNewBlend(m_config.newBlend);
 
       frame_t outputFrame = 0;
       for (frame_t frame : m_roi.selectedFrames()) {
@@ -903,7 +907,7 @@ void FileOp::postLoad()
       base::SharedPtr<Palette> palette(
         render::create_palette_from_sprite(
           sprite, frame_t(0), sprite->lastFrame(), true,
-          nullptr, nullptr, Preferences::instance().experimental.newBlend()));
+          nullptr, nullptr, m_config.newBlend));
 
       sprite->resetPalettes();
       sprite->setPalette(palette.get(), false);
@@ -915,10 +919,10 @@ void FileOp::postLoad()
   app::gen::ColorProfileBehavior behavior =
     app::gen::ColorProfileBehavior::DISABLE;
 
-  if (m_preserveColorProfile) {
+  if (m_config.preserveColorProfile) {
     // Embedded color profile
     if (this->hasEmbeddedColorProfile()) {
-      behavior = Preferences::instance().color.filesWithProfile();
+      behavior = m_config.filesWithProfile;
       if (behavior == app::gen::ColorProfileBehavior::ASK) {
 #ifdef ENABLE_UI
         if (m_context && m_context->isUIAvailable()) {
@@ -944,7 +948,7 @@ void FileOp::postLoad()
     }
     // Missing color space
     else {
-      behavior = Preferences::instance().color.missingProfile();
+      behavior = m_config.missingProfile;
       if (behavior == app::gen::ColorProfileBehavior::ASK) {
 #ifdef ENABLE_UI
         if (m_context && m_context->isUIAvailable()) {
@@ -982,7 +986,7 @@ void FileOp::postLoad()
 
     case app::gen::ColorProfileBehavior::CONVERT: {
       // Convert to the working color profile
-      auto gfxCS = get_working_rgb_space_from_preferences();
+      auto gfxCS = m_config.workingCS;
       if (!gfxCS->nearlyEqual(*spriteCS))
         cmd::convert_color_profile(sprite, gfxCS);
       break;
@@ -990,7 +994,7 @@ void FileOp::postLoad()
 
     case app::gen::ColorProfileBehavior::ASSIGN: {
       // Convert to the working color profile
-      auto gfxCS = get_working_rgb_space_from_preferences();
+      auto gfxCS = m_config.workingCS;
       sprite->setColorSpace(gfxCS);
       m_document->notifyColorSpaceChanged();
       break;
@@ -1178,7 +1182,9 @@ bool FileOp::isStop() const
   return stop;
 }
 
-FileOp::FileOp(FileOpType type, Context* context)
+FileOp::FileOp(FileOpType type,
+               Context* context,
+               const FileOpConfig* config)
   : m_type(type)
   , m_format(nullptr)
   , m_context(context)
@@ -1190,11 +1196,17 @@ FileOp::FileOp(FileOpType type, Context* context)
   , m_oneframe(false)
   , m_createPaletteFromRgba(false)
   , m_ignoreEmpty(false)
-  , m_preserveColorProfile(Preferences::instance().color.manage())
   , m_embeddedColorProfile(false)
-  , m_newBlend(Preferences::instance().experimental.newBlend())
-  , m_defaultSliceColor(Preferences::instance().slices.defaultColor())
 {
+  if (config)
+    m_config = *config;
+  else if (ui::is_ui_thread())
+    m_config.fillFromPreferences();
+  else {
+    LOG(VERBOSE, "FILE: Using a file operation with default configuration\n");
+    ASSERT(false);
+  }
+
   m_seq.palette = nullptr;
   m_seq.image.reset();
   m_seq.progress_offset = 0.0f;

src/app/file/file.h

@@ -10,6 +10,8 @@
 #pragma once
 
 #include "app/color.h"
+#include "app/file/file_op_config.h"
+#include "app/pref/preferences.h"
 #include "base/mutex.h"
 #include "base/paths.h"
 #include "base/shared_ptr.h"
@@ -102,7 +104,8 @@ namespace app {
   public:
     static FileOp* createLoadDocumentOperation(Context* context,
                                                const std::string& filename,
-                                               int flags);
+                                               const int flags,
+                                               const FileOpConfig* config = nullptr);
 
     static FileOp* createSaveDocumentOperation(const Context* context,
                                                const FileOpROI& roi,
@@ -114,7 +117,7 @@ namespace app {
 
     bool isSequence() const { return !m_seq.filename_list.empty(); }
     bool isOneFrame() const { return m_oneframe; }
-    bool preserveColorProfile() const { return m_preserveColorProfile; }
+    bool preserveColorProfile() const { return m_config.preserveColorProfile; }
 
     const std::string& filename() const { return m_filename; }
     const base::paths& filenames() const { return m_seq.filename_list; }
@@ -175,11 +178,13 @@ namespace app {
     void setEmbeddedColorProfile() { m_embeddedColorProfile = true; }
     bool hasEmbeddedColorProfile() const { return m_embeddedColorProfile; }
 
-    bool newBlend() const { return m_newBlend; }
+    bool newBlend() const { return m_config.newBlend; }
 
   private:
     FileOp();                   // Undefined
-    FileOp(FileOpType type, Context* context);
+    FileOp(FileOpType type,
+           Context* context,
+           const FileOpConfig* config);
 
     FileOpType m_type;          // Operation type: 0=load, 1=save.
     FileFormat* m_format;
@@ -204,18 +209,10 @@ namespace app {
     bool m_createPaletteFromRgba;
     bool m_ignoreEmpty;
 
-    // Return if we've to save/embed the color space of the document
-    // in the file.
-    bool m_preserveColorProfile;
-
     // True if the file contained a color profile when it was loaded.
     bool m_embeddedColorProfile;
 
-    // True if we should render each frame to save it with the new
+    FileOpConfig m_config;
-    // blend mode.
-    bool m_newBlend;
-
-    app::Color m_defaultSliceColor;
 
     base::SharedPtr<FormatOptions> m_formatOptions;
 

src/app/file/file_op_config.cpp

@@ -0,0 +1,27 @@
+// Aseprite
+// Copyright (C) 2019  Igara Studio S.A.
+//
+// This program is distributed under the terms of
+// the End-User License Agreement for Aseprite.
+
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
+#include "app/file/file_op_config.h"
+
+#include "app/color_spaces.h"
+
+namespace app {
+
+void FileOpConfig::fillFromPreferences()
+{
+  preserveColorProfile = Preferences::instance().color.manage();
+  filesWithProfile = Preferences::instance().color.filesWithProfile();
+  missingProfile = Preferences::instance().color.missingProfile();
+  newBlend = Preferences::instance().experimental.newBlend();
+  defaultSliceColor = Preferences::instance().slices.defaultColor();
+  workingCS = get_working_rgb_space_from_preferences();
+}
+
+} // namespace app

src/app/file/file_op_config.h

@@ -0,0 +1,40 @@
+// Aseprite
+// Copyright (C) 2019  Igara Studio S.A.
+//
+// This program is distributed under the terms of
+// the End-User License Agreement for Aseprite.
+
+#ifndef APP_FILE_FILE_OP_CONFIG_H_INCLUDED
+#define APP_FILE_FILE_OP_CONFIG_H_INCLUDED
+#pragma once
+
+#include "app/color.h"
+#include "app/pref/preferences.h"
+#include "gfx/color_space.h"
+
+namespace app {
+
+  // Options that came from Preferences but can be used in the non-UI thread.
+  struct FileOpConfig {
+    // True if we have to save/embed the color space of the document
+    // in the file.
+    bool preserveColorProfile = true;
+
+    // Configuration of what to do when we load a file with a color
+    // profile or without a color profile.
+    app::gen::ColorProfileBehavior filesWithProfile = app::gen::ColorProfileBehavior::EMBEDDED;
+    app::gen::ColorProfileBehavior missingProfile = app::gen::ColorProfileBehavior::ASSIGN;
+    gfx::ColorSpacePtr workingCS = gfx::ColorSpace::MakeSRGB();
+
+    // True if we should render each frame to save it with the new
+    // blend mode.h
+    bool newBlend = true;
+
+    app::Color defaultSliceColor = app::Color::fromRgb(0, 0, 255);
+
+    void fillFromPreferences();
+  };
+
+} // namespace app
+
+#endif

src/app/file/palette_file.cpp

@@ -1,5 +1,5 @@
 // Aseprite
-// Copyright (C) 2018  Igara Studio S.A.
+// Copyright (C) 2018-2019  Igara Studio S.A.
 // Copyright (C) 2001-2018  David Capello
 //
 // This program is distributed under the terms of
@@ -52,7 +52,8 @@ base::paths get_writable_palette_extensions()
   return paths;
 }
 
-Palette* load_palette(const char* filename)
+Palette* load_palette(const char* filename,
+                      const FileOpConfig* config)
 {
   dio::FileFormat dioFormat = dio::detect_format(filename);
   Palette* pal = nullptr;
@@ -89,7 +90,8 @@ Palette* load_palette(const char* filename)
           nullptr, filename,
           FILE_LOAD_CREATE_PALETTE |
           FILE_LOAD_SEQUENCE_NONE |
-          FILE_LOAD_ONE_FRAME));
+          FILE_LOAD_ONE_FRAME,
+          config));
 
       if (fop && !fop->hasError()) {
         fop->operate(nullptr);

src/app/file/palette_file.h

@@ -1,4 +1,5 @@
 // Aseprite
+// Copyright (C) 2019  Igara Studio S.A.
 // Copyright (C) 2001-2018  David Capello
 //
 // This program is distributed under the terms of
@@ -15,11 +16,13 @@ namespace doc {
 }
 
 namespace app {
+  struct FileOpConfig;
 
   base::paths get_readable_palette_extensions();
   base::paths get_writable_palette_extensions();
 
-  doc::Palette* load_palette(const char *filename);
+  doc::Palette* load_palette(const char *filename,
+                             const FileOpConfig* config = nullptr);
   bool save_palette(const char *filename, const doc::Palette* pal,
                     int columns);
 

src/app/res/palettes_loader_delegate.cpp

@@ -1,4 +1,5 @@
 // Aseprite
+// Copyright (C) 2019  Igara Studio S.A.
 // Copyright (C) 2001-2017  David Capello
 //
 // This program is distributed under the terms of
@@ -20,9 +21,17 @@
 #include "base/fs.h"
 #include "base/scoped_value.h"
 #include "doc/palette.h"
+#include "ui/system.h"
 
 namespace app {
 
+PalettesLoaderDelegate::PalettesLoaderDelegate()
+{
+  // Necessary to load preferences in the UI-thread which will be used
+  // in a FileOp executed in a background thread.
+  m_config.fillFromPreferences();
+}
+
 void PalettesLoaderDelegate::getResourcesPaths(std::map<std::string, std::string>& idAndPath) const
 {
   // Include extension palettes
@@ -55,7 +64,7 @@ void PalettesLoaderDelegate::getResourcesPaths(std::map<std::string, std::string
 Resource* PalettesLoaderDelegate::loadResource(const std::string& id,
                                                const std::string& path)
 {
-  doc::Palette* palette = load_palette(path.c_str());
+  doc::Palette* palette = load_palette(path.c_str(), &m_config);
   if (palette)
     return new PaletteResource(id, path, palette);
   else

src/app/res/palettes_loader_delegate.h

@@ -1,4 +1,5 @@
 // Aseprite
+// Copyright (C) 2019  Igara Studio S.A.
 // Copyright (C) 2001-2017  David Capello
 //
 // This program is distributed under the terms of
@@ -8,16 +9,22 @@
 #define APP_RES_PALETTES_LOADER_DELEGATE_H_INCLUDED
 #pragma once
 
+#include "app/file/file_op_config.h"
 #include "app/res/resources_loader_delegate.h"
 
 namespace app {
 
   class PalettesLoaderDelegate : public ResourcesLoaderDelegate {
   public:
+    PalettesLoaderDelegate();
+
     // ResourcesLoaderDelegate impl
     virtual void getResourcesPaths(std::map<std::string, std::string>& idAndPath) const override;
     virtual Resource* loadResource(const std::string& id,
                                    const std::string& path) override;
+
+  private:
+    FileOpConfig m_config;
   };
 
 } // namespace app