Drop selection when we hide a layer that is being transformed (fix #4179, fix #3254)

This fixes several problems in MovingPixelsState where hidden layers
were transformed anyway when we switched the visibility of a layer in
this state.

Other fix was tried before in #3254 but we needed the onBefore/After
layer visibility change notifications to make this work properly
(i.e. drop pixels when the visiblity of a layer is changed).

The only drawback at this moment is that changing the visibility of
the non-active layer when we are transforming multiple cels/timeline
range can be confused because we don't have #2144/#2865 implemented
yet.

This bug was originally reported here: https://community.aseprite.org/t/20621

src/app/commands/cmd_layer_visibility.cpp

@@ -1,4 +1,5 @@
 // Aseprite
+// Copyright (C) 2024  Igara Studio S.A.
 // Copyright (C) 2001-2017  David Capello
 //
 // This program is distributed under the terms of
@@ -8,7 +9,6 @@
 #include "config.h"
 #endif
 
-#include "app/app.h"
 #include "app/commands/command.h"
 #include "app/context_access.h"
 #include "app/modules/gui.h"
@@ -49,7 +49,7 @@ bool LayerVisibilityCommand::onChecked(Context* context)
     return false;
 
   SelectedLayers selLayers;
-  auto range = App::instance()->timeline()->range();
+  DocRange range = context->activeSite().range();
   if (range.enabled()) {
     selLayers = range.selectedLayers();
   }
@@ -67,24 +67,25 @@ bool LayerVisibilityCommand::onChecked(Context* context)
 void LayerVisibilityCommand::onExecute(Context* context)
 {
   ContextWriter writer(context);
+  Doc* doc = writer.document();
   SelectedLayers selLayers;
-  auto range = App::instance()->timeline()->range();
+  DocRange range = context->activeSite().range();
   if (range.enabled()) {
     selLayers = range.selectedLayers();
   }
   else {
     selLayers.insert(writer.layer());
   }
+
   bool anyVisible = false;
   for (auto layer : selLayers) {
     if (layer->isVisible())
       anyVisible = true;
   }
-  for (auto layer : selLayers) {
-    layer->setVisible(!anyVisible);
-  }
 
-  update_screen_for_document(writer.document());
+  const bool newState = !anyVisible;
+  for (auto layer : selLayers)
+    doc->setLayerVisibilityWithNotifications(layer, newState);
 }
 
 Command* CommandFactory::createLayerVisibilityCommand()

src/app/doc.cpp

@@ -1,5 +1,5 @@
 // Aseprite
-// Copyright (C) 2018-2023  Igara Studio S.A.
+// Copyright (C) 2018-2024  Igara Studio S.A.
 // Copyright (C) 2001-2018  David Capello
 //
 // This program is distributed under the terms of
@@ -193,6 +193,16 @@ color_t Doc::bgColor(Layer* layer) const
     return layer->sprite()->transparentColor();
 }
 
+//////////////////////////////////////////////////////////////////////
+// Modifications with notifications
+
+void Doc::setLayerVisibilityWithNotifications(Layer* layer, const bool visible)
+{
+  notifyBeforeLayerVisibilityChange(layer, visible);
+  layer->setVisible(visible);
+  notifyAfterLayerVisibilityChange(layer);
+}
+
 //////////////////////////////////////////////////////////////////////
 // Notifications
 
@@ -244,6 +254,20 @@ void Doc::notifyLayerMergedDown(Layer* srcLayer, Layer* targetLayer)
   notify_observers<DocEvent&>(&DocObserver::onLayerMergedDown, ev);
 }
 
