Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions node-graph/libraries/no-std-types/src/color/color_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -360,8 +360,8 @@ impl Pixel for Color {

fn from_bytes(bytes: &[u8]) -> Self {
// `Image<Color>` pixel convention is linear-light with associated (premultiplied) alpha.
let srgba = SRGBA8::new(bytes[0], bytes[1], bytes[2], bytes[3]);
Color::from(srgba).apply_opacity(bytes[3] as f32 / 255.)
let color = Color::from(SRGBA8::new(bytes[0], bytes[1], bytes[2], bytes[3]));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P3: This change duplicates the SRGBA8-to-premultiplied-Color conversion already used by image-data decoding and GPU readback. Extract the conversion into a shared helper and call it from all paths so future color or alpha fixes cannot leave these implementations inconsistent.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At node-graph/libraries/no-std-types/src/color/color_types.rs, line 363:

<comment>This change duplicates the SRGBA8-to-premultiplied-`Color` conversion already used by image-data decoding and GPU readback. Extract the conversion into a shared helper and call it from all paths so future color or alpha fixes cannot leave these implementations inconsistent.</comment>

<file context>
@@ -360,8 +360,8 @@ impl Pixel for Color {
 		// `Image<Color>` pixel convention is linear-light with associated (premultiplied) alpha.
-		let srgba = SRGBA8::new(bytes[0], bytes[1], bytes[2], bytes[3]);
-		Color::from(srgba).apply_opacity(bytes[3] as f32 / 255.)
+		let color = Color::from(SRGBA8::new(bytes[0], bytes[1], bytes[2], bytes[3]));
+		color.map_rgb(|channel| channel * color.a())
 	}
</file context>

color.map_rgb(|channel| channel * color.a())
}
fn byte_size() -> usize {
4
Expand Down
4 changes: 2 additions & 2 deletions node-graph/libraries/raster-types/src/image.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,8 +148,8 @@ impl Image<Color> {
.chunks_exact(4)
.map(|v| {
// `Image<Color>` pixels are stored linear-light with premultiplied alpha
let srgba = SRGBA8::new(v[0], v[1], v[2], v[3]);
Color::from(srgba).apply_opacity(v[3] as f32 / 255.)
let color = Color::from(SRGBA8::new(v[0], v[1], v[2], v[3]));
color.map_rgb(|channel| channel * color.a())

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P3: This premultiplication fix changes stored alpha semantics (no longer alpha-squared) and is the kind of subtle color-space regression that can silently reappear. Add a round-trip test that runs from_image_data on a known semi-transparent RGBA pixel and asserts the stored Color has alpha == v[3]/255 with premultiplied linear RGB (e.g. white at alpha 128 → rgb ≈ 0.502, alpha ≈ 0.502), and that to_png()/to_flat_u8 recovers the original u8 values.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At node-graph/libraries/raster-types/src/image.rs, line 152:

<comment>This premultiplication fix changes stored alpha semantics (no longer alpha-squared) and is the kind of subtle color-space regression that can silently reappear. Add a round-trip test that runs from_image_data on a known semi-transparent RGBA pixel and asserts the stored Color has alpha == v[3]/255 with premultiplied linear RGB (e.g. white at alpha 128 → rgb ≈ 0.502, alpha ≈ 0.502), and that to_png()/to_flat_u8 recovers the original u8 values.</comment>

<file context>
@@ -148,8 +148,8 @@ impl Image<Color> {
-				let srgba = SRGBA8::new(v[0], v[1], v[2], v[3]);
-				Color::from(srgba).apply_opacity(v[3] as f32 / 255.)
+				let color = Color::from(SRGBA8::new(v[0], v[1], v[2], v[3]));
+				color.map_rgb(|channel| channel * color.a())
 			})
 			.collect();
</file context>

})
.collect();
Image {
Expand Down
10 changes: 5 additions & 5 deletions node-graph/libraries/wgpu-executor/src/texture_conversion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,14 @@ use raster_types::Image;
use raster_types::{CPU, GPU, Raster, Texture};
use wgpu::{Extent3d, TextureFormat};

/// Uploads CPU image data to a GPU texture
/// Uploads CPU image data to a GPU texture as gamma sRGB with unassociated alpha, the convention for all GPU raster textures.
fn upload_to_texture(executor: &WgpuExecutor, queue: &wgpu::Queue, image: &Raster<CPU>) -> Texture {
let rgba8_data: Vec<SRGBA8> = image.data.iter().map(|x| (*x).into()).collect();
let rgba8_data = image.to_flat_u8().0;

let texture = executor.request_texture_with_format(glam::UVec2::new(image.width, image.height), TextureFormat::Rgba8UnormSrgb);
queue.write_texture(
texture.as_image_copy(),
bytemuck::cast_slice(rgba8_data.as_slice()),
&rgba8_data,
wgpu::TexelCopyBufferLayout {
offset: 0,
bytes_per_row: Some(4 * image.width),
Expand Down Expand Up @@ -116,8 +116,8 @@ impl RasterGpuToRasterCpuConverter {
let row_slice = &view[start..start + row_bytes];
for px in row_slice.chunks_exact(4) {
// `Image<Color>` pixels are stored linear-light with associated (premultiplied) alpha
let srgba = SRGBA8::new(px[0], px[1], px[2], px[3]);
cpu_data.push(Color::from(srgba).apply_opacity(px[3] as f32 / 255.));
let color = Color::from(SRGBA8::new(px[0], px[1], px[2], px[3]));
cpu_data.push(color.map_rgb(|channel| channel * color.a()));
}
}

Expand Down
8 changes: 6 additions & 2 deletions node-graph/node-macro/src/shader_nodes/per_pixel_adjust.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,11 @@ impl PerPixelAdjustCodegen<'_> {
.iter()
.map(|Param { ident, param_type, item_wrapped, .. }| {
let bare_value = match param_type {
ParamType::Image { .. } => quote!(Color::from_vec4(#ident.fetch_with(texel_coord, lod(0)))),
// Textures hold unassociated alpha but node functions take premultiplied `Color`
ParamType::Image { .. } => quote!({
let texel = Color::from_vec4(#ident.fetch_with(texel_coord, lod(0)));
texel.map_rgb(|channel| channel * texel.a())
}),
ParamType::Uniform => quote!(uniform.#ident),
};
if *item_wrapped { quote!(Item::new_from_element(#bare_value)) } else { bare_value }
Expand Down Expand Up @@ -184,7 +188,7 @@ impl PerPixelAdjustCodegen<'_> {
let uniform = <Uniform as #gcore_shaders::shaders::buffer_struct::BufferStruct>::read(*uniform);
let texel_coord = frag_coord.xy().as_uvec2();
let color: Color = #fn_name(#context, #(#call_args),*)#unwrap_result;
*color_out = color.to_vec4();
*color_out = color.to_unassociated_alpha().to_vec4();
}
}
})
Expand Down
6 changes: 3 additions & 3 deletions node-graph/nodes/gstd/src/platform_application_io.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,9 +174,9 @@ fn decode_image(_: impl Ctx, data: Item<Resource>) -> Item<Raster<CPU>> {
data: image
.chunks(4)
.map(|pixel| {
// Decoded bytes are unassociated gamma sRGB; premultiply in gamma then lift to linear
let a = pixel[3];
Color::from_gamma_srgb_channels(pixel[0] * a, pixel[1] * a, pixel[2] * a, a)
// Decoded bytes are unassociated gamma sRGB, so lift to linear before premultiplying
let color = Color::from_gamma_srgb_channels(pixel[0], pixel[1], pixel[2], pixel[3]);
color.map_rgb(|channel| channel * color.a())
})
.collect(),
width: image.width(),
Expand Down
4 changes: 2 additions & 2 deletions node-graph/nodes/raster/src/std_nodes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -260,8 +260,8 @@ pub fn image<'a: 'n>(_: impl Ctx, resource: Item<Resource>) -> Item<Raster<CPU>>
data: image
.chunks(4)
.map(|pixel| {
let alpha = pixel[3];
Color::from_gamma_srgb_channels(pixel[0] * alpha, pixel[1] * alpha, pixel[2] * alpha, alpha)
let color = Color::from_gamma_srgb_channels(pixel[0], pixel[1], pixel[2], pixel[3]);
color.map_rgb(|channel| channel * color.a())
})
.collect(),
width: image.width(),
Expand Down
Loading