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

Jump to bottom

vello_encoding panics while loading CIA World Map in debug build with_winit #875

Closed
xorgy opened this issue Mar 22, 2025 · 8 comments · Fixed by #877
Closed

vello_encoding panics while loading CIA World Map in debug build with_winit #875

xorgy opened this issue Mar 22, 2025 · 8 comments · Fixed by #877

Comments

@xorgy
Copy link
Member

xorgy commented Mar 22, 2025

When I run with_winit, it panics due to an arithmetic overflow in vello_encoding/src/path.rs while encoding the scene.
@waywardmonkeys
Copy link
Collaborator

Can you give a download link for that file? A stack trace and more detail on the error would be cool too.

@xorgy
Copy link
Member Author

xorgy commented Mar 23, 2025

Can you give a download link for that file?

I forgot that the downloader was removed.. here it is (github won't let me upload it because it's 37 megs, and I don't remember where it came from):

https://qui.suis.je/drop/cia-world-map.svg

@xorgy
Copy link
Member Author

xorgy commented Mar 23, 2025

and FWIW it doesn't even load properly for me in release mode, even though it doesn't panic.

@DJMcNab
Copy link
Member

DJMcNab commented Mar 24, 2025 •
edited
Loading

I think it's

vello/examples/scenes/src/download/default_downloads.rs

Lines 9 to 17 in ba855c5

SVGDownload {
builtin:Some(BuiltinSvgProps {
info: "https://commons.wikimedia.org/wiki/File:CIA_WorldFactBook-Political_world.svg",
license: "Public Domain",
expected_size: 12771150,
}),
url: "https://upload.wikimedia.org/wikipedia/commons/7/72/Political_Map_of_the_World_%28august_2013%29.svg".to_string(),
name: "CIA World Map".to_string()
},

And yes, for this kind of error more details than "it panicks" would be helpful

@xorgy xorgy closed this as completed Mar 24, 2025
@waywardmonkeys
Copy link
Collaborator

If I download your file (35M) and run cargo run -p with_winit -- cia-world-map.svg, it does crash on the let new_len = ... here:

if self.state == PathState::MoveTo {
    let new_len = self.data.len() - 8;
    self.data.truncate(new_len);
}

That's pretty curious. Probably worth leaving this open.

@DJMcNab
Copy link
Member

DJMcNab commented Mar 25, 2025

This looks like something which I missed in #817. Should be a trivial s/8/2 here

@waywardmonkeys
Copy link
Collaborator

Checking and will submit PR if it fixes it.

@waywardmonkeys
Copy link
Collaborator

Yes!

waywardmonkeys added a commit to waywardmonkeys/vello that referenced this issue Mar 25, 2025
@waywardmonkeys
Follow-up fix for encoding data as u32
efced13 
The original work was done in PR linebender#817 and as noticed in issue linebender#875,
there was still a place where there was a length manipulation that
was in terms of `u8` rather than `u32`.

Fixes linebender#875.
@waywardmonkeys waywardmonkeys mentioned this issue Mar 25, 2025
Merged
github-merge-queue bot pushed a commit that referenced this issue Mar 26, 2025
@waywardmonkeys
Follow-up fix for encoding data as u32 (#877)
97eab3c 
The original work was done in PR #817 and as noticed in issue #875,
there was still a place where there was a length manipulation that was
in terms of `u8` rather than `u32`.

Fixes #875.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Follow-up fix for encoding data as u32 waywardmonkeys/vello
3 participants
@waywardmonkeys @xorgy @DJMcNab