Fix regression showing the selection feedback above all layers

Without this fix we'll see the selection stroke feedback in the same
level as the active layer. With this patch the preview is displayed at
the top just as in the main branch.

src/app/ui/editor/drawing_state.cpp

@@ -68,18 +68,24 @@ void DrawingState::initToolLoop(Editor* editor,
 {
   Tileset* tileset = m_toolLoop->getDstTileset();
 
+  // For selection inks we don't use a "the selected layer" for
+  // preview purposes, because we want the selection feedback to be at
+  // the top of all layers.
+  Layer* previewLayer = (m_toolLoop->getInk()->isSelection() ? nullptr:
+                                                               m_toolLoop->getLayer());
+
   // Prepare preview image (the destination image will be our preview
   // in the tool-loop time, so we can see what we are drawing)
   editor->renderEngine().setPreviewImage(
-    m_toolLoop->getLayer(),
+    previewLayer,
     m_toolLoop->getFrame(),
     tileset ? nullptr: m_toolLoop->getDstImage(),
     tileset,
     m_toolLoop->getCelOrigin(),
-    (m_toolLoop->getLayer() &&
-     m_toolLoop->getLayer()->isImage() ?
+    (previewLayer &&
+     previewLayer->isImage() ?
      static_cast<LayerImage*>(m_toolLoop->getLayer())->blendMode():
-     BlendMode::NEG_BW));
+     BlendMode::NEG_BW)); // To preview the selection ink we use the negative black & white blender
 
   ASSERT(!m_toolLoopManager->isCanceled());
 

src/app/ui/editor/tool_loop_impl.cpp

@@ -60,6 +60,7 @@
 #include "ui/ui.h"
 
 #include <algorithm>
+#include <memory>
 
 namespace app {
 
@@ -449,7 +450,7 @@ class ToolLoopImpl : public ToolLoopBase,
   gfx::Point m_maskOrigin;
   bool m_internalCancel = false;
   Tx m_tx;
-  ExpandCelCanvas* m_expandCelCanvas;
+  std::unique_ptr<ExpandCelCanvas> m_expandCelCanvas;
   Image* m_floodfillSrcImage;
   bool m_saveLastPoint;
 
@@ -470,7 +471,6 @@ public:
              m_ink->isSlice() ||
              m_ink->isZoom()) ? DoesntModifyDocument:
                                 ModifyDocument))
-    , m_expandCelCanvas(nullptr)
     , m_floodfillSrcImage(nullptr)
     , m_saveLastPoint(saveLastPoint)
   {
@@ -503,8 +503,8 @@ public:
       }
     }
 
-    m_expandCelCanvas = new ExpandCelCanvas(
-      site, site.layer(),
+    m_expandCelCanvas.reset(new ExpandCelCanvas(
+      site, m_layer,
       m_docPref.tiled.mode(),
       m_tx,
       ExpandCelCanvas::Flags(
@@ -517,7 +517,9 @@ public:
          site.tilemapMode() == TilemapMode::Pixels &&
          site.tilesetMode() == TilesetMode::Manual &&
          !m_ink->isSelection() ? ExpandCelCanvas::TilesetPreview:
-                                 ExpandCelCanvas::None)));
+                                 ExpandCelCanvas::None) |
+        (m_ink->isSelection() ? ExpandCelCanvas::SelectionPreview:
+                                ExpandCelCanvas::None))));
 
     if (!m_floodfillSrcImage)
       m_floodfillSrcImage = const_cast<Image*>(getSrcImage());
@@ -539,10 +541,12 @@ public:
     m_sprayWidth = m_toolPref.spray.width();
     m_spraySpeed = m_toolPref.spray.speed();
 
-    if (m_ink->isSelection())
+    if (m_ink->isSelection()) {
       m_useMask = false;
-    else
+    }
+    else {
       m_useMask = m_document->isMaskVisible();
+    }
 
     // Start with an empty mask if the user is selecting with "default selection mode"
     if (m_ink->isSelection() &&
@@ -576,7 +580,6 @@ public:
 
     if (m_floodfillSrcImage != getSrcImage())
       delete m_floodfillSrcImage;
-    delete m_expandCelCanvas;
   }
 
   // IToolLoop interface

