From b980e386fa9a95c3887b847da22febfa1d600ab3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mart=C3=ADn=20Capello?= Date: Thu, 10 Jun 2021 17:05:23 -0300 Subject: [PATCH] Fix pixel-perfect in manual tile mode (fix #2741) --- src/app/tools/intertwine.cpp | 15 ++- src/app/tools/intertwine.h | 3 +- src/app/tools/intertwiners.h | 29 ++++- src/app/tools/tool_loop.h | 7 +- src/app/ui/editor/tool_loop_impl.cpp | 161 ++++++++++++++++++++------- src/app/util/cel_ops.cpp | 9 +- src/app/util/cel_ops.h | 3 +- src/app/util/expand_cel_canvas.cpp | 5 +- src/app/util/expand_cel_canvas.h | 2 +- 9 files changed, 179 insertions(+), 55 deletions(-) diff --git a/src/app/tools/intertwine.cpp b/src/app/tools/intertwine.cpp index e4d9f6042..4b0a5951e 100644 --- a/src/app/tools/intertwine.cpp +++ b/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 @@ -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 diff --git a/src/app/tools/intertwine.h b/src/app/tools/intertwine.h index 99e47113f..03d34b0e1 100644 --- a/src/app/tools/intertwine.h +++ b/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); diff --git a/src/app/tools/intertwiners.h b/src/app/tools/intertwiners.h index c71fdd07f..72da75b09 100644 --- a/src/app/tools/intertwiners.h +++ b/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. - if (c == m_pts.size() - 1) { - loop->savePointshapeStrokePtArea(c, m_pts[c]); + + // For the last point we need to store the source image content at that + // point so we can restore it when erasing a point because of + // 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 diff --git a/src/app/tools/tool_loop.h b/src/app/tools/tool_loop.h index 95612ff00..6658c8d37 100644 --- a/src/app/tools/tool_loop.h +++ b/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; }; diff --git a/src/app/ui/editor/tool_loop_impl.cpp b/src/app/ui/editor/tool_loop_impl.cpp index 6a794c3ee..9b669aa38 100644 --- a/src/app/ui/editor/tool_loop_impl.cpp +++ b/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 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 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 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(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 - // of each stroke. The idea is to have the state of the image (only the + void clearPointshapeStrokePtAreas() override { + 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 { - if (m_savedAreas.size() > 0 && m_savedAreas[0].pos == pt) - return; + void savePointshapeStrokePtArea(const tools::Stroke::Pt& pt) override { + gfx::Rect r; + getPointShape()->getModifiedArea(this, pt.x, pt.y, r); - m_savedAreas.clear(); - m_lastPti = pti; + gfx::Region rgn(r); + // 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) { - gfx::Rect r; - getPointShape()->getModifiedArea(this, pt.x, pt.y, r); + for (auto a : rgn) { + a.offset(-m_celOrigin); - gfx::Region rgn(r); - // 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); - - for (auto a : rgn) { - a.offset(-m_celOrigin); - 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}); + if (m_tempTileset) { + forEachTilePos( + m_grid.tilesInCanvasRegion(gfx::Region(a)), + [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())); + }); } - }; - tools::Symmetry* symmetry = getSymmetry(); - if (symmetry) { - // Convert the point to the sprite position so we can apply the - // 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]); + 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}); } - 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; icopy(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& tilesPts, + const std::function& 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(); } diff --git a/src/app/util/cel_ops.cpp b/src/app/util/cel_ops.cpp index 28200838e..f63de62cc 100644 --- a/src/app/util/cel_ops.cpp +++ b/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; diff --git a/src/app/util/cel_ops.h b/src/app/util/cel_ops.h index acb6b9221..a2374415e 100644 --- a/src/app/util/cel_ops.h +++ b/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, diff --git a/src/app/util/expand_cel_canvas.cpp b/src/app/util/expand_cel_canvas.cpp index 6fb08177e..64695ab40 100644 --- a/src/app/util/expand_cel_canvas.cpp +++ b/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); } } diff --git a/src/app/util/expand_cel_canvas.h b/src/app/util/expand_cel_canvas.h index b3b6109ec..b18363b2e 100644 --- a/src/app/util/expand_cel_canvas.h +++ b/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);