Rollback some changes to keep the Site::grid() origin in the cel position

Now ExpandCelCanvas uses a correctly modified doc::Grid that matches
the new and temporal cel position while we are drawing.

src/app/site.cpp

@@ -78,18 +78,8 @@ Grid Site::grid() const
 {
   if (m_layer && m_layer->isTilemap()) {
     doc::Grid grid = static_cast<LayerTilemap*>(m_layer)->tileset()->grid();
-    if (const Cel* cel = m_layer->cel(m_frame)) {
-      gfx::Point gridOrigin(0, 0);
-      int remainderX = cel->position().x % grid.tileSize().w;
-      int remainderY = cel->position().y % grid.tileSize().h;
-
-      if (remainderX)
-        gridOrigin.x = remainderX - (remainderX > 0 ? grid.tileSize().w : 0);
-      if (remainderY)
-        gridOrigin.y = remainderY - (remainderY > 0 ? grid.tileSize().h : 0);
-
-      grid.origin(gridOrigin);
-    }
+    if (const Cel* cel = m_layer->cel(m_frame))
+      grid.origin(grid.origin() + cel->position());
     return grid;
   }
 

src/app/tools/point_shapes.h

@@ -47,8 +47,6 @@ public:
     const doc::Grid& grid = loop->getGrid();
     gfx::Point newPos = grid.canvasToTile(pt.toPoint());
 
-    if (loop->getInk()->needsCelCoordinates())
-      newPos += loop->getCelOrigin();
     loop->getInk()->prepareForPointShape(loop, true, newPos.x, newPos.y);
     doInkHline(newPos.x, newPos.y, newPos.x, loop);
   }

src/app/ui/editor/brush_preview.cpp

@@ -5,6 +5,8 @@
 // This program is distributed under the terms of
 // the End-User License Agreement for Aseprite.
 
+#define BP_TRACE(...) // TRACEARGS(__VA_ARGS__)
+
 #ifdef HAVE_CONFIG_H
 #include "config.h"
 #endif
@@ -28,6 +30,7 @@
 #include "app/ui/editor/tool_loop_impl.h"
 #include "app/ui_context.h"
 #include "app/util/wrap_value.h"
+#include "base/debug.h"
 #include "doc/algo.h"
 #include "doc/blend_internals.h"
 #include "doc/brush.h"
@@ -35,6 +38,7 @@
 #include "doc/image_impl.h"
 #include "doc/layer.h"
 #include "doc/primitives.h"
+#include "gfx/rect_io.h"
 #include "os/display.h"
 #include "render/render.h"
 #include "ui/manager.h"
@@ -225,26 +229,11 @@ void BrushPreview::show(const gfx::Point& screenPos)
     Site site = m_editor->getSite();
 
     // TODO add support for "tile-brushes"
-    gfx::Rect origBrushBounds;
-    gfx::Rect brushBounds;
-    doc::Grid grid;
-    if (isFloodfill) {
-      origBrushBounds = gfx::Rect(0, 0, 1, 1);
-      brushBounds = origBrushBounds;
-      brushBounds.offset(spritePos);
-    }
-    else if (site.tilemapMode() == TilemapMode::Tiles) {
-      grid = site.grid();
-      brushBounds = gfx::Rect(grid.tileSize());
-      gfx::Point tileInGrid = grid.canvasToTile(spritePos);
-      brushBounds.offset(grid.tileBoundsInCanvas(tileInGrid).origin());
-      origBrushBounds.setOrigin(brushBounds.origin());
-    }
-    else {
-      origBrushBounds = brush->bounds();
-      brushBounds = origBrushBounds;
-      brushBounds.offset(spritePos);
-    }
+    gfx::Rect origBrushBounds =
+      (isFloodfill || site.tilemapMode() == TilemapMode::Tiles ? gfx::Rect(0, 0, 1, 1):
+                                                                 brush->bounds());
+    gfx::Rect brushBounds = origBrushBounds;
+    brushBounds.offset(spritePos);
     gfx::Rect extraCelBoundsInCanvas = brushBounds;
 
     // Tiled mode might require a bigger extra cel (to show the tiled)
@@ -270,12 +259,19 @@ void BrushPreview::show(const gfx::Point& screenPos)
     gfx::Rect extraCelBounds;
     if (site.tilemapMode() == TilemapMode::Tiles) {
       ASSERT(layer->isTilemap());
-      extraCelBounds = grid.canvasToTile(brushBounds);
+      doc::Grid grid = site.grid();
+      extraCelBounds = grid.canvasToTile(extraCelBoundsInCanvas);
+      extraCelBoundsInCanvas = grid.tileToCanvas(extraCelBounds);
     }
     else {
       extraCelBounds = extraCelBoundsInCanvas;
     }
 
