Fix tiled mode when moving pixels by martincapello · Pull Request #5369 · aseprite/aseprite

martincapello · 2025-08-19T17:25:44Z

This PR doesn't include changes to make the fix work with tilemaps. Should this be addressed? If so, IMO it would be better to do it in a separate issue.

aseprite-bot

clang-tidy made some suggestions

aseprite-bot · 2025-08-19T17:30:00Z


  auto corners = transformation.transformedCorners();
  gfx::Rect bounds = corners.bounds(transformation.cornerThick());
+  gfx::Rect unexpandedBounds = bounds;


warning: variable 'unexpandedBounds' of type 'gfx::Rect' (aka 'RectT') can be declared 'const' [misc-const-correctness]

Suggested change

gfx::Rect unexpandedBounds = bounds;

gfx::Rect const unexpandedBounds = bounds;

aseprite-bot · 2025-08-19T17:30:00Z

+
+    // Temporal image. Used for copying several times the pixels being transformed by the user
+    //  when tile mode is active.
+    doc::ImageRef tmp;


warning: variable 'tmp' of type 'doc::ImageRef' (aka 'shared_ptr') can be declared 'const' [misc-const-correctness]

Suggested change

doc::ImageRef tmp;

doc::ImageRef const tmp;

aseprite-bot

clang-tidy made some suggestions

aseprite-bot · 2025-08-20T14:50:30Z

