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/master #8093

Merged
merged 117 commits into from
Aug 20, 2024
Merged

Internal/master #8093

merged 117 commits into from
Aug 20, 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.

Richard-Horton and others added 30 commits July 2, 2024 17:00
Adds documentation pages to URP and HDRP docs for STP and associated functionality
Jira: [UUM-74375](https://jira.unity3d.com/browse/UUM-74375)

Variadic attributes such as `scale`, `pivot`, `angle` and `angularVelocity` should not be supported in custom hlsl code.
But the big refactor done when the [custom attributes were introduced in the blackboard](https://github.cds.internal.unity3d.com/unity/unity/pull/25236) changed this behavior and led to unexpected behavior.

Now, when the user uses variadic attributes, the VFX do not compile and an error feedback is displayed in the graph.
![Unity_IsptmUvhFU](https://media.github.cds.internal.unity3d.com/user/4003/files/d8a3d6a8-9707-4e96-bc5e-69110d3af270)
…t for Android

The ShaderGraph_Android_Vulkan_Standalone_il2cpp_Linear pipeline is failing due to ShaderGraphGraphicsTests.TransformNode test failing. In order to stabilise the pipeline, we will disable this test for Android.

The initiative is under: UUM-74903
Fixes the ToC link to Render Requests
Fix [this issue](https://jira.unity3d.com/browse/UUM-74949) raised by this [forum discussion](https://forum.unity.com/threads/solved-set-reflection-probe-resolution-in-script.1606875/).
TLDR:
ProbeSettings.cubeResolution was internal, making it impossible to change the resolution of a cube reflection probe at runtime, unless using Reflection or ASMRef.
The use case for this is to change the resolution of a realtime or on demand probe depending on it's distance to the camera, which is quite a valid use to save a bit of performances.
Even though HDRP have a limit 16 you will see all defined Rendering Layers but you can't select ones that are greater than 16.
…dering

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.
This PR implements the Multiview Per View Viewports (MVPVV) extension for Quest. The goal of Phase 1 is to get MVPVV working on the BiRP and URP template scenes for the base use case; which is forward rendering into eye textures and only the last pass on URP.

MVPVV allows developers to set multiple viewports and scissors per viewport. In the context of VR rendering, this enables the drivers to skip shader invocations (and rendering work) for screen areas outside of the user’s view (these are the nasal regions that the user can't see within the headset). Note that it only works with symmetric Field of View (FOV).

As part of Phase 1, our current enablement is guarded by the "-enable-vulkan-mvpvv" argument.

This change is also needed in xr.sdk.oculus to pass the two different viewports for each eye: https://github.cds.internal.unity3d.com/unity/xr.sdk.oculus/commit/64c4079096f110172f0dea6fe465323b4c70cd56

There are no issues running without the above xr.sdk.oculus change, but we won't be reducing any rendering work.
However, if we apply the above xr.sdk.oculus change without this engine change (+"-enable-vulkan-mvpvv"), then the views will be distorted.
… set to None

If there are several instances of a VFX that have a exposed texture used in the Output context, and one of them is overriding the exposed texture value with an empty texture (None), this VFX will not render with the expected texture, and will use any of the textures used by the other instances.
This occurs because NULL textures are ignored when set directly during rendering.
This PR avoid using NULL textures and selects the correct default texture early.
The expected texture color should be:
- Using Shader Graph: Defined by the default value of the exposed texture in SG (White, Black, Blue...)
- Not using Shader Graph: Gray 50%

The issue is easy to reproduce, just create 2 or more instances of a VFX using a exposed texture in an Output context and override the exposed texture in one of them with None. More than one VFX should be visible at the same time for the issue to reproduce.
Depending on the version, the issue could be a random flickering texture or a stable wrong texture. This fix works in both cases.
fix BlitAndSwapColorPass sample that used editor namespace and couldn't build
When duplicating a scene, the PerSceneData gameojbect serialized inside a scene will contain out of date information
The baking set may have changed (eg. if we are in single scene mode) and the scene guid will obviously always be different.
We're fixing the failing SRP jobs using a GPU. I fixed some light unrelated issues along the way.

**Threads**: 
- https://unity.slack.com/archives/C3KQ4UD6V/p1719409357316169
- https://unity.slack.com/archives/C6Y79CZM0/p1719409151788219

**Fix**:
- **SG Builtin Foundation**
    - Add failing test scenes ([vulkan](https://unity-ci.cds.internal.unity3d.com/project/3/branch/trunk/jobDefinition/.yamato%2Fsrp%2Fshadergraph-builtin-foundation.yml%23shadergraph_builtin_foundation_linux_vulkan_playmode_mono_linear), [dx11](https://unity-ci.cds.internal.unity3d.com/project/3/branch/trunk/jobDefinition/.yamato%2Fsrp%2Fshadergraph-builtin-foundation.yml%23shadergraph_builtin_foundation_win_dx11_playmode_mono_linear)) to the Shadergraph Builtin Foundation project test filter.
- **HDRP Perf**
    - Update the BuildPlayer method of the graphics performance test framework to target the development player, as this is what the hdrp job dependency is providing (ABV Subset). Bump the framework package to 2.1.2 as a result.
    - Move the job to a `-sp1` bokken image flavor for less standard deviation
    - Move the job to weekly cadence as it's a bit long now that we increased the timeout for the job to pass.

**Cleanup**:
- Add missing .meta files in the Shadergraph Builtin Foundation project
- Remove invalid use of `{{test_filters.urp_postpro_editmode}}` to make the `yamato-parser` command work again
- Remove missing scenes from the Shadergraph Builtin Foundation project test filter 

![image](https://media.github.cds.internal.unity3d.com/user/2280/files/956d1662-e550-4014-b918-b406464a0214)
Added a 5pixels wide gaussian blur pass to the atmospheric scattering buffer to fix artefacts
Initially reported here https://unity.slack.com/archives/C022VGK09HT/p1718883356960729

Note that the gaussian blur parameters can be adjusted if needed
…o new files, removed uids from includes in the snippets folder.

Changed the prefix from "urp-docfx-" to just "urp-". Added uids to two new files, removed uids from includes in the snippets folder.
Quick PR to fix the wrong log messages, as we discussed :-)
RTX light cluster was 64x64x32 which means less resolution on the z axis than the x axis which doesn't really make sense
This was likely an error, so switched the cluster to 64x32x64 to have less resolution on the y axis, as clustering on the y axis is usually less effective

Additionally, fixed light culling that was not happening when raytracing is enabled. This means all probes/lights/decals were uploaded to the gpu, even if they are outside of the light cluster. For pathtracing, we always skip reflection probes as they are not required, and since the light cluster size is much bigger for PT, it would always throw errors.
Note: i didn't implement cpu culling for decals but it would be nice to do it
JIRA: https://jira.unity3d.com/browse/UUM-55243

There were some issues with Universal Graphics Test 262 but it looks like it has been fixed! This PR reenables the test for MacOS and IPhone
Fix for UUM-19101

The shader utility function Linear01DepthFromNear does not work for APIs with reversed Z buffer (all of our APIs except OpenGL/GLES). I changed the function into one that works and updated some comments in shader code.

```
float Linear01DepthFromNear(float depth, float4 zBufferParam)
{
    // Old:
    // return 1.0 / (zBufferParam.x + zBufferParam.y / depth);

    // New:
    #if UNITY_REVERSED_Z
    return (1.0 - depth) / (zBufferParam.x * depth + zBufferParam.y);
    #else
    return depth / (zBufferParam.x * depth + zBufferParam.y);
    #endif
}
```

![image](https://media.github.cds.internal.unity3d.com/user/7119/files/beac76da-e6e4-4db1-8d3c-f4c6ef08b37d)
Fixing a small issue that was overlooked on the timeofday script in recently published environement sample. 
This prevented for the speed to have any effect in the time of day.
Added the missing API `RequestAsyncReadbackIntoNativeArray` in the UnsafePass.
I used the `CommandBufferGenerator.cs` to regenerate the file.
- Add missing graphics settings in URP (shader variant logging, render graph, probe volume streaming assets)
- Clarify shader variant logging information, and split BIRP, URP and HDRP information as per our content model

Jira ticket: https://jira.unity3d.com/browse/DOCG-5763
Fixes [UUM-74727](https://jira.unity3d.com/browse/UUM-74727).

Disc lights are Lambertian emitters, which the disc light area calculation doesn't account for. This small change fixes this.
Add note that GPU Resident Drawer doesn't support OpenGLES 

Jira ticket: https://jira.unity3d.com/browse/DOCG-5722
Document the Adaptive Probe Volume Seam Reduction feature.
Fix clouds rendering on top of geometry.
Fix depth pyramid allocated with the wrong size (it uses the rt size instead of the actual viewport size)
Fixed water debug modes
- Added What's new entry to SRP Core package for Unity 6.
… support)

Documentation for custom post-processing effect template (with Volume support).
Document the Adaptive Probe Volume Seam Reduction feature.
alelievr and others added 26 commits July 31, 2024 10:02
Fixed offscreen scaling when enabling HDR
Let's consider a simple scenario, a render graph with 3 passes:
- `myPass1` (index 0 in the contextData.passData `NativeList`)
- `myPass2` (index 1 in the contextData.passData `NativeList`)
- `myPass3`  (index 2 in the contextData.passData `NativeList`)

Until now, when merging multiple passes together (so the 3 previous passes into 1 NativePassData; assuming the merge can be done), we were not considering the fact that a pass can be culled.

So without any pass being culled, when merging the passes together, it gives a `NativePassData`, with the value:
```
myNativePass.firstGraphPass = 0; (index 0, representing `myPass1`)
myNativePass.numGraphPasses = 3; (since we are merging 3 passes together)

// You can retrieve the last index by doing:
var lastGraphPass = myNativePass.firstGraphPass + myNativePass.numGraphPasses - 1;
// So it gives lastGraphPass = 0 + 3 - 1 = 2 (index 2, representing `myPass3`)
```
So when iterating through all the passes, like this:
```
for (int i = 0; i < nativePass.numGraphPasses - 1; i++)
```

It works well when we don't have any culled passes, since for this scenario, `numGraphPasses` is equal to 3 as expected.

Now let's assume `myPass2` has been culled; the previous `for` doesn't work anymore, since `numGraphPasses` is now equal to 2. 
So we will iterate the first 2 passes of `contextData.passData`, so `myPass1` and `myPass2`, but it's wrong. We should iterate over `myPass1` and `myPass3`, skipping `myPass2` since it has been culled.

This wrong logic creates many bugs in the core RenderGraph system and the viewer.

Here's how this PR fixes this bug. 
Now, in addition to storing the index of the first pass (`nativePass.firstGraphPass`) and the number of passes (`nativePass.numGraphPasses`), we also store the index of the last pass (`nativePass.lastGraphPass`). 

So we can now calculate the `real count` of passes, including the culled ones, by doing :
```
var countPasses = nativePass.lastGraphPass - nativePass.firstGraphPass + 1;
```

Which gives, for our previous example (myPass2 being culled):
```
var countPasses = myPass3 (index 2) - myPass1 (index 0) + 1;
var countPasses = 2 - 1 + 1 = 3 passes
// This is now correct, it includes the culled pass. If we were using nativePass.numGraphPasse, it would gives 2, which is wrong since myPass2 has been culled.
```

With this fix, when iterating through passes, we simply check if the pass has been culled or not, avoiding using it or updating it as we were doing before. And we are now updating the non-culled pass, since now we iterate through them.

Here's a gif with the problem in action, before resolving it with this PR:

![WeirdMerge1](https://media.github.cds.internal.unity3d.com/user/1911/files/5a461c19-286d-4df2-8495-ecd480763c7a)
With Flipbook player block improvements, the flipbook size property in Output changed from Vector2 to Flipbook.
This affected upgrading, because those types were not compatible at that time, and links would be broken.
This PR adds compatibility, and ensures safe upgrading.
Minor tweaks to URP and GfxDevice to enable HDR on visionOS. This is currently in draft because I have some unanswered questions I'd like to discuss with area experts (graphics) before I go much further. Questions are below in comments to reviewers. There is a [slack thread](https://unity.slack.com/archives/C89KFUUCT/p1717028887300969) with some more context here. I've created a [bug ticket](https://jira.unity3d.com/browse/UUM-73695) as well.

Basically, without this fix, when you enable HDR while XR rendering is active, you see this:
![image](https://media.github.cds.internal.unity3d.com/user/59/files/8f5d92f1-19bd-4215-a704-9ecb70b48d38)

And with it, you see this:

<img width="457" alt="Screenshot 2024-06-06 at 12 49 35 PM" src="https://media.github.cds.internal.unity3d.com/user/59/files/fc51a75f-beba-4060-809c-e364275a94b4">
JIRA Tickets:
[UUM-75218](https://jira.unity3d.com/browse/UUM-75218) - Errors with Additional Lights Cookie Atlas
[UUM-75205](https://jira.unity3d.com/browse/UUM-75205) -  Errors with Motion Vector Map Overlay

In the render graph rendering debugger, use blit utils such as AddCopyPass and AddBlitPass instead of doing our own blit
*Updates the Shader Graph window topic to include changes made to the SG window UI.*
…'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)
…ostProcessing.

Jira: [UUM-66399](https://jira.unity3d.com/browse/UUM-66399)

Cause : There are same name("_BlitTexture") textures in s_propertyBlock(MaterialPropertyBlock) and global texture.

If you use BlitTexture() which uses RenderTargetIdentifier (not RTHandle), it will set global "_BlitTexture" which will be overwritten by s_propertyBlock's "_BlitTexture".

This makes source texture for BlitTexture() not set.
Especially, If you are using blit after the postprocessing (UberPost), the blitter will use render texture before the postprocessing as a source texture. 

Solution : use different materialPropertyBlock for BlitTexture() with RenderTargetIdentifier.
Fix UUM-74448
The RenderGraph implementation of the full screen render feature is inefficient with memory.

It uses all global textures, which is unneeded in most if not all cases, increasing memory usage because the lifetime of the resources is longer than needed.

This PR uses the input requirements set on the feature to set what global is used. 

This can be a significant memory improvement/optimization when using the full screen render feature under certain circumstances.

Ideally, we avoid the copy when fetching the current camera color. We can do this by swapping the camera color resource. However, we would need to be sure that the new resource is completely overwritten, not only partially which requires the copy. That requires user input, ie, new UI. Additionally, we could optimize the copy using framebuffer fetch using RenderGraph.AddCopyPass utility. This is out of scope for this PR.

This PR also improves the 176_FullScreenPass_MultiPass_StencilTest automated tests. The green checkerboard pass doesn't require the color fetch so it's turned off now. This ensures we also test that code path, increasing coverage. 


before
![image](https://media.github.cds.internal.unity3d.com/user/1031/files/f25b8da4-c960-423d-8328-ca5d266cf57c)

after
![image](https://media.github.cds.internal.unity3d.com/user/1031/files/23ad240c-dd64-4aa7-9539-eb242d1902ab)
[Jira: https://jira.unity3d.com/browse/UUM-75655](https://jira.unity3d.com/browse/UUM-75690)
Addressed camera end callback issue in XR.
PR handles 3 cases:
Multipass last pass: pass ID == 1, viewCount == 1
Singlepass last pass: pass ID == 0, viewCount ==2
Emptypass(non-XR) last pass: pass ID == 0, viewCount == 0
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.
This PR fixes some seams and leaks that can occur between two subdivision levels when using Rendering Layers. 
https://jira.unity3d.com/browse/UUM-76513

By default, APV hides these seams at bake time: probes on the boundary between two subdivision levels can have their values modified, with smaller subdivision levels receiving pre-interpolated values from the larger level.

However, this mechanism did not take Rendering Layers into account. Probes could receive values from other probes even if they did not share any common rendering layer, resulting in seams and leaks at the boundary between subdivisions. This PR fixes this, ensuring that values are only interpolated between probes that share at least one common rendering layer.

Let's take a look at this example, there are two subdivision levels in the room
![image](https://media.github.cds.internal.unity3d.com/user/5677/files/8ba70616-a9c8-4a82-9ccb-9603010bba5f)


Before: The seam reduction mechanism did not work correctly and created additional leaks. Dark probes from other rooms (with different Rendering Layers) were used when interpolating the values for the probes at the boundary between two levels, creating some dark leak and a visible seam. 
![RenderingLayer_Seams](https://media.github.cds.internal.unity3d.com/user/5677/files/162b38c6-15e5-48b6-9530-64002e522b33)

After : The seam reduction mechanism is only using probes that share at least a common Rendering Layers (in this case, only probes from this room) resulting in no leak or seam
![RenderingLayer_NoSeams](https://media.github.cds.internal.unity3d.com/user/5677/files/1636e25a-b236-4593-9574-c44ad285c9a4)
Address a few issues with shader graph shader variant limit.
This change removes the option to use Depth Priming in URP when MSAA is also enabled. If MSAA is enabled, a new GUI warning box will appear when trying to enabled Depth Priming, informing the user that this configuration is not possible.

The reason for this change is that Depth Priming with MSAA is not fully implemented and causes several bugs, but it is not performant either, which undermines the point of using Depth Priming in the first place.

![image](https://media.github.cds.internal.unity3d.com/user/7119/files/375d0bfc-b462-4feb-ab38-583c6b74863f)

This fixes multiple bugs:
https://jira.unity3d.com/browse/UUM-59480
https://jira.unity3d.com/browse/UUM-65800
https://jira.unity3d.com/browse/UUM-72556

Additionally, a few URP Depth Priming tests have been updated to ensure that they do not use MSAA so they remain functional. One test has been disabled for XR because that configuration forces MSAA.

Docs ticket: https://jira.unity3d.com/browse/DOCG-5943
POI: https://jira.unity3d.com/browse/POI-585
Epic: https://jira.unity3d.com/browse/GFXFEAT-206
Ticket: https://jira.unity3d.com/browse/GFXFEAT-687

As this PR description will be public, see instead the details under [the epic](https://jira.unity3d.com/browse/GFXFEAT-206).

**NOTE:** This is a continuation of https://github.cds.internal.unity3d.com/unity/unity/pull/49473, and the final PR with batched changes we intend to land.
…setting

This PR addresses the issue, suggested by a partner, of Forward+ in URP always blending reflection probes regardless of what setting is chosen. This now enables Forward+ to use non-blended reflection probes, saving performance on platforms that do not require blending.

Jira: GFXFOUND-730
This PR fixes https://jira.unity3d.com/browse/UUM-75670.

When an auxiliary window like a color picker shows up, it takes focus away from the SG window. This causes the main preview to stop updating, and does not let the user see the color picker result until it is closed.

By instead using visibility (determined by the parent window's BecameVisible/BecameInvisible events) as the determiner, we can display the result but also stop processing when the window is hidden.
…ocessing feature template

URP RenderGraph [Jira](https://jira.unity3d.com/browse/URP-1297).
This is documentation in the form of sample code in the asset template.

This PR provides more performant sample code that is part of the template to generate a new URP Post Processing Effect (with Volume).

This change removes the cameraColor copy from the template. This code is meant as a sample and a starting point for users to implement their own. 

The assumption is that this is the most common usage case, the full screen (every pixel) is written to, so we can save the copy and use the new ContextContainer resource swapping.

![image](https://media.github.cds.internal.unity3d.com/user/1031/files/fd65699e-01c7-460f-8a3a-09acd275d08b)

After (no copy, new resource is used by next pass)
![image](https://media.github.cds.internal.unity3d.com/user/1031/files/e4b8a732-7881-4913-aa17-d1b3cf311881)
https://jira.unity3d.com/browse/PLATGRAPH-3569

WebGPU errors when the SSAO blur mode is Gaussian (Medium) or Kawase (Low).

SSAO.hlsl  HorizontalGaussianBlur,  VerticalGaussianBlur, and KawaseBlur return half.

The RenderTexture created in ScreenSpaceAmbientOcclusionPass.cs is RGBA8_SRGB when the color space is linear, and R8 when the color space is gamma. This is because there is no R8_SRGB texture format, and it switches the texture format to match the SRGB mode of the camera texture.

WebGPU strictly validates that the number of channels a shader outputs must be >= the number of channels of the color texture target.

With RGBA8_SRGB, the color texture target has 4 channels, and the shader outputs 1 channel, WebGPU throws a validation error.

The fix is to have the blur functions return half4 instead of half. Even when the color texture target is R8 and has 1 channel, the validation rule still passes and there is no error.

This fixes the 202_SSAO* tests in UniversalGraphicsTest_Foundation for WebGPU.
- I never was able to reproduce the issue described by a user.
- I asked them for a repro project, but they moved to a different API (ImportTexture) and are not interested to spend more time on the `ImportBuffer` one.
- They told me they will be able to send us their project in the near future, so we might want to investigate what happened (spending some time to try to reproduce the problem described)
- Meanwhile, I'm pushing this Unit Test, proving the API works well.

This PR is also fixing the Gizmo test, which was failing from time to time on Yamato and 100% of the time on my computer, locally.
@UnityAljosha UnityAljosha requested review from a team as code owners August 20, 2024 12:43
@UnityAljosha UnityAljosha merged commit fc267ad into master Aug 20, 2024
2 of 3 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.