Fix pixel-perfect in manual tile mode (fix #2741)

src/app/tools/intertwine.cpp

@@ -18,6 +18,7 @@
 #include "app/tools/tool_loop.h"
 #include "base/pi.h"
 #include "doc/algo.h"
+#include "doc/layer.h"
 
 #include <cmath>
 
@@ -58,7 +59,11 @@ gfx::Rect Intertwine::getStrokeBounds(ToolLoop* loop, const Stroke& stroke)
   return stroke.bounds();
 }
 
-// static
+void Intertwine::doTransformPoint(const Stroke::Pt& pt, ToolLoop* loop)
+{
+  loop->getPointShape()->transformPoint(loop, pt);
+}
+
 void Intertwine::doPointshapeStrokePt(const Stroke::Pt& pt, ToolLoop* loop)
 {
   Symmetry* symmetry = loop->getSymmetry();
@@ -73,11 +78,11 @@ void Intertwine::doPointshapeStrokePt(const Stroke::Pt& pt, ToolLoop* loop)
     for (const auto& stroke : strokes) {
       // We call transformPoint() moving back each point to the cel
       // origin.
-      loop->getPointShape()->transformPoint(loop, stroke[0]);
+      doTransformPoint(stroke[0], loop);
     }
   }
   else {
-    loop->getPointShape()->transformPoint(loop, pt);
+    doTransformPoint(pt, loop);
   }
 }
 
@@ -87,14 +92,14 @@ void Intertwine::doPointshapePoint(int x, int y, ToolLoop* loop)
   Stroke::Pt pt(x, y);
   pt.size = loop->getBrush()->size();
   pt.angle = loop->getBrush()->angle();
-  doPointshapeStrokePt(pt, loop);
+  loop->getIntertwine()->doPointshapeStrokePt(pt, loop);
 }
 
 // static
 void Intertwine::doPointshapePointDynamics(int x, int y, Intertwine::LineData* data)
 {
   data->doStep(x, y);
-  doPointshapeStrokePt(data->pt, data->loop);
+  data->loop->getIntertwine()->doPointshapeStrokePt(data->pt, data->loop);
 }
 
 // static

src/app/tools/intertwine.h

@@ -43,7 +43,8 @@ namespace app {
       };
 
     protected:
-      static void doPointshapeStrokePt(const Stroke::Pt& pt, ToolLoop* loop);
+      virtual void doTransformPoint(const Stroke::Pt& pt, ToolLoop* loop);
+      virtual void doPointshapeStrokePt(const Stroke::Pt& pt, ToolLoop* loop);
       // The given point must be relative to the cel origin.
       static void doPointshapePoint(int x, int y, ToolLoop* loop);
       static void doPointshapePointDynamics(int x, int y, LineData* data);

src/app/tools/intertwiners.h

@@ -429,6 +429,7 @@ class IntertwineAsPixelPerfect : public Intertwine {
   // we have to ignore printing the first pixel of the line.
   bool m_retainedTracePolicyLast = false;
   Stroke m_pts;
+  bool m_saveStrokeArea = false;
 
 public:
   // Useful for Shift+Ctrl+pencil to draw straight lines and snap
@@ -458,7 +459,10 @@ public:
     else if (stroke.size() == 1) {
       if (m_pts.empty())
         m_pts = stroke;
+
+      m_saveStrokeArea = false;
       doPointshapeStrokePt(stroke[0], loop);
+
       return;
     }
     else {
@@ -511,10 +515,15 @@ public:
       // the SHIFT key))
       if (c == 0 && m_retainedTracePolicyLast)
         continue;
-      // For the last point we store the source image content at that point so we
+
-      // can restore it when erasing a point because of pixel-perfect.
+      // For the last point we need to store the source image content at that
-      if (c == m_pts.size() - 1) {
+      // point so we can restore it when erasing a point because of
-        loop->savePointshapeStrokePtArea(c, m_pts[c]);
+      // pixel-perfect. So we set the following flag to indicate this, and
+      // use it in doTransformPoint.
+      m_saveStrokeArea = (c == m_pts.size() - 1 && !m_retainedTracePolicyLast);
+      if (m_saveStrokeArea) {
+        loop->clearPointshapeStrokePtAreas();
+        loop->setLastPtIndex(c);
       }
       doPointshapeStrokePt(m_pts[c], loop);
     }
@@ -532,6 +541,18 @@ public:
       v.size()/2, &v[0],
       loop, (AlgoHLine)doPointshapeHline);
   }
+
+protected:
+  void doTransformPoint(const Stroke::Pt& pt, ToolLoop* loop) override {
+    if (m_saveStrokeArea)
+      loop->savePointshapeStrokePtArea(pt);
+
+    Intertwine::doTransformPoint(pt, loop);
+
+    if (loop->getLayer()->isTilemap()) {
+      loop->updateTempTileset(pt);
+    }
+  }
 };
 
 } // namespace tools

