Skip to content
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

Internal/2021.3/staging #8108

Merged
merged 7 commits into from
Nov 1, 2024
Merged

Conversation

UnityAljosha
Copy link
Collaborator

Please read the Contributing guide before making a PR.

Checklist for PR maker

  • Have you added a backport label (if needed)? For example, the need-backport-* label. After you backport the PR, the label changes to backported-*.
  • Have you updated the changelog? Each package has a CHANGELOG.md file.
  • Have you updated or added the documentation for your PR? When you add a new feature, change a property name, or change the behavior of a feature, it's best practice to include related documentation changes in the same PR. If you do add documentation, make sure to add the relevant Graphics Docs team member as a reviewer of the PR. If you are not sure which person to add, see the Docs team contacts sheet.
  • Have you added a graphic test for your PR (if needed)? When you add a new feature, or discover a bug that tests don't cover, please add a graphic test.

Purpose of this PR

Why is this PR needed, what hard problem is it solving/fixing?


Testing status

Describe what manual/automated tests were performed for this PR


Comments to reviewers

Notes for the reviewers you have assigned.

mseonkim-unity and others added 7 commits September 20, 2024 22:33
…d boundaries.

Jira: [UUM-70628](https://jira.unity3d.com/browse/UUM-70628)
Parent PR: [#48000](https://github.cds.internal.unity3d.com/unity/unity/pull/48000)

As reported in JIRA, the sun flicker problem occurs when the directional light is close to cloud boundaries. This can only be reproduced if `perceptual blending` is enabled on volumetric clouds. The transmittance of each pixel changes every frame due to ray marching noise and temporal reprojection. When perceptual blending is enabled, it divides the transmittance by the current luminance of the target pixel, so it reduces the RGB value dramatically when the luminance is really high.

![image](https://media.github.cds.internal.unity3d.com/user/3842/files/c2c067db-d228-4500-b28d-4f9d0295c004)

When the `perceptual blending` is disabled, we simply use the original input transmittance, so that neighboring pixels have slightly different values. However, we reduce the transmittance by luminance when `perceptual blending` is enabled. This causes a huge pixel value change for neighboring pixels at the cloud boundaries, so the sun flickers.
![image](https://media.github.cds.internal.unity3d.com/user/3842/files/06816951-ac8b-4523-a0e6-36db227fc146)

By softening the transmittance attenuation curve with a `pow(transmittance, 6)` clamp, we can prevent sun flicker and slightly improve the perceptual blending where the sun is behind the clouds.
![image](https://media.github.cds.internal.unity3d.com/user/3842/files/cce785be-a336-48b9-871b-aa53443c58b2)


| Before | After |
| -- | -- |
| ![image](https://media.github.cds.internal.unity3d.com/user/3842/files/594c9648-dd7e-4fa0-aa72-4f4cd3643ac3)|![image](https://media.github.cds.internal.unity3d.com/user/3842/files/cb52d81c-82c6-4b7a-bdff-d8dda24d7bf6)|
…r targets(UUM-65523)

Fixes UUM-65523.
Backport of #46039
Shader Graph importer can throw exceptions during asset import, which result in an asset state that is more difficult for a user to correct. I'm suppressing exceptions around precision mismatch between node dependencies, as in most cases this situation is neither reachable nor user actionable nor is the behavior actually erroneous.
…(DOCATT-589 and DOCATT-1045

*[This PR is intended to fix broken links and formatting issues in the Shader Graph documentation for the following JIRA tickets:

- DOCATT-589
- DOCATT-1045 ]*
…'s speed tree shaders have the same c-buffer layout in vertex and fragment shaders when GPU instancing and Light Layer are enabled

JIRA: [UUM-64059](https://jira.unity3d.com/browse/UUM-64059)

This PR is to fix the error case where. shader compilation error logs about c-buffer layout mispatch in vertex and fragment shaders of URP speed tree 7 shader are repetitively printed out when the material enables GPU instancing and Use Rendering Layer in URP RP asset's lighting section is on.

Because the speed tree shader is not compatible with SRP Batcher, the shader is supported by InstancingBatcher.
When rendering request comes, the batcher collects layout informations about 'unity_Builtins0Array' through array of GPU program parameters defined by shader compilation result.
For its vertex shader variant, `unity_LODFadeArray` is the first member of the c-buffer because the shader always enables `LOD_FADE_PERCENTAGE`.
In addtion to that, `unity_RenderingLayerArray` becomes the second member because Light Layer, a `multi_compile` keyword is active, 
Unlikely, for its fragment shader variant, it still requires `unity_Builtins0Array`, but the `LOD_FADE_PERCENTAGE` keyword doesn't exist here, so the first member is determined to be `unity_RenderingLayerArray`, and the mismatch case occurs.

For avoiding this, I simply changed their pragma action from `multi_compile_vertex` to `multi_compile`.
And when I tested further, there are more places where the same problem happens, I applied this there too.
So, affected places are...

- `ForwardLit` pass in URP's SpeedTree7 shader
- `GBuffer` pass in URP's SpeedTree7 shader
- `ForwardLit` pass in URP's SpeedTree8 shader
- `GBuffer` pass in URP's SpeedTree8 shader
@UnityAljosha UnityAljosha requested review from a team as code owners November 1, 2024 15:16
@UnityAljosha UnityAljosha merged commit 3d53173 into 2021.3/staging Nov 1, 2024
4 checks passed
Copy link

github-actions bot commented Nov 1, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants