[Impeller] align SSBOs to 16 bytes to fix iOS shader validation issue. #39654
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. |
There was a problem hiding this comment.
I believe DefaultUniformAlignment will give you the right alignment. The name is an unfortunate artifact of a time when we didn't have SSBOs. Just uniforms which have the same alignment rules. I see EmplaceUniform and EmplaceStorageBuffer which apply this alignment correctly but I guess we didn't use it here because these don't have a vector specialization.
EmplaceUniform and EmplaceStorageBuffer are identical and should be aliased. They should also have a vector or contiguous collection specialization. Can we file a bug to fix both and bring the naming conventions of impeller::HostBuffer into the modern age?
W.r.t this patch, lets replace kSSBOAlignOf with impeller::DefaultUniformAlignment (also unfortunately named and we can fix in a refactoring pass later) so its right on more platforms. Then we can land this while we wait for a refactor.
|
Done |
jonahwilliams
added
the
autosubmit
…120920) * 9ee3221d0 [Impeller] align SSBOs to 16 bytes to fix iOS shader validation issue. (flutter/engine#39654) * 65a79237f Roll Dart SDK from 09361aea2ca8 to 3ec7f1f92216 (1 revision) (flutter/engine#39686) * e213d1c5c Uses `int64_t` instead of `int` for the |view_id| parameter. (flutter/engine#39618) * 94e33ae50 test (flutter/engine#39630) * bddfc1c4d [Impeller] Match position attribute name across runtime effect stages (flutter/engine#39552)
Fix flutter/flutter#120817 by doing what the nice shader validation error message says