Skip to content

vello_hybrid: Avoid unnecessarily allocating Uint32Arrays #1498

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

Merged
laurenz-canva merged 2 commits into from
Mar 18, 2026
Merged

vello_hybrid: Avoid unnecessarily allocating Uint32Arrays #1498

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
33f83ff
vello_hybrid: Avoid unnecessarily allocating `Uint32Array`s
laurenz-canva Mar 10, 2026
aa190ba
Encapsulate
laurenz-canva Mar 12, 2026
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
86 changes: 53 additions & 33 deletions sparse_strips/vello_hybrid/src/render/webgl.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1072,22 +1072,12 @@ impl WebGlPrograms {
Some(&self.resources.alphas_texture),
);

// Pack alpha values into RGBA uint32 texture
let alpha_data_as_u32 = bytemuck::cast_slice::<u8, u32>(alphas);
let packed_array = js_sys::Uint32Array::from(alpha_data_as_u32);

gl.tex_image_2d_with_i32_and_i32_and_i32_and_format_and_type_and_opt_array_buffer_view(
WebGl2RenderingContext::TEXTURE_2D,
0,
WebGl2RenderingContext::RGBA32UI as i32,
alpha_texture_width as i32,
alpha_texture_height as i32,
0,
WebGl2RenderingContext::RGBA_INTEGER,
WebGl2RenderingContext::UNSIGNED_INT,
Some(&packed_array),
)
.unwrap();
upload_data_to_rgba32_texture(
gl,
bytemuck::cast_slice::<u8, u32>(alphas),
alpha_texture_width,
alpha_texture_height,
);

// Truncate back to the original size.
alphas.truncate(original_len);
Expand All @@ -1111,23 +1101,12 @@ impl WebGlPrograms {
Some(&self.resources.encoded_paints_texture),
);

// Pack encoded paints into RGBA uint32 texture
let encoded_paints_data_as_u32 =
bytemuck::cast_slice::<u8, u32>(&self.encoded_paints_data);
let packed_array = js_sys::Uint32Array::from(encoded_paints_data_as_u32);

gl.tex_image_2d_with_i32_and_i32_and_i32_and_format_and_type_and_opt_array_buffer_view(
WebGl2RenderingContext::TEXTURE_2D,
0,
WebGl2RenderingContext::RGBA32UI as i32,
encoded_paints_texture_width as i32,
encoded_paints_texture_height as i32,
0,
WebGl2RenderingContext::RGBA_INTEGER,
WebGl2RenderingContext::UNSIGNED_INT,
Some(&packed_array),
)
.unwrap();
upload_data_to_rgba32_texture(
gl,
bytemuck::cast_slice::<u8, u32>(&self.encoded_paints_data),
encoded_paints_texture_width,
encoded_paints_texture_height,
);
}
}

Expand Down Expand Up @@ -2235,3 +2214,44 @@ fn copy_to_texture_array_layer(
// Clean up
gl.delete_framebuffer(Some(&read_framebuffer));
}

// Upload the data to the currently bound texture assuming a RGBA32UI format.
fn upload_data_to_rgba32_texture(
gl: &WebGl2RenderingContext,
data: &[u32],
texture_width: u32,
texture_height: u32,
) {
// Safety: This calling `Uint32Array::view` is unsafe because it provides a view into
// WASM linear memory, and any additional allocations might invalidate that view.
// In our case, this is not an issue because we only use this view once for uploading
// data to the GPU below, and no allocations happen between that.
// The `tex_image_2d` method is synchronous in the sense that once it returns, it is guaranteed
// that all necessary data has already been read, so any allocations that happen
// after this block don't affect this anymore.
//
// See also: https://wikis.khronos.org/opengl/Synchronization
// >> There are several OpenGL functions that can pull data directly from client-side memory,
// >> or push data directly into client-side memory. Functions like `glTexSubImage2D`,
// >> `glReadPixels`, `glBufferSubData` and so forth.
//
// >> Because OpenGL is defined to be synchronous, when any of these functions have
// >> returned, they must have finished with the client memory. When `glReadPixels` returns,
// >> the pixel data is in your client memory (unless you are reading into a buffer object).
// >> When `glBufferSubData` returns, you can immediately modify or delete whatever memory
// >> pointer you gave it, as OpenGL has already read as much as it wants.
let packed_array = unsafe { js_sys::Uint32Array::view(data) };

gl.tex_image_2d_with_i32_and_i32_and_i32_and_format_and_type_and_opt_array_buffer_view(
WebGl2RenderingContext::TEXTURE_2D,
0,
WebGl2RenderingContext::RGBA32UI as i32,
texture_width as i32,
texture_height as i32,
0,
WebGl2RenderingContext::RGBA_INTEGER,
WebGl2RenderingContext::UNSIGNED_INT,
Some(&packed_array),
)
.unwrap();
}
Comment on lines +2219 to +2257

@grebmeg grebmeg Mar 16, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

One thought: I think maybe you can avoid the unsafe entirely by using the _opt_u8_array overload instead. As I understand it when web-sys receives a &[u8] parameter, wasm-bindgen passes it as a zero-copy Uint8Array view into wasm linear memory, same perf as the manual Uint32Array::view(). WebGL interprets the raw bytes according to the type parameter (UNSIGNED_INT), not the JS typed array type, so a Uint8Array view works correctly for RGBA32UI textures.

This would simplify the helper to:

fn upload_data_to_rgba32_texture(
    gl: &WebGl2RenderingContext,
    data: &[u8],
    texture_width: u32,
    texture_height: u32,
) {
    gl.tex_image_2d_with_i32_and_i32_and_i32_and_format_and_type_and_opt_u8_array(
        WebGl2RenderingContext::TEXTURE_2D,
        0,
        WebGl2RenderingContext::RGBA32UI as i32,
        texture_width as i32,
        texture_height as i32,
        0,
        WebGl2RenderingContext::RGBA_INTEGER,
        WebGl2RenderingContext::UNSIGNED_INT,
        Some(data),
    )
    .unwrap();
}

@laurenz-canva laurenz-canva Mar 16, 2026

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.

Oh! Lemme try this out!

@LaurenzV LaurenzV Mar 16, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Unfortunately, that doesn't seem to work correctly due to the fact that we are using RGBA32 textures.

@grebmeg grebmeg Mar 16, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Got it, thanks for checking! I think the next option might work, but I’m not sure that making two calls is better than the current approach.

fn upload_data_to_rgba32_texture(
    gl: &WebGl2RenderingContext,
    data: &[u8],
    texture_width: u32,
    texture_height: u32,
    pixel_unpack_buffer: &WebGlBuffer,
) {
    gl.bind_buffer(
        WebGl2RenderingContext::PIXEL_UNPACK_BUFFER,
        Some(pixel_unpack_buffer),
    );
    gl.buffer_data_with_u8_array(
        WebGl2RenderingContext::PIXEL_UNPACK_BUFFER,
        data,
        WebGl2RenderingContext::STREAM_DRAW,
    );

    gl.tex_image_2d_with_i32_and_i32_and_i32_and_format_and_type_and_f64(
        WebGl2RenderingContext::TEXTURE_2D,
        0,
        WebGl2RenderingContext::RGBA32UI as i32,
        texture_width as i32,
        texture_height as i32,
        0,
        WebGl2RenderingContext::RGBA_INTEGER,
        WebGl2RenderingContext::UNSIGNED_INT,
        0.0,
    )
    .unwrap();

    gl.bind_buffer(WebGl2RenderingContext::PIXEL_UNPACK_BUFFER, None);
}

@laurenz-canva laurenz-canva Mar 18, 2026

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.

Hmm, but wouldn't this still invoke another copy to the unpack buffer first before copying it to the GPU, whch is what we've been trying to avoid?

Since you both approved I think I'll merge it in its current state, but let me know if you would like to see adjustments.

Loading