Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rotation inaccuracy when using a center pivot #3041

Closed
IgnacioLuxP opened this issue Nov 3, 2021 · 9 comments
Closed

Rotation inaccuracy when using a center pivot #3041

IgnacioLuxP opened this issue Nov 3, 2021 · 9 comments

Comments

@IgnacioLuxP
Copy link

IgnacioLuxP commented Nov 3, 2021

When rotating a shape there's a slight inaccuracy when rotating by 90°, 180° or 270° degrees. The resulting rotated shape is slightly skewed. This only occurs if the default center pivot is selected.

Might be related to #2738

Steps to repro

  1. Create a square sprite.
  2. Select the shape, press CTRL+T to transform.
  3. Select the center pivot.
  4. Rotate holding Shift to snap rotation to 90°.
  5. Notice the shape is slightly skewed.

Observations:

  • Does not happen in the latest stable version (1.2.30).
  • Does not occur when selecting a different pivot position other than center.
  • Fast Rotation or rotSprite produce the same results in terms of the shape silhouette.
  • Rotating via Edit > Rotate > 90° CC produces the same results as rotating manually.
  • The tooltips all show the sprite is being rotated by whole integer degrees (i.e., 90° and not 91.3°).
  • Selection shape or dimensions don't seem to matter.
  • Does not occur when rotating canvas.

Aseprite and System version

  • Aseprite version: 1.3.beta-6, 1.3-beta7 dev

Attachments:

Sample sprite: spriteRotation example.zip

Videos:

Bug demo in ver 1.3beta-7 (100% repro)
rotation 1 3beta7

EDIT: With Fast Rotation: rotation fastrot 1.3beta7.gif

Bug demo in ver 1.2.30 (Cannot reproduce)
rotation 1 2 30

@Omar-Abdul-Azeez
Copy link
Contributor

Omar-Abdul-Azeez commented Nov 3, 2021

further testing reveal:
2x2 normal
4x4 normal
8x8 normal
14x14 bugged
16x16 normal (???)
18x18 normal (???)
32x32 bugged
64x64 bugged
96x96 bugged
128x128 bugged
256x256 bugged
(need further testing for numbers other than powers of 2)
The higher you go it seems to stay buggy even though it's harder to see because it's always off by one pixel

@Gasparoken
Copy link
Member

Thank you for this report! Related to https://community.aseprite.org/t/rotate-90-should-not-use-rotsprite/3267.

For 90 degrees rotation, you should always use 'Fast Rotation' instead of 'RotSprite'.
Be careful @IgnacioLuxP: both videos were done under different rotation modes, that's why you got different results.

@Gasparoken
Copy link
Member

Anyway we need to review the RotSprite algorithm and evaluate if is needed to do exceptions in some angles or if we can adjust something else. Here I found other issue related to RotSprite: #2733

@IgnacioLuxP
Copy link
Author

@Gasparoken

For 90 degrees rotation, you should always use 'Fast Rotation' instead of 'RotSprite'.
Be careful @IgnacioLuxP: both videos were done under different rotation modes, that's why you got different results.

Sorry, I mentioned it in the observations but the video only shows Rotsprite. It actually occurs in either rotation mode, as seen here: rotation fastrot 1.3beta7.gif

@behreajj
Copy link
Contributor

behreajj commented Nov 4, 2021

At first I thought the condition for a 90 deg angle wasn't being hit properly. (The conditional could still stand to be clarified, btw.) The next thought I had is to round real numbers to integers rather than truncating. From:

doc::algorithm::parallelogram(

    case tools::RotationAlgorithm::FAST: {
      const auto diff0 = corners.leftTop() - leftTop;
      const auto diff1 = corners.rightTop() - leftTop;
      const auto diff2 = corners.rightBottom() - leftTop;
      const auto diff3 = corners.leftBottom() - leftTop;

      LOG(VERBOSE, "diff0: %.6f, %.6f\n", diff0.x, diff0.y);
      LOG(VERBOSE, "diff1: %.6f, %.6f\n", diff1.x, diff1.y);
      LOG(VERBOSE, "diff2: %.6f, %.6f\n", diff2.x, diff2.y);
      LOG(VERBOSE, "diff3: %.6f, %.6f\n", diff3.x, diff3.y);

      doc::algorithm::parallelogram(
        dst, src, (mask ? mask->bitmap(): nullptr),
        std::lround(diff0.x),
        std::lround(diff0.y),
        std::lround(diff1.x),
        std::lround(diff1.y),
        std::lround(diff2.x),
        std::lround(diff2.y),
        std::lround(diff3.x),
        std::lround(diff3.y));
      break;
    }

Not all that familiar with C++ yet, so I just used the first reference that came to hand. Looks like the differences could be negative, so that's why I didn't just bias the truncation by adding 0.5.

Edit: Looks like this makes 90 and 180 less common, but skewing can still happen.

@Gasparoken
Copy link
Member

Gasparoken commented May 31, 2022

Sorry, I mentioned it in the observations but the video only shows Rotsprite. It actually occurs in either rotation mode, as seen here: rotation fastrot 1.3beta7.gif

You're righ! I'll increase the priority. 'Fast rotation' also has an issue in the beta version.

@kyrieru
Copy link

kyrieru commented Jun 1, 2022

Something I noticed when encountering this is that moving the pivot of the selection will also make it change the result, and you can see it in real-time.
https://i.gyazo.com/c7198e7c7bd09b3246d6d8576e7ed9a3.mp4

@oceanhahn
Copy link

Confirmed that this happens with v.1.3-b16-x64 as well, with the [ Edit/Rotate/180 ] command, too.
I can work around this with Flip Horizontal + Flip Vertical but I thought you ought to know that it hits this one, too.

@Gnumaru
Copy link

Gnumaru commented Mar 2, 2023

This inaccuracy while rotating in multiples of 90 degrees seems to happen when the selection is far from the canvas center. If the selection is near the center it is less likely to happen. It happens both with the fast rotation or the RotSprite algorithms.

Even though the user can achieve the same result of a 180deg rotation by both flipping horizontally and vertically, there is no way to achieve a 90deg or 270deg rotation with flipping only (it would require transposition, reinterpreting pixel rows as columns).

This was recorded on RC1.

2023-03-02_08-54-39.mkv.mp4

@dacap dacap self-assigned this Mar 2, 2023
@dacap dacap modified the milestones: v1.3, v1.3-rc2 Mar 2, 2023
@dacap dacap moved this to Todo in Aseprite v1.3.x Mar 7, 2023
@dacap dacap removed their assignment Mar 8, 2023
@martincapello martincapello self-assigned this Mar 13, 2023
@martincapello martincapello moved this from Todo to In Progress in Aseprite v1.3.x Mar 16, 2023
martincapello added a commit to martincapello/aseprite that referenced this issue Mar 16, 2023
@martincapello martincapello moved this from In Progress to Done in Aseprite v1.3.x Mar 16, 2023
@dacap dacap closed this as completed in 7b63651 Mar 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

9 participants