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

Improve incremental bspline builder #6055

Merged
merged 4 commits into from Nov 21, 2023

Conversation

Tom94
Copy link
Collaborator

@Tom94 Tom94 commented Nov 20, 2023

This PR makes the following improvements to the incremental BSpline builder:

  1. the last vertex of the BSpline now follows the mouse perfectly without any jitter. Vertices are still inserted with FD_EPSILON * 2 spacing to ensure derivatives are computed stably; only the currently last vertex is affected by this change.
  2. the control point spacing algorithm has been tweaked to prevent global dependencies along the path. This gets rid of jittering that the previous batch of "improvements" introduced. See comparison videos below.

Before this PR:
https://github.com/ppy/osu-framework/assets/4923655/69f60092-f24f-4f62-a296-143a3058fe2c

After this PR:
https://github.com/ppy/osu-framework/assets/4923655/8d8c72d0-0822-4364-8929-1c2e9598c6d2

@bdach
Copy link
Collaborator

bdach commented Nov 21, 2023

I don't really trust myself to properly gauge what is the desired UX of the drawing process at this point so I'm deferring review of this one.

@peppy peppy self-requested a review November 21, 2023 01:28
@pull-request-size pull-request-size bot added size/S and removed size/M labels Nov 21, 2023
peppy
peppy previously approved these changes Nov 21, 2023
@peppy peppy enabled auto-merge November 21, 2023 02:05
@peppy
Copy link
Sponsor Member

peppy commented Nov 21, 2023

This behaves much better than before, so let's go with it 👍 .

@peppy peppy force-pushed the improve-incremental-bspline-builder branch from 1b48007 to eaf0bf7 Compare November 21, 2023 02:23
@peppy peppy disabled auto-merge November 21, 2023 02:40
@peppy peppy merged commit 66b9058 into ppy:master Nov 21, 2023
18 of 21 checks passed
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.

None yet

3 participants