Skip to content

Support BGRA images in Vello Classic#1173

Merged
sagudev merged 6 commits into
linebender:mainfrom
sagudev:bgra
Oct 2, 2025
Merged

Support BGRA images in Vello Classic#1173
sagudev merged 6 commits into
linebender:mainfrom
sagudev:bgra

Conversation

@sagudev

@sagudev sagudev commented Aug 23, 2025

Copy link
Copy Markdown
Contributor

This is possible because of linebender/peniko#120.

@sagudev sagudev added the C-classic Applies to the classic `vello` crate label Sep 18, 2025
@sagudev sagudev changed the title Support Bgra in vello classic Support Bgra images in vello classic Sep 18, 2025
@sagudev sagudev changed the title Support Bgra images in vello classic Support BGRA images in vello classic Sep 18, 2025
@sagudev sagudev marked this pull request as ready for review September 18, 2025 12:53
@DJMcNab DJMcNab changed the title Support BGRA images in vello classic Support BGRA images in Vello Classic Sep 19, 2025

@DJMcNab DJMcNab left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This implementation looks reasonable to me.

We will need to bring this up at renderer office hours, but I think we can merge it optimistically before then.

However, I would like to see at least one test which uses this before landing.
The changelog update will need to be folded in with #1231.

Comment thread vello_shaders/shader/fine.wgsl Outdated
Comment thread vello_shaders/shader/fine.wgsl
Comment thread vello_shaders/shader/fine.wgsl

@DJMcNab DJMcNab left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

To clarify the state, this is still lightly blocked on a test. This only needs to be similar to:

params.resolution = Some(Vec2::new(1100., 1100.));
params.base_color = Some(palette::css::WHITE);
let mut blob: Vec<u8> = Vec::new();
[
palette::css::RED,
palette::css::BLUE,
palette::css::CYAN,
palette::css::MAGENTA,
]
.iter()
.for_each(|c| {
blob.extend(c.to_rgba8().to_u8_array());
});
let data = Blob::new(Arc::new(blob));
let image = ImageBrush::new(ImageData {
data,
format: ImageFormat::Rgba8,
width: 2,
height: 2,
alpha_type: ImageAlphaType::Alpha,
});

That is, it can manually construct the image, rather than loading from a file.

@sagudev

sagudev commented Sep 24, 2025

Copy link
Copy Markdown
Contributor Author

To clarify the state, this is still lightly blocked on a test.

I know, I am currently on vacation so progress is slow. I will rerequest review when ready.

This only needs to be similar to:

This is great example, thanks.

@DJMcNab

DJMcNab commented Sep 24, 2025

Copy link
Copy Markdown
Member

Yeah, there's no rush! I'm just noting it e.g. for office hours, so we can keep track of an accurate state

@DJMcNab DJMcNab left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you - this looks good. There are just a few details we should fix before landing this.

The concern about ImageQuality is especially blocking (plus CI, but as noted, just doing a hack for that is fine).

Comment thread vello_encoding/src/encoding.rs Outdated
Comment thread vello_shaders/shader/fine.wgsl Outdated
Comment thread vello_tests/tests/smoke_snapshots.rs Outdated
Comment thread vello_tests/tests/smoke_snapshots.rs Outdated
Comment thread vello_tests/tests/smoke_snapshots.rs Outdated
Comment thread vello_tests/tests/smoke_snapshots.rs Outdated
Signed-off-by: sagudev <16504129+sagudev@users.noreply.github.com>
Signed-off-by: sagudev <16504129+sagudev@users.noreply.github.com>
Signed-off-by: sagudev <16504129+sagudev@users.noreply.github.com>
Signed-off-by: sagudev <16504129+sagudev@users.noreply.github.com>

@DJMcNab DJMcNab left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this PR just needs the change to mask the qualities correctly after 9b37898, then we're done!

Thank you

Signed-off-by: sagudev <16504129+sagudev@users.noreply.github.com>
Signed-off-by: sagudev <16504129+sagudev@users.noreply.github.com>
@sagudev sagudev requested a review from DJMcNab October 2, 2025 10:23

@DJMcNab DJMcNab left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. Thanks!

@sagudev sagudev added this pull request to the merge queue Oct 2, 2025
Merged via the queue into linebender:main with commit 462fd4e Oct 2, 2025
17 checks passed
@sagudev sagudev deleted the bgra branch October 2, 2025 11:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-classic Applies to the classic `vello` crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants