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

New test scene: hugepath #542

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

New test scene: hugepath #542

wants to merge 2 commits into from

Conversation

armansito
Copy link
Collaborator

This test draws lines with very large floating point y-coordinates (spanning 12345678901234567890.0 and -9.8765432109876543210e+19 respectively). This causes an overflow in bump estimation and results in rendering issues in Vello.

Bump Estimation

This test causes a panic in debug builds with the bump_estimate feature enabled:

thread 'main' panicked at /Users/armansito/work/vello/crates/encoding/src/estimate.rs:157:26:
attempt to multiply with overflow

This happens because the line generates an exorbitant amount of tile crossings which causes the segment count to overflow. This can be avoided by taking the viewport size and culling into account during estimation since a majority of the shape lies outside the viewport. See #541.

Rendering Issues

  1. The frame rate (on my M1 Pro) doesn't go higher than 6 fps for this scene.
  2. The expected rendering result is similar to the following (barring colors):

Screenshot 2024-04-02 at 10 36 57 PM

With vello, the top 3 strokes (winding from positive to negative) are missing under the default transform:

Screenshot 2024-04-02 at 10 41 09 PM

Under rotation, (particularly >90°) causes artifacts that look like this:

Screenshot 2024-04-02 at 10 45 46 PM

@DJMcNab
Copy link
Member

DJMcNab commented Apr 3, 2024

I think I see the opposite problem (?) when pressing the C key in with_winit, where the strokes for the frame times are scaled to 1/u64::MAX, which seems to lead to terrible performance (~200ms frame times)

Copy link
Member

@DJMcNab DJMcNab left a comment

Choose a reason for hiding this comment

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

Thanks, this looks like it brings in a real case where we have issues

I don't know what the path towards fixing this would be

@armansito
Copy link
Collaborator Author

Thanks, this looks like it brings in a real case where we have issues

I don't know what the path towards fixing this would be

There are some ideas, all mainly point towards implementing some type of viewport culling. For this particular test, we probably want to clamp the coordinates down to a reasonable range such that the ULP is sufficiently small for math.

This test draws lines with very large floating point y-coordinates.
This causes an overflow in bump estimation and rendering issues in
Vello.
@DJMcNab
Copy link
Member

DJMcNab commented Apr 10, 2024

The only blocker is the clippy failure now

auto-merge was automatically disabled April 22, 2024 09:06

Merge queue setting changed

@waywardmonkeys waywardmonkeys added this to the Vello 0.2 release milestone May 3, 2024
@waywardmonkeys
Copy link
Contributor

@armansito Is this something that we want to land? Trying to clear up some PRs before doing a Vello 0.2 release...

@DJMcNab
Copy link
Member

DJMcNab commented May 13, 2024

This only impacts the examples, so is entirely orthogonal to the release imo

But we should get this out of limbo.

@DJMcNab DJMcNab removed this from the Vello 0.2 release milestone May 16, 2024
@DJMcNab DJMcNab mentioned this pull request Jun 10, 2024
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants