Use FlattenLayers for MergeDownLayerCommand (#4643)

Rework of the cmd::FlattenLayers implementation to accommodates the
'Merge Down' command as a special case.

src/app/cmd/flatten_layers.cpp

@@ -12,12 +12,18 @@
 #include "app/cmd/flatten_layers.h"
 
 #include "app/cmd/add_layer.h"
+#include "app/cmd/add_cel.h"
 #include "app/cmd/configure_background.h"
-#include "app/cmd/copy_rect.h"
 #include "app/cmd/move_layer.h"
 #include "app/cmd/remove_layer.h"
-#include "app/cmd/set_layer_flags.h"
+#include "app/cmd/remove_cel.h"
+#include "app/cmd/replace_image.h"
 #include "app/cmd/set_layer_name.h"
+#include "app/cmd/set_layer_opacity.h"
+#include "app/cmd/set_layer_blend_mode.h"
+#include "app/cmd/set_cel_opacity.h"
+#include "app/cmd/set_cel_zindex.h"
+#include "app/cmd/set_cel_position.h"
 #include "app/cmd/unlink_cel.h"
 #include "app/doc.h"
 #include "app/i18n/strings.h"
@@ -34,10 +40,10 @@ namespace cmd {
 
 FlattenLayers::FlattenLayers(doc::Sprite* sprite,
                              const doc::SelectedLayers& layers0,
-                             const bool newBlend)
+                             const Options options)
   : WithSprite(sprite)
 {
-  m_newBlendMethod = newBlend;
+  m_options = options;
   doc::SelectedLayers layers(layers0);
   layers.removeChildrenIfParentIsSelected();
 
@@ -66,8 +72,27 @@ void FlattenLayers::onExecute()
   if (list.empty())
     return;                     // Do nothing
 
+  // Set the drawable area to a union of all cel bounds
+  // when this option is enabled
+  ImageSpec spec = sprite->spec();
+  gfx::Rect area;
+  if (m_options.dynamicCanvas) {
+    for (frame_t frame(0); frame<sprite->totalFrames(); ++frame) {
+      for (Layer* layer : layers) {
+        Cel* cel = layer->cel(frame);
+        if (cel)
+          area |= cel->bounds();
+      }
+    }
+    spec.setSize(area.size());
+  }
+  // Otherwise use the sprite's bounds
+  else {
+    area.setSize(spec.size());
+  }
+
   // Create a temporary image.
-  ImageRef image(Image::create(sprite->spec()));
+  ImageRef image(Image::create(spec));
 
   LayerImage* flatLayer;  // The layer onto which everything will be flattened.
   color_t bgcolor;        // The background color to use for flatLayer.
@@ -78,6 +103,12 @@ void FlattenLayers::onExecute()
     // There exists a visible background layer, so we will flatten onto that.
     bgcolor = doc->bgColor(flatLayer);
   }
+  // Get bottom layer when merging layers in-place, but only if
+  // we are not flattening into the background layer
+  else if (m_options.inplace) {
+    flatLayer = static_cast<LayerImage*>(list.front());
+    bgcolor = sprite->transparentColor();
+  }
   else {
     // Create a new transparent layer to flatten everything onto it.
     flatLayer = new LayerImage(sprite);
@@ -88,7 +119,7 @@ void FlattenLayers::onExecute()
   }
 
   render::Render render;
-  render.setNewBlend(m_newBlendMethod);
+  render.setNewBlend(m_options.newBlendMethod);
   render.setBgOptions(render::BgOptions::MakeNone());
 
   {
@@ -97,44 +128,97 @@ void FlattenLayers::onExecute()
     RestoreVisibleLayers restore;
     restore.showSelectedLayers(sprite, layers);
 
+    // Map draw area to image coords
+    const gfx::ClipF area_to_image(0, 0, area);
+
     // Copy all frames to the background.
     for (frame_t frame(0); frame<sprite->totalFrames(); ++frame) {
       // Clear the image and render this frame.
       clear_image(image.get(), bgcolor);
-      render.renderSprite(image.get(), sprite, frame);
+      render.renderSprite(image.get(), sprite, frame, area_to_image);
+
+      // Get exact bounds for rendered frame
+      gfx::Rect bounds = image->bounds();
+      const bool shrink = doc::algorithm::shrink_bounds(
+        image.get(), image->maskColor(), nullptr,
+        image->bounds(), bounds);
+
+      // Skip when fully transparent
+      Cel* cel = flatLayer->cel(frame);
+      if (!shrink) {
+        if (!newFlatLayer && cel)
+          executeAndAdd(new cmd::RemoveCel(cel));
+
+        continue;
+      }
+
+      // Apply shrunk bounds to new image
+      const ImageRef new_image(doc::crop_image(
+        image.get(), bounds, image->maskColor()));
+
+      // Replace image on existing cel
+      if (cel) {
 
         // TODO Keep cel links when possible
 
-      ImageRef cel_image;
-      Cel* cel = flatLayer->cel(frame);
-      if (cel) {
         if (cel->links())
           executeAndAdd(new cmd::UnlinkCel(cel));
 
-        cel_image = cel->imageRef();
+        const ImageRef cel_image = cel->imageRef();
         ASSERT(cel_image);
 
-        executeAndAdd(
+        // Reset cel properties when flattening in-place
-          new cmd::CopyRect(cel_image.get(), image.get(),
+        if (!newFlatLayer) {
-                            gfx::Clip(0, 0, image->bounds())));
+          if (cel->opacity() != 255)
+            executeAndAdd(new cmd::SetCelOpacity(cel, 255));
+
+          if (cel->zIndex() != 0)
+            executeAndAdd(new cmd::SetCelZIndex(cel, 0));
+
+          executeAndAdd(new cmd::SetCelPosition(cel,
+            area.x+bounds.x, area.y+bounds.y));
+        }
+
+        // Modify destination cel
+        executeAndAdd(new cmd::ReplaceImage(sprite, cel_image, new_image));
+      }
+      // Add new cel on null
+      else {
+        cel = new Cel(frame, new_image);
+        cel->setPosition(area.x+bounds.x, area.y+bounds.y);
+
+        // No need to undo adding this cel when flattening onto
+        // a new layer, as the layer itself would be destroyed,
+        // hence the lack of a command
+        if (newFlatLayer) {
+          flatLayer->addCel(cel);
         }
         else {
-        gfx::Rect bounds(image->bounds());
+          executeAndAdd(new cmd::AddCel(flatLayer, cel));
-        if (doc::algorithm::shrink_bounds(
-              image.get(), image->maskColor(), nullptr, bounds)) {
-          cel_image.reset(
-            doc::crop_image(image.get(), bounds, image->maskColor()));
-          cel = new Cel(frame, cel_image);
-          cel->setPosition(bounds.origin());
-          flatLayer->addCel(cel);
         }
       }
     }
   }
 
+  // Notify observers when merging down
+  if (m_options.mergeDown)
+    doc->notifyLayerMergedDown(list.back(), flatLayer);
+
   // Add new flatten layer
-  if (newFlatLayer)
+  if (newFlatLayer) {
-    executeAndAdd(new cmd::AddLayer(list.front()->parent(), flatLayer, list.front()));
+    executeAndAdd(new cmd::AddLayer(
+      list.front()->parent(), flatLayer, list.front()));
+
+  }
+  // Reset layer properties when flattening in-place
+  else {
+    if (flatLayer->opacity() != 255)
+      executeAndAdd(new cmd::SetLayerOpacity(flatLayer, 255));
+
+    if (flatLayer->blendMode() != doc::BlendMode::NORMAL)
+      executeAndAdd(new cmd::SetLayerBlendMode(
+        flatLayer, doc::BlendMode::NORMAL));
+  }
 
   // Delete flattened layers.
   for (Layer* layer : layers) {

src/app/cmd/flatten_layers.h

@@ -20,16 +20,31 @@ namespace cmd {
   class FlattenLayers : public CmdSequence
                       , public WithSprite {
   public:
+
+    struct Options {
+      bool newBlendMethod: 1;
+      bool inplace: 1;
+      bool mergeDown: 1;
+      bool dynamicCanvas: 1;
+
+      Options():
+        newBlendMethod(false),
+        inplace(false),
+        mergeDown(false),
+        dynamicCanvas(false) {
+      }
+    };
+
     FlattenLayers(doc::Sprite* sprite,
                   const doc::SelectedLayers& layers,
-                  const bool newBlendMethod);
+                  const Options options);
 
   protected:
     void onExecute() override;
 
   private:
     doc::ObjectIds m_layerIds;
-    bool m_newBlendMethod;
+    Options m_options;
   };
 
 } // namespace cmd

src/app/commands/cmd_change_pixel_format.cpp

@@ -693,10 +693,13 @@ void ChangePixelFormatCommand::onExecute(Context* context)
     if (flatten) {
       Tx tx(Tx::LockDoc, context, doc);
       const bool newBlend = Preferences::instance().experimental.newBlend();
+      cmd::FlattenLayers::Options options;
+      options.newBlendMethod = newBlend;
+
       SelectedLayers selLayers;
       for (auto layer : sprite->root()->layers())
         selLayers.insert(layer);
-      tx(new cmd::FlattenLayers(sprite, selLayers, newBlend));
+      tx(new cmd::FlattenLayers(sprite, selLayers, options));
     }
 
     job.startJobWithCallback(

src/app/commands/cmd_flatten_layers.cpp

@@ -81,9 +81,11 @@ void FlattenLayersCommand::onExecute(Context* context)
       }
     }
     const bool newBlend = Preferences::instance().experimental.newBlend();
+    cmd::FlattenLayers::Options options;
+    options.newBlendMethod = newBlend;
     tx(new cmd::FlattenLayers(sprite,
                               range.selectedLayers(),
-                              newBlend));
+                              options));
     tx.commit();
   }
 

