10000 Implement `Image::from_peniko_image` by tomcur · Pull Request #996 · linebender/vello · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Implement Image::from_peniko_image #996

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

Merged
merged 2 commits into from
May 9, 2025
Merged

Implement Image::from_peniko_image #996

merged 2 commits into < 8000 span title="linebender/vello:main" class="commit-ref css-truncate user-select-contain expandable ">main from
May 9, 2025

Conversation

tomcur
Copy link
Member
@tomcur tomcur commented May 8, 2025

Probably a temporary method until there's better resource binding; in the meantime this is useful for clients already using Peniko::Image.

Perhaps Pixmap should get a method to do the conversion to premultiplied RGBA8 from &[Rgba8], or perhaps Vec<Rgba8> using Bytemuck with the extern_crate_alloc feature (though the latter doesn't help with Peniko::Image, which stores data in an Arc).

Copy link
Contributor
@LaurenzV LaurenzV left a comment

Choose a reason for hiding this comment

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

Seems fine as an intermediate solution!

/// This panics if `image` has a `width` or `height` greater than `u16::MAX`.
pub fn from_peniko_image(image: &peniko::Image) -> Self {
if image.format != peniko::ImageFormat::Rgba8 {
unimplemented!("Unsupported image format: {:?}", image.format);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't we return an Option instead and return None in this case?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm a bit torn. I think we do ideally have a non-fallible function for this and we should try to make sure to support all the Peniko image formats (currently there's only Rgba8). Perhaps peniko::ImageFormat growing should be considered breaking, not least because there's no existing mechanism in Peniko to convert it to something known.

Copy link
Member Author

Choose a reason for hiding this comment

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

(Vello Classic also has infallible methods taking Image (through Brush), https://docs.rs/vello/0.5.0/vello/struct.Scene.html#method.fill).

Probably a temporary method until there's better resource binding; in
the meantime this is useful for clients already using `Peniko::Image`.

Perhaps `Pixmap` should get a method to do the conversion to
premultiplied RGBA8 from `&[Rgba8]`, or perhaps `Vec<Rgba8>` using
Bytemuck with the `extern_crate_alloc` feature (though the latter
doesn't help with `Peniko::Image`, which stores data in an `Arc`).
@tomcur tomcur force-pushed the image-peniko-image branch from 7f3cabc to 9172733 Compare May 9, 2025 16:40
@tomcur tomcur added this pull request to the merge queue May 9, 2025
Merged via the queue into main with commit b3e173e May 9, 2025
17 checks passed
@tomcur tomcur deleted the image-peniko-image branch May 9, 2025 17:19
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.

2 participants
0