Repository navigation
Conversation
Pad clamped the sample position to [0, size - 1] before the bilinear half-texel offset, so everything past the right/bottom edge blended the last two texels 50/50 instead of repeating the edge texel. The samplers already clamp each tap to the image bounds, so leave the position unclamped for Pad and clamp the nearest-neighbour load instead. Adds image_bilinear_pad and image_bicubic_pad snapshot tests and updates two GPU-only external texture snapshots whose right/bottom edge pixels change slightly (max difference 4).
Collaborator
|
Is this not fixed by #1963? |
Collaborator
|
I think it should be. It will still take a bit but I hope I can land it next week! 😄 |
Contributor
Author
Might well be. Bug is present in 0.3. I didn't check for open PRs. |
Collaborator
|
@nicoburns Could you check whether this works fine on newest main? |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug noticed when smoke testing AnyRender upgrade to
vello_gpu0.3.Fix entirely generated by LLM (but it's very small).
Summary
If an image paint uses
Extend::PadwithImageQuality::Medium,vello_gpufills everything past the image's right and bottom edges with one flat colour: an exact 50/50 blend of the last two texels.vello_cpurepeats the edge texels, which is the expected output.(8×8 checkerboard, scaled 8×, filled into a rect 4×3 times the image size.)
Cause:
extend_modeclamped the Pad position to[0, size - 1]before the bilinear sampler subtracted the half-texel offset. For an 8-wide image, every point past the edge became7.0 - 0.5 = 6.5, so the sampler blended texels 6 and 7 atfract = 0.5. Bicubic had the same problem.Fix:
external_bilinear_sampleandexternal_bicubic_samplealready clamp each tap to the image bounds, which matches whatvello_cpudoes with per-tapextend_mode. So Pad no longer clamps the position at all, and the only path that relied on that clamp, the nearest-neighbourtextureLoad, now clamps to the image bounds itself:// helpers/extend.wesl case EXTEND_PAD: { - return clamp(t, 0.0, max - 1.0); + return t; } // helpers/external_texture.wesl, sample_external_image -return textureLoad(external_texture, vec2<u32>(coords), 0); +return textureLoad(external_texture, vec2<u32>(clamp(coords, image_offset, image_max)), 0);Tests:
image_bilinear_padandimage_bicubic_padsnapshot tests: a 4×4 checkerboard on a 100×100 canvas, with snapshots of 368 B and 1 KB. Both fail on GPU before this change and pass after it.external_texture_skewedandexternal_texture_with_scene_transformwere regenerated. Only their right and bottom edge pixels change, by at most 4/255, because of the half-texel fix at those edges.