[Impeller] Use fast path for CPU generated textures where possible#36466
[Impeller] Use fast path for CPU generated textures where possible#36466auto-submit[bot] merged 8 commits intoflutter:mainfrom
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!). If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
@jonahwilliams this might be useful for the Gradient textures as well, although those are tiny. |
|
They can end up quite big sometimes, if the stops are close together. |
|
This needs some work around how the row bytes are handled. Metal expects them to be aligned to a particular value. |
|
This pull request has been changed to a draft. The currently pending flutter-gold status will not be able to resolve until a new commit is pushed or the change is marked ready for review again. |
|
The tests are passing locally on an M1 mac, going to see if I can try them on an x64... |
|
Testing that the pixmap dimensions match the texture mipmap size doesn't work across platforms anymore, because we might be allocating rowbytes beyond what the texture dimensions actually are because the platform needs us to do that to keep the alignment requirements for MTLBuffer/MTLTexture working. |
|
(The check happened to work out fine on arm64 environments but not x64). |
|
I've refactored this to update the DeviceBuffer API to make the allocation make more sense. The GLES could use https://registry.khronos.org/OpenGL/extensions/ARB/ARB_pixel_buffer_object.txt to improve things here, but I'm not sure if we want to pursue that. Vulkan should have something better but that can wait for a subsequent patch (@iskakaushik fyi) |
|
I had thought of a variant of the AsTexture interface but eventually gave up because it couldn't be implemented across all APIs. The workaround you mention where a texture upload is performed in OpenGL leaves you susepctible to change to the host buffer you make after calling the AsTexture method not actually updating the texture in OpenGL as they would with Metal. I am nervous of having APIs not behaving consistently across all backends. Perhaps AsTexture should just fail on OpenGL and the caller then has to figure out to do the copy. We can have a wrapper method that makes this convenient of course. |
Maybe something on the context about whether this is supported or not, like offscreen MSAA? There are pixel buffer object extensions for GL as well but it's not clear to me how available they are (although anything on EGL 3 would have them). |
|
Hard to tell exactly, we'll need another few data points due to noise in the chart. That said, looks like a win so far |
|
That benchmark is unfortunately noisy - locally I had some runs look better and others not. But looking at a different app that just repeatedly pumped that frame, the frame times seemed better on average. |


Offers a slight improvement on glyph atlas rendering by making the texture upload basically free. Shows improvements locally on our text based benchmarks.