+void Doc::notifyBeforeLayerVisibilityChange(Layer* layer, bool newState)
+{
+  DocEvent ev(this);
+  ev.layer(layer);
+  notify_observers<DocEvent&, bool>(&DocObserver::onBeforeLayerVisibilityChange, ev, newState);
+}
+
+void Doc::notifyAfterLayerVisibilityChange(Layer* layer)
+{
+  DocEvent ev(this);
+  ev.layer(layer);
+  notify_observers<DocEvent&>(&DocObserver::onAfterLayerVisibilityChange, ev);
+}
+
 void Doc::notifyCelMoved(Layer* fromLayer, frame_t fromFrame, Layer* toLayer, frame_t toFrame)
 {
   DocEvent ev(this);

src/app/doc.h

@@ -1,5 +1,5 @@
 // Aseprite
-// Copyright (C) 2018-2023  Igara Studio S.A.
+// Copyright (C) 2018-2024  Igara Studio S.A.
 // Copyright (C) 2001-2018  David Capello
 //
 // This program is distributed under the terms of
@@ -107,6 +107,14 @@ namespace app {
 
     os::ColorSpaceRef osColorSpace() const { return m_osColorSpace; }
 
+    //////////////////////////////////////////////////////////////////////
+    // Modifications with notifications
+
+    // Use this function to change the layer visibility and notify all
+    // DocObservers about this change (e.g. so the Editor can be
+    // invalidated/redrawn, MovingPixelsState can drop pixels, etc.)
+    void setLayerVisibilityWithNotifications(Layer* layer, const bool visible);
+
     //////////////////////////////////////////////////////////////////////
     // Notifications
 
@@ -116,6 +124,8 @@ namespace app {
     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);
+    void notifyBeforeLayerVisibilityChange(Layer* layer, bool newState);
+    void notifyAfterLayerVisibilityChange(Layer* layer);
     void notifyCelMoved(Layer* fromLayer, frame_t fromFrame, Layer* toLayer, frame_t toFrame);
     void notifyCelCopied(Layer* fromLayer, frame_t fromFrame, Layer* toLayer, frame_t toFrame);
     void notifySelectionChanged();

src/app/doc_observer.h

@@ -1,5 +1,5 @@
 // Aseprite
-// Copyright (C) 2018-2023  Igara Studio S.A.
+// Copyright (C) 2018-2024  Igara Studio S.A.
 // Copyright (C) 2001-2018  David Capello
 //
 // This program is distributed under the terms of
@@ -97,6 +97,10 @@ namespace app {
     // The collapsed/expanded flag of a specific layer changed.
     virtual void onLayerCollapsedChanged(DocEvent& ev) { }
 
+    // The visibility flag of a specific layer is going to change/changed.
+    virtual void onBeforeLayerVisibilityChange(DocEvent& ev, bool newState) { }
+    virtual void onAfterLayerVisibilityChange(DocEvent& ev) { }
+
     // The tileset was remapped (e.g. when tiles are re-ordered).
     virtual void onRemapTileset(DocEvent& ev, const doc::Remap& remap) { }
 

src/app/script/layer_class.cpp

@@ -1,5 +1,5 @@
 // Aseprite
-// Copyright (C) 2018-2023  Igara Studio S.A.
+// Copyright (C) 2018-2024  Igara Studio S.A.
 // Copyright (C) 2018  David Capello
 //
 // This program is distributed under the terms of
@@ -329,7 +329,9 @@ int Layer_set_isEditable(lua_State* L)
 int Layer_set_isVisible(lua_State* L)
 {
   auto layer = get_docobj<Layer>(L, 1);
-  layer->setVisible(lua_toboolean(L, 2));
+  const bool newState = lua_toboolean(L, 2);
+  Doc* doc = static_cast<Doc*>(layer->sprite()->document());
+  doc->setLayerVisibilityWithNotifications(layer, newState);
   return 0;
 }
 

src/app/ui/doc_view.cpp

@@ -1,5 +1,5 @@
 // Aseprite
-// Copyright (C) 2018-2023  Igara Studio S.A.
+// Copyright (C) 2018-2024  Igara Studio S.A.
 // Copyright (C) 2001-2018  David Capello
 //
 // This program is distributed under the terms of
@@ -478,6 +478,15 @@ void DocView::onTotalFramesChanged(DocEvent& ev)
 
 void DocView::onLayerRestacked(DocEvent& ev)
 {
+  if (hasContentInActiveFrame(ev.layer()))
+    m_editor->invalidate();
+}
+
+void DocView::onAfterLayerVisibilityChange(DocEvent& ev)
+{
+  // If there is no cel for this layer in the current frame, there is
+  // no need to redraw the editor
+  if (hasContentInActiveFrame(ev.layer()))
     m_editor->invalidate();
 }
 
@@ -653,4 +662,19 @@ void DocView::onCancel(Context* ctx)
   }
 }
 
+bool DocView::hasContentInActiveFrame(const doc::Layer* layer) const
+{
+  if (!layer)
+    return false;
+  else if (layer->cel(m_editor->frame()))
+    return true;
+  else if (layer->isGroup()) {
+    for (const doc::Layer* child : static_cast<const doc::LayerGroup*>(layer)->layers()) {
+      if (hasContentInActiveFrame(child))
+        return true;
+    }
+  }
+  return false;
+}
+
 } // namespace app