-    if (int(m_mode) & int(filters::TiledMode::X_AXIS))
-      rgn.offset(m_canvas->width() * (1 - (rgn.bounds().x / m_canvas->width())), 0);
+    if (int(m_mode) & int(filters::TiledMode::X_AXIS)) {
+      int w = rgn.bounds().origin().x < 0 ? (rgn.bounds().w - m_canvas->width()) : 0;


warning: variable 'w' of type 'int' can be declared 'const' [misc-const-correctness]

Suggested change

int w = rgn.bounds().origin().x < 0 ? (rgn.bounds().w - m_canvas->width()) : 0;

int const w = rgn.bounds().origin().x < 0 ? (rgn.bounds().w - m_canvas->width()) : 0;

aseprite-bot · 2025-08-20T14:50:31Z

+    }
+
+    if (int(m_mode) & int(filters::TiledMode::Y_AXIS)) {
+      int h = rgn.bounds().origin().y < 0 ? (rgn.bounds().h - m_canvas->height()) : 0;


warning: variable 'h' of type 'int' can be declared 'const' [misc-const-correctness]

Suggested change

int h = rgn.bounds().origin().y < 0 ? (rgn.bounds().h - m_canvas->height()) : 0;

int const h = rgn.bounds().origin().y < 0 ? (rgn.bounds().h - m_canvas->height()) : 0;

aseprite-bot · 2025-08-20T14:50:31Z

+  // When Y axis tiled mode is enable, the analog algorithm is used for the Y coordinates.
+  void wrapTransformation(Transformation* transformation) const
+  {
+    gfx::Rect bounds = transformation->transformedBounds();


warning: variable 'bounds' of type 'gfx::Rect' (aka 'RectT') can be declared 'const' [misc-const-correctness]

Suggested change

gfx::Rect bounds = transformation->transformedBounds();

gfx::Rect const bounds = transformation->transformedBounds();

aseprite-bot · 2025-08-20T14:50:31Z

+    doc::ImageRef tmp;
+    Mask tmpMask;
+    tmpMask.freeze();
+    int x = (hasModeFlag(TiledMode::X_AXIS) ? m_canvas->width() : 0);


warning: variable 'x' of type 'int' can be declared 'const' [misc-const-correctness]

Suggested change

int x = (hasModeFlag(TiledMode::X_AXIS) ? m_canvas->width() : 0);

int const x = (hasModeFlag(TiledMode::X_AXIS) ? m_canvas->width() : 0);

aseprite-bot · 2025-08-20T14:50:31Z

+    Mask tmpMask;
+    tmpMask.freeze();
+    int x = (hasModeFlag(TiledMode::X_AXIS) ? m_canvas->width() : 0);
+    int y = (hasModeFlag(TiledMode::Y_AXIS) ? m_canvas->height() : 0);


warning: variable 'y' of type 'int' can be declared 'const' [misc-const-correctness]

Suggested change

int y = (hasModeFlag(TiledMode::Y_AXIS) ? m_canvas->height() : 0);

int const y = (hasModeFlag(TiledMode::Y_AXIS) ? m_canvas->height() : 0);

aseprite-bot · 2025-08-20T14:50:31Z

+    tmpMask.freeze();
+    int x = (hasModeFlag(TiledMode::X_AXIS) ? m_canvas->width() : 0);
+    int y = (hasModeFlag(TiledMode::Y_AXIS) ? m_canvas->height() : 0);
+    int w = dst->width() - 2 * x;


warning: variable 'w' of type 'int' can be declared 'const' [misc-const-correctness]

Suggested change

int w = dst->width() - 2 * x;

int const w = dst->width() - 2 * x;

aseprite-bot · 2025-08-20T14:50:31Z

+    int x = (hasModeFlag(TiledMode::X_AXIS) ? m_canvas->width() : 0);
+    int y = (hasModeFlag(TiledMode::Y_AXIS) ? m_canvas->height() : 0);
+    int w = dst->width() - 2 * x;
+    int h = dst->height() - 2 * y;


warning: variable 'h' of type 'int' can be declared 'const' [misc-const-correctness]

Suggested change

int h = dst->height() - 2 * y;

int const h = dst->height() - 2 * y;

Gasparoken · 2025-08-27T15:58:58Z

Works great! A minor observation: Perhaps it would be better to use the new hasModeFlag() function in more places instead the expression ((int(m_mode) & int(filters::TiledMode::SOME_MODE))).

Throughout testing, I discovered that the pivot point isn't positioned correctly in tiled mode, but that's a different issue (#2316); I'll look into a fix for this in another PR.

dacap · 2025-10-15T15:03:23Z

It works great 👍 it would be nice to move the whole TiledModeHelper impl in its own .cpp file. Adjusting the required #includes (e.g. #include "doc/algorithm/rotate.h" can be moved to the .cpp)

aseprite-bot

clang-tidy made some suggestions

aseprite-bot · 2025-10-17T18:03:08Z

+                                              const render::Projection* proj) const
+{
+  gfx::Region tile = rgn;
+  const bool xTiled = (int(m_mode) & int(filters::TiledMode::X_AXIS));


warning: implicit conversion 'int' -> 'bool' [readability-implicit-bool-conversion]

Suggested change

const bool xTiled = (int(m_mode) & int(filters::TiledMode::X_AXIS));

const bool xTiled = (int(m_mode) & int(filters::TiledMode::X_AXIS)) != 0;

aseprite-bot · 2025-10-17T18:03:08Z

+{
+  gfx::Region tile = rgn;
+  const bool xTiled = (int(m_mode) & int(filters::TiledMode::X_AXIS));
+  const bool yTiled = (int(m_mode) & int(filters::TiledMode::Y_AXIS));


warning: implicit conversion 'int' -> 'bool' [readability-implicit-bool-conversion]

Suggested change

const bool yTiled = (int(m_mode) & int(filters::TiledMode::Y_AXIS));

const bool yTiled = (int(m_mode) & int(filters::TiledMode::Y_AXIS)) != 0;

aseprite-bot · 2025-10-17T18:03:09Z

+    return;
+
+  if (int(m_mode) & int(filters::TiledMode::X_AXIS)) {
+    int w = rgn.bounds().origin().x < 0 ? (rgn.bounds().w - m_canvas->width()) : 0;


warning: variable 'w' of type 'int' can be declared 'const' [misc-const-correctness]

Suggested change

int w = rgn.bounds().origin().x < 0 ? (rgn.bounds().w - m_canvas->width()) : 0;

int const w = rgn.bounds().origin().x < 0 ? (rgn.bounds().w - m_canvas->width()) : 0;

aseprite-bot · 2025-10-17T18:03:09Z

+  }
+
+  if (int(m_mode) & int(filters::TiledMode::Y_AXIS)) {
+    int h = rgn.bounds().origin().y < 0 ? (rgn.bounds().h - m_canvas->height()) : 0;


warning: variable 'h' of type 'int' can be declared 'const' [misc-const-correctness]

Suggested change

int h = rgn.bounds().origin().y < 0 ? (rgn.bounds().h - m_canvas->height()) : 0;

int const h = rgn.bounds().origin().y < 0 ? (rgn.bounds().h - m_canvas->height()) : 0;

aseprite-bot · 2025-10-17T18:03:09Z

+// including its pivot.
+void TiledModeHelper::wrapTransformation(Transformation* transformation) const
+{
+  gfx::Rect bounds = transformation->transformedBounds();


warning: variable 'bounds' of type 'gfx::Rect' (aka 'RectT') can be declared 'const' [misc-const-correctness]

Suggested change

gfx::Rect bounds = transformation->transformedBounds();

gfx::Rect const bounds = transformation->transformedBounds();

aseprite-bot · 2025-10-17T18:03:09Z

+  doc::ImageRef tmp;
+  Mask tmpMask;
+  tmpMask.freeze();
+  int x = (hasModeFlag(TiledMode::X_AXIS) ? m_canvas->width() : 0);


warning: variable 'x' of type 'int' can be declared 'const' [misc-const-correctness]

Suggested change

int x = (hasModeFlag(TiledMode::X_AXIS) ? m_canvas->width() : 0);

int const x = (hasModeFlag(TiledMode::X_AXIS) ? m_canvas->width() : 0);

aseprite-bot · 2025-10-17T18:03:09Z

+  Mask tmpMask;
+  tmpMask.freeze();
+  int x = (hasModeFlag(TiledMode::X_AXIS) ? m_canvas->width() : 0);
+  int y = (hasModeFlag(TiledMode::Y_AXIS) ? m_canvas->height() : 0);


warning: variable 'y' of type 'int' can be declared 'const' [misc-const-correctness]

Suggested change

int y = (hasModeFlag(TiledMode::Y_AXIS) ? m_canvas->height() : 0);

int const y = (hasModeFlag(TiledMode::Y_AXIS) ? m_canvas->height() : 0);

aseprite-bot · 2025-10-17T18:03:09Z

+  tmpMask.freeze();
+  int x = (hasModeFlag(TiledMode::X_AXIS) ? m_canvas->width() : 0);
+  int y = (hasModeFlag(TiledMode::Y_AXIS) ? m_canvas->height() : 0);
+  int w = dst->width() - 2 * x;


warning: variable 'w' of type 'int' can be declared 'const' [misc-const-correctness]

Suggested change

int w = dst->width() - 2 * x;

int const w = dst->width() - 2 * x;

aseprite-bot · 2025-10-17T18:03:09Z

+  int x = (hasModeFlag(TiledMode::X_AXIS) ? m_canvas->width() : 0);
+  int y = (hasModeFlag(TiledMode::Y_AXIS) ? m_canvas->height() : 0);
+  int w = dst->width() - 2 * x;
+  int h = dst->height() - 2 * y;


warning: variable 'h' of type 'int' can be declared 'const' [misc-const-correctness]

Suggested change

int h = dst->height() - 2 * y;

int const h = dst->height() - 2 * y;

dacap · 2025-11-19T21:30:16Z

LGTM! 👍

aseprite-bot reviewed Aug 19, 2025

View reviewed changes

aseprite-bot reviewed Aug 20, 2025

View reviewed changes

martincapello marked this pull request as ready for review August 20, 2025 21:09

martincapello requested a review from dacap as a code owner August 20, 2025 21:09

martincapello assigned Gasparoken Aug 20, 2025

martincapello requested a review from Gasparoken August 20, 2025 21:14

Gasparoken assigned martincapello and unassigned Gasparoken Aug 27, 2025

Gasparoken approved these changes Aug 27, 2025

View reviewed changes

martincapello assigned dacap and unassigned martincapello Aug 27, 2025

dacap linked an issue Oct 15, 2025 that may be closed by this pull request

Ability to "loop" when moving selection into tiled area #1457

Closed

dacap assigned martincapello and unassigned dacap Oct 15, 2025

martincapello added 6 commits October 17, 2025 10:49

Fix tiled mode when moving pixels (fix aseprite#1457)

f38df60

Make wrapPosition function consider region size

8495911

Remove unnecessary region offset

15036b6

Refactor, comment, and improve performance of code

d11f384

Fix logic to detect if tiled mode is enabled

74d2755

Fix wrapPosition behavior

ea70862

martincapello force-pushed the loop-selection-into-tiled-area branch from 9df632b to ea70862 Compare October 17, 2025 13:50

Move TiledModeHelper impl from header to cpp file

c2232d8

martincapello assigned dacap and unassigned martincapello Oct 17, 2025

aseprite-bot reviewed Oct 17, 2025

View reviewed changes

dacap merged commit a0749d4 into aseprite:beta Nov 19, 2025
12 checks passed

	gfx::Rect unexpandedBounds = bounds;
	gfx::Rect const unexpandedBounds = bounds;

	int w = rgn.bounds().origin().x < 0 ? (rgn.bounds().w - m_canvas->width()) : 0;
	int const w = rgn.bounds().origin().x < 0 ? (rgn.bounds().w - m_canvas->width()) : 0;

	int h = rgn.bounds().origin().y < 0 ? (rgn.bounds().h - m_canvas->height()) : 0;
	int const h = rgn.bounds().origin().y < 0 ? (rgn.bounds().h - m_canvas->height()) : 0;

	gfx::Rect bounds = transformation->transformedBounds();
	gfx::Rect const bounds = transformation->transformedBounds();

	int x = (hasModeFlag(TiledMode::X_AXIS) ? m_canvas->width() : 0);
	int const x = (hasModeFlag(TiledMode::X_AXIS) ? m_canvas->width() : 0);

	int y = (hasModeFlag(TiledMode::Y_AXIS) ? m_canvas->height() : 0);
	int const y = (hasModeFlag(TiledMode::Y_AXIS) ? m_canvas->height() : 0);

	int w = dst->width() - 2 * x;
	int const w = dst->width() - 2 * x;

	int h = dst->height() - 2 * y;
	int const h = dst->height() - 2 * y;

	const bool xTiled = (int(m_mode) & int(filters::TiledMode::X_AXIS));
	const bool xTiled = (int(m_mode) & int(filters::TiledMode::X_AXIS)) != 0;

	const bool yTiled = (int(m_mode) & int(filters::TiledMode::Y_AXIS));
	const bool yTiled = (int(m_mode) & int(filters::TiledMode::Y_AXIS)) != 0;

Uh oh!

Uh oh!

Conversation

martincapello commented Aug 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aseprite-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aseprite-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Gasparoken commented Aug 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dacap commented Oct 15, 2025

Uh oh!

aseprite-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dacap commented Nov 19, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

martincapello commented Aug 19, 2025 •

edited

Loading

Gasparoken commented Aug 27, 2025 •

edited

Loading