src/app/tools/tool_loop.h

@@ -254,9 +254,12 @@ namespace app {
       // Called when the user release the mouse on SliceInk
       virtual void onSliceRect(const gfx::Rect& bounds) = 0;
 
-      virtual void savePointshapeStrokePtArea(const int pti, const Stroke::Pt& pt) = 0;
+      // The following functions are used in pixel perfect mode
-
+      virtual void clearPointshapeStrokePtAreas() = 0;
+      virtual void setLastPtIndex(const int pti) = 0;
+      virtual void savePointshapeStrokePtArea(const Stroke::Pt& pt) = 0;
       virtual void restoreLastPts(const int pti, const tools::Stroke::Pt& pt) = 0;
+      virtual void updateTempTileset(const tools::Stroke::Pt& pt) = 0;
 
       virtual const app::TiledModeHelper& getTiledModeHelper() = 0;
     };

src/app/ui/editor/tool_loop_impl.cpp

@@ -145,6 +145,9 @@ protected:
   // Holds the areas saved by savePointshapeStrokePtArea method and restored by
   // restoreLastPts method.
   std::vector<SavedArea> m_savedAreas;
+  // When a SavedArea is restored we add its Rect to this Region, then we use
+  // this to expand the modified region when editing a tilemap manually.
+  gfx::Region m_restoredRegion;
   // Last point index.
   int m_lastPti;
 
@@ -435,10 +438,16 @@ public:
 
   void onSliceRect(const gfx::Rect& bounds) override { }
 
-  void savePointshapeStrokePtArea(const int pti, const tools::Stroke::Pt& pt) override { }
+  void clearPointshapeStrokePtAreas() override { }
+
+  void setLastPtIndex(const int pti) override { }
+
+  void savePointshapeStrokePtArea(const tools::Stroke::Pt& pt) override { }
 
   void restoreLastPts(const int pti, const tools::Stroke::Pt& pt) override { }
 
+  void updateTempTileset(const tools::Stroke::Pt& pt) override { }
+
   const app::TiledModeHelper& getTiledModeHelper() override {
     return m_tiledModeHelper;
   }
@@ -480,6 +489,9 @@ class ToolLoopImpl : public ToolLoopBase,
   std::unique_ptr<ExpandCelCanvas> m_expandCelCanvas;
   Image* m_floodfillSrcImage;
   bool m_saveLastPoint;
+  // Temporal tileset with latest changes to be used by pixel perfect only when
+  // modifying a tilemap in Manual mode.
+  std::unique_ptr<Tileset> m_tempTileset;
 
 public:
   ToolLoopImpl(Editor* editor,
@@ -500,6 +512,7 @@ public:
                                 ModifyDocument))
     , m_floodfillSrcImage(nullptr)
     , m_saveLastPoint(saveLastPoint)
