From 944634542eebd4de35bef389297e7fb3081d5af9 Mon Sep 17 00:00:00 2001 From: David Capello Date: Tue, 1 Oct 2019 09:24:03 -0300 Subject: [PATCH] Refresh screen/current palette automatically on Transaction::commit() (#378) Removed app_update_current_palette() with this patch. --- src/app/app.cpp | 24 ++++++---------- src/app/app.h | 1 - src/app/cmd/set_palette.cpp | 9 ++++++ src/app/cmd/set_palette.h | 2 ++ src/app/commands/cmd_add_color.cpp | 14 +++++----- src/app/commands/cmd_color_quantization.cpp | 11 +------- src/app/commands/cmd_palette_size.cpp | 13 ++------- src/app/doc.cpp | 15 +++++----- src/app/doc.h | 1 + src/app/doc_observer.h | 2 +- src/app/transaction.cpp | 31 +++++++++++++++++++++ src/app/transaction.h | 17 +++++++++-- src/app/ui/color_bar.cpp | 3 -- src/app/ui_context.cpp | 7 +++-- 14 files changed, 89 insertions(+), 61 deletions(-) diff --git a/src/app/app.cpp b/src/app/app.cpp index e20c78ec3..6fb660734 100644 --- a/src/app/app.cpp +++ b/src/app/app.cpp @@ -639,26 +639,18 @@ InputChain& App::inputChain() } #endif -void app_update_current_palette() -{ -#ifdef ENABLE_UI - Context* context = UIContext::instance(); - ASSERT(context != NULL); - - Site site = context->activeSite(); - - if (Palette* pal = site.palette()) - set_current_palette(pal, false); - else - set_current_palette(nullptr, false); -#endif // ENABLE_UI -} - // Updates palette and redraw the screen. void app_refresh_screen() { #ifdef ENABLE_UI - app_update_current_palette(); + Context* ctx = UIContext::instance(); + ASSERT(ctx != NULL); + + Site site = ctx->activeSite(); + if (Palette* pal = site.palette()) + set_current_palette(pal, false); + else + set_current_palette(nullptr, false); // Invalidate the whole screen. ui::Manager::getDefault()->invalidate(); diff --git a/src/app/app.h b/src/app/app.h index c3a822a4a..b7ea917f1 100644 --- a/src/app/app.h +++ b/src/app/app.h @@ -147,7 +147,6 @@ namespace app { #endif }; - void app_update_current_palette(); void app_refresh_screen(); void app_rebuild_documents_tabs(); PixelFormat app_get_current_pixel_format(); diff --git a/src/app/cmd/set_palette.cpp b/src/app/cmd/set_palette.cpp index 7c22466c2..bca10e9c2 100644 --- a/src/app/cmd/set_palette.cpp +++ b/src/app/cmd/set_palette.cpp @@ -1,4 +1,5 @@ // Aseprite +// Copyright (C) 2019 Igara Studio S.A. // Copyright (C) 2001-2015 David Capello // // This program is distributed under the terms of @@ -10,6 +11,7 @@ #include "app/cmd/set_palette.h" +#include "app/doc.h" #include "base/serialization.h" #include "doc/palette.h" #include "doc/sprite.h" @@ -77,5 +79,12 @@ void SetPalette::onUndo() palette->incrementVersion(); } +void SetPalette::onFireNotifications() +{ + doc::Sprite* sprite = this->sprite(); + Doc* doc = static_cast(sprite->document()); + doc->notifyPaletteChanged(); +} + } // namespace cmd } // namespace app diff --git a/src/app/cmd/set_palette.h b/src/app/cmd/set_palette.h index 591c123ab..bd8a6a9a9 100644 --- a/src/app/cmd/set_palette.h +++ b/src/app/cmd/set_palette.h @@ -1,4 +1,5 @@ // Aseprite +// Copyright (C) 2019 Igara Studio S.A. // Copyright (C) 2001-2015 David Capello // // This program is distributed under the terms of @@ -32,6 +33,7 @@ namespace cmd { protected: void onExecute() override; void onUndo() override; + void onFireNotifications() override; size_t onMemSize() const override { return sizeof(*this) + sizeof(doc::color_t) * (m_oldColors.size() + diff --git a/src/app/commands/cmd_add_color.cpp b/src/app/commands/cmd_add_color.cpp index 78ff4e944..a0c27a9e8 100644 --- a/src/app/commands/cmd_add_color.cpp +++ b/src/app/commands/cmd_add_color.cpp @@ -16,14 +16,12 @@ #include "app/context.h" #include "app/context_access.h" #include "app/i18n/strings.h" -#include "app/modules/palettes.h" #include "app/tx.h" #include "app/ui/color_bar.h" #include "app/ui/context_bar.h" #include "app/ui/editor/editor.h" #include "doc/palette.h" #include "fmt/format.h" -#include "ui/manager.h" namespace app { @@ -87,8 +85,13 @@ void AddColorCommand::onExecute(Context* ctx) break; } + Palette* pal = ctx->activeSite().palette(); + ASSERT(pal); + if (!pal) + return; + try { - Palette* newPalette = get_current_palette(); // System current pal + std::unique_ptr newPalette(new Palette(*pal)); color_t color = doc::rgba( appColor.getRed(), appColor.getGreen(), @@ -122,12 +125,9 @@ void AddColorCommand::onExecute(Context* ctx) frame_t frame = writer.frame(); Tx tx(writer.context(), friendlyName(), ModifyDocument); - tx(new cmd::SetPalette(sprite, frame, newPalette)); + tx(new cmd::SetPalette(sprite, frame, newPalette.get())); tx.commit(); } - - set_current_palette(newPalette, true); - ui::Manager::getDefault()->invalidate(); } catch (base::Exception& e) { Console::showException(e); diff --git a/src/app/commands/cmd_color_quantization.cpp b/src/app/commands/cmd_color_quantization.cpp index 9c3762859..5bf34b858 100644 --- a/src/app/commands/cmd_color_quantization.cpp +++ b/src/app/commands/cmd_color_quantization.cpp @@ -16,7 +16,6 @@ #include "app/context.h" #include "app/context_access.h" #include "app/job.h" -#include "app/modules/palettes.h" #include "app/pref/preferences.h" #include "app/sprite_job.h" #include "app/transaction.h" @@ -25,7 +24,6 @@ #include "doc/palette.h" #include "doc/sprite.h" #include "render/quantization.h" -#include "ui/manager.h" #include "palette_from_sprite.xml.h" @@ -156,7 +154,7 @@ void ColorQuantizationCommand::onExecute(Context* ctx) std::unique_ptr newPalette( new Palette(createPal ? tmpPalette: - *get_current_palette())); + *site.palette())); if (createPal) { entries = PalettePicks(newPalette->size()); @@ -172,13 +170,6 @@ void ColorQuantizationCommand::onExecute(Context* ctx) if (*curPalette != *newPalette) job.tx()(new cmd::SetPalette(sprite, frame, newPalette.get())); - -#ifdef ENABLE_UI - if (ui) { - set_current_palette(newPalette.get(), false); - ui::Manager::getDefault()->invalidate(); - } -#endif // ENABLE_UI } catch (const base::Exception& e) { Console::showException(e); diff --git a/src/app/commands/cmd_palette_size.cpp b/src/app/commands/cmd_palette_size.cpp index 3884e2429..258f51f9b 100644 --- a/src/app/commands/cmd_palette_size.cpp +++ b/src/app/commands/cmd_palette_size.cpp @@ -13,11 +13,9 @@ #include "app/commands/command.h" #include "app/commands/params.h" #include "app/context_access.h" -#include "app/modules/palettes.h" #include "app/tx.h" #include "doc/palette.h" #include "doc/sprite.h" -#include "ui/manager.h" #include "palette_size.xml.h" @@ -51,8 +49,9 @@ void PaletteSizeCommand::onLoadParams(const Params& params) void PaletteSizeCommand::onExecute(Context* context) { ContextReader reader(context); - frame_t frame = reader.frame(); - Palette palette(*reader.sprite()->palette(frame)); + const frame_t frame = reader.frame(); + ASSERT(reader.palette()); + Palette palette(*reader.palette()); int ncolors = (m_size != 0 ? m_size: palette.size()); #ifdef ENABLE_UI @@ -76,12 +75,6 @@ void PaletteSizeCommand::onExecute(Context* context) Tx tx(context, "Palette Size", ModifyDocument); tx(new cmd::SetPalette(writer.sprite(), frame, &palette)); tx.commit(); - - set_current_palette(&palette, false); -#ifdef ENABLE_UI - if (context->isUIAvailable()) - ui::Manager::getDefault()->invalidate(); -#endif } Command* CommandFactory::createPaletteSizeCommand() diff --git a/src/app/doc.cpp b/src/app/doc.cpp index c96a0388b..13c449c83 100644 --- a/src/app/doc.cpp +++ b/src/app/doc.cpp @@ -146,6 +146,13 @@ void Doc::notifyColorSpaceChanged() notify_observers(&DocObserver::onColorSpaceChanged, ev); } +void Doc::notifyPaletteChanged() +{ + DocEvent ev(this); + ev.sprite(sprite()); + notify_observers(&DocObserver::onPaletteChanged, ev); +} + void Doc::notifySpritePixelsModified(Sprite* sprite, const gfx::Region& region, frame_t frame) { DocEvent ev(this); @@ -549,14 +556,6 @@ void Doc::updateOSColorSpace(bool appWideSignal) context()->activeDocument() == this) { App::instance()->ColorSpaceChange(); } - - if (ui::is_ui_thread()) { - // As the color space has changed, we might need to upate the - // current palette (because the color space conversion might be - // came from a cmd::ConvertColorProfile, so the palette might be - // changed). This might generate a PaletteChange() signal. - app_update_current_palette(); - } } // static diff --git a/src/app/doc.h b/src/app/doc.h index a8cb03ab6..78995f0c4 100644 --- a/src/app/doc.h +++ b/src/app/doc.h @@ -95,6 +95,7 @@ namespace app { void notifyGeneralUpdate(); void notifyColorSpaceChanged(); + void notifyPaletteChanged(); void notifySpritePixelsModified(Sprite* sprite, const gfx::Region& region, frame_t frame); void notifyExposeSpritePixels(Sprite* sprite, const gfx::Region& region); void notifyLayerMergedDown(Layer* srcLayer, Layer* targetLayer); diff --git a/src/app/doc_observer.h b/src/app/doc_observer.h index 4a7be7b26..591d2aba2 100644 --- a/src/app/doc_observer.h +++ b/src/app/doc_observer.h @@ -24,8 +24,8 @@ namespace app { virtual void onGeneralUpdate(DocEvent& ev) { } virtual void onColorSpaceChanged(DocEvent& ev) { } - virtual void onPixelFormatChanged(DocEvent& ev) { } + virtual void onPaletteChanged(DocEvent& ev) { } virtual void onAddLayer(DocEvent& ev) { } virtual void onAddFrame(DocEvent& ev) { } diff --git a/src/app/transaction.cpp b/src/app/transaction.cpp index ef8c0f86b..1ccdae0a7 100644 --- a/src/app/transaction.cpp +++ b/src/app/transaction.cpp @@ -15,7 +15,9 @@ #include "app/context_access.h" #include "app/doc.h" #include "app/doc_undo.h" +#include "app/modules/palettes.h" #include "doc/sprite.h" +#include "ui/manager.h" #include "ui/system.h" #define TX_TRACE(...) @@ -88,6 +90,8 @@ void Transaction::commit() TX_TRACE("TX: Commit <%s>\n", m_cmds->label().c_str()); m_cmds->updateSpritePositionAfter(); + const SpritePosition sprPos = m_cmds->spritePositionAfterExecute(); + m_undo->add(m_cmds); m_cmds = nullptr; @@ -96,6 +100,23 @@ void Transaction::commit() m_doc->resetTransformation(); m_doc->generateMaskBoundaries(); } + +#ifdef ENABLE_UI + if (int(m_changes) & int(Changes::kColorChange)) { + ASSERT(m_doc); + ASSERT(m_doc->sprite()); + + Palette* pal = m_doc->sprite()->palette(sprPos.frame()); + ASSERT(pal); + if (pal) + set_current_palette(pal, false); + else + set_current_palette(nullptr, false); + + if (m_ctx->isUIAvailable()) + ui::Manager::getDefault()->invalidate(); + } +#endif } void Transaction::rollbackAndStartAgain() @@ -141,4 +162,14 @@ void Transaction::onSelectionChanged(DocEvent& ev) m_changes = Changes(int(m_changes) | int(Changes::kSelection)); } +void Transaction::onColorSpaceChanged(DocEvent& ev) +{ + m_changes = Changes(int(m_changes) | int(Changes::kColorChange)); +} + +void Transaction::onPaletteChanged(DocEvent& ev) +{ + m_changes = Changes(int(m_changes) | int(Changes::kColorChange)); +} + } // namespace app diff --git a/src/app/transaction.h b/src/app/transaction.h index 1cacf40e5..b577465f2 100644 --- a/src/app/transaction.h +++ b/src/app/transaction.h @@ -31,6 +31,11 @@ namespace app { // whole operation if something fails (e.g. an exceptions is thrown) // in the middle of the procedure. // + // This class is a DocObserver because it listen and accumulates the + // changes in the Doc (m_changes), and when the transaction ends, it + // processes those changes as UI updates (so widgets are + // invalidated/updated correctly to show the new Doc state). + // // You have to wrap every call to an transaction with a // ContextWriter. The preferred usage is as follows: // @@ -79,13 +84,21 @@ namespace app { private: // List of changes during the execution of this transaction - enum class Changes { kNone = 0, - kSelection = 1 }; + enum class Changes { + kNone = 0, + // The selection has changed so we have to re-generate the + // boundary segments. + kSelection = 1, + // The color palette or color space has changed. + kColorChange = 2 + }; void rollback(CmdTransaction* newCmds); // DocObserver impl void onSelectionChanged(DocEvent& ev) override; + void onColorSpaceChanged(DocEvent& ev) override; + void onPaletteChanged(DocEvent& ev) override; Context* m_ctx; Doc* m_doc; diff --git a/src/app/ui/color_bar.cpp b/src/app/ui/color_bar.cpp index 4271c0b10..3d48a9578 100644 --- a/src/app/ui/color_bar.cpp +++ b/src/app/ui/color_bar.cpp @@ -633,9 +633,6 @@ void ColorBar::setPalette(const doc::Palette* newPalette, const std::string& act catch (base::Exception& e) { Console::showException(e); } - - set_current_palette(newPalette, false); - manager()->invalidate(); } void ColorBar::setTransparentIndex(int index) diff --git a/src/app/ui_context.cpp b/src/app/ui_context.cpp index 1cc425da2..ca170d441 100644 --- a/src/app/ui_context.cpp +++ b/src/app/ui_context.cpp @@ -125,9 +125,10 @@ void UIContext::setActiveView(DocView* docView) m_lastSelectedView = docView; // TODO all the calls to functions like updateUsingEditor(), - // setPixelFormat(), app_refresh_screen(), updateDisplayTitleBar() - // Can be replaced with a ContextObserver listening to the - // onActiveSiteChange() event. + // setPixelFormat(), app_refresh_screen(), updateDisplayTitleBar(), + // etc. could be replaced with the Transaction class, which is a + // DocObserver and handles updates on the screen processing the + // observed changes. notifyActiveSiteChanged(); }