Universal/light layers #3677

kaychang-unity · 2021-02-27T01:57:09Z

Read the Graphics repository & Yamato FAQ.

Purpose of this PR

This adds support for light layers.
UniversalRenderPipelineAsset has a new checkbox to enable light layers. In this mode, the culling mask property of lights is switched to a light layer property (bitmask). A new shadow layer also appear for the light if it casts shadows.

Testing status

Need a test scene

Comments to reviewers

There was a bug where when static batching is enabled where unity_RenderingLayer are not always correctly set to different meshes. This does not happens anymore with recent trunk revision.
Should light layer names be renamable by users? They are currently hardcoded.
Should we add the code related to decals?
How to convert a light culling mask into a light layer?

github-actions · 2021-02-27T01:57:37Z

It appears that you made a non-draft PR!
Please convert your PR to draft (button on the right side of the page)
and cancel any jobs that started on Yamato.
See the PR template for more information.
Thank you!

kaychang-unity · 2021-02-27T02:18:50Z

UniversalRenderPipelineAsset has new checkbox:

kaychang-unity · 2021-02-27T02:19:51Z

When light layer is enabled, Light "Culling Mask" option is replaced by light layer option:

pbbastian

@kaychang-unity Thank you for implementing this. I have some questions:

Are there any risks, performance considerations (including build times), or dependencies involved?
Are there any other technical areas this has an impact on? (e.g. decals)
Which platforms is this targeting?
Does this differ from the HDRP light layers in any way?
Is documentation planned? @oleks-k
Which rendering layers are getting exposed as light layers? The first 8?
Does this mean that some of the 4/8 lights available per object might be wasted due to non matching light layer masks?
Should this include any analytics?
Is this feature complete, or is there more that needs to be implemented later?
Should this be on or off by default?

Regarding your questions:

Should light layer names be renamable by users? They are currently hardcoded.

I think that should be possible, especially since it's possible in HDRP.

Should we add the code related to decals?

@lukaschod

How to convert a light culling mask into a light layer?

Hmm, I'm not sure if this is possible. Maybe the HDRP people has some experience with this? @JulienIgnace-Unity

Also, make sure to add the test scene mentioned before this lands :)

com.unity.render-pipelines.universal/ShaderLibrary/UnityInput.hlsl

com.unity.render-pipelines.universal/CHANGELOG.md

com.unity.render-pipelines.universal/Shaders/Lit.shader

com.unity.render-pipelines.universal/Shaders/Nature/SpeedTree7.shader

com.unity.render-pipelines.universal/ShaderLibrary/Input.hlsl

com.unity.render-pipelines.universal/ShaderLibrary/Lighting.hlsl

hkr · 2021-03-23T16:02:47Z

Is this feature supported for OpenGL ES 2.0?
The additional uniform array might cause problems. Also the integer/bit ops that are used.

sebastienlagarde · 2021-03-23T16:12:42Z

Should light layer names be renamable by users? They are currently hardcoded.

For this you will need a default settings asset. URP don't have one yet. It come with various workflows. It would be nice to be able to rename light layers, but I will suggest to do it in a separate PR, and then to evaluate if you want to introduce a default settings asset in 21.2 (it have consequence as you need to have it in template etc...).

Should we add the code related to decals?
I don't see the relationship? Light layers and decal layers are differents thing if this is what you refer too, you shouldn't have to worry about decal in this PR.

How to convert a light culling mask into a light layer?
I let you judge but it can be a very tricky problem. We haven't look at it for HDRP so we don't have experience about it, but clearly, if we have done dedicated light layers it was because of issue with light culling mask, so I don't expect you can get a 1-1 matching. Users can do what they want, with naming they want for culling mask.

sebastienlagarde

Regarding the change compare to HDRP I am agree with the approach and how it is implemented. But the final decision must come from URP people and take into account URP platform

Co-authored-by: Peter Bay Bastian <pbbastian@users.noreply.github.com>

…hnologies/Graphics into universal/light-layers

kaychang-unity · 2021-03-23T22:36:31Z

@pbbastian