+    , m_tempTileset(nullptr)
   {
     if (m_pointShape->isFloodFill()) {
       if (m_tilesMode) {
@@ -595,6 +608,11 @@ public:
     if (m_editor)
       m_editor->add_observer(this);
 #endif
+
+    if (m_layer->isTilemap() && site.tilesetMode() == TilesetMode::Manual) {
+      const Tileset* srcTileset = static_cast<LayerTilemap*>(m_layer)->tileset();
+      m_tempTileset.reset(Tileset::MakeCopyCopyingImages(srcTileset));
+    }
   }
 
   ~ToolLoopImpl() {
@@ -693,7 +711,7 @@ public:
     m_expandCelCanvas->validateDestCanvas(rgn);
   }
   void validateDstTileset(const gfx::Region& rgn) override {
-    m_expandCelCanvas->validateDestTileset(rgn);
+    m_expandCelCanvas->validateDestTileset(rgn, m_restoredRegion);
   }
   void invalidateDstImage() override {
     m_expandCelCanvas->invalidateDestCanvas();
@@ -748,53 +766,51 @@ public:
     m_internalCancel = true;
   }
 
-  // Saves the destination image's areas that will be updated by the last point
+  void clearPointshapeStrokePtAreas() override {
-  // of each stroke. The idea is to have the state of the image (only the
+    m_savedAreas.clear();
+  }
+
+  void setLastPtIndex(const int pti) override {
+    m_lastPti = pti;
+  }
+
+  // Saves the destination image's area that will be updated by the point
+  // passed. The idea is to have the state of the image (only the
   // portion modified by the stroke's point shape) before drawing the last
   // point of the stroke, then if that point has to be deleted by the
   // pixel-perfect algorithm, we can use this image to restore the image to the
   // state previous to the deletion. This method is used by
   // IntertwineAsPixelPerfect.joinStroke() method.
-  void savePointshapeStrokePtArea(const int pti, const tools::Stroke::Pt& pt) override {
+  void savePointshapeStrokePtArea(const tools::Stroke::Pt& pt) override {
-    if (m_savedAreas.size() > 0 && m_savedAreas[0].pos == pt)
+    gfx::Rect r;
-      return;
+    getPointShape()->getModifiedArea(this, pt.x, pt.y, r);
 
-    m_savedAreas.clear();
+    gfx::Region rgn(r);
-    m_lastPti = pti;
+    // By wrapping the modified area's position when tiled mode is active, the
+    // user can draw outside the canvas and still get the pixel-perfect
+    // effect.
+    m_tiledModeHelper.wrapPosition(rgn);
+    m_tiledModeHelper.collapseRegionByTiledMode(rgn);
 
-    auto saveArea = [this](const tools::Stroke::Pt& pt) {
+    for (auto a : rgn) {
-      gfx::Rect r;
+      a.offset(-m_celOrigin);
-      getPointShape()->getModifiedArea(this, pt.x, pt.y, r);
 
-      gfx::Region rgn(r);
+      if (m_tempTileset) {
-      // By wrapping the modified area's position when tiled mode is active, the
+        forEachTilePos(
-      // user can draw outside the canvas and still get the pixel-perfect
+          m_grid.tilesInCanvasRegion(gfx::Region(a)),
-      // effect.
+          [this](const doc::ImageRef existentTileImage,
-      m_tiledModeHelper.wrapPosition(rgn);
+                 const gfx::Point tilePos) {
-      m_tiledModeHelper.collapseRegionByTiledMode(rgn);
+            getDstImage()->copy(existentTileImage.get(),
-
+                                gfx::Clip(tilePos.x, tilePos.y, 0, 0,
-      for (auto a : rgn) {
+                                          existentTileImage.get()->width(),
-        a.offset(-m_celOrigin);
+                                          existentTileImage.get()->height()));
-        ImageRef i(Image::create(getDstImage()->pixelFormat(), a.w, a.h));
+          });
-        i->copy(getDstImage(), gfx::Clip(0, 0, a));
-        m_savedAreas.push_back(SavedArea{ i, pt, a});
       }
-    };
 
-    tools::Symmetry* symmetry = getSymmetry();
+      ImageRef i(Image::create(getDstImage()->pixelFormat(), a.w, a.h));
-    if (symmetry) {
+      i->copy(getDstImage(), gfx::Clip(0, 0, a));
-      // Convert the point to the sprite position so we can apply the
+      m_savedAreas.push_back(SavedArea{ i, pt, a});
-      // symmetry transformation.
-      tools::Stroke main_stroke;
-      main_stroke.addPoint(pt);
-
-      tools::Strokes strokes;
-      symmetry->generateStrokes(main_stroke, strokes, this);
-      for (const auto& stroke : strokes)
-        saveArea(stroke[0]);
     }
-    else
-      saveArea(pt);
   }
 
   // Takes the images saved by savePointshapeStrokePtArea and copies them to
@@ -806,16 +822,85 @@ public:
     if (m_savedAreas.empty() || pti != m_lastPti || m_savedAreas[0].pos != pt)
       return;
 
+    m_restoredRegion.clear();
+
     tools::Stroke::Pt pos;
     for (int i=0; i<m_savedAreas.size(); ++i) {
       getDstImage()->copy(m_savedAreas[i].img.get(),
                           gfx::Clip(m_savedAreas[i].r.origin(),
                                     m_savedAreas[i].img->bounds()));
+
+      if (m_tempTileset) {
+        auto r = m_savedAreas[i].r;
+        forEachTilePos(
+          m_grid.tilesInCanvasRegion(gfx::Region(r)),
+          [this, i, r](const doc::ImageRef existentTileImage,
+                       const gfx::Point tilePos) {
+            existentTileImage->copy(m_savedAreas[i].img.get(), gfx::Clip(r.x - tilePos.x, r.y - tilePos.y, 0, 0, r.w, r.h));
+          });
+      }
+
+      m_restoredRegion |= gfx::Region(m_savedAreas[i].r);
+    }
+  }
+
+  void updateTempTileset(const tools::Stroke::Pt& pt) override {
+    if (!m_tempTileset)
+      return;
+
+    gfx::Rect r;
+    getPointShape()->getModifiedArea(this, pt.x, pt.y, r);
+
+    auto tilesPts = m_grid.tilesInCanvasRegion(gfx::Region(r));
+    forEachTilePos(
+      tilesPts,
+      [this, r](const doc::ImageRef existentTileImage,
+                const gfx::Point tilePos) {
+        existentTileImage->copy(getDstImage(), gfx::Clip(r.x - tilePos.x, r.y - tilePos.y, r.x, r.y, r.w, r.h));
+      });
+
+    if (tilesPts.size() > 1) {
+      forEachTilePos(
+        tilesPts,
+        [this](const doc::ImageRef existentTileImage,
+               const gfx::Point tilePos) {
+          getDstImage()->copy(existentTileImage.get(), gfx::Clip(tilePos.x, tilePos.y, 0, 0,
+                                                                 existentTileImage.get()->width(),
+                                                                 existentTileImage.get()->height()));
+        });
     }
   }
 
 private:
 
+  // Loops over the points in tilesPts, and for each one calls the provided
+  // processTempTileImage callback passing to it the corresponding temp tile
+  // image and canvas position.
+  void forEachTilePos(const std::vector<gfx::Point>& tilesPts,
+                      const std::function<void(const doc::ImageRef existentTileImage,
+                                               const gfx::Point tilePos)>& processTempTileImage) {
+    auto cel = m_expandCelCanvas->getCel();
+    for (const gfx::Point& tilePt : tilesPts) {
+      // Ignore modifications outside the tilemap
+      if (!cel->image()->bounds().contains(tilePt.x, tilePt.y))
+        continue;
+
+      const doc::tile_t t = cel->image()->getPixel(tilePt.x, tilePt.y);
+      if (t == doc::notile)
+        continue;
+
+      const doc::tile_index ti = doc::tile_geti(t);
+      const doc::ImageRef existentTileImage = m_tempTileset->get(ti);
+      if (!existentTileImage) {
+        continue;
+      }
+
+      auto tilePos = m_grid.tileToCanvas(tilePt);
+
+      processTempTileImage(existentTileImage, tilePos);
+    }
+  }
+
 #ifdef ENABLE_UI
   // EditorObserver impl
   void onScrollChanged(Editor* editor) override { updateAllVisibleRegion(); }

src/app/util/cel_ops.cpp

@@ -449,7 +449,8 @@ void modify_tilemap_cel_region(
   doc::Tileset* tileset,
   const gfx::Region& region,
   const TilesetMode tilesetMode,
-  const GetTileImageFunc& getTileImage)
+  const GetTileImageFunc& getTileImage,
+  const gfx::Region& forceRegion)
 {
   OPS_TRACE("modify_tilemap_cel_region %d %d %d %d\n",
             region.bounds().x, region.bounds().y,
@@ -678,6 +679,12 @@ void modify_tilemap_cel_region(
       // Keep only the modified region for this specific modification
       tileRgn &= diffRgn;
 
+      if (!forceRegion.isEmpty()) {
+        gfx::Region fr(forceRegion);
+        fr.offset(-tileInCanvasRc.origin());
+        tileRgn |= fr;
+      }
+
       if (!tileRgn.isEmpty()) {
         if (addUndoToTileset) {
           Mod mod;

src/app/util/cel_ops.h

@@ -71,7 +71,8 @@ namespace app {
     doc::Tileset* previewTileset, // Temporary tileset that can be used for preview
     const gfx::Region& region,
     const TilesetMode tilesetMode,
-    const GetTileImageFunc& getTileImage);
+    const GetTileImageFunc& getTileImage,
+    const gfx::Region& forceRegion = gfx::Region());
 
   void clear_mask_from_cel(
     CmdSequence* cmds,

src/app/util/expand_cel_canvas.cpp

@@ -661,7 +661,7 @@ void ExpandCelCanvas::validateDestCanvas(const gfx::Region& rgn)
   m_validDstRegion.createUnion(m_validDstRegion, rgnToValidate);
 }
 
-void ExpandCelCanvas::validateDestTileset(const gfx::Region& rgn)
+void ExpandCelCanvas::validateDestTileset(const gfx::Region& rgn, const gfx::Region& forceRgn)
 {
   EXP_TRACE("ExpandCelCanvas::validateDestTileset", rgn.bounds(), m_dstTileset);
 
@@ -676,7 +676,8 @@ void ExpandCelCanvas::validateDestTileset(const gfx::Region& rgn)
       [this](const doc::ImageRef& origTile,
              const gfx::Rect& tileBoundsInCanvas) -> doc::ImageRef {
         return trimDstImage(tileBoundsInCanvas);
-      });
+      },
+      forceRgn);
   }
 }
 

src/app/util/expand_cel_canvas.h

@@ -85,7 +85,7 @@ namespace app {
 
     void validateSourceCanvas(const gfx::Region& rgn);
     void validateDestCanvas(const gfx::Region& rgn);
-    void validateDestTileset(const gfx::Region& rgn);
+    void validateDestTileset(const gfx::Region& rgn, const gfx::Region& forceRgn);
     void invalidateDestCanvas();
     void invalidateDestCanvas(const gfx::Region& rgn);
     void copyValidDestToSourceCanvas(const gfx::Region& rgn);