[impeller] move gaussian blur vertex uniforms to fragment uniforms #35201
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. |
bdero
left a comment
bdero
left a comment
There was a problem hiding this comment.
Thanks! This was on my cleanup scratch pad. It was written at a time where there were bugs preventing uniform structs from working when present in both shaders.
(Also, please verify that Play/EntityTest.GaussianBlurFilter/* doesn't break if you haven't done so yet!)
| v_inner_blur_factor = frame_info.inner_blur_factor; | ||
| v_outer_blur_factor = frame_info.outer_blur_factor; | ||
| } | ||
| } |
|
I've verified to the best of my ability that this is still working as expected 😓 😅 maybe golden tests aren't that bad? |
|
actually, I see some differences in this particular app but that is in a comparison with skia. Let me double check |
|
nvm the difference is consistent between the change shaders, so its just an existing impeller bug - perhaps due to missing scaling on the sigma values |
jonahwilliams
added
the
autosubmit
2 participants
The blur shader is currently passing uniforms from the vertex shader through fragment shader inputs. Instead make the fragment shader take these as a uniform - which I think is theoretically faster and cleaner.
From local debugging, this reduces the amount of time spent in WaitForNextDrawable from ~830 ms to ~660 ms in a customer application.
TOT ios_debug
With change ios_debug
Of course, maybe its all just noise!