src/app/ui/doc_view.h

@@ -1,5 +1,5 @@
 // Aseprite
-// Copyright (C) 2019-2023  Igara Studio S.A.
+// Copyright (C) 2019-2024  Igara Studio S.A.
 // Copyright (C) 2001-2018  David Capello
 //
 // This program is distributed under the terms of
@@ -15,6 +15,10 @@
 #include "app/ui/workspace_view.h"
 #include "ui/box.h"
 
+namespace doc {
+  class Layer;
+}
+
 namespace ui {
   class View;
 }
@@ -86,6 +90,7 @@ namespace app {
     void onAfterRemoveCel(DocEvent& ev) override;
     void onTotalFramesChanged(DocEvent& ev) override;
     void onLayerRestacked(DocEvent& ev) override;
+    void onAfterLayerVisibilityChange(DocEvent& ev) override;
     void onTilesetChanged(DocEvent& ev) override;
 
     // InputChainElement impl
@@ -105,6 +110,8 @@ namespace app {
     bool onProcessMessage(ui::Message* msg) override;
 
   private:
+    bool hasContentInActiveFrame(const doc::Layer* layer) const;
+
     Type m_type;
     Doc* m_document;
     ui::View* m_view;

src/app/ui/editor/editor.cpp

@@ -1,5 +1,5 @@
 // Aseprite
-// Copyright (C) 2018-2023  Igara Studio S.A.
+// Copyright (C) 2018-2024  Igara Studio S.A.
 // Copyright (C) 2001-2018  David Capello
 //
 // This program is distributed under the terms of
@@ -2438,6 +2438,12 @@ void Editor::onRemoveSlice(DocEvent& ev)
   }
 }
 
+void Editor::onBeforeLayerVisibilityChange(DocEvent& ev, bool newState)
+{
+  if (m_state)
+    m_state->onBeforeLayerVisibilityChange(this, ev.layer(), newState);
+}
+
 void Editor::setCursor(const gfx::Point& mouseDisplayPos)
 {
   Rect vp = View::getView(this)->viewportBounds();

src/app/ui/editor/editor.h

@@ -1,5 +1,5 @@
 // Aseprite
-// Copyright (C) 2018-2023  Igara Studio S.A.
+// Copyright (C) 2018-2024  Igara Studio S.A.
 // Copyright (C) 2001-2018  David Capello
 //
 // This program is distributed under the terms of
@@ -351,6 +351,7 @@ namespace app {
     void onAddTag(DocEvent& ev) override;
     void onRemoveTag(DocEvent& ev) override;
     void onRemoveSlice(DocEvent& ev) override;
+    void onBeforeLayerVisibilityChange(DocEvent& ev, bool newState) override;
 
     // ActiveToolObserver impl
     void onActiveToolChange(tools::Tool* tool) override;

src/app/ui/editor/editor_state.h

@@ -1,5 +1,5 @@
 // Aseprite
-// Copyright (C) 2019-2023  Igara Studio S.A.
+// Copyright (C) 2019-2024  Igara Studio S.A.
 // Copyright (C) 2001-2016  David Capello
 //
 // This program is distributed under the terms of
@@ -26,6 +26,7 @@ namespace ui {
 }
 
 namespace doc {
+  class Layer;
   class Tag;
 }
 
@@ -145,6 +146,11 @@ namespace app {
     // collection.
     virtual void onBeforeRemoveLayer(Editor* editor) { }
 
+    // Called when the visibility of a specific layer is changed.
+    virtual void onBeforeLayerVisibilityChange(Editor* editor,
+                                               doc::Layer* layer,
+                                               bool newState) { }
+
   private:
     DISABLE_COPYING(EditorState);
   };

src/app/ui/editor/moving_pixels_state.cpp

@@ -1,5 +1,5 @@
 // Aseprite
-// Copyright (C) 2019-2023  Igara Studio S.A.
+// Copyright (C) 2019-2024  Igara Studio S.A.
 // Copyright (C) 2001-2018  David Capello
 //
 // This program is distributed under the terms of
@@ -578,6 +578,25 @@ bool MovingPixelsState::acceptQuickTool(tools::Tool* tool)
      tool->getInk(0)->isZoom());
 }
 
