Skip to content

fix: remove dangerous unwrap#989

Open
Its-Just-Nans wants to merge 6 commits intolinebender:mainfrom
Its-Just-Nans:fix-unwrap
Open

fix: remove dangerous unwrap#989
Its-Just-Nans wants to merge 6 commits intolinebender:mainfrom
Its-Just-Nans:fix-unwrap

Conversation

@Its-Just-Nans
Copy link
Copy Markdown
Contributor

@Its-Just-Nans Its-Just-Nans commented Dec 14, 2025

Fix #937
Fix #941

This change the API of render() but is still compatible I think
(returns None instead of crashing)

Maybe we could add #[must_use] ?

let mut pixmap = tiny_skia::PixmapMut::from_bytes(pixmap, width, height).unwrap();

resvg::render(&tree.0, transform.to_tiny_skia(), &mut pixmap)
.expect("Failed to render to pixmap");
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Should we panic or fail quielty?

size.height() as f32 / tree.size().height() as f32,
);
resvg::render(&tree, render_ts, &mut pixmap.as_mut());
resvg::render(&tree, render_ts, &mut pixmap.as_mut()).expect("Failed to render");
Copy link
Copy Markdown
Contributor Author

@Its-Just-Nans Its-Just-Nans Dec 14, 2025

Choose a reason for hiding this comment

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

The best would be to resturn a Option<usize> or a Result<usize, String>

I don't really like to .expect() but I do it now for this MR for no breaking change

@Its-Just-Nans Its-Just-Nans changed the title checked operations Jan 13, 2026
@Its-Just-Nans
Copy link
Copy Markdown
Contributor Author

I understand that I made multiple MRs to improve safety but they contains breaking changes.

Another solution would be to add more public method which are safer instead of editing the current ones (I can do that).

Aka

Instead of modifying render.
Create a new method render_with_result

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

1 participant