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 #8065

Merged
merged 39 commits into from
Apr 29, 2024
Merged

Internal/master #8065

merged 39 commits into from
Apr 29, 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.

arttu-peltonen and others added 30 commits April 15, 2024 20:37
…priming

Fix https://jira.unity3d.com/browse/UUM-55171

This PR fixes issue where when Render Graph is active, animation preview window draws the grid on top of the preview object. This is because the grid is rendered in BiRP after rendering the preview object, and the depth buffer is not passed correctly when using URP RG. This only happens with Forward & depth priming active.

The fix has two parts:
1) UniversalRendererRenderGraph.cs: Added missing check to enable FinalCopyDepthPass. This is the same bugfix as was done earlier to fix grid depth composition: https://github.cds.internal.unity3d.com/unity/unity/pull/16980 - however the original fix was only made on non-RG codepath, so we need the same fix on the RG codepath.

2) CopyDepthPass.cs: Doing the fix above, we start to get this warning from C++: `Attempting to render to a depth only surface with no dummy color attachment` (see [GfxDevice.cpp](https://github.cds.internal.unity3d.com/unity/unity/blob/trunk/Runtime/GfxDevice/GfxDevice.cpp#L1545)). To fix this, the same fix is applied to preview camera as was done for scene view camera here: https://github.cds.internal.unity3d.com/unity/unity/pull/36665

After these two fixes, the BiRP code that renders the grid gets the URP-rendered depth buffer, and the grid is rendered correctly.

Before:
![image](https://media.github.cds.internal.unity3d.com/user/3380/files/b122b988-1f4e-4f1b-9f26-24593e9f7b62)

After:
![image](https://media.github.cds.internal.unity3d.com/user/3380/files/1e0c018b-64d2-4ad2-85d2-45453774e5b1)
Added an option in hdrp asset to trace clouds at half res instead of quarter res when dynamic res scaling goes under a certain threshold (defaults to 50%). This helps reduce artefacts.


Video below is DLSS rendering at 25% (toggled manually, this was a wip version, but result is the same)

https://media.github.cds.internal.unity3d.com/user/2154/files/fa04df77-9fac-43db-b39a-593862729667
Fix vertex color in Sprite shadergraph targets
Fixes:
- VFX error on underwater sample
- NaN errors when using scripting intercations
- Various memory leaks
- Deformation on first frame
Adds the following URP APV 2023.3 features:

- Lighting scenarios (ported from existing HDRP documentation)
- Disk streaming (ported from existing HDRP documentation)
- Sky occlusion (edited version of existing HDRP documentation)
- Replacement URP-specific to show per-object vs per-pixel lighting

Jira ticket: https://jira.unity3d.com/browse/DOCG-5305
Fixes - Setting shadow rendering layer is not changing shadow when using “shadowRenderingLayers” in the script
https://jira.unity3d.com/browse/UUM-28061

Added logic similar to UI. Where we replace base light component renderingLayerMask with either shadow or light layers. Which later used for shadow drawing.
Fixed issue when building the acceleration structure with renderes that don't have a mesh filter. https://unity.slack.com/archives/C06TQ1HNE/p1712352499863349

Also fix an issue with some meshes reported here https://forum.unity.com/threads/adaptive-probe-volumes-apvs-experimental-release-for-hdrp-in-2021-2.1238824/page-17#post-9704507

And fix formatting in doc
*[Corrects the output type of the Branch node as per DOCATT-2055.]*
Update Rendering Layer documentation following the changes to the UI in 2023.3.

Jira ticket: https://jira.unity3d.com/browse/DOCG-4356
This PR is the first pass of an initiative to improve VFX Graph documentation for Unity 6.
It is doc only and does not modify the code.

This PR contains the following addition/improvements to doc
- [volumetric output](https://github.cds.internal.unity3d.com/unity/unity/pull/43553)
- [what's new](https://github.cds.internal.unity3d.com/unity/unity/pull/44163)
- [type color scheme](https://github.cds.internal.unity3d.com/unity/unity/pull/44081)
- [activation ports](https://github.cds.internal.unity3d.com/unity/unity/pull/44450)
- [experimental feature](https://github.cds.internal.unity3d.com/unity/unity/pull/38982)
- [shader graph output](https://github.cds.internal.unity3d.com/unity/unity/pull/43637)
- [instancing](https://github.cds.internal.unity3d.com/unity/unity/pull/44241)
- [Getting started improvements](https://github.cds.internal.unity3d.com/unity/unity/pull/44228)
- [in graph profiling](https://github.cds.internal.unity3d.com/unity/unity/pull/44453)
- [Update graph logic and philosophy](https://github.cds.internal.unity3d.com/unity/unity/pull/44867)
…Jira

This PR renames the `XR SRP` area to [SRP XR](https://jira.unity3d.com/secure/insight/assets/UCW-71535) to reflect the changes in Jira.
…pport

Only package sample changes.
This PR is a forward porting of 2022.3 PR https://github.cds.internal.unity3d.com/unity/unity/pull/36366, which added 3 examples in the URP package samples > Renderer Feature.
This PR add RenderGraph support, with the Compatibility Mode (Render Graph disabled) still working.
…or RG

This PR is related to the [URP-2105](https://jira.unity3d.com/browse/URP-2105) jira epic.
The purpose of this PR is to add missing functionality by adding mip level and slices to RG's  SetInputAttachment and SetRenderAttachment functions.

This PR also introduces helper functions for Copy and Blit as well as a test to ensure the newly added functionality and the helper functions work as expected.

There is also a fix for invalid texture description for imported textures
…fest

Slack thread: https://unity.slack.com/archives/C02AS13NMGC/p1713351585445409 

> Each time we need to open the SmokeTest project with rider, we would need to add it

This PR reverts the removal of the package from the manifest, that was part of PR https://github.cds.internal.unity3d.com/unity/unity/pull/41829
Fix https://jira.unity3d.com/browse/UUM-69066

This bug happens when an user calls `Submit()` inside `ScriptableRenderPass.Execute()`. This is causing the profiler marker order to be ill-formed.

Here is more detailled explanation of what happens. URP `ScriptableRenderer.Execute` function looks like this:

```
void ScriptableRendererExecute()
{
    using (new ProfilingScope(cmd, profilingExecute))
    {
        /* ... */

        foreach (ScriptableRenderPass renderPass in renderPassBlock)
        {
            renderPass.Execute();
        }

        /* ... */
    }
}
```
And `ProfilingScope` ctor/dtor look like this:

```
ProfilingScope(CommandBuffer cmd, ProfilingSampler sampler)
{
    cmd.BeginSample(sampler); 
    sampler.inlineSampler.Begin();
}

void Dispose()
{
    cmd.EndSample(sampler);
    sampler.inlineSampler.End();
}
```
So here is what happens with this setup when Submit() is called inside ScriptableRenderPass.Execute() :
```
void ScriptableRendererExecute()
{
    cmd.BeginSample(sampler); // Deferred profiler_begin() which will be called in Submit()
    sampler.inlineSampler.Begin(); // Calls profiler_begin() immediately

    // Inside renderPass.Execute()
    context.Submit(); // Calls the deferred profiler_begin()

    cmd.EndSample(sampler); // Deferred profiler_end() which will be called in Submit()
    sampler.inlineSampler.End(); / Calls profiler_end() immediately -> Error. The previous profiler_begin() call has no corresponding profiler_end()
}
```
Since we have no way to know in advance whether a render pass will call `Submit()` or not, we can't know if it is fine to use command buffer based profiling scope without causing errors. So the fix proposed here is to simply not use that when there is a risk that Submit() is called inside of the profiling scope. We can simply stick to normal immediate profiler markers instead.
…riants for builtin-deferred

Unnecessary duplicate variants may be created due to the simultaneous usage of 
- `multi_compile_prepassfinal`
- `LIGHTMAP_ON` and `DIRLIGHTMAP_COMBINED` in different lines.

for deferred mode in built-in RP.

This PR removes the usage of `LIGHTMAP_ON` and `DIRLIGHTMAP_COMBINED` to prevent the creation of these duplicate variants.

[POI-828 [Shader System] Shader Variant Management](https://jira.unity3d.com/browse/POI-828)
Update rendergraph samples to use the new helper functions to show users how to minimize writing boiler plate code. The samples are used as documentation to demonstrate how to best use RenderGraph. This PR does not change anything in URP or the product, it improves the learning materials.
…oadable scenes for tests

This PR aims to add changes to the Graphics Performance Tests Package. The changes were made while working on the performance tests for the URP version of the Fantasy Kingdom demo. Those tests will live in the Fantasy Kingdom repo. 

**PerformanceTests.cs** : Bug where scene name is the a substring in another scene name

**TestSceneAsset.cs**: Mostly come from Remi's PR. Added additionable scenes to add in build settings, added manual alias for RPAssets that shows up in test names

**TestSceneAssetEditor.cs**: Update editor to UIToolkit
This PR fixes scene-view filtering's greyed-out rendering when using URP + Deferred (RG compatibility-mode, a.k.a. RenderGraph disabled). The scene-view filtering is masked based on the color-attachment alpha-channel, which needs to be cleared to zero before re-rendering the filtered-scene on top of it.
This PR fixes an issue where RPCore_PerformanceTests test project didn't include the top-level markers  after PR https://github.cds.internal.unity3d.com/unity/unity/pull/42503 refactored them.
…when URP + NRP RG (Vulkan/DX12)

Two coupled fixes:

1. GfxDeviceClient fix:
In `GfxDeviceClient`, unlike other `GfxDevice` implementations, native render pass workflow is not correctly supported by `GetActiveRenderSurfaceWidth()`/`GetActiveREnderSurfaceHeight()`/`IsRenderingToBackBuffer()`. For those calls, even if made in a middle of a render pass, `GfxDeviceClient` will analyze the current render target and not the color attachment of the native render pass currently rendered. 

Thus, it can generate visual mismatch if the non-active render target size differs from the color attachment size of the current active render pass. It was noticeable within UI Overlay native render pass in URP where the UI items were overstretched because of that.

2. URP Fix :
When a game was launched with an initial scene with no camera, UI Overlay was wrongly always rendered later in the engine and not in URP for the subsequent scenes. This means a loss in performance due to the absence of pass merging between Final Blit and UI Overlay pass (extra framebuffer load/store) in those scenes. To prevent this, we now enforce, at every frame, UI overlay in URP except if XR or no camera. We used to set this as the initial behavior but only enforcing it at every frame for HDR scenes. 
Because of this, UI was rendered after URP in some cases like HappyHarvest with an initial menu with no camera, and rendering UI outside of URP meant rendering it after NRPs (m_ActiveRenderPassIndex == -1) so the canvas was wrongly retrieving RenderTarget size and not NRP one.
To be fully transparent, this second fix is not the perfect one for this situation but 1. it should prevent the bug from happening again except in some obscure corner cases (user forcing UI outside of URP + NRP + UITK), 2. we need to tackle this URP bug anyway, 3. it fixes the initial bug while still limiting changes at GfxDeviceClient level which are a bit more risky.
…ertain parameters

Our user was getting flickering when using accumulation motion blur with certain parameters:
https://jira.unity3d.com/browse/UUM-59115

The root cause of this was that we were not properly handling the case that the first sample in the Monte Carlo accumulation has zero weight. I've re-arranged the formula to output zero in this case. 

Path tracing without motion blur always uses a weight of one for all frames, so it should not be affected. @pieterjan-bartels if you think there is an issue let me know.
 
I've also fixed an error in the example script in the documentation: it was capturing a screenshot in two places, only one is needed.
…Clouds.

While developping arctic demo, we noticed that the transmission in our icy moutains was still glowing eventhough they were in the volumetric clouds shadows, this was a huge discrepancy in lighting which is now eaisly fixed by this PR thanks to @adrien-tocqueville snippet.

This wasn't the case for cloud layer because it's using the cookie of the directionnal light. 


![c5784cf2073607debfbe678b852b052e](https://media.github.cds.internal.unity3d.com/user/1764/files/7819d363-b832-4a2c-856b-e9d0d4599b8e)

Fix hard earth shadow
![image](https://media.github.cds.internal.unity3d.com/user/2154/files/5adb6416-6dff-4ad4-9afc-bd154048475d)
![image](https://media.github.cds.internal.unity3d.com/user/2154/files/64573645-fe94-48e1-bfa2-de8aa97af159)
Issue introduced at https://github.cds.internal.unity3d.com/unity/unity/pull/40913

The support of `CustomHLSL` was partial and didn't consider custom HLSL and includes workflow.

The exact repro was about usage in update leaking into output generated but the issue was reproducible actually using SG in output.

➕  Additionally, the `defines` wasn't applied in SG (but it doesn't have any effect, so far, only [FillAabbBuffer](https://github.cds.internal.unity3d.com/unity/unity/blob/05bbb1aacd661908468d5fe51f2a83f96c0649f2/Packages/com.unity.visualeffectgraph/Shaders/VFXBoundsUtils.hlsl#L194) and it isn't called in any vertex or pixel shader. I introduced the define declaration for consistency.

➕ Move the `VFXCommonOutput.hlsl` in case of SG because it wasn't consistent with builtin generated code.
Fixes GC dynamic memory allocation that happens on every frame during GRD update.
![Screenshot 2024-04-08 at 13 43 24](https://media.github.cds.internal.unity3d.com/user/1206/files/d86fab3b-0b66-4d54-9597-32796463861e)
Initially code from [hackweek](https://drive.google.com/file/d/1g8j6nYsJwYIe80wRt4tBNlVbEayeGqom/view?usp=drive_link) project but made it clean because the improvement worth it and it fixes a couple of issue.

Fix some usage in CustomHLSL which is now allowing:
- `StructuredBuffer<CustomType>`: this is about UUM-64315
- `RWStructuredBuffer<float>`: Prevent usage of global buffer for this common case
- `StructuredBuffer<uint3>`: Even if we don't have `uint3` in VFX, we are allowing this kind of declaration in CustomHLSL
- `RWByteAddressBuffer`, `AppendStructuredBuffer`, `ConsumeStructuredBuffer`, `Buffer<>`, `RWBuffer<>`: I covered all common type of HLSL

🎁 Bonus, fix regression at https://github.cds.internal.unity3d.com/unity/unity/pull/45766/commits/ce1a39b236bcddaebfbc3cadb8c9e8ccdc6eaa0c which is simply a revert from https://github.cds.internal.unity3d.com/unity/unity/commit/517c85322917cc0ff86a75e2c5887d5685e2db04

## Behind the curtain

There was an illegal change of expression within custom hlsl, see this code in trunk (BuildExpression modifies parent expression):
https://github.cds.internal.unity3d.com/unity/unity/blob/eb1023001e78a637400bfa08cb0f8f4b287c381a/Packages/com.unity.visualeffectgraph/Editor/Models/Operators/Implementations/CustomHLSL.cs#L246

I changed the approach to use an intermediate expression which link the source graphicsBuffer to a usage.
The resolution of this issue has been done with this commit https://github.cds.internal.unity3d.com/unity/unity/pull/42668/commits/5f4581f13ee481f24aa27b3be8ccf1d5ac2d84ab

Here a sum up of the situation:
![image](https://media.github.cds.internal.unity3d.com/user/42/files/1f6e36a5-3725-4152-9185-f4cc874f010b)

Let's take the example of this graph:
![image](https://media.github.cds.internal.unity3d.com/user/42/files/fc4da21b-a69a-4b46-a9c9-c93d889b2ba3)

It will generates an expression graph like this:
![image](https://media.github.cds.internal.unity3d.com/user/42/files/bb629785-c551-413d-a0f7-c648086d2887)
_N.B.: In reality, if GraphicsBufferUsage are identical, expressions `1.` & `2.` are the same reference._

We are now going to track `VFXExpressionGraphicsBufferUsage` in compilation to associate every GraphicsBuffer with their future declaration.

The same approach is use in CustomHLSL block, operator but also with SampleBuffer.


## Known Limitation
- There is an issue regarding include in custom HLSL => UUM-64570, there is a repro in `026_CellularAutomaton` (which a workaround)
- Diverging usage of buffer can be marked as illegal when it's technically possible:
![image](https://media.github.cds.internal.unity3d.com/user/42/files/16eb1b4a-7dc6-4671-9383-44478694a680)
_N.B.: I think we can considered this limitation as a bug, we are gathering usage globally while it should be done only per context._ , this is an issue which can be fixed with UUM-66620 (this also an issue where usage should be computed per context)
- We aren't supporting `RWTexture2D<float>` and other, it would require the integration of texture usage pattern, I would like to explore this in the future.
To allow the console HDRP VFX tests to run a set of images are provided and a small fix applied
We now inform users about light limits in URP when running in the editor.
https://jira.unity3d.com/browse/UUM-56213
alex-vazquez-unity3d and others added 9 commits April 24, 2024 18:43
… rendering into a texture and having a renderScale.

When Render scale != 1 and we are rendering to a Render Texture. The NativeRender pass was creating the RenderTextureDescriptor with an invalid dimensions.

Make sure that Attachements sizes are the same as pass sizes.

https://jira.unity3d.com/browse/UUM-61468
Fixes the issue with accumulating ambient probe lighting when sky light multiplier is not equal to one.

https://jira.unity3d.com/browse/UUM-69385
…public

Fixing URP frame resource setters that should not be public. This avoids many kinds of user errors and bugs due to unexpected behavior. It also minimizes the issue of frame resource handles and global texture reference becoming out-of-sync. This is a partial fix to https://jira.unity3d.com/browse/UUM-69948

This is new API that landed in 23.3 but that accidently made setters public that should not be, since it's incorrect user behavior to set these. This PR adds guardrails to guide users better. Since this is new API, and a very advanced use case, this should only impact a handful of users at most.
* Significant performance optimization for loading scenes. On the avalanche demo scene, APV loading goes from **15 minutes** to a few seconds.

* Fixed subdivision that would create bricks of unwanted subdivision level when using the max amount of subdivisions.

* Fixed file writing error occuring randomly that would make the bake result not be written to disk

* Fixed validity sampling not taking light layers into account

* Fixed rounding error in probe volume sampling

* Fixed floating point issue when computing the size of cells during baking which would lead to asserts during runtime

* Minor optimization of runtime sampling code

* Fixed error with sample gi node in builtin

* Improve debug views by adding NdotL to probes
Skip the foam reprojection pass if foam region hasn't moved.
Saves like half a millisecond on PS4 without visual change
Update the documentation to remove mentions of Unity 2023.3 and replace it by Unity 6
…enabling another mode

Fix https://jira.unity3d.com/browse/UUM-70780

HDRP Rendering Debugger UI maps multiple different enum dropdowns into a single internal enum, and therefore these modes are mutually exclusive. The mechanism to do this relies on enumIndices that get reset. In this case the enumindex reset was missing, causing the enum dropdown UI drawer to get stuck in the previous value even though the internal enum value had changed.
Platform image updates for SRP testing
@UnityAljosha UnityAljosha requested review from a team as code owners April 29, 2024 09:15
@UnityAljosha UnityAljosha merged commit c8df1d8 into master Apr 29, 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.