Are there any risks, performance considerations (including build times), or dependencies involved?
This adds extra shader variant _LIGHT_LAYERS but the shader preprocessor strips them when light layers feature is enabled.
Are there any other technical areas this has an impact on? (e.g. decals)
What's the status for URP decals? Is URP team is planning to implement decals using HDRP implementation?
Additionally, users wants to use rendering layer masks for their own purposes (but currently they can't), and this light layer implementation also consumes 8 bits of it.
Which platforms is this targeting?
This is working for both Forward and Deferred renderers. So for Forward mode -> all platforms - GLES2 (doesn't seem to like the bit operations), for deferred mode -> only platform supporting deferred (NOT GL, GLES2, GLES3, DX10).
Does this differ from the HDRP light layers in any way?
No.
Is documentation planned?
Yes, @oleks-k
Which rendering layers are getting exposed as light layers? The first 8?
Yes, same as HDRP
Does this mean that some of the 4/8 lights available per object might be wasted due to non matching light layer masks?
Good point, in case there is few light count, bits in the rendering layer mask are wasted. Also not that implementation of light layers in forward renderer is inefficient and requires an extra dynamic branch, which does not happen when using the legacy culling mask. The issue has been shared with @phi-lira , it is expected.
Should this include any analytics?
It could, sure. However, this is not expected to be performant.
Is this feature complete, or is there more that needs to be implemented later?
I hope so. Unless HDRP team has more plans in progress regarding it.
Should this be on or off by default?
Off by default. It is never efficient to use light layers in neither URP forward nor deferred. It is only implemented for feature parity between URP forward and URP deferred (URP forward has culling mask support, URP deferred has not), as well as compatibility with HDRP.
Should light layer names be renamable by users? They are currently hardcoded.
I am asking because I could not find a way to rename the light layers in HDRP, unless you switch the HDRP render pipeline asset to "debug" view, in which case the light layers names appear and can be renamed but have a big "Obsolete" prefix in front of them.
Note that culling masks can be renamed but appear under "layers" in the UI (top left corner) which can be confusing (this is not the rendering layer mask).

I will work on adding a proper test scene.

In HDRP, on the HD Render Pipeline Asset in Debug view:

Edit: as Seb pointed out, it's possible to rename light layers using default settings asset, which does not exist in URP (leave the renaming feature for another PR).

Jonasmortensen

Overall looks good to me.
Only a couple of thoughts:

Light Layer property can be removed from light inspector when light is baked
Interaction with Shadowmask can give unexpected results. I'm not sure if it is possible for the mask to take light/shadow layers into account but would be nice.
About renaming light layers: The layer names should be hardcoded as long as URP doesn't have Global Settings

ernestasKupciunas

It seems to work as expected. I was able to play around with the Light Layers in the editor, set different objects to be affected by particular light layers. Rendering on mobile devices looks good too.
Tested using Boat attack and Winter projects.
Winter:

BoatAttack:

Unity:
Version: 2021.2.0a14.1944
Revision: trunk 9f66320aebca
Built: Fri, 09 Apr 2021 17:39:30 GMT

Devices under testing:
iPad Air4, SoC: A14, iOS: 14.0
iPhone 12Pro, SoC: A14, iOS: 14.1
VLNQA00217, Razer Phone 2, 9.0.0, Snapdragon 845 SDM845, Adreno 630
VLNQA00268, Samsung Galaxy S10+, 10.0.0, Exynos 9 9820, Mali-G76
OnePlus Nord, 10, Snapdragon 765G SM7250-AB, Adreno 620
Samsung Galaxy Z Fold2 5G, 10, Snapdragon 865 SM8250, Adreno 650

simon-engelbrecht-soerensen

Working as expected!

kroatoa · 2021-04-19T12:15:19Z

I'm assuming this relates to https://unity3d.atlassian.net/browse/UR-1297. That roadmap item is missing the Product manager approval, so if you could poke the relevant one to sign off, that would be good. Otherwise we risk adding the delay for that approval to the weekly bundling.

Other than that I have no complaints.

…ode does so too). Added missing "instancing_options renderinglayer" to terrain shaders.

…roken on iOS device with light layer shader variant addition.

phi-lira · 2021-04-21T15:33:40Z

URP Update top assets is known issue already fixed on master. It's ok to merge with this one red.
HDRP tests, this PR doesn't touch any HDRP file so ignoring.

phi-lira · 2021-04-21T15:38:10Z

@kaychang-unity wait for test to finish before resolving conflict. Once ABV is green, resolve conflict and don't run test again. looking at conflict file we dont' need to rerun graphics test due to that.

* Initial light layer support. * Updated changelog. * Auto-formatting. * Update com.unity.render-pipelines.universal/CHANGELOG.md Co-authored-by: Peter Bay Bastian <pbbastian@users.noreply.github.com> * Minor fixes. * Adjusted code to compile out IsMatchingLightLayer() when light layers are disabled (confirmed the code is compiled out on Switch). * Refactoring. Fixed small bug with gbuffer index calculation when shadowMask and lightLayers are enabled. Updated comment for unity_RenderingLayer. * Added test scenes for light layers (148_Lighting_LightLayers and 148_Lighting_LightLayers_deferred). * Formating. * Disable filtering for lightmaps. * Added 148_Lighting_LightLayers and 148_Lighting_LightLayers_deferred in list of scenes to build and test. * AdditionalLightData is now unconditionally created when OnEnable() is called, removed unecessary code to create it when light layers are enabled. * Added some reference images. * Added fake quality level that references UniversalRPAssetLightLayers.asset as a workaround for ShaderPreprocessor otherwise stripping too much. * Moved UI element for light layer mask in Light inspector. Renamed linkLightLayer for shadow to customShadowLayer. * Detect invalid platforms (GLES2) and display error message. * More reference images. * Reference images. * Wait for 2 frames before taking screenshots. This fixes the test scenes failing on Android GLES3. * Reference images. * Tweaked error message. * Removed duplicate assets. * Updated UX to match HDRP. SwapPipelineAsset file redirected to new path. * Fixed null reference in Player mode. * Update com.unity.render-pipelines.universal/Runtime/DeferredLights.cs Fixed typo in comment. Co-authored-by: Felipe Lira <felipedrl@gmail.com> * Added comment. * Moved rendering layer texture from 32 to 8 bits as currently only light layers are stored inside. * A lot of internal renaming from lightLayers to layerMask. * Hide light layers for baked lights. * Fixed formatting to pass yamato format test. * Terrain vertex lit ignores light layers (because default vertex lit code does so too). Added missing "instancing_options renderinglayer" to terrain shaders. * More missing instancing_option renderinglayer. * Temporarily sable 153_Lighting_EnlightenTerrain.unity because it is broken on iOS device with light layer shader variant addition. Co-authored-by: Peter Bay Bastian <pbbastian@users.noreply.github.com> Co-authored-by: Felipe Lira <felipedrl@gmail.com>

kaychang-unity added 2 commits February 26, 2021 15:53

Initial light layer support.

2578116

Merge branch 'master' into universal/light-layers

4507696

kaychang-unity requested a review from a team as a code owner February 27, 2021 01:57

github-actions bot added the universal label Feb 27, 2021

kaychang-unity marked this pull request as draft February 27, 2021 01:58

Updated changelog.

aa9d82e

kaychang-unity requested review from JulienIgnace-Unity, phi-lira and sebastienlagarde February 27, 2021 02:10

kaychang-unity mentioned this pull request Mar 3, 2021

[10.x.x] Adding Rendering Layer Mask to the Render Objects renderer feature #1891

Closed

kaychang-unity added 2 commits March 18, 2021 18:46

Auto-formatting.

ade05b1

Merge branch 'master' into universal/light-layers

8ab4cec

kaychang-unity self-assigned this Mar 23, 2021

kaychang-unity marked this pull request as ready for review March 23, 2021 04:59

kaychang-unity requested a review from a team as a code owner March 23, 2021 04:59

pbbastian suggested changes Mar 23, 2021

View reviewed changes

com.unity.render-pipelines.universal/ShaderLibrary/UnityInput.hlsl Show resolved Hide resolved

com.unity.render-pipelines.universal/CHANGELOG.md Outdated Show resolved Hide resolved

com.unity.render-pipelines.universal/Shaders/Lit.shader Show resolved Hide resolved

phi-lira requested a review from hkr March 23, 2021 11:05

hkr reviewed Mar 23, 2021

View reviewed changes

com.unity.render-pipelines.universal/Shaders/Nature/SpeedTree7.shader Show resolved Hide resolved

hkr reviewed Mar 23, 2021

View reviewed changes

com.unity.render-pipelines.universal/ShaderLibrary/Input.hlsl Show resolved Hide resolved

hkr reviewed Mar 23, 2021

View reviewed changes

com.unity.render-pipelines.universal/ShaderLibrary/Lighting.hlsl Outdated Show resolved Hide resolved

sebastienlagarde approved these changes Mar 23, 2021

View reviewed changes

kaychang-unity and others added 3 commits March 23, 2021 14:26

Update com.unity.render-pipelines.universal/CHANGELOG.md

6c7decb

Co-authored-by: Peter Bay Bastian <pbbastian@users.noreply.github.com>

Merge branch 'universal/light-layers' of https://github.com/Unity-Tec…

40926e3

…hnologies/Graphics into universal/light-layers

Minor fixes.

b360f0c

thomas-zeng approved these changes Apr 14, 2021

View reviewed changes

A lot of internal renaming from lightLayers to layerMask.

e6a344d

kaychang-unity requested review from pbbastian and phi-lira April 14, 2021 17:08

hkr approved these changes Apr 14, 2021

View reviewed changes

phi-lira approved these changes Apr 15, 2021

View reviewed changes

JulienIgnace-Unity approved these changes Apr 15, 2021

View reviewed changes

Jonasmortensen approved these changes Apr 15, 2021

View reviewed changes

Hide light layers for baked lights.

ee81c8d

ernestasKupciunas approved these changes Apr 16, 2021

View reviewed changes

simon-engelbrecht-soerensen approved these changes Apr 16, 2021

View reviewed changes

pbbastian approved these changes Apr 16, 2021

View reviewed changes

Merge branch 'master' into universal/light-layers

5c72df6

phi-lira requested a review from a team April 19, 2021 09:26

phi-lira added 2 commits April 19, 2021 17:13

Merge branch 'master' into universal/light-layers-test

41d9a7d

Fixed formatting to pass yamato format test.

f1c308b

phi-lira mentioned this pull request Apr 19, 2021

[Light Layer Yamato Fix] #4229

Merged

jenniferd-unity mentioned this pull request Apr 20, 2021

First pass on URP Global Settings with Light Layer Names #4199

Merged

kaychang-unity added 3 commits April 20, 2021 21:46

Terrain vertex lit ignores light layers (because default vertex lit c…

f5c389c

…ode does so too). Added missing "instancing_options renderinglayer" to terrain shaders.

More missing instancing_option renderinglayer.

4ca80bc

Temporarily sable 153_Lighting_EnlightenTerrain.unity because it is b…

722dd17

…roken on iOS device with light layer shader variant addition.

Merge branch 'master' into universal/light-layers

810b205

kaychang-unity merged commit 3da4810 into master Apr 22, 2021

kaychang-unity deleted the universal/light-layers branch April 22, 2021 02:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Universal/light layers #3677

Universal/light layers #3677

kaychang-unity commented Feb 27, 2021 •

edited

github-actions bot commented Feb 27, 2021

kaychang-unity commented Feb 27, 2021

kaychang-unity commented Feb 27, 2021

pbbastian left a comment •

edited

hkr commented Mar 23, 2021

sebastienlagarde commented Mar 23, 2021

sebastienlagarde left a comment

kaychang-unity commented Mar 23, 2021 •

edited

Jonasmortensen left a comment

ernestasKupciunas left a comment •

edited

simon-engelbrecht-soerensen left a comment

kroatoa commented Apr 19, 2021

phi-lira commented Apr 21, 2021

phi-lira commented Apr 21, 2021

Universal/light layers #3677

Universal/light layers #3677

Conversation

kaychang-unity commented Feb 27, 2021 • edited

Purpose of this PR

Testing status

Comments to reviewers

github-actions bot commented Feb 27, 2021

kaychang-unity commented Feb 27, 2021

kaychang-unity commented Feb 27, 2021

pbbastian left a comment • edited

Choose a reason for hiding this comment

hkr commented Mar 23, 2021

sebastienlagarde commented Mar 23, 2021

sebastienlagarde left a comment

Choose a reason for hiding this comment

kaychang-unity commented Mar 23, 2021 • edited

Jonasmortensen left a comment

Choose a reason for hiding this comment

ernestasKupciunas left a comment • edited

Choose a reason for hiding this comment

simon-engelbrecht-soerensen left a comment

Choose a reason for hiding this comment

kroatoa commented Apr 19, 2021

phi-lira commented Apr 21, 2021

phi-lira commented Apr 21, 2021

kaychang-unity commented Feb 27, 2021 •

edited

pbbastian left a comment •

edited

kaychang-unity commented Mar 23, 2021 •

edited

ernestasKupciunas left a comment •

edited