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.

Sign up for GitHub

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
Merged

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

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
06af8cb
interpolate parts in local space to avoid broken trails
Cai1Hsu Aug 2, 2024
4b5c163
remove unnecessary LayoutValue
Cai1Hsu Aug 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 14 additions & 5 deletions osu.Game.Rulesets.Osu/UI/Cursor/CursorTrail.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ public CursorTrail()
}

AddLayout(partSizeCache);
AddLayout(scaleRatioCache);
}

[BackgroundDependencyLoader]
Expand Down Expand Up @@ -154,8 +155,16 @@ protected override bool OnMouseMove(MouseMoveEvent e)
return base.OnMouseMove(e);
}

private readonly LayoutValue<Vector2> scaleRatioCache = new LayoutValue<Vector2>(Invalidation.DrawInfo | Invalidation.RequiredParentSizeToFit | Invalidation.Presence);

private Vector2 scaleRatio => scaleRatioCache.IsValid
? scaleRatioCache.Value
: (scaleRatioCache.Value = DrawInfo.MatrixInverse.ExtractScale().Xy);

protected void AddTrail(Vector2 position)
{
position = ToLocalSpace(position);

if (InterpolateMovements)
{
if (!lastPosition.HasValue)
Expand All @@ -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 •
edited
Loading

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 •
edited
Loading

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

float stopAt = distance - (AvoidDrawingNearCursor ? interval.Length : 0);

for (float d = interval; d < stopAt; d += interval)
for (Vector2 d = interval; d.Length < stopAt; d += interval)
{
lastPosition = pos1 + direction * d;
addPart(lastPosition.Value);
Expand All @@ -191,9 +200,9 @@ protected void AddTrail(Vector2 position)
}
}

private void addPart(Vector2 screenSpacePosition)
private void addPart(Vector2 localSpacePosition)
{
parts[currentIndex].Position = ToLocalSpace(screenSpacePosition);
parts[currentIndex].Position = localSpacePosition;
parts[currentIndex].Time = time + 1;
++parts[currentIndex].InvalidationID;

Expand Down
Loading