-
Notifications
You must be signed in to change notification settings - Fork 840
[VFX] Mobile fixes #5149
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[VFX] Mobile fixes #5149
Conversation
Hi! This comment will help you figure out which jobs to run before merging your PR. The suggestions are dynamic based on what files you have changed. HDRP VFX Depending on the scope of your PR, you may need to run more jobs than what has been suggested. Please speak to your lead or a Graphics SDET (#devs-graphics-automation) if you are unsure. |
It appears that you made a non-draft PR! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
Hey @julienf-unity! Here's the WIP Test Doc. Main issue of extended particles on Android is fixed, it haven't occured on any of the devices during first QA pass. Whew! Speaking further, some of the issues are already summarized (included minor and major issues) with provided reproduction and can be worked on. Some of them still need some attention Also, if there's anything I can do to make the investigation on your side easier, let me know. |
Yes this is what this PR is fixing. Of course there's still lots of other issues but it shouldnt block the PR. If the "extended particle" fix is verified, I would like to merge it before branching off. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving as the main issue is fixed, other issues will be looked into in another fix pass.
#5582 must be backported at the same time as this one |
Purpose of this PR
Some mobile fixes for VFX Graph:
This is a first pass, and other fixes are required for mobile devices (in particular some C++ changes)
For 1348666, I've checked with @hkr, and decided to guard functions using cubemap arrays between SHADER_AVAILABLE_CUBEARRAY check. They may revert to old behavior possibly at some point.
For 1149057, The issue is caused by driver bug for certain integer instructions (in this case, it's with bitFieldExtract). I've worked around them by using float instruction instead.
I've tested all outputs: There's still a potential issue with cube output uv computation, but as it is an experimental feature that has other bugs, I don't have an immediate fix for that (and not sure if it's worth it anyway)
Testing status
Tested locally on Oculus Quest 2 with GLES3.1 (Vulkan does not exhibit the issue)
Comments to reviewers
Notes for the reviewers you have assigned.