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/2022.3/staging #8099

Merged
merged 42 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.

…om Function node's body field

This PR fixes https://jira.unity3d.com/browse/UUM-76270.

The Custom Function node's body field was difficult to use due to a combination of broken sizing and a lack of scrollbars. This fix prevents the body field from growing beyond the inspector width, and enables horizontal/vertical scrollbars.

![image](https://media.github.cds.internal.unity3d.com/user/3965/files/053d0c59-a251-4484-b532-f72ab0f37eaf)
…ct feature UUM-76307

fix camera offset in the UI editor for the RenderObject feature UUM-76307
Jira: UUM-76346

Steps to reproduce:

1. Create a new HDRP project
2. Observe Console

Actual results: Errors are thrown to the console
Expected results: Console does not contain VFX Graph errors

Reproducible with versions: 2022.3.39f1
Not reproducible with versions: 6000.0.12f1
…a wasn't updated properly for Asset Bundles (UUM-52890)

Fixes UUM-52890
This PR fixes an issue where some leftover shadow-rendering uniforms aren't properly cleared when using the `Unlit Draw Mode` in the scene-view, leading to some visible artifacts when sampling the empty shadow-map set for this mode.
… cbuffer

Shader Graph allows to declare properties in a global context. These properties can be set from script, globally to all shaders at the same time. (see [Shader.SetGlobalVector](https://docs.unity3d.com/ScriptReference/Shader.SetGlobalVector.html for instance)).
These global properties work fine when used with a regular ShaderGraph material, but not with VFX outputs that use ShaderGraph.
The reason is that we were grouping them in a structure with the other exposed properties, when these should be read directly from the constant buffer. Just changing the declaration in the shader makes it work fine.
…erial upgrader

Fixes https://jira.unity3d.com/browse/UUM-78379.

LOD Cross-fade is a per-material keyword in BiRP, but a pipeline-wide setting in URP. Therefore, the BiRP -> URP upgrader should omit this keyword from the upgraded material. This PR adds this logic to the upgrader.
… renderContext s…

This is a manual backport of https://github.cds.internal.unity3d.com/unity/unity/pull/52682. There was a small merge conflict because the `EndCameraRendering()` callback is called inside an `if` condition in Unity 6.0, but there's no such condition in 2022.3. The intent of the original PR is to move the callback down, so that it is called after the render context is submitted. I have retained the spirit of that change in this port.
… version upgrade.

This PR is intended to remove the VCS local checkout (due to `AssetDatabase.MakeEditable`) that happened on any reimport of SG assets.

Jira: https://jira.unity3d.com/browse/UUM-17912
…e rendering.

This PR aims to fix scene-view wireframe rendering issues when using forward/forward+ and depth-priming together.

This bug was caused by depth-priming forcing the depth-test comparison function to `Equal` (instead of `LEqual` of the original wireframe-rendering shader) for opaque-rendering ([here](https://github.cds.internal.unity3d.com/unity/unity/blob/833b9fd62c679e02f6be2dc94d6150525ce88553/Packages/com.unity.render-pipelines.universal/Runtime/Passes/DrawObjectsPass.cs#L218-L223)), assuming that valid depth-values would have been rendered beforehand to the bound depth-attachment. This is not the case for wireframe rendering, and thus, nothing renders.

Jira: https://jira.unity3d.com/browse/UUM-74999
…uild platforms are selected.

This PR fixes an issue in URP when selecting Switch or Android build target were forcing the camera depth render target to be reallocated every frame. 

`UniversalRenderPipelineCore.CreateRenderTextureDescriptor` by default sets depth buffer 32 bit format, however in `UniversalRenderer.CreateCameraRenderTarget` the descriptor depth buffer format is overridden to a 24 bit format when Switch or Android build targets are selected triggering depth buffer re-allocation. The change to 24 bit format was implemented in https://github.cds.internal.unity3d.com/unity/unity/pull/15935 to solve crashes and performance issues on Android, but is not necessary when running in the Editor where instead it causes unnecessary depth buffer reallocations, leading to memory leaks under certain circumstances.
JIRA:  https://jira.unity3d.com/browse/UUM-76242

An issue occurred because the API connected via Reflection was changed, causing the function to be undetectable. The code has been updated to align with the new API, and a unit test has been added to prevent future issues

Before
![Unity_LJ7DIyXku5](https://media.github.cds.internal.unity3d.com/user/761/files/e8558a2a-3547-43a3-aef1-c8e5a1c14f59)

After
![AfterFix](https://media.github.cds.internal.unity3d.com/user/761/files/b5be202a-24f6-4f30-b99c-6c3b132c2c5a)
…rrect shader code for 16-bit half precision.

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

This PR fixes a bug that Normal From Height node in a shader graph might return an invalid value when using 16-bit half precision.

<table><thead>
  <tr>
    <th><img src="https://media.github.cds.internal.unity3d.com/user/7006/files/2082dd6b-3b91-4f48-9c05-f440ccfea156" /></th>
    <th><img src="https://media.github.cds.internal.unity3d.com/user/7006/files/b31992b2-c6ae-45c9-989e-bacd86ac97c3" /></th>
  </tr></thead>
<tbody>
  <tr>
    <td>before</td>
    <td>after</td>
  </tr>
</tbody>
</table>
…backup' errors on entering Playmode with shader graphs open.

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

This PR fixes 'Objects are trying to be loaded during a domain backup' errors due to invalid serialization within SerializableTexture. One possible case is, as reported in JIRA, entering Play mode with PhysicalHair_Cards shadergraph from HDRP material sample open.
…Name.

Jira: https://jira.unity3d.com/browse/UUM-76081

Added a displayName to AbstractMaterialNode to use when displaying it in the Graph Inspector. displayName defaults to name, and is overridden by BlockNode to create it using shaderStage and displayName.
![image](https://media.github.cds.internal.unity3d.com/user/761/files/74c417ea-4dc5-43e9-8637-65ecb678eb63)

The changed name looks like this:
<table>
<tr><th>Before</th><th>After</th></tr>
<tr><td>VertexDescription.Position Node</td><td>Vertex Position Node</td></tr>
<tr><td>VertexDescription.Normal Node</td><td>Vertex Normal Node</td></tr>
<tr><td>VertexDescription.Tangent Node</td><td>Vertex Tangent Node</td></tr>
<tr><td>SurfaceDescription.BaseColor Node</td><td>Fragment Base Color Node</td></tr>
<tr><td>SurfaceDescription.NormalTS Node</td><td>Fragment Normal (Tangent Space) Node</td></tr>
<tr><td>SurfaceDescription.Metalilc Node</td><td>Fragment Metallic Node</td></tr>
<tr><td>SurfaceDescription.Smoothness Node</td><td>Fragment Smoothness Node</td></tr>
<tr><td>SurfaceDescription.Emission Node</td><td>Fragment Emissiton Node</td></tr>
<tr><td>SurfaceDescription.Occlusion Node</td><td>Fragment Ambient Occlusion Node</td></tr>
</table>
…nfinity

This PR fixes a freeze when the `Camera farClip` is set to `Infinity`.

Basically, the Light Forward+ algorithm was freezing the editor because the farClip becomes negative, which is not a valid scenario, so most of the calculations are not ending (freeze).

Clamping it to a minimum value of 0 fixes the freeze without any drawback.
…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)|
@UnityAljosha UnityAljosha requested review from a team as code owners September 19, 2024 09:06
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).
See the PR template for more information.
Thank you!

…er used when rendering

This PR changes the URP behaviour in ReflectionProbeManager to ignore custom probes without a texture as that can lead to some issues on platforms where the reflection probe atlas is not cleared by the driver.
* Fixing the wrong texture being set (due to resource name leaking) for half res downsample: instead of using _CameraDepthTexture which could be anything, using an new explicit name _SourceDownsampleDepth
* When hardware DRS is on, we can safely assume that the ratio between the input and output textures is 1 because the GPU hardware sizes are watertight.
Jira: https://jira.unity3d.com/browse/UUM-64133
JIRA: [UUM-29958](https://jira.unity3d.com/browse/UUM-29958)

Blit uses wrong texture source when using blit that sets texture using SetGlobalTexture right after a blit that sets texture using property block.
Based on the comments on the task, it looks like the expected behavior was that property blocks are cleared after each blit, but that wasn't happening. I added a clear after each blit that uses property block, so now they are indeed cleared after each blit and don't interfere with blits that use SetGlobalTexture
…n Api

The issue is, Depth texture is flickering while using screen recording option on adreno device(Adreno 730) which uses graphics API as Vulkan.

We have brought in specific changes by referring to the PR: https://github.cds.internal.unity3d.com/unity/unity/pull/33610 related to UniversalRenderer.cs 
if the `copyDepthPassEvent` is before `AfterRenderingTransparents` we change `CopyDepthMode` to `AfterOpaques`. These changes to `copyDepthPassEvent` fixing the issue at hand related to Samsung S22 that has Adreno 730 GPU

Bug : https://jira.unity3d.com/browse/UUM-77428
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.
To fix an issue with URP instancing

The transfer of the instancing ID was using the original instance ID and not the one modifed by the GetMeshAndElementIndex function. In this PR we change the code to use the modified value
… when entering Play Mode without saving changes

This PR fixes the reported bug and add a unit test.

Why the bug occurs: If a SG window (a `MaterialGraphEditWindow`) is open and its `m_GraphObject` field is null, then it reverts to the last saved state. `m_GraphObject` becomes null in `MaterialGraphEditWindow.OnDisable()`, which is called as a result of Domain Reload.

Fix: Release `m_GraphObject` only in `OnDestroy()`

Another issue: When a SG window is closed, `MaterialGraphEditWindow.OnDestroy()` is supposed to release `m_GraphObject` and its revelant resources. But `OnDisable()` makes only `m_GraphObject` null therefore the cleanup in `OnDestroy()` does not run. Same goes for `m_GraphEditorView`.
…irtual

allow render graph passes with reflection
This PR tackles 3 different issues related to strips:

- Some strip attributes (ParticleIndexInStrip, StripIndex and ParticleCountinStrip) return 0 when they are being used on an output that is not a strip. This was never implemented, but there is no technical reason why they can't be used, as long as the particle system connected to it is using strips, and it can be useful in different scenarios.

- ParticleCountinStrip returns the wrong value when used in Initialize context with a variable strip index. The reason for this is that we can't get a consistent count while we are still spawning new particles. When the strip index is constant, ParticleCountinStrip can estimate the count using the previous count + all the new particles, but this is not possible if particles are divided in different strips with a variable strip index. This PR adds a visual warning in Initialize context when ParticleCountinStrip is being used and stripIndex slot is connected to a variable expression.

- These changes allow us to use particle count to discard unused particles, when rendering strips as particles (previously rendering entire capacity, see  UUM-76014)

- A new simple operator RatioOverStrip was added, similar to AgeOverLifetime, to simplify a common use case.
…s when using Vulkan Graphics API

This PR fixes the issue where Overlay Camera Clear Depth ignored on some Android devices when using Vulkan Graphics API

Bug: https://jira.unity3d.com/browse/UUM-79014
Backport: https://jira.unity3d.com/browse/UUM-79416
Fixes UUM-72639 by ignoring material variants when converting, and logging an informative message to the console for awareness:
![image](https://media.github.cds.internal.unity3d.com/user/696/files/4ae2bcd6-e62e-480b-8f83-59164578bbd2)
[UUM-84842] Disabling HDRP_GraphicTestRunner.003-VirtualTexturing-NoMV on PS4
…'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
…trip data

When rendering particle strips, the last particle is ignored. This is because N particles produce N-1 strip segments.
However, if the particle strip system is connected to a regular particle system output, we have to render all particles.

Before, we were adding 1 to the max particle index before testing if the particle must be discarded.
Now, we only do that if the particle is a particle strip output (HAS_STRIPS). We already do similar adjustments in the same function.

The separate path for Shader Graph outputs was already taking this into account, that is why there was a discrepancy.

In the provided repro case, there are 2 particle outputs connected to the same strip system, one with Shader Graph output (left), the other with a regular output(right). The strips behind are only there as a reference.

Before:
![image](https://media.github.cds.internal.unity3d.com/user/2805/files/307ae729-135e-4a39-b2f4-6b0b27f70c00)

After:
![image](https://media.github.cds.internal.unity3d.com/user/2805/files/5b291f82-fa72-4ded-a829-84e04275a8a1)
…(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 ]*
…r passes (UUM-84287)

Fixing UUM-84287.
This is a followup of #46297 PR that fixed UUM-63267.
The fix in that PR fixed the issue given there by setting the WorldToCamera matrix before rendering shadows. However in this case the user also needs the CameratoWorld matrix.
… working with Decals (UUM-81517)

Fixes UUM-81517.
…iltin

https://jira.unity3d.com/browse/UUM-78803

This PR adds an implementation of the Main Light Direction node on builtin. I couldn't find any official indication it was unsupported, and `_WorldSpaceLightPos0` works fine.
Port of https://github.cds.internal.unity3d.com/unity/unity/pull/53318 for UUM-76172

Fixed Shader Graph undeclared identifier error around foveated rendering area.

Bug: UUM-76172
Port: UUM-84653
@UnityAljosha UnityAljosha merged commit 40a072c into 2022.3/staging Nov 1, 2024
1 check passed
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.