src/app/commands/cmd_merge_down_layer.cpp

@@ -10,23 +10,18 @@
 #endif
 
 #include "app/app.h"
-#include "app/cmd/add_cel.h"
+#include "app/pref/preferences.h"
-#include "app/cmd/replace_image.h"
+#include "app/cmd/flatten_layers.h"
-#include "app/cmd/set_cel_position.h"
-#include "app/cmd/unlink_cel.h"
 #include "app/commands/command.h"
 #include "app/context_access.h"
 #include "app/doc.h"
 #include "app/doc_api.h"
+#include "app/doc_range.h"
 #include "app/modules/gui.h"
 #include "app/tx.h"
 #include "doc/blend_internals.h"
-#include "doc/cel.h"
-#include "doc/image.h"
 #include "doc/layer.h"
-#include "doc/primitives.h"
 #include "doc/sprite.h"
-#include "render/rasterize.h"
 #include "ui/ui.h"
 
 namespace app {
@@ -81,86 +76,18 @@ void MergeDownLayerCommand::onExecute(Context* context)
 
   Tx tx(writer, friendlyName(), ModifyDocument);
 
-  for (frame_t frpos = 0; frpos<sprite->totalFrames(); ++frpos) {
+  DocRange range;
-    // Get frames
+  range.selectLayer(writer.layer());
-    Cel* src_cel = src_layer->cel(frpos);
+  range.selectLayer(dst_layer);
-    Cel* dst_cel = dst_layer->cel(frpos);
 
-    // Get images
+  const bool newBlend = Preferences::instance().experimental.newBlend();
-    Image* src_image;
+  cmd::FlattenLayers::Options options;
-    if (src_cel != NULL)
+  options.newBlendMethod = newBlend;
-      src_image = src_cel->image();
+  options.inplace = true;
-    else
+  options.mergeDown = true;
-      src_image = NULL;
+  options.dynamicCanvas = true;
-
-    ImageRef dst_image;
-    if (dst_cel)
-      dst_image = dst_cel->imageRef();
-
-    // With source image?
-    if (src_image) {
-      int t;
-      int opacity;
-      opacity = MUL_UN8(src_cel->opacity(), src_layer->opacity(), t);
-
-      // No destination image
-      if (!dst_image) {  // Only a transparent layer can have a null cel
-        // Copy this cel to the destination layer...
-
-        // Creating a copy of the image
-        dst_image.reset(
-          render::rasterize_with_cel_bounds(src_cel));
-
-        // Creating a copy of the cell
-        dst_cel = new Cel(frpos, dst_image);
-        dst_cel->setPosition(src_cel->x(), src_cel->y());
-        dst_cel->setOpacity(opacity);
-
-        tx(new cmd::AddCel(dst_layer, dst_cel));
-      }
-      // With destination
-      else {
-        gfx::Rect bounds;
-
-        // Merge down in the background layer
-        if (dst_layer->isBackground()) {
-          bounds = sprite->bounds();
-        }
-        // Merge down in a transparent layer
-        else {
-          bounds = src_cel->bounds().createUnion(dst_cel->bounds());
-        }
-
-        doc::color_t bgcolor = app_get_color_to_clear_layer(dst_layer);
-
-        ImageRef new_image(doc::crop_image(
-            dst_image.get(),
-            bounds.x-dst_cel->x(),
-            bounds.y-dst_cel->y(),
-            bounds.w, bounds.h, bgcolor));
-
-        // Draw src_cel on new_image
-        render::rasterize(
-          new_image.get(), src_cel,
-          -bounds.x, -bounds.y, false);
-
-        // First unlink the dst_cel
-        if (dst_cel->links())
-          tx(new cmd::UnlinkCel(dst_cel));
-
-        // Then modify the dst_cel
-        tx(new cmd::SetCelPosition(dst_cel,
-            bounds.x, bounds.y));
-
-        tx(new cmd::ReplaceImage(sprite,
-            dst_cel->imageRef(), new_image));
-      }
-    }
-  }
-
-  document->notifyLayerMergedDown(src_layer, dst_layer);
-  document->getApi(tx).removeLayer(src_layer); // src_layer is deleted inside removeLayer()
 
+  tx(new cmd::FlattenLayers(sprite, range.selectedLayers(), options));
   tx.commit();
 
   update_screen_for_document(document);

src/app/script/sprite_class.cpp

@@ -339,7 +339,9 @@ int Sprite_flatten(lua_State* L)
     range.selectLayer(layer);
 
   Tx tx(sprite);
-  tx(new cmd::FlattenLayers(sprite, range.selectedLayers(), true));
+  cmd::FlattenLayers::Options options;
+  options.newBlendMethod = true;
+  tx(new cmd::FlattenLayers(sprite, range.selectedLayers(), options));
   tx.commit();
   return 0;
 }

tests/scripts/merge_down_bugs.lua

@@ -25,3 +25,24 @@ do
   assert(#s.cels == 2)
   assert(s.cels[1].image:isEqual(s.cels[2].image))
 end
+
+-- Checks that merge down accounts for z-index (issue #4468)
+do
+  local s = Sprite(32, 32)
+  local a = s.layers[1]
+  app.useTool{ color=red, layer=a, points={ Point(1, 1) }}
+
+  local b = s:newLayer()
+  app.useTool{ color=blue, layer=b, points={ Point(1, 1) }}
+
+  a.cels[1].zIndex = 1
+  b.cels[1].zIndex = 0
+
+  local before = s.cels[1].image:clone()
+
+  app.activeLayer = b
+  app.command.MergeDownLayer()
+
+  local after = s.cels[1].image
+  assert(before:isEqual(after))
+end