Skip to content

HDRP Metal fixes [Hold] #2044

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

Draft
wants to merge 5 commits into
base: HDRP/staging
Choose a base branch
from
Draft

Conversation

jessebarker
Copy link
Contributor


Purpose of this PR

  • Enable precision variants for TEXTURE2D_ARRAY_FLOAT/TEXTURECUBE_ARRAY_FLOAT/TEXTURE2D_ARRAY_HALF/TEXTURECUBE_ARRAY_HALF, it's been supported by Unity shader compiler a long time now
  • Add a DeviceInfo helper class that's enabling us to do potentially GPU/Target specific runtime changes that don't make sense to be a pipeline settings option. Allow picking up compute kernels based on GPU preference.
  • Refactor some older Nintendo Switch/iOS assumptions to use DeviceInfo. Apple GPU optimized variant of FPTL is in use on iOS atm, but it's not yet enabled by runtime for Apple GPU on macOS.
  • Because of above, now multi-compiling PLATFORM_LANE_COUNT_32 variants of FPTL. If necessary, should be easy to strip away on platforms not needing one.
  • Add 3D texture max dimensions clamping into VolumetricLighting.cs to avoid Metal API Validation in the editor while dynamically changing the resolution
  • Fix a rendering issue with FPTL when running as thread32 configuration against Apple GPUs. After speaking with Morten, it sounds like we should do a cleanup pass for our GroupMemoryBarrierWithGroupSync() usage, but that can wait to another PR.
  • Remove _DebugDisplayPad0 from ShaderVariablesDebugDisplay cbuffer, was causing alignment issue and doesn't seem to be needed.

With this, should be able to update a bunch of the reference images for MacEditor in HDRP_Tests, but will do that in a separate PR unless reviewers feel strongly.

Testing status

Manual testing with local projects.
Run of HDRP_Tests on Intel GPU to match Bokken agents


Comments to reviewers

This is most of !1795 with some of the pieces removed that are being handled separately/differently. Please let me know if there are still tweaks you'd like to see.

@github-actions
Copy link

It appears that you made a non-draft PR!
Please convert your PR to draft (button on the right side of the page)
and cancel any jobs that started on Yamato.
See the PR template for more information.
Thank you!

@sebastienlagarde
Copy link
Contributor

I really want to do a pass on this. I don't like the way platform 32 vs 64 is handled in this PR.
if we go the multi-compile road, then let's do it the proper way and add it everywhere it is required + add a stripping step.
(and in addition let's find a way to detect if we have a nvidia or amd on PC to select appropriate shader :)).

Question. The multicompile 32, is it really a fix or is it just an optimziation?

@jessebarker
Copy link
Contributor Author

I really want to do a pass on this. I don't like the way platform 32 vs 64 is handled in this PR.
if we go the multi-compile road, then let's do it the proper way and add it everywhere it is required + add a stripping step.
(and in addition let's find a way to detect if we have a nvidia or amd on PC to select appropriate shader :)).

Question. The multicompile 32, is it really a fix or is it just an optimziation?

I think there are two issues here. One is that we may need a runtime decision based on the actual GPU we see there (versus the Mac that the player was built on). The other is optimization (from the DM thread we had with Antti, he said running with 32 lane count was good for 1-2 ms, but not sure exactly what platform he was testing on).

@sebastienlagarde sebastienlagarde changed the title HDRP Metal fixes HDRP Metal fixes [Hold] Feb 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants