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

Interpolate parts in local space to avoid broken cursor trails #29253

Merged
merged 2 commits into from
Aug 2, 2024

Conversation

Cai1Hsu
Copy link
Contributor

@Cai1Hsu Cai1Hsu commented Aug 2, 2024

fixes #29214
The trail parts were originally interpolated in the screen space but stored in local space. With Barrel Roll on, even if we are adding trails at the same position, the new parts have different angles compared to the previous parts because the whole playfield is rotating. This leads to the incorrect interpolation direction(position - lastPosition) because the lastPosition was actually rotated back a little relatively.
before

This PR interpolates parts in the local space, so the interpolation always follows the correct direction.

2024-08-02.17-09-12.mp4

@@ -174,10 +183,10 @@ protected void AddTrail(Vector2 position)
float distance = diff.Length;
Vector2 direction = diff / distance;

float interval = partSize.X / 2.5f * IntervalMultiplier;
float stopAt = distance - (AvoidDrawingNearCursor ? interval : 0);
Vector2 interval = partSize.X / 2.5f * IntervalMultiplier * scaleRatio;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It sounds like you should be able to remove the scale from partSize, and, along with that, use Texture.DisplayWidth directly (and remove the two LayoutValues? This is the only usage of it after all.

Have you also made sure that the trail visually looks the same as in current master?

Copy link
Contributor Author

@Cai1Hsu Cai1Hsu Aug 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you also made sure that the trail visually looks the same as in current master?

yes, that's why I use extract scaleRatio from DrawInfo.MatrixInverse

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use Texture.DisplayWidth directly (and remove the two LayoutValues? This is the only usage of it after all.

do I have to use a vector? I don't know if the DisplayWidth is guaranteed equal to DisplayHeight.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's three choices:

  • new Vector2(Texture.DisplayWidth, Texture.DisplayHeight)
  • new Vector2(Texture.DisplayWidth)
  • Or go back to using float and just Texture.DisplayWidth

Personally, I would choose (3) just so that we don't inadvertently break something else with this PR, but I'm not sure why you've gone to using a vector other than that it looks more correct, which I would be willing to entertain.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, if there's a strong reason not to, then I would suggest doing (3) for a "fix" PR.

Copy link
Contributor Author

@Cai1Hsu Cai1Hsu Aug 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but I'm not sure why you've gone to using a vector other than that it looks more correct, which I would be willing to entertain.

not for special reasons, just because the extracted scale is a vector and in case some parents scale the X-axis and Y-axis differently. I'll go with (3) then

@smoogipoo smoogipoo merged commit a2ce480 into ppy:master Aug 2, 2024
13 checks passed
@hachuyenden
Copy link

Very smooth, thanks for the fix. ❤️

@Cai1Hsu Cai1Hsu deleted the local-interpolate-trail-parts branch August 2, 2024 10:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cursortrail broken (intermittent) in Barrel Roll 12 RPM mod.
3 participants