Skip to content

Conversation

@anvilfolk
Copy link
Contributor

@anvilfolk anvilfolk commented May 29, 2023

Currently, CurveEdit uses a couple of different ways to ensure that points and lines are drawn in the right space. This makes the code harder to understand and maintain.

This PR makes the curve be drawn in view coordinates by transforming every point from world/curve-space into view-space. This ensures there is a single way to scale from curve-space to view-space and vice-versa. By drawing in view-space, we can keep anti-aliasing, which was the main point of the previous changes! 🥳

Other minor changes, resulting in fewer lines of code overall:

  • Removed CanvasItemPlotCurve, since it was only used in a single place
  • Moved plot_curve_accurate to CurveEdit so it can make use of _world_to_view transform and related functions
  • Did a pass to make sure vectors are passed around as const Vector2 &

@MewPurPur
Copy link
Contributor

For reference, "World" is the space of the curve, i.e. x from 0 to 1, and y = f(x). "View" is the pixel coordinates of stuff on the screen, relative to the top left corner of the curve editor.

@anvilfolk anvilfolk marked this pull request as ready for review May 29, 2023 17:16
@anvilfolk anvilfolk force-pushed the curvywurvy branch 2 times, most recently from fbfa331 to f49be2b Compare May 29, 2023 18:33
@anvilfolk anvilfolk force-pushed the curvywurvy branch 2 times, most recently from 862a1d0 to 27dbf0c Compare May 29, 2023 19:25
@MewPurPur
Copy link
Contributor

Looks good to me

Currently, `CurveEdit` uses a couple of different ways to ensure that
points and lines are drawn in the right space. This makes the code
harder to understand and maintain.

This PR makes the curve be drawn in view coordinates by transforming
every point from world/curve-space into view-space. By drawing in
view-space, we can keep enable anti-aliasing and use a single way to
transform coordinates from the two different spaces.
@anvilfolk anvilfolk requested a review from a team as a code owner September 29, 2024 00:46
@anvilfolk
Copy link
Contributor Author

Rebased. This has been reviewed & tested by the person who last worked intensely on the curve editor. It is on the path towards #67857, which has decent support and I'd like to revive since I need it for myself :) Do we need reviews from someone else?

@anvilfolk
Copy link
Contributor Author

Closed as included in #67857.

@anvilfolk anvilfolk closed this Nov 10, 2024
@AThousandShips AThousandShips removed this from the 4.x milestone Nov 10, 2024
@anvilfolk anvilfolk deleted the curvywurvy branch January 4, 2025 18:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants