Skip to content

Render more than ~5300 images #1467

Merged
taj-p merged 4 commits into
linebender:mainfrom
taj-p:tajp/fixImageShader
Feb 23, 2026
Merged

Render more than ~5300 images #1467
taj-p merged 4 commits into
linebender:mainfrom
taj-p:tajp/fixImageShader

Conversation

@taj-p
Copy link
Copy Markdown
Contributor

@taj-p taj-p commented Feb 23, 2026

Fixes #1468

There is a bug in the shader code - we only check the first row of the encoded paints texture. On my machine, that gives me ~5400 images before I start drawing empty rectangles.

This PR modifies our textureLoad calls to consider more than the first row of texels.

On my machine, I used the following test to verify the fix.

#[vello_test(width = 10, height = 10)]
fn image_many_unique_paints_wrap_encoded_texture(ctx: &mut impl Renderer) {
    const NUM_IMAGES: usize = 6000;
    let sampler = ImageSampler {
        x_extend: Extend::Pad,
        y_extend: Extend::Pad,
        quality: ImageQuality::Low,
        alpha: 1.0,
    };

    let dummy_rect = Rect::new(0.0, 0.0, 1.0, 1.0);
    for i in 0..NUM_IMAGES {
        let mut pix = Pixmap::new(1, 1);
        let val = (i % 255 + 1) as u8;
        pix.set_pixel(0, 0, PremulRgba8::from_u32(u32::from_be_bytes([val, val, val, 255])));
        let source = ctx.get_image_source(Arc::new(pix));
        ctx.set_paint(Image {
            image: source,
            sampler,
        });
        ctx.fill_rect(&dummy_rect);
    }

    let mut final_pix = Pixmap::new(1, 1);
    final_pix.set_pixel(0, 0, PremulRgba8::from_u32(u32::from_be_bytes([255, 0, 0, 255])));
    let final_source = ctx.get_image_source(Arc::new(final_pix));
    ctx.set_paint(Image {
        image: final_source,
        sampler,
    });
    ctx.fill_rect(&Rect::new(0.0, 0.0, 10.0, 10.0));
}

@taj-p taj-p requested a review from grebmeg February 23, 2026 06:04
Copy link
Copy Markdown
Collaborator

@grebmeg grebmeg left a comment

Choose a reason for hiding this comment

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

LGTM

sampler,
});
ctx.fill_rect(&Rect::new(0.0, 0.0, 10.0, 10.0));
} No newline at end of file
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.

Missing new line


// Convert a flat texel index to 2D texture coordinates for the encoded paints texture.
fn encoded_paint_coord(flat_idx: u32) -> vec2<u32> {
let w = textureDimensions(encoded_paints_texture).x;
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.

I’d be interested in exploring passing the texture dimensions as a uniform later. I’m curious how it performs across different machines.

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.

Ohhh great idea. This would mimic what we do for alphas_tex_width_bits. Implemented in 050add1

// Convert a flat texel index to 2D texture coordinates for the encoded paints texture.
fn encoded_paint_coord(flat_idx: u32) -> vec2<u32> {
let w = textureDimensions(encoded_paints_texture).x;
return vec2<u32>(flat_idx % w, flat_idx / w);
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.

If we can rely on max_texture_dimension_2d always being a power of two, we can use bitwise operations. We use the same pattern for alphas_texture.

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.

Great idea! Thank you! Implemented in 050add1

@taj-p taj-p added this pull request to the merge queue Feb 23, 2026
Merged via the queue into linebender:main with commit 4a32b89 Feb 23, 2026
17 checks passed
@taj-p taj-p deleted the tajp/fixImageShader branch February 23, 2026 20:12
}

/// Configuration for the GPU renderer.
#[repr(C)]
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.

I think if we set this to align(16), bytemuck will automatically complain in case there is not enough padding, which could be useful in case we change it again in the future

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.

Addressed in #1474

Comment on lines +117 to +119
_padding0: u32,
_padding1: u32,
_padding2: u32,
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.

Can we not use an array<3> here?

Copy link
Copy Markdown
Contributor Author

@taj-p taj-p Feb 23, 2026

Choose a reason for hiding this comment

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

That's not possible. Apparently for downleveling uniforms to WebGL2, arrays require a stride of 16 bytes. See the table in this section .

array<S> requires alignment roundUp(16, AlignOf(S)).

edit: How wild that it requires so much memory. A big pain point for uniforms!

github-merge-queue Bot pushed a commit that referenced this pull request Feb 24, 2026
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.

Only ~5400 paints supported by Hybrid

3 participants