UI texture atlas slice shader #14990
Conversation
* Extract `ImageScaleMode` component, retrieve image size in process function and calculate slice geometry there.
alice-i-cecile
added
the
S-Needs-Review
Yes, the UI fix seemed much more important though. Once this is merged it should be relatively simple to apply the same changes to the sprite renderer. |
alice-i-cecile
added
S-Ready-For-Final-Review
| let mut flip = Vec2::new(1.0, -1.0); | ||
| let [mut flip_x, mut flip_y] = [false; 2]; | ||
| if image.flip_x { | ||
| flip.x *= -1.0; | ||
| flip_x = true; | ||
| } | ||
| if image.flip_y { | ||
| flip.y *= -1.0; | ||
| flip_y = true; | ||
| } |
There was a problem hiding this comment.
@ickshonpe thanks for the PR, using a shader is much better than CPU bound logic.
I don't see an equivalent to this flip image logic anywhere in the new code, so it might be missing from the migration
There was a problem hiding this comment.
Hi felix, yes you're right, I took image flipping out temporarily to make things easier and meant to add it back in at the end but completely forgot about it 😓. I think it must be really rare to combine nine slicing with flipping though so it's probably not too critical and shouldn't be difficult to add support in a followup pr.
Fixes #14183 Reimplement the UI texture atlas slicer using a shader. The problems with #14183 could be fixed more simply by hacking around with the coordinates and scaling but that way is very fragile and might get broken again the next time we make changes to the layout calculations. A shader based solution is more robust, it's impossible for gaps to appear between the image slices with these changes as we're only drawing a single quad. I've not tried any benchmarks yet but it should much more efficient as well, in the worst cases even hundreds or thousands of times faster. Maybe could have used the UiMaterialPipeline. I wrote the shader first and used fat vertices and then realised it wouldn't work that way with a UiMaterial. If it's rewritten it so it puts all the slice geometry in uniform buffer, then it might work? Adding the uniform buffer would probably make the shader more complicated though, so don't know if it's even worth it. Instancing is another alternative. The examples are working and it seems to match the old API correctly but I've not used the texture atlas slicing API for anything before, I reviewed the PR but that was back in January. Needs a review by someone who knows the rendering pipeline and wgsl really well because I don't really have any idea what I'm doing.
Objective
Fixes #14183
Solution
Reimplement the UI texture atlas slicer using a shader.
The problems with #14183 could be fixed more simply by hacking around with the coordinates and scaling but that way is very fragile and might get broken again the next time we make changes to the layout calculations. A shader based solution is more robust, it's impossible for gaps to appear between the image slices with these changes as we're only drawing a single quad.
I've not tried any benchmarks yet but it should much more efficient as well, in the worst cases even hundreds or thousands of times faster.
Maybe could have used the UiMaterialPipeline. I wrote the shader first and used fat vertices and then realised it wouldn't work that way with a UiMaterial. If it's rewritten it so it puts all the slice geometry in uniform buffer, then it might work? Adding the uniform buffer would probably make the shader more complicated though, so don't know if it's even worth it. Instancing is another alternative.
Testing
The examples are working and it seems to match the old API correctly but I've not used the texture atlas slicing API for anything before, I reviewed the PR but that was back in January.
Needs a review by someone who knows the rendering pipeline and wgsl really well because I don't really have any idea what I'm doing.