src/app/util/expand_cel_canvas.cpp

@@ -83,9 +83,6 @@ ExpandCelCanvas::ExpandCelCanvas(
   , m_cel(nullptr)
   , m_celImage(nullptr)
   , m_celCreated(false)
-  // When editing the tiles of a tilemap manually we use a tileset as
-  // preview, see getDestTileset() for details.
-  , m_tilesetPreview((flags & TilesetPreview) == TilesetPreview)
   , m_flags(flags)
   , m_srcImage(nullptr)
   , m_dstImage(nullptr)
@@ -107,7 +104,7 @@ ExpandCelCanvas::ExpandCelCanvas(
 
   create_buffers();
 
-  if (m_layer && m_layer->isImage()) {
+  if (previewSpecificLayerChanges()) {
     m_cel = m_layer->cel(site.frame());
     if (m_cel)
       m_celImage = m_cel->imageRef();
@@ -163,7 +160,7 @@ ExpandCelCanvas::ExpandCelCanvas(
   // We have to adjust the cel position to match the m_dstImage
   // position (the new m_dstImage will be used in RenderEngine to
   // draw this cel).
-  if (!m_tilesetPreview)
+  if (!isTilesetPreview())
     m_cel->setPosition(m_bounds.origin());
 
   EXP_TRACE("ExpandCelCanvas",
@@ -177,7 +174,7 @@ ExpandCelCanvas::ExpandCelCanvas(
 
     m_cel->data()->setImage(m_dstImage, m_layer);
 
-    if (m_layer && m_layer->isImage())
+    if (previewSpecificLayerChanges())
       static_cast<LayerImage*>(m_layer)->addCel(m_cel);
   }
   else if (m_layer->isTilemap() &&
@@ -189,11 +186,11 @@ ExpandCelCanvas::ExpandCelCanvas(
   // of the tilemap image).
   else if (m_layer->isTilemap() &&
            m_tilemapMode == TilemapMode::Pixels &&
-           !m_tilesetPreview) {
+           !isTilesetPreview()) {
     getDestCanvas();
     m_cel->data()->setImage(m_dstImage, m_layer);
   }
-  else if (m_tilesetPreview) {
+  else if (isTilesetPreview()) {
     getDestTileset();
   }
 }
@@ -235,7 +232,7 @@ void ExpandCelCanvas::commit()
     // don't have a m_celImage)
     validateDestCanvas(gfx::Region(m_bounds));
 
-    if (m_layer->isImage()) {
+    if (previewSpecificLayerChanges()) {
       // We can temporary remove the cel.
       static_cast<LayerImage*>(m_layer)->removeCel(m_cel);
 
@@ -271,9 +268,11 @@ void ExpandCelCanvas::commit()
         m_cmds->executeAndAdd(new cmd::AddCel(m_layer, m_cel));
       }
     }
-    // We are selecting inside a layer group...
+    // We are selecting...
     else {
-      // Just delete the created layer
+      ASSERT(isSelectionPreview());
+
+      // Just delete the created cel for preview purposes of the selection
       delete m_cel;
       m_cel = nullptr;
     }
@@ -283,7 +282,7 @@ void ExpandCelCanvas::commit()
     m_cel->setPosition(m_origCelPos);
 
 #ifdef _DEBUG
-    if (m_layer->isTilemap() && !m_tilesetPreview) {
+    if (m_layer->isTilemap() && !isTilesetPreview()) {
       ASSERT(m_cel->image() != m_celImage.get());
     }
     else {
@@ -428,7 +427,7 @@ void ExpandCelCanvas::rollback()
   m_cel->setPosition(m_origCelPos);
 
   if (m_celCreated) {
-    if (m_layer && m_layer->isImage())
+    if (previewSpecificLayerChanges())
       static_cast<LayerImage*>(m_layer)->removeCel(m_cel);
 
     delete m_cel;
@@ -446,7 +445,7 @@ void ExpandCelCanvas::rollback()
 
 gfx::Point ExpandCelCanvas::getCelOrigin() const
 {
-  if (m_tilesetPreview)
+  if (isTilesetPreview())
     return m_bounds.origin();
   else
     return m_cel->position();
@@ -494,12 +493,12 @@ Tileset* ExpandCelCanvas::getDestTileset()
 {
   EXP_TRACE("ExpandCelCanvas::getDestTileset()"
             "celCreated", m_celCreated,
-            "tilesetPreview", m_tilesetPreview);
+            "tilesetPreview", isTilesetPreview());
 
   // When we edit the pixels in manual mode, we can create a tileset
   // that will be used for preview purposes to see changes in all
   // instances of the same tile.
-  if (!m_celCreated && m_tilesetPreview) {
+  if (!m_celCreated && isTilesetPreview()) {
     // Copy the whole tileset
     const Tileset* srcTileset = static_cast<LayerTilemap*>(m_layer)->tileset();
 
@@ -537,7 +536,7 @@ void ExpandCelCanvas::validateSourceCanvas(const gfx::Region& rgn)
   rgnToValidate.createSubtraction(rgnToValidate, m_validSrcRegion);
   rgnToValidate.createIntersection(rgnToValidate, gfx::Region(m_srcImage->bounds()));
 
-  if (m_celImage) {
+  if (m_celImage && previewSpecificLayerChanges()) {
     gfx::Region rgnToClear;
     rgnToClear.createSubtraction(
       rgnToValidate,

src/app/util/expand_cel_canvas.h

@@ -1,5 +1,5 @@
 // Aseprite
-// Copyright (C) 2019-2020  Igara Studio S.A.
+// Copyright (C) 2019-2021  Igara Studio S.A.
 // Copyright (C) 2001-2018  David Capello
 //
 // This program is distributed under the terms of
@@ -14,6 +14,7 @@
 #include "doc/frame.h"
 #include "doc/grid.h"
 #include "doc/image_ref.h"
+#include "doc/layer.h"
 #include "filters/tiled_mode.h"
 #include "gfx/point.h"
 #include "gfx/rect.h"
@@ -52,8 +53,11 @@ namespace app {
       PixelsBounds = 2,
       // True if you want to preview the changes in the tileset. Only
       // useful in TilesetMode::Manual mode when editing tiles in a
-      // tilemap.
+      // tilemap. See getDestTileset() for details.
       TilesetPreview = 4,
+      // Enable when we are going to use the expanded cel canvas for
+      // preview purposes of a selection tools.
+      SelectionPreview = 8,
     };
 
     ExpandCelCanvas(Site site, Layer* layer,
@@ -94,6 +98,22 @@ namespace app {
     ImageRef trimDstImage(const gfx::Rect& bounds) const;
     void copySourceTilestToDestTileset();
 
+    bool isTilesetPreview() const {
+      return ((m_flags & TilesetPreview) == TilesetPreview);
+    }
+
+    bool isSelectionPreview() const {
+      return ((m_flags & SelectionPreview) == SelectionPreview);
+    }
+
+    // This is the common case where we want to preview a change in
+    // the given layer of ExpandCelCanvas ctor.
+    bool previewSpecificLayerChanges() const {
+      return (m_layer &&
+              m_layer->isImage() &&
+              !isSelectionPreview());
+    }
+
     Doc* m_document;
     Sprite* m_sprite;
     Layer* m_layer;
@@ -101,7 +121,6 @@ namespace app {
     Cel* m_cel;
     ImageRef m_celImage;
     bool m_celCreated;
-    bool m_tilesetPreview;
     gfx::Point m_origCelPos;
     Flags m_flags;
     gfx::Rect m_bounds;