Internal/master #7947
Merged
Internal/master #7947
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is a prerequisite before I can start some experiments.
The goal of this PR is to improve and fix HDRP runtime tests. - Cleaned up usage of HDRP assets. There were many more variants than required which led to increased build time and inconsistencies (scenes were run in forward instead of deferred or vice versa). Also made sure to only use assets from the RuntimeTest Projects and not the shared test package (changing anything in those can lead to failures in other test projects without knowing). - Removed NoMotionVector variants of Virtual Texturing tests as they are no long relevant. - Moved to forward without MSAA most scenes that were using MSAA (saves memory and avoid crash on some devices) - Kept MSAA for 001 and 002 as they are testing a lot of different things. - Moved 001 and 002 as a temporary fix for out of memory issues on Xbox (Chaining MSAA scenes then non MSAA scenes cause a memory spike before MSAA textures are deallocated which leads to crashes when low memory is available on some platforms). This fixes Xbox runtime test crash. - Increased threshold of 001 to avoid instability. - Small changes made 002 pick up a lighting failure on PS4/PS5 that was actually already present. Disabled the test and filed a case: https://jira.unity3d.com/browse/UUM-45673 - Moved TestAnimationCurveVolumeComponent to the test file because it would cause serialization issues. - Regenerated the full shader variant list - Update some assets to new versions. - Only other known remaining failures are on Mac where MSAA is broken. Also renamed a lot of HDRP assets in the common test package to make them consistent with their rendering mode (deferred/forward/both).
…e original Bloom mip While making this [simple update to the HDRP Template](#7934), we discovered an issue on the bloom when the Bloom Mip Bias was > 0 on the Screen Space Lens Flare override. (cf [video](https://unity.gyazo.com/9b281901f2c92793cd1f06c494db6d18)). In the video, the bloom on the overexposed part shouldn't change when adding SSLF, only the flares should be added. Recently, a change was made to save costs. This change was basically to remove the SSLF buffer and write (adding) its result directly to the bloom buffer so that everything is composited once on the Uber shader. Before this PR, we were: - Passing the bloom mip X to the lens flare function - Doing the lens flare stuff based on mip X - Adding the result on the bloom mip X texture - Writing the result back on the original bloom texture. Which was bad if we used a mip other than 0, since it was basically writing higher mips into the orignal "crisp" bloom texture. Now, after this PR, we are: - Passing the bloom mip X to the lens flare function **alongside the original bloom texture** - Doing the lens flare stuff based on mip X - Adding the result on the **original bloom texture** - Writing the result back on the original bloom texture. Before => [video](https://unity.gyazo.com/f472eb289829bb1787c6c39a0636d873) (Observe the bloom on the post light) After => [video ](https://unity.gyazo.com/d064999e4dbfb1be2bd44e9e8b4a9803)(Now the bloom on the post light doesn't change when adding / removing SSLF This PR was also the opportunity to do 2 small refactor for better future maintenance, basically moving things from SRPs side to Core. This does not save costs, but avoid modifying things on one end and forgetting to do it on the other side: - First one was to move ShaderIDs related to the SSLF shader to Core and remove them from HDStringConstants.cs and PostProcessPass.cs - Second one was to move the small computation we do with the lens flare parameters to core as well since they are the same between SRPs
Fixes an issue where the LightProbeGroup inspector had a help box spilling over text. Mitigated it by removing an unnecessary linebreak and made the help box resizable. <img width="341" alt="Screenshot 2023-08-03 at 11 35 50" src="https://media.github.cds.internal.unity3d.com/user/3133/files/922831c4-d02b-4d24-8dd8-3306b46a3a62">
…or is not available On standalone there is a potential modification of the HDRP Global Settings from the Compositor. Removed the automatic registration of the Custom PP into the Global Settings. Instead we are using the Render Pipeline Asset.
… association for Android and WebGL platforms in srp test projects [The URP team want to add WebGL to their automated testing](https://docs.google.com/document/d/1FGe_FWmklt3szCr6Mgr0pfljw4ADRA-5S30VCNRopsc/edit#heading=h.n6dr00fcffwu), specifically the QV set in order to detect potential issues before we make it to trunk. This PR updates the local UTF reference for SRP jobs to the to the latest [com.unity.testframework.graphics package, version 8.0.0-exp.1.](https://github.cds.internal.unity3d.com/unity/com.unity.testframework.graphics/pull/163). This updated version introduces new asynchronous asset bundle loading method for Android and WebGL in the following test projects: - the [com.unity.testing.urp package test base class ](https://github.cds.internal.unity3d.com/unity/unity/blob/ant/seans/add-webgl-support/Tests/SRPTests/Packages/com.unity.testing.urp/Scripts/Runtime/UniversalGraphicsTests.cs)and thus all projects that use it, eg URP Foundation, PostPro, Global Illumination - [VFX Graph](https://github.cds.internal.unity3d.com/unity/unity/blob/ant/seans/add-webgl-support/Tests/SRPTests/Packages/com.unity.testing.visualeffectgraph/Tests/Runtime/VFXGraphicsTests.cs#L46) - [Multiple SRP](https://github.cds.internal.unity3d.com/unity/unity/blob/ant/seans/add-webgl-support/Tests/SRPTests/Projects/MultipleSRP_Tests/Assets/Common/Editor/BaseGraphicsTests.cs#L20) - [Shadergraph](https://github.cds.internal.unity3d.com/unity/unity/blob/ant/seans/add-webgl-support/Tests/SRPTests/Projects/ShaderGraph/Assets/CommonAssets/Scripts/ShaderGraphGraphicsTests.cs#L13) Additional updates to support this include - Update UTR version to 1.21.1; this version includes the WebGL updates from the test framework team - Update WebGL bokken images to automation-tooling/win-10-21H1-unity:v2.945135 with browsers installed.
Fixes https://jira.unity3d.com/browse/UUM-45842 When you create/clone the default volume profile through global settings, the code was missing AddObjectToAsset call for each created component. This caused the new asset on disk to be empty even though the volume profile appears to have all components inside the editor. Adding the missing call (it is required because VolumeComponent is also a ScriptableObject) fixes the issue.
This PR aims to optimize the BatchRendererGroup shader variant code, especially for low-end platforms such as Android. Specifically, I've been running with Quest 2 on my side. The PR mainly adds two optimizations.
### Caching instanced property loads
This one attempts to fix the shader code-gen by caching some property loads. With BRG shader variants, we override material properties such as `_BaseColor` like this:
`#define _BaseColor UNITY_ACCESS_DOTS_INSTANCED_PROP_WITH_DEFAULT(float4, _BaseColor)`
This macro is expanded to a function which is going to branch and either load the property directly from the material cbuffer, or run some ALU operations to compute a buffer address based on the current instanceID + property metadata. It then loads the property from this buffer.
This computation is not free, and it seems that using for example `_BaseColor` multiple times in the shader will results in this function being run multiple times. It looks like the compiler is unable to optimize this case by itself.
This PR tries to fix the problem by calling this function only once per property (if ever used) at the beginning of the shader, and then store the result in a static value. The macro is then defined directly as `#define _BaseColor MyStaticValue` so that there is no risk for the loading computation to be regenerated.
This optimization improved GPU time by ~10% on Quest 2 with the URPLitProperties scene. FPS counter also went from 50 to 55 with this setup.
### Static branch for instanced property loads
The second optimization is about turning the dynamic branch in the instanced property loading code to a static branch when possible. Loading a property such as the `_BaseColor` can be done either from the material cbuffer, or from the big raw buffer. Selecting from where the data must be loaded is done dynamically by checking the metadata high-bit in the shader.
The cost of this branch has been negligible on high/mid-end platforms, but for low-end platforms it has a pretty big impact. The thing is that not all properties usually need to be instanced in the game. You might want to instantiate the `_BaseColor` but not the `_Smoothness` for example. It depends entirely on the game and this is something users need to be able to control so that they don't pay the cost of the expensive dynamic branch for a feature they do not use.
Ideally we would have a nice UI for this, but multiple discussions suggest that there is a lot of technical problem with that. So the goal here is to provide some simple utilities allowing users to control which properties they want to instantiate by modifying the shader themselves. It's not ideal UX wise, but at least it's something we can do now.
This PR introduces 4 new macros:
- `UNITY_DOTS_INSTANCED_PROP_OVERRIDE_DISABLED`
- `UNITY_DOTS_INSTANCED_PROP_OVERRIDE_ENABLED`
- `UNITY_DOTS_INSTANCED_PROP_OVERRIDE_REQUIRED`
- `UNITY_DOTS_INSTANCED_PROP_OVERRIDE_DISABLED_BY_DEFAULT`
The three first ones allow you to specify in the metadata cbuffer definition which properties can or must be overridden per instance. The last one is a config define that allows you to change how the default `UNITY_DOTS_INSTANCED_PROP` macro behaves. So for example we could imagine a cbuffer definition like this one:
```
UNITY_DOTS_INSTANCING_START(MaterialPropertyMetadata)
UNITY_DOTS_INSTANCED_PROP_OVERRIDE_ENABLED(float4, _BaseColor)
UNITY_DOTS_INSTANCED_PROP_OVERRIDE_DISABLED(float4, _SpecColor)
UNITY_DOTS_INSTANCED_PROP_OVERRIDE_REQUIRED(float4, _EmissionColor)
UNITY_DOTS_INSTANCED_PROP(float , _Cutoff)
UNITY_DOTS_INSTANCING_END(MaterialPropertyMetadata)
```
Here is what this declaration means:
- The `_BaseColor` property can be either instanced or not. A dynamic branch will be emitted so that the shader is able to fetch the data from the material cbuffer or the `unity_DOTSInstanceData` buffer depending on the metadata high-bit value.
- The ` _SpecColor` property is not instantiable. The property will always be loaded from the material cbuffer, and no dynamic branch is emitted in the code.
- The `_EmissionColor` property **must** be instanced. The property will always be loaded from the `unity_DOTSInstanceData` buffer, and no dynamic branch is emitted in the code.
- The `_Cutoff` property can be instantiable or not depending on the config define.
If `UNITY_DOTS_INSTANCED_PROP_OVERRIDE_DISABLED_BY_DEFAULT` is defined, then `_Cutoff` is not instantiable will behave just like `_SpecColor` in this example.
If `UNITY_DOTS_INSTANCED_PROP_OVERRIDE_DISABLED_BY_DEFAULT` is **not** defined, then `_Cutoff` is instantiable and will behave just like `_BaseColor` in this example.
Disabling instancing for every material property improved the GPU time by ~20 to 40% on Quest 2 with the URPLitProperties scene. So this is likely something you want to keep disabled for the majority of the material properties on low-end platforms considering the performance impact it has.
Fix https://jira.unity3d.com/browse/UUM-46030 NullReferenceException would happen when clicking on the context menu next to "Volume Profile" if the currently assigned VolumeProfile is "None". This PR fixes that.
This is a fix for the reopened bug UUM-2386. The previous fix only fixed the issue going forward but didn't fix the users who already had the issue. This fix will help the remaining users.
* [POI-817](https://jira.unity3d.com/browse/POI-817) Render Graph Adoption in URP * [Jira Epic](https://jira.unity3d.com/browse/URP-1805) * [TDD](https://docs.google.com/document/d/1TGxzYZ4Ah9LYLFSEfkOHd_2qPVTC52edONKrb5Qj5Po) This PR does the following: * Moves all frame resources from the `FrameResources` class in to `UniversalResourcesData/Universal2DResourcesData`. * Makes URP's internal code use the resourcesData directly instead of `FrameResources` * Makes `FrameResources` log an error if `GetTexture()` or `SetTexture()` are called.
…er results This PR fixes an instability in the test "Ensure_Camera_Command_Culled" that started occuring recently. Concretely it adds some waiting frames before starting to record profiler markers used in the test. The structure is now more similar to what is done in VFXCheckGarbage.
So due to the fact that soft shadow quality levels are controlled by dynamic branches, this has a huge effect on lower end platforms. This PR tries to fix that by disabling per-light quality levels and use only the global setting in the URP Asset. This affects HL/Quest, but can be expanded easily to other platforms as well. This PR adds some tests for this as well in the shader stripping tests. The keyword number shouldn't change too much for all platforms, as the shader stripper should remove all the new additions on other platforms and remove the old variant on the Quest/HL Adds warning box as well when there is at least one XR provider 
APV In general, supportRuntimeDebug, must only be checked on Standalone builds. As when the editor is present the debug shaders are available. This was making that if in the Global Settings you had strip runtime debug shaders. The Rendering Debugger was not showing the panel for Probe Volumes Besieds this, the HDRenderPipeline. was initializing the ProbeVolume by getting the if the APV was supported from the asset. This can leak resources, and not unregistering the debug panels if, you change the asset value to not support APV. As the disposal of the pipeline is done after the modification of the RP asset. Render Graph, The Render Graph was duplicated into Rendering and Render Graph panels. Making sure it only appears in Render Graph panel.
This PR Fixes an invalid motion vector frame index when graphics jobs are enabled, it is the reason why URP_Terrain tests 300_Motion_Vectors_SpeedTree is failing on some platforms as described https://jira.unity3d.com/browse/UUM-40684 I have also updated the 3 reference images for these tests , and also added a missing reference image.
Fix billboard depth prepass for SpeedTree7 and grass details on terrain system. Added test scenes for depth prepass and reference images.
Part of JIRA https://jira.unity3d.com/browse/XPIPELINE-794 / https://jira.unity3d.com/browse/POI-816 Avoid calling multiple times on the rendering loop: - m_GlobalSettings.renderPipelineRayTracingResources - m_GlobalSettings.renderPipelineResources Right now the only fetch is done on initialization and 1 time per render.
Regular docs sync to main.
This PR aims to address the majority of bugs and functionality shortcomings related with the FullScreenPassRendererfeature which seems to have been released in a non production-ready state (see [this doc](https://docs.google.com/document/d/1UzxIKAci6-8_MVUEEy6S3jU8qNDZCM4_W4fj04w_4sg/edit?usp=sharing) for a detailed list of problems) The full list of fixes done in this PR: - Fix using multiple features in a single renderer (previously the pass material was static so the last one was used for all passes) - Fixed null reference exception in the Render Graph path when "None" is used in the input requirements mask - Added missing Depth-Stencil support (users can still opt-out of binding the depth-stencil buffer per feature) - Fixed the custom feature names not showing up in the frame debugger - Moved the material field to the original design location (before the material pass name at the end) - FullScreenPassRendererFeature will no longer render into reflection probes - FullScreenpassRendererFeature will no longer try to execute for preview cameras in the RG path - Fixed the "Additional Properties" getting opened for all full-screen-features at once - Made the "Additional Properties" section not reset the additional state when "Show Additional Properties" is disabled - Renamed "Pass Index" to "Pass" in the UI (as the dropdown for this field shows pass names) - Removed an unnecessary MSAA resolve when sampling the screen color when MSAA is enabled - Fixed the "MotionVector" texture access not working in the Fullscreen Shader Graph target in XR - Added range clamping for the material pass index in the UI (which could go out of bounds when switching to a material with fewer passes) - Added tooltips for all the UI fields - Made sure Undo works with "Pass" (which was renamed to "Pass Index") - Fixed missing "_BlitScaleBias" upload which was causing no rendering with user shaders relying on our CoreRP Blit.hlsl header - Added upgrader for the "FullScreenPassRendererFeature" to move the "Color" flag from the "requirements" mask into a new "Fetch Color Buffer" checkbox This PR also: - Adds a workaround script to the `com.unity.testing.urp` to be able to run XR tests with MSAA disabled (so they would match the non XR tests; see [commit](https://github.cds.internal.unity3d.com/unity/unity/pull/26221/commits/289399c725f14f0b6a37f5727a5ae58fcaad1013) for details) The updated UI looks as follows: 
Regular docs sync to main.
A simple cleanup PR to update various `.asset` `.meta` and `.mat` files in URP's Foundation, Lighting and PostPro projects. Done by simply opening the projects and closing.
Trying to access VFX renderer materials from scripts currently return a variable number of materials, depending on the number of active systems. This behavior is a side effect of an optimization, and it seems to be less intuitive than returning the entire lists of materials. We are restoring the previous behavior (return the entire list) while implementing an alternative optimization. The original problem: - VFX with sleeping systems still set up the shader, even though they will not be rendering anything - Renderer nodes by default create one "drawcall" per material, but it is possible to change it in the renderer. - SRP renderer can handle a number different from the material count, but editor rendering (picking, selection) can not. First optimization: - Renderer node uses a variable number of materials (GetMaterialCount and GetMaterial are virtual), only counting the materials of systems that are active. - Works with SRP renderer, that can handle a variable number of drawcalls, and the editor renderer, because the material count now matches the active - "materials" and "sharedMaterials" use the same functions, so the number of materials returned is variable, depending on the active systems New optimization: - Renderer node uses one "drawcall" per material, default behavior for renderers - Inactive systems replace the material data with a special material that is not using any passes, so it gets ignored by all renderers - "materials" and "sharedMaterials" go back to default behavior. The special empty material is not included in these lists
…ectory for VFX Performance Tests - The SceneList.txt file is not written to the Resources directory in the VFX Performance Tests, so we need to retrieve them from there using the RuntimeGraphicsTestCaseProvider.GetScenePaths method. Also guard the PrebuildSetup for only UNITY_EDITOR to reduce noise in build. - Added update to python package code for switch - Added Editor guard for prebuild setup attribute to remove noise of failure to find method in runtime Jira https://jira.unity3d.com/browse/ANT-1521 I have a PR in review for the Graphics Test Framework that needs to land first, then I'll update this PR to use the newly published version https://github.cds.internal.unity3d.com/unity/com.unity.testframework.graphics/pull/169
The main purpose of the PR is to: - Add 8K Shadow Texture option for Main and Additional Lights. - Address user feedback regarding API being internal but should be made public. These are: - mainLightShadowmapResolution - additionalLightsShadowmapResolution - cascade2Split - cascade3Split - cascade4Split On top of that the PR: - cleans up the code around properties in `UniversalRenderPipelineAsset.cs`. - adds a callback interface for any script that needs to do runtime changes in our test projects - adds a Shadow Setting test script to change the new exposed API - adds a test scene using 8K shadow textures
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Please read the Contributing guide before making a PR.
Checklist for PR maker
need-backport-*label. After you backport the PR, the label changes tobackported-*.CHANGELOG.mdfile.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.