+    BP_TRACE("BrushPreview:",
+             "brushBounds", brushBounds,
+             "extraCelBounds", extraCelBounds,
+             "extraCelBoundsInCanvas", extraCelBoundsInCanvas);
+
     // Create the extra cel to show the brush preview
     Cel* cel = site.cel();
 
@@ -303,7 +299,7 @@ void BrushPreview::show(const gfx::Point& screenPos)
     Image* extraImage = m_extraCel->image();
     if (extraImage->pixelFormat() == IMAGE_TILEMAP) {
       extraImage->setMaskColor(tile_i_notile);
-      clear_image(extraImage,tile_i_notile);
+      clear_image(extraImage, tile_i_notile);
     }
     else {
       extraImage->setMaskColor(mask_index);

src/app/ui/editor/drawing_state.cpp

@@ -72,8 +72,7 @@ void DrawingState::initToolLoop(Editor* editor,
     m_toolLoop->getLayer(),
     m_toolLoop->getFrame(),
     m_toolLoop->getDstImage(),
-    m_toolLoop->getDstImage()->pixelFormat() == IMAGE_TILEMAP ?
-      m_toolLoop->getGrid().origin() : m_toolLoop->getCelOrigin(),
+    m_toolLoop->getCelOrigin(),
     (m_toolLoop->getLayer() &&
      m_toolLoop->getLayer()->isImage() ?
      static_cast<LayerImage*>(m_toolLoop->getLayer())->blendMode():

src/app/ui/editor/tool_loop_impl.cpp

@@ -124,7 +124,7 @@ protected:
 
 public:
   ToolLoopBase(Editor* editor,
-               Site& site, const doc::Grid& grid,
+               Site& site,
                ToolLoopParams& params)
     : m_editor(editor)
     , m_tool(params.tool)
@@ -143,8 +143,8 @@ public:
     , m_contiguous(params.contiguous)
     , m_snapToGrid(m_docPref.grid.snap())
     , m_isSelectingTiles(false)
-    , m_grid(grid)
-    , m_gridBounds(grid.origin(), grid.tileSize())
+    , m_grid(site.grid())
+    , m_gridBounds(m_grid.origin(), m_grid.tileSize())
     , m_button(params.button)
     , m_ink(params.ink->clone())
     , m_controller(params.controller)
@@ -454,11 +454,10 @@ class ToolLoopImpl : public ToolLoopBase {
 public:
   ToolLoopImpl(Editor* editor,
                Site& site,
-               const doc::Grid& grid,
                Context* context,
                ToolLoopParams& params,
                const bool saveLastPoint)
-    : ToolLoopBase(editor, site, grid, params)
+    : ToolLoopBase(editor, site, params)
     , m_context(context)
     , m_canceled(false)
     , m_tx(m_context,
@@ -549,6 +548,10 @@ public:
     m_maskOrigin = (!m_mask->isEmpty() ? gfx::Point(m_mask->bounds().x-m_celOrigin.x,
                                                     m_mask->bounds().y-m_celOrigin.y):
                                          gfx::Point(0, 0));
+
+    // Setup the new grid of ExpandCelCanvas which can be displaced to
+    // match the new temporal cel position (m_celOrigin).
+    m_grid = m_expandCelCanvas->getGrid();
   }
 
   ~ToolLoopImpl() {
@@ -714,7 +717,6 @@ tools::ToolLoop* create_tool_loop(
   const bool selectTiles)
 {
   Site site = editor->getSite();
-  doc::Grid grid = site.grid();
 
   ToolLoopParams params;
   params.tool = editor->getCurrentEditorTool();
@@ -820,7 +822,7 @@ tools::ToolLoop* create_tool_loop(
 
     ASSERT(context->activeDocument() == editor->document());
     auto toolLoop = new ToolLoopImpl(
-      editor, site, grid, context, params, saveLastPoint);
+      editor, site, context, params, saveLastPoint);
 
     if (selectTiles)
       toolLoop->forceSnapToTiles();
@@ -859,8 +861,7 @@ tools::ToolLoop* create_tool_loop_for_script(
 
     Site site2(site);
     return new ToolLoopImpl(
-      nullptr, site2, site2.grid(),
-      context, params, false);
+      nullptr, site2, context, params, false);
   }
   catch (const std::exception& ex) {
     Console::showException(ex);
@@ -885,7 +886,7 @@ public:
     ToolLoopParams& params,
     Image* image,
     const gfx::Point& celOrigin)
-    : ToolLoopBase(editor, site, site.grid(), params)
+    : ToolLoopBase(editor, site, params)
     , m_image(image)
   {
     m_celOrigin = celOrigin;

src/app/util/expand_cel_canvas.cpp

@@ -5,6 +5,8 @@
 // This program is distributed under the terms of
 // the End-User License Agreement for Aseprite.
 
+#define EXP_TRACE(...) // TRACEARGS(__VA_ARGS__)
+
 #ifdef HAVE_CONFIG_H
 #include "config.h"
 #endif
@@ -22,6 +24,7 @@
 #include "app/site.h"
 #include "app/util/cel_ops.h"
 #include "app/util/range_utils.h"
+#include "base/debug.h"
 #include "doc/algorithm/shrink_bounds.h"
 #include "doc/cel.h"
 #include "doc/image.h"
@@ -33,11 +36,11 @@
 #include "doc/tileset.h"
 #include "doc/tileset_hash_table.h"
 #include "doc/tilesets.h"
+#include "gfx/point_io.h"
 #include "gfx/rect_io.h"
+#include "gfx/size_io.h"
 #include "render/render.h"
 
-#define EXP_TRACE(...) // TRACEARGS
-
 namespace {
 
 // We cannot have two ExpandCelCanvas instances at the same time
@@ -129,35 +132,29 @@ ExpandCelCanvas::ExpandCelCanvas(
   }
 
   if (m_tilemapMode == TilemapMode::Tiles) {
-    // m_bounds represents the bounds of the m_srcImage/m_dstImage in tiles.
-    gfx::Size srcSize = m_grid.canvasToTile(spriteBounds).size();
-    m_bounds = gfx::Rect(m_grid.origin().x, m_grid.origin().y, srcSize.w, srcSize.h);
-    // cel position will be according to the site.grid() (which is the grid origin,
-    // alocated in the top-left corner of the canvas, it can be fit in the corner or
-    // located a fraction outside of a tile.
-    // m_grid.origin() represents that fraction in pixels units.
-    //
-    //  grid.origin()
-    //  example = (-1, -1)
-    //          O---------|---------|-----   <--- grid (example: tile size = 2x2)
-    //          |     ____________________
-    //          |    |                       <-- sprite canvas
-    //          -    |    .---------.
-    //          |    |    |         |
-    //          |    |    |         |    <------- cel example= cel->position() = (1, 1)
-    //          -    |    '---------'
-    //          |    |
-    m_cel->setPosition(m_cel->position().x - m_grid.origin().x, m_cel->position().y - m_grid.origin().y);
-    // following the example, cel->position() = (2, 2)
-    // Here, we have a mixed units cel->bounds(): position in pixels, size in tiles.
-  }
-  else {
-    // 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).
-    m_cel->setPosition(m_bounds.x, m_bounds.y);
+    // Bounds of the canvas in tiles.
+    m_bounds = m_grid.canvasToTile(m_bounds);
+
+    // As the grid origin depends on the current cel position (see
+    // Site::grid()), and we're going to modify the m_cel position
+    // temporarily, we need to adjust the grid to the new temporal
+    // grid origin matching the new m_dstImage position.
+    auto newCelPosition = m_grid.tileToCanvas(m_bounds.origin());
+    m_grid.origin(m_grid.origin() - m_cel->position() + newCelPosition);
+
+    // The origin of m_bounds must be in canvas position
+    m_bounds.setOrigin(newCelPosition);
   }
 
+  // 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).
+  m_cel->setPosition(m_bounds.origin());
+  EXP_TRACE("ExpandCelCanvas",
+            "m_cel->bounds()=", m_cel->bounds(),
+            "m_bounds=", m_bounds,
+            "m_grid=", m_grid.origin(), m_grid.tileSize());
+
   if (m_celCreated) {
     getDestCanvas();
     m_cel->data()->setImage(m_dstImage, m_layer);

src/app/util/expand_cel_canvas.h

@@ -74,6 +74,7 @@ namespace app {
     void copyValidDestToSourceCanvas(const gfx::Region& rgn);
 
     const Cel* getCel() const { return m_cel; }
+    const doc::Grid& getGrid() const { return m_grid; }
 
   private:
     gfx::Rect getTrimDstImageBounds() const;