+void MovingPixelsState::onBeforeLayerVisibilityChange(Editor* editor,
+                                                      doc::Layer* layer,
+                                                      bool newState)
+{
+  if (!isActiveDocument())
+    return;
+
+  // If the layer visibility of any selected layer changes, we just
+  // drop the pixels (it's the easiest way to avoid modifying hidden
+  // pixels).
+  if (m_pixelsMovement) {
+    const Site& site = m_pixelsMovement->site();
+    if (site.layer() == layer ||
+        site.range().contains(layer)) {
+      dropPixels();
+    }
+  }
+}
+
 // Before executing any command, we drop the pixels (go back to standby).
 void MovingPixelsState::onBeforeCommandExecution(CommandExecutionEvent& ev)
 {

src/app/ui/editor/moving_pixels_state.h

@@ -1,5 +1,5 @@
 // Aseprite
-// Copyright (C) 2019-2022  Igara Studio S.A.
+// Copyright (C) 2019-2024  Igara Studio S.A.
 // Copyright (C) 2001-2017  David Capello
 //
 // This program is distributed under the terms of
@@ -21,7 +21,7 @@
 #include "ui/timer.h"
 
 namespace doc {
-  class Image;
+  class Layer;
 }
 
 namespace app {
@@ -54,35 +54,37 @@ namespace app {
     void updateTransformation(const Transformation& t);
 
     // EditorState
-    virtual void onEnterState(Editor* editor) override;
+    void onEnterState(Editor* editor) override;
-    virtual void onEditorGotFocus(Editor* editor) override;
+    void onEditorGotFocus(Editor* editor) override;
-    virtual LeaveAction onLeaveState(Editor* editor, EditorState* newState) override;
+    LeaveAction onLeaveState(Editor* editor, EditorState* newState) override;
-    virtual void onActiveToolChange(Editor* editor, tools::Tool* tool) override;
+    void onActiveToolChange(Editor* editor, tools::Tool* tool) override;
-    virtual bool onMouseDown(Editor* editor, ui::MouseMessage* msg) override;
+    bool onMouseDown(Editor* editor, ui::MouseMessage* msg) override;
-    virtual bool onMouseUp(Editor* editor, ui::MouseMessage* msg) override;
+    bool onMouseUp(Editor* editor, ui::MouseMessage* msg) override;
-    virtual bool onMouseMove(Editor* editor, ui::MouseMessage* msg) override;
+    bool onMouseMove(Editor* editor, ui::MouseMessage* msg) override;
-    virtual bool onSetCursor(Editor* editor, const gfx::Point& mouseScreenPos) override;
+    bool onSetCursor(Editor* editor, const gfx::Point& mouseScreenPos) override;
-    virtual bool onKeyDown(Editor* editor, ui::KeyMessage* msg) override;
+    bool onKeyDown(Editor* editor, ui::KeyMessage* msg) override;
-    virtual bool onKeyUp(Editor* editor, ui::KeyMessage* msg) override;
+    bool onKeyUp(Editor* editor, ui::KeyMessage* msg) override;
-    virtual bool onUpdateStatusBar(Editor* editor) override;
+    bool onUpdateStatusBar(Editor* editor) override;
-    virtual bool acceptQuickTool(tools::Tool* tool) override;
+    bool acceptQuickTool(tools::Tool* tool) override;
-    virtual bool requireBrushPreview() override { return false; }
+    bool requireBrushPreview() override { return false; }
+    void onBeforeLayerVisibilityChange(Editor* editor, doc::Layer* layer, bool newState) override;
+
 
     // EditorObserver
-    virtual void onDestroyEditor(Editor* editor) override;
+    void onDestroyEditor(Editor* editor) override;
-    virtual void onBeforeFrameChanged(Editor* editor) override;
+    void onBeforeFrameChanged(Editor* editor) override;
-    virtual void onBeforeLayerChanged(Editor* editor) override;
+    void onBeforeLayerChanged(Editor* editor) override;
 
     // TimelineObserver
-    virtual void onBeforeRangeChanged(Timeline* timeline) override;
+    void onBeforeRangeChanged(Timeline* timeline) override;
 
     // ContextBarObserver
-    virtual void onDropPixels(ContextBarObserver::DropAction action) override;
+    void onDropPixels(ContextBarObserver::DropAction action) override;
 
     // PixelsMovementDelegate
-    virtual void onPivotChange() override;
+    void onPivotChange() override;
 
-    virtual Transformation getTransformation(Editor* editor) override;
+    Transformation getTransformation(Editor* editor) override;
 
   private:
     // DelayedMouseMoveDelegate impl

src/app/ui/editor/pixels_movement.cpp

@@ -1,5 +1,5 @@
 // Aseprite
-// Copyright (C) 2019-2023  Igara Studio S.A.
+// Copyright (C) 2019-2024  Igara Studio S.A.
 // Copyright (C) 2001-2018  David Capello
 //
 // This program is distributed under the terms of
@@ -787,9 +787,10 @@ void PixelsMovement::stampImage(bool finalStamp)
     cels.push_back(currentCel);
   }
 
-  if (currentCel && currentCel->layer() &&
+  if (currentCel &&
+      currentCel->layer() &&
       currentCel->layer()->isImage() &&
-      !currentCel->layer()->isEditableHierarchy()) {
+      !currentCel->layer()->canEditPixels()) {
     Transformation initialCelPos(gfx::Rect(m_initialMask0->bounds()), m_currentData.cornerThick());
     redrawExtraImage(&initialCelPos);
     stampExtraCelImage();

src/app/ui/editor/pixels_movement.h

@@ -1,5 +1,5 @@
 // Aseprite
-// Copyright (C) 2019-2021  Igara Studio S.A.
+// Copyright (C) 2019-2024  Igara Studio S.A.
 // Copyright (C) 2001-2018  David Capello
 //
 // This program is distributed under the terms of
@@ -75,6 +75,8 @@ namespace app {
                    const Mask* mask,
                    const char* operationName);
 
+    const Site& site() { return m_site; }
+
     HandleType handle() const { return m_handle; }
     bool canHandleFrameChange() const { return m_canHandleFrameChange; }
 

src/app/ui/timeline/timeline.cpp

@@ -1,5 +1,5 @@
 // Aseprite
-// Copyright (C) 2018-2023  Igara Studio S.A.
+// Copyright (C) 2018-2024  Igara Studio S.A.
 // Copyright (C) 2001-2018  David Capello
 //
 // This program is distributed under the terms of
@@ -696,7 +696,9 @@ bool Timeline::onProcessMessage(Message* msg)
           bool newVisibleState = !allLayersVisible();
           for (Layer* topLayer : m_sprite->root()->layers()) {
             if (topLayer->isVisible() != newVisibleState) {
-              topLayer->setVisible(newVisibleState);
+              m_document->setLayerVisibilityWithNotifications(
+                topLayer, newVisibleState);
+
               if (topLayer->isGroup())
                 regenRows = true;
             }
@@ -825,11 +827,11 @@ bool Timeline::onProcessMessage(Message* msg)
                 for (Row& row : m_rows) {
                   Layer* l = row.layer();
                   if (l->hasFlags(LayerFlags::Internal_WasVisible)) {
-                    l->setVisible(true);
+                    m_document->setLayerVisibilityWithNotifications(l, true);
                     l->switchFlags(LayerFlags::Internal_WasVisible, false);
                   }
                   else {
-                    l->setVisible(false);
+                    m_document->setLayerVisibilityWithNotifications(l, false);
                   }
                 }
               }
@@ -838,7 +840,7 @@ bool Timeline::onProcessMessage(Message* msg)
                 for (Row& row : m_rows) {
                   Layer* l = row.layer();
                   l->switchFlags(LayerFlags::Internal_WasVisible, l->isVisible());
-                  l->setVisible(false);
+                  m_document->setLayerVisibilityWithNotifications(l, false);
                 }
               }
 
@@ -2016,6 +2018,14 @@ void Timeline::onLayerCollapsedChanged(DocEvent& ev)
   invalidate();
 }
 
+void Timeline::onAfterLayerVisibilityChange(DocEvent& ev)
+{
+  layer_t layerIdx = getLayerIndex(ev.layer());
+  if (layerIdx >= 0)
+    invalidateRect(getPartBounds(Hit(PART_ROW_EYE_ICON, layerIdx))
+                   .offset(origin()));
+}
+
 void Timeline::onStateChanged(Editor* editor)
 {
   m_aniControls.updateUsingEditor(editor);
@@ -4475,12 +4485,10 @@ void Timeline::setLayerVisibleFlag(const layer_t l, const bool state)
   if (!layer)
     return;
 
-  bool redrawEditors = false;
   bool regenRows = false;
 
   if (layer->isVisible() != state) {
-    layer->setVisible(state);
+    m_document->setLayerVisibilityWithNotifications(layer, state);
-    redrawEditors = true;
 
     // Regenerate rows because might change the flag of the children
     // (the flag is propagated to the children in m_inheritedFlags).
@@ -4493,9 +4501,8 @@ void Timeline::setLayerVisibleFlag(const layer_t l, const bool state)
       layer = layer->parent();
       while (layer) {
         if (!layer->isVisible()) {
-          layer->setVisible(true);
+          m_document->setLayerVisibilityWithNotifications(layer, true);
           regenRows = true;
-          redrawEditors = true;
         }
         layer = layer->parent();
       }
@@ -4506,9 +4513,6 @@ void Timeline::setLayerVisibleFlag(const layer_t l, const bool state)
     regenerateRows();
     invalidate();
   }
-
-  if (redrawEditors)
-    m_document->notifyGeneralUpdate();
 }
 
 void Timeline::setLayerEditableFlag(const layer_t l, const bool state)

src/app/ui/timeline/timeline.h

@@ -1,5 +1,5 @@
 // Aseprite
-// Copyright (C) 2018-2023  Igara Studio S.A.
+// Copyright (C) 2018-2024  Igara Studio S.A.
 // Copyright (C) 2001-2018  David Capello
 //
 // This program is distributed under the terms of
@@ -163,6 +163,7 @@ namespace app {
     void onTagChange(DocEvent& ev) override;
     void onTagRename(DocEvent& ev) override;
     void onLayerCollapsedChanged(DocEvent& ev) override;
+    void onAfterLayerVisibilityChange(DocEvent& ev) override;
 
     // app::Context slots.
     void onBeforeCommandExecution(CommandExecutionEvent& ev);

src/app/util/layer_utils.cpp

@@ -1,5 +1,5 @@
 // Aseprite
-// Copyright (C) 2020  Igara Studio S.A.
+// Copyright (C) 2020-2024  Igara Studio S.A.
 //
 // This program is distributed under the terms of
 // the End-User License Agreement for Aseprite.
@@ -45,14 +45,34 @@ Layer* candidate_if_layer_is_deleted(
 bool layer_is_locked(Editor* editor)
 {
   Layer* layer = editor->layer();
-  if (layer && !layer->isEditableHierarchy()) {
+  if (!layer)
+    return false;
+
 #ifdef ENABLE_UI
-    if (auto statusBar = StatusBar::instance())
+  auto statusBar = StatusBar::instance();
+#endif
+
+  if (!layer->isVisibleHierarchy()) {
+#ifdef ENABLE_UI
+    if (statusBar) {
       statusBar->showTip(
-        1000, fmt::format(Strings::statusbar_tips_layer_locked(), layer->name()));
+        1000, fmt::format(Strings::statusbar_tips_layer_x_is_hidden(),
+                          layer->name()));
+    }
 #endif
     return true;
   }
+
+  if (!layer->isEditableHierarchy()) {
+#ifdef ENABLE_UI
+    if (statusBar) {
+      statusBar->showTip(
+        1000, fmt::format(Strings::statusbar_tips_layer_locked(), layer->name()));
+    }
+#endif
+    return true;
+  }
+
   return false;
 }