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/6000.0/staging #8111

Merged
merged 39 commits into from
Dec 18, 2024
Merged

Internal/6000.0/staging #8111

merged 39 commits into from
Dec 18, 2024

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.

…ipelineRuntimeXRResources definition

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

Since the class UniversalRenderPipelineRuntimeXRResources is wrapped in the #if ENABLE_VR && ENABLE_XR_MODULE preprocessor, it will not compile on platforms where ENABLE_VR is not defined.

This is why on platforms like tvOS, where ENABLE_VR is not defined, a warnning is shown that no matching type found for the UniversalRenderPipelineRuntimeXRResources data added to Assets/Settings/UniversalRenderPipelineGlobalSettings.asset.

The UniversalRenderPipelineRuntimeXRResources class definition should compile regardless of platform, so I removed the #if ENABLE_VR && ENABLE_XR_MODULE preprocessor.



### Before:
<img width="872" alt="image" src="https://media.github.cds.internal.unity3d.com/user/6374/files/1886efc7-b93e-4bbe-8ab2-4471f56c50b5">

Warning shows up when open the URP template project on tvOS platform.

### After:
No warning shows up.
…OpenGLES3

UUM-84431

In OpenGLES3, Lens Flare does not render properly when the Screen Space Occlusion option is enabled.
The following warning message appears:
```
CommandBuffer: temporary render texture  not found while executing  (SetGlobalTexture)
UnityEngine.GUIUtility:ProcessEvent (int,intptr,bool&)
```

OpenGLES3, OpenGLCore, and WebGPU do not initialize occlusionRT. Fixed occlusion to be enabled when occluionRT is initialized.
…(box, sphere, bow.. etc) as Foam Generator and Water Deformers

Following the Water Decal improvement in Unity 6, we handled the migration of old foam generators and water deformers properly by converting them to a shader graph. However, users starting in Unity 6 don't have this possibility (to start from the migration shader graph), so following the pattern in the custom pass material added by this PR (#46391), I improved the workflow allowing users to either create a new blank water decal or start from the deformer and foam migration shader graph to get the default shapes that was there before U6 (box, sphere, bow.. etc)

Before: 

https://media.github.cds.internal.unity3d.com/user/1764/files/fe721506-6342-4e21-a7f1-5b515873db12


After: 
(Empty water decal is the same path as the "new" button before)
https://media.github.cds.internal.unity3d.com/user/1764/files/de95d1b1-9dfc-47d0-9ef9-e1dacbbf5eb2
### Bug 1
Jira: UUM-83782
When a custom hlsl block has parameter with the same name as a built-in attribute, the VFX does not compile.

- import the provided [package](https://jira.unity3d.com/secure/attachment/1556961/VFXG_HLSL_AttributeRedefinition.unitypackage)
- open the asset named `VFXG_HLSL_AttributeRedefinition`
- try to compile

Actual results: VFX doesn't compile with errors in the console:

> Shader error in '[VFXG_HLSL_AttributeRedefinition] [Simple Burst] Initialize Particles': redefinition of 'direction' at kernel CSMain at VFXG_HLSL_AttributeRedefinition.vfx(191) (on d3d11)

Expected results: 
User should be able to set input names that match the attributes name.

### Bug 2
Jira: UUM-83676

When creating a Custom HLSL operator with two outputs, and that those two outputs are plugged, then the same function is copied twice in the generated compute shader.

Steps to reproduce:
1. Create a new VFX Graph asset (use the simple loop template)
2. Open this VFX Graph
3. Add a Custom HLSL operator
4. Replace the default code by this one:

>void FloatFunction(in float value, out float a, out float b)
{
  a = value * 2;
  b = value * 3;
}
 
5. Connect the output a and output b to the gravity Force input (X and Y)

Actual results: 
The VFX Graph do not compile because the function is duplicated
Expected results: 
The VFX Graph works fine
…rom cachedShadowManager

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

When preserveCachedShadow is set to true on an HDAdditionalLightData, it shouldn't be evicted from the cachedShadowManager. This PR adds a missing check for this in RefreshCachedShadow().
…Graph mode

This PR fixes the GBuffer pass for VFX outputs when URP is using the Render Graph : when Render Graph is enabled, the Render Target 4 expects to receive the pixel depth. 

The PR also fixes the GBuffer pass for the Decal output, making it match the non-VFX implementation.
Fixes [UUM-86522](https://jira.unity3d.com/browse/UUM-86522).

Since the water exclusion shader was developed, it hasn't been tested with rendering as an entity, and thus gave a compilation error. In this PR, I've fixed this error.

<img width="884" alt="image" src="https://media.github.cds.internal.unity3d.com/user/2726/files/e9471984-2082-4c87-b066-fbf32f18dce0">
The issue is that certain GPU drivers (specifically, Power VR drivers in this case) delay sending commands to the hardware until a specific threshold is met.
This PR resolves the issue by inserting a call to `glFlush()` before invoking `glClientWaitSync()` to ensure that the queued commands are sent to the hardware.
This PR also reverts the changes done for https://jira.unity3d.com/browse/UUM-59572

Bug: https://jira.unity3d.com/browse/UUM-35986
Backport: https://jira.unity3d.com/browse/UUM-86487
Disabling 270_ScreenSpace_UI_Overlay.
This test was disabled on trunk but not on 6000.0.
https://jira.unity3d.com/browse/UUM-85648
https://jira.unity3d.com/browse/PLATGRAPH-3929
One of the big fixes in this PR is to make sure to submit any command encoders before uploading data to the same texture, otherwise there would be data race issues and the texture ends up looking incorrect on the GPU.

https://jira.unity3d.com/browse/PLATGRAPH-3930
A few other BiRP bugs popped up when fixing our webgpu backend, turns out we previously had black temp template images and never fixed a BiRP graphics tests bugs. Here is a list of bugs also fixed in this PR:
- BiRP test 877 fixed to use `RWTexture2D<half4>` instead of `RWTexture2D<float4>` since the texture that is bound uses a 16 bit format
- Implemented `CreateStencilViewForPlatform` and `DestroyStencilViewForPlatform`. This lets us have stencil only views of depth render surfaces in webgpu, and fixes test 873
- Added additional checks for the `S8_UINT` format as a depth stencil attachment, as webgpu is strict on what settings are enabled (ie load/store ops) when using a stencil only format (test 891)
- Fallback to point samplers instead of linear samplers for mip map generation when generating mip maps of a texture that has an unfilterable texture format (test 506)
- fix tests that use mip maps for the render texture and limit the mip limit to 1. The Web platform graphics apis, unlike other graphics apis, do not allow color attachment texture views and depth stencil attachment views bound to the render pipeline to have different sizes, so binding a non 1st level mip causes validation errors. (tests 764 506)
- fix texture copy logic to use blitting instead of a `copyTexturetoTexture` command when the source dimensions of a texture are smaller the block size of a compressed texture (test 434)
- add a WGSL parser that updates reflection info correctly for depth texture sampling usage.
…ribute sanitize issue

Jira: UUM-83849
When a VFX asset from 2022.3 or previous version is imported, and that this VFX asset uses a custom attribute with the same name as a built-in attribute, the sanitization does not work as expected. 

How to reproduce:
- Import the attached [package](https://jira.unity3d.com/secure/attachment/1557681/Custom%20Attribute%20Upgradability.unitypackage) to 6000.x version
- Open Smoke1 VFX
- Observe that there are custom attributes of 'Seed' and 'Seed_1" and error message in the console
Add a general dynamic resolution/upscaling page to the manual.

Jira ticket: https://jira.unity3d.com/browse/DOCG-5783
…e count is higher than 64 due to k_MaxCubeReflectionsOnScreen when Raytracing is enabled

JIRA: https://jira.unity3d.com/browse/UUM-86779
Slack: https://unity.slack.com/archives/C6Y79CZM0/p1730454767182339

Out of bounds asserts for _PlanarCaptureVPWL and _CubeScaleOffsetWL were using an incorrect size. Fixed by using the right constants matching their actual size.
…exture array slices

Fix UUM-77828
The render graph compiler was not correctly handling sub-resources as attachments both in c# and c++. This could cause rendering to slice x and y to be merged into a single render pass that rendered only to slice x. This is fixed by comparing not only the resource but also the array slice and mip index.

Also fixes the following minor issues discovered during investigation of this bug:
- Add Slice/Mip data to the render graph debugger which made it difficult to debug this
- Add some extra validation to the descriptor to avoid silently failing when creating invalid texture descriptors
- Renderpass C++ api validation was broken and considered some valid setups to be invalid since it was not comparing the aray slice and mip when testing attachments for equality.
URP Renderer2D sets the depthStencilFormat to D32_SFloat_S8_UInt except for switch, embedded, and QNX, in which case it uses D24_UNorm_S8_UInt.

D32_SFloat_S8_UInt is not supported on a number of Android devices, such as the Galaxy Note 8 or Galaxy Note 9.

Because RenderTexture will try to find a fallback format by **increasing** the bits of the depth format, and there are no formats with more than 32 bits, the RenderTexture will fail to be created, causing the render pass to fail.

Everywhere else in Unity D24_UNorm_S8_UInt is used for Android, so we should have Android use that in Renderer2D, too.
…terial

Currently, the only way to use the default shader graph in the HDRP package for custom PBR Sky is to use the menu (that no one knows about). 
![image](https://media.github.cds.internal.unity3d.com/user/1764/files/2beff507-dad9-40c0-bf69-468041483d5a)

And the default material is in the HDRP package making it readonly.
Currently : 
![image](https://media.github.cds.internal.unity3d.com/user/1764/files/35eb2961-ba4b-4331-be82-c43cc185c112)

After the PR: 
What this PR does is add the pattern that we already have elsewhere (custom pass, water surface, water decal) to basically create a NEW material based on a shader graph that is in the HDRP package using the NEW button.
![image](https://media.github.cds.internal.unity3d.com/user/1764/files/11ad1dab-19c2-4459-9618-f777b25a93b7)


After clicking on new, a new folder is created using the name of the current scene, assign it and highlight it. 
This is the material created, it should help users not knowing how to use the PBR Sky master stack by default by not starting from an empty master stack. 

![image](https://media.github.cds.internal.unity3d.com/user/1764/files/b79f107d-7b51-42c0-a838-5396d1f9e71a)

This PR also fix a minor discrepancy, between setting no material and overidding the material property and setting no material and not overriding the property. The two gave 2 different render whereas they should have been the same. (cf [commit](https://github.cds.internal.unity3d.com/unity/unity/pull/56435/commits/df0fc3e234358fdbf5210fcaf00b369baada6da1))
…ggle wasn't working (UUM-79471)

Fixes UUM-79471.
The issue is that for Main Light Shadows, it changes the shadow fade to accomplish the goal of making transparent objects not receive shadows. While Additional Light Shadows do this by setting the shadow strength to 0 and marking every light as a non shadow casting. This discrepancy causes issue for users making custom shaders that don't use the Shadow Fade for main light shadows. To fix the issue, shadow strength is now set to 0 for the transparent passes if the toggle is disabled (so transparent objects should not receive shadows).

This PR also cleans up the shadow codebase a little bit, similar to #56069
…y Based Sky (incorrect exposure in later frames)

This PR fixes the issue of removing the celestial data twice resulting in a black sky cubemap.

The cleanup calling order was changed after this PR(https://github.cds.internal.unity3d.com/unity/unity/pull/49443) was applied.
The ordering change caused the removal of the celestial data for sky rendering in the middle of the render pipeline.

So it has been fixed to call the cleanup only once to maintain the celestial body data.
Backport of https://github.cds.internal.unity3d.com/unity/unity/pull/56758

Previously, the internal method `OnDrawCommand()` used for BatchRendererGroup picking and outline expected a pointer to a `BatchDrawCommand` (i.e. a regular direct draw), which it expects to be able to modify, at least for the material and mesh fields.

When non-direct (procedural and/or indirect) draws were added to BRG, this code was not changed, and draws were simply skipped if they were not of the regular direct type.

The existing `OnDrawCommand` implementations did not actually require the draw to be direct. Instead, they need to:
* Select the Material the draw is performed with, which can differ from the original Material.
* Skip over some draws, e.g. in case the draw is present in an ignore list.
* Know the submesh index so it can be communicated to the shader.

This PR modifies `OnDrawCommand` to accept an immutable `BatchDrawCommandRef` instead, which it can use to read any relevant data from the draw command. Instead of being able to modify the draw command (which was bug prone to begin with, since modifying some parts like the flags would not actually work), it now returns the `BatchMaterialID` the draw should be performed with, or a null material ID in case the draw should be skipped. The mesh can no longer be modified, but this was not actually being used in practice, and would not be well defined for procedural draw types.

The code which needs to know the submesh index can now check if the draw actually has one, and default to zero in case it doesn't.
Fix for https://jira.unity3d.com/browse/UUM-83521

Nesting subgraph blocks was not possible anymore and all blocks in subgraph block contexts appeared as "invalid" (detoured in red)
After investigation it appeared that this had never really working. However before "the regression" the issue was less obvious: Subgraph blocks could accept too many blocks regardless of their compatibility, and the red detour feedback was not working correctly. There was a big ambiguity in code about context type and compatible context types. I tried to refactor this a bit so that it's clearer and less hacky.

- Accepting blocks should work with all contexts now. For subgraph block context, it can only accept a block that is compatible with *all* its suitable contexts. 
- Invalid visual feedback should work and be refreshed correctly now (so for instance when switching the suitable context setting of the subblock context)
… TAA is disabled

Currently STP upscaler can be enabled only if temporal anti-aliasing (TAA) is enabled.

This PR fixes corner case situations where, despite being selected, STP is not enabled due to TAA pre-requisite conditions (needs motion vector, no camera stacking, no dynamic resolution...) not being fulfilled. 

Root cause is due to the imperfect junction between TAA and STP enablement: we end up with cases where `IsTAAEnabled()` is false but `IsSTPEnabled()` is true, generating various mis-behaviors. 

In this given issue, URP wrongly tries to use upscaled color buffer despite STP being disabled due to TAA pre-requisite missing. At runtime, STP upscaler is not used but `IsSTPEnabled()` remains true, triggering wrong resource connections.

Current PR fixes this by: 
- clarifying terminology: supported/selected/enabled. Support is hardware/gfx API specific (`IsSTPSupported()`), selection is user choice (`IsTAA/STPSelected()`), enablement (`IsTAA/STPEnabled()`) is our choice based on support/selection and other internal reasons.
- clarifiying logic: STP is enabled only if TAA is enabled
- adding IsTAA/STPSelected helpers in cameraData

PR is also adjusting TAA warning when being disabled to mention STP if selected, helping users to understand why STP is not called despite being selected. In the given situation, both STP and TAA are disabled because of camera stacking (first message only appearing for TAA+noSTP case:
![image](https://media.github.cds.internal.unity3d.com/user/5521/files/f0c278af-b37d-47b4-b185-d8c9cb111bcc)
… running in play…

JIRA: https://jira.unity3d.com/browse/UUM-86959
Slack: https://unity.slack.com/archives/C02FVPP6M34/p1731665800856559
Fix ambient probe not being updated when changing its intensity when running in play mode with static batching and GPU resident drawer enabled. Instances that are part of a static batch where explicitely discarded in the ProbesUpdateJob, which is incorrect since the ambient probe affects all gameobjects (static or not). The mistake probably happened because the job was written to be very similar to the TransformUpdateJob where it is valid to discard instances that are part of static batch
Following recent QoL PR, for coherence we decided to add a new button also for the Decal Projector component. 
This material creation follows the UX pattern of the custom passes shaders, i.e:
- Creating the asset in the current folder
- Focus on renaming right after creation

![image](https://media.github.cds.internal.unity3d.com/user/1764/files/cc1d3ca1-d56c-43ea-98d5-a619235de601)

also did the same on URP for feature parity. The only difference is that there's no URP/Decal material so we just create a material based on the default decal material SG located here : com.unity.render-pipelines.universal\Shaders\Decal.shadergraph

![image](https://media.github.cds.internal.unity3d.com/user/1764/files/136a33a3-1879-4428-82fa-b202f97528f6)
Fixed the CameraDepthAttachment Texture for Dx11, which was rendering as black earlier.
The issue is the Camera Depth Attachment texture is referring to the depth buffer and when the shader is trying to access the texture the texture is black.

Bug: https://jira.unity3d.com/browse/UUM-64316
Backport: https://jira.unity3d.com/browse/UUM-65829
… Combine pass

Unity forces depth buffer binding when binding MRTs. 'Volumetric Clouds Combine' pass uses MRTs, but there were 2 errors:
- There are two places to bind dummy depth buffers but only one place was handled.
- The bound texture has a color format, not a depth format.

This PR binds a dummy texture with a depth format in both places.
…samples

Fix errors when building XR Player with the repro project in [UUM-86061](https://jira.unity3d.com/browse/UUM-86061).
- SRP: ProbeVolumeDebugBase.hlsl was missing an include statement for stereo rendering.
- HDRP: WaterDecalSubTarget.cs was not generating stereo rendering related keywords.
…ionMatrices() available in RasterRenderPass

This PR makes `SetViewAndProjectionMatrices(int, int, RasterCommandBuffer)` API public to users. They already have access to `SetViewAndProjectionMatrices(int, int, CommandBuffer)` which directly calls the internal overload mentioned above, but having direct access to the `RasterCommandBuffer` overload allows them to call it within their RasterRenderPass. This API being kept internal is an omission probably forgotten when releasing URP RG in Unity 6, there is no real reason of keeping it internal.
…tencil shadows

Add documentation for how shadow matte interacts with stencil and ray traced shadows.

The 'regression' happened when Anis added stencil for unlit materials https://github.cds.internal.unity3d.com/unity/unity/pull/16463/files#diff-f8f08e5234e1f79654d09936f18d3a903cdb9eccf0ab92b29d8e377a24c66845. The user can check Shadow Matte on the custom shader graph and it will work again. 

https://jira.unity3d.com/browse/UUM-72348
Fixing issues on the side panel of the Render Graph Viewer.
To fix an issue where baking for probe volumes would fail in the Editor when Switch was selected as a runtime platform

Although the shader is used in Editor, the asset import mechanism requires that the shader is valid for the selected runtime platform. We add support for Nintendo Switch in the same way that other runtime platforms are supported by the shader
Jira: UUM-83852

- Open a VFX or create a new one
- Create an operator like Noise 3D
- Connect any input port
- Collapse the node
- Save and close the VFX
- Reopen the VFX

![image-2024-10-08-11-50-42-583](https://media.github.cds.internal.unity3d.com/user/4003/files/6ac4f566-97ce-47f3-8cdf-72082e2ff3d2)

Actual result:
The port's label is not visible
Expected result:
The port's label is visible
…ar and bindMs

The values were always false despite they should be true.
…iew Prepass

This PR updates the Final Depth Copy logic in URP to always source its depth data from the depth attachment rather than the depth texture. It also updates the logic that decides if we allocate a depth attachment or render to backbuffer to ensure that the depth attachment is always available in the cases where a final depth copy is required.

In addition, this PR also removes conditions from the depth prepass logic that seem like they were intended to handle the final depth copy requirements. Now that the depth attachment allocation logic explicitly knows about the final depth copy use case, it's no longer necessary to force a prepass to indirectly allocate an attachment / intermediate depth texture.
I made a fix for https://jira.unity3d.com/browse/UUM-68986 that was too aggressive - clearing the material property block after each blit actually clears properties that are needed between blits, in the case for example of edge detection. This created another bug (https://jira.unity3d.com/browse/UUM-85970) where TexelSize property was cleared between two blits in the same pass.

Luckily, in the meantime, a better fix was implemented in Unity 6 and 7 (#51608)
This works better because it doesn't clear property blocks after each blit, only when a RenderTargetIdentifier is used, which is less aggressive than the fix I had in the first place
Editor: when doing scene filter we reuse depth from the "normal" render pass to draw selection highlights in "filter" pass, so it needs to be preserved in render graph
@UnityAljosha UnityAljosha requested review from a team as code owners November 28, 2024 14:37
@UnityAljosha UnityAljosha merged commit a65be2c into 6000.0/staging Dec 18, 2024
2 checks 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.

7 participants