vello_gpu: Fix methods for bilinear and bicubic image sampling - #1963
Draft
laurenz-canva wants to merge 3 commits into
Draft
laurenz-canva wants to merge 3 commits into
laurenz-canva wants to merge 3 commits into
Conversation
laurenz-canva
marked this pull request as draft
September 29, 2026 14:58
laurenz-canva
added this pull request to stack #1965
September 29, 2026 14:59
laurenz-canva
force-pushed
the
laurenz/fix_bilinear_image
branch
from
September 29, 2026 15:08
8ae8442 to
690d88d
Compare
LaurenzV
force-pushed
the
laurenz/fix_bilinear_image
branch
from
September 30, 2026 06:18
690d88d to
93e02c4
Compare
LaurenzV
reviewed
Sep 30, 2026
| @@ -1,5 +1,6 @@ | |||
| { | |||
| "timeouts": { | |||
| "pageLoad": 1200000, | |||
Collaborator
There was a problem hiding this comment.
Had to add this to address another timeout failure resulting from our newly added tests: https://github.com/linebender/vello/actions/runs/36677557384/job/109765766086
laurenz-canva
force-pushed
the
laurenz/fix_bilinear_image
branch
from
September 30, 2026 07:36
f50d0ce to
4ee3473
Compare
LaurenzV
reviewed
Sep 30, 2026
Collaborator
There was a problem hiding this comment.
The border in the new snapshot is expected. The test uses bilinear sampling with repeat. This means that for example, for the top-right part, as we approach the right border of the yellow pixel, it should slowly start to fade into the red of the top-left pixel, hence why it becomes orange.
LaurenzV
marked this pull request as ready for review
September 30, 2026 08:02
LaurenzV
marked this pull request as draft
October 1, 2026 09:29
laurenz-canva
force-pushed
the
laurenz/fix_bilinear_image
branch
from
October 1, 2026 10:58
4ee3473 to
0fa8fe9
Compare
laurenz-canva
removed this pull request from stack #1965
October 1, 2026 10:59
laurenz-canva
added this pull request to stack #1969
October 1, 2026 11:00
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.
This PR makes the following changes:
This does unfortunately regress performance: Rendering a full-sized image on my Android phone goes from around 23FPS to 13FPS. See the below videos.
Before:
IMG_1296.MOV
After:
IMG_1297.MOV
But the previous approach was just fundamentally wrong. However, the good news is that with #1964, I will introduce a fast path that uses GPU-native bilinear sampling for images that are sampled from the whole external texture, which will not only undo this slowdown, but in fact make image rendering more than 2x faster compared to current main, when using external textures and sampling the whole texture! See that PR for more information. Since rendering whole images is the most common operation (except for glyph caching, but this is experimental right now, anyway. And nearest-neighbor sampling is less affected than bilinear sampling), in my opinion this is a trade-off worth taking.
Using the image atlas will unfortunately stay slow, even with #1964, but I think that's something we have to accept for now. We could improve this in the future by restricting the possible image sampling modes for such images (for example, only allowing extend mode
Pad). But the problem is simply that when we sample from an arbitrary subregion of an atlas, we have to emulate correct extension all ourselves, which is much slower than letting the hardware do it, so this should be avoided anyway.