Skip to content

Improve light AABB generation #1574

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.

Sign up for GitHub

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

Closed
wants to merge 22 commits into from
Closed

Improve light AABB generation #1574

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
c084cdd
Implement clipping and culling (does not consider view frustum corners)
EvgeniiG Jul 20, 2020
a6f00ce
Support orthographic projection
EvgeniiG Aug 5, 2020
f8eea29
k_identity -> k_Identity
EvgeniiG Aug 5, 2020
ccbb9e1
Turn 'scaleXY' into a scalar
EvgeniiG Aug 5, 2020
1559d1c
Test corners of the view volume
EvgeniiG Aug 6, 2020
b8e8c83
Improve the placeholder for the linear depth
EvgeniiG Aug 6, 2020
676eb5a
Fix aspect
EvgeniiG Aug 6, 2020
433e27e
Bugfix
EvgeniiG Aug 6, 2020
8a2458a
Optimize
EvgeniiG Aug 7, 2020
0453c6c
Also store view space Z
EvgeniiG Aug 7, 2020
7aa331c
Optimize orthographic
EvgeniiG Aug 8, 2020
05a222e
Optimize LUT
EvgeniiG Aug 10, 2020
1e90134
Add wave intrinsic support
EvgeniiG Aug 11, 2020
9e4b8c6
Fix group count
EvgeniiG Aug 11, 2020
f52d29b
Reduce the kernel count to 1
EvgeniiG Aug 11, 2020
64f50ba
Remove old code
EvgeniiG Aug 11, 2020
1a3e172
Bounds check
EvgeniiG Aug 12, 2020
3e28378
Add a profiling marker
EvgeniiG Aug 12, 2020
06a8d70
Fix lane masks
EvgeniiG Aug 12, 2020
a7fcd99
Fix compiler warning
EvgeniiG Aug 12, 2020
71004f0
Remove GPU Pro reference
EvgeniiG Aug 12, 2020
21c2481
No instrinsics on Xbox
EvgeniiG Aug 26, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions com.unity.render-pipelines.core/ShaderLibrary/Common.hlsl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -852,11 +852,11 @@ void CompositeOver(real3 colorFront, real3 alphaFront,
// Space transformations
// ----------------------------------------------------------------------------

static const float3x3 k_identity3x3 = {1, 0, 0,
static const float3x3 k_Identity3x3 = {1, 0, 0,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so I know this is not great but we can't rename it without breaking existing code of users (this is also used by URP).

Here there is not a lot of choice, either we don't rename it, or we do the following.
in common.hlsl:

static const float3x3 k_Identity3x3 = {

(i.e your change).

And in CommonDeprecated.hlsl
we add :
// This is obsolete, don't used, keep for compatiblity.
static const float3x3 k_identity3x3 = k_Identity3x3 ;

this will work and not break existing code.

Copy link
Contributor Author

@EvgeniiG EvgeniiG Sep 16, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this is a fix. I actually couldn't find this constant because it was capitalized in the wrong way.
I can fix URP (or already have). I propose we just add a comment in this file. If someone's code breaks, it is trivial to fix.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

being it a fix or not isn't the problem. The issue is that it will break users custom shader. The proposal is to allow to fix the nomenclature and still not break existing user code (by putting old version as k_identity3x3 )

"If someone's code breaks, it is trivial to fix"
We can't afford this anymore. Any change like this must be added to the upgrade guide if there is no automatic upgrade, and for this little change, I will suggest we don't break code.

And yes URP code should be fix in this case as well. You should consider make a separate PR for this change as it isn't related to improve AABB generation and touch a lot of files.

0, 1, 0,
0, 0, 1};

static const float4x4 k_identity4x4 = {1, 0, 0, 0,
static const float4x4 k_Identity4x4 = {1, 0, 0, 0,
0, 1, 0, 0,
0, 0, 1, 0,
0, 0, 0, 1};
Expand All @@ -880,7 +880,7 @@ float4 ComputeClipSpacePosition(float2 positionNDC, float deviceDepth)
// (position = positionCS) => (clipSpaceTransform = use default)
// (position = positionVS) => (clipSpaceTransform = UNITY_MATRIX_P)
// (position = positionWS) => (clipSpaceTransform = UNITY_MATRIX_VP)
float4 ComputeClipSpacePosition(float3 position, float4x4 clipSpaceTransform = k_identity4x4)
float4 ComputeClipSpacePosition(float3 position, float4x4 clipSpaceTransform = k_Identity4x4)
{
return mul(clipSpaceTransform, float4(position, 1.0));
}
Expand All @@ -890,7 +890,7 @@ float4 ComputeClipSpacePosition(float3 position, float4x4 clipSpaceTransform = k
// (position = positionCS) => (clipSpaceTransform = use default)
// (position = positionVS) => (clipSpaceTransform = UNITY_MATRIX_P)
// (position = positionWS) => (clipSpaceTransform = UNITY_MATRIX_VP)
float3 ComputeNormalizedDeviceCoordinatesWithZ(float3 position, float4x4 clipSpaceTransform = k_identity4x4)
float3 ComputeNormalizedDeviceCoordinatesWithZ(float3 position, float4x4 clipSpaceTransform = k_Identity4x4)
{
float4 positionCS = ComputeClipSpacePosition(position, clipSpaceTransform);

Expand All @@ -912,7 +912,7 @@ float3 ComputeNormalizedDeviceCoordinatesWithZ(float3 position, float4x4 clipSpa
// (position = positionCS) => (clipSpaceTransform = use default)
// (position = positionVS) => (clipSpaceTransform = UNITY_MATRIX_P)
// (position = positionWS) => (clipSpaceTransform = UNITY_MATRIX_VP)
float2 ComputeNormalizedDeviceCoordinates(float3 position, float4x4 clipSpaceTransform = k_identity4x4)
float2 ComputeNormalizedDeviceCoordinates(float3 position, float4x4 clipSpaceTransform = k_Identity4x4)
{
return ComputeNormalizedDeviceCoordinatesWithZ(position, clipSpaceTransform).xy;
}
Expand Down
4 changes: 2 additions & 2 deletions com.unity.render-pipelines.core/ShaderLibrary/ImageBasedLighting.hlsl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ void SampleVisibleAnisoGGXDir(real2 u,
real3x3 viewToLocal;
if (VeqN)
{
viewToLocal = k_identity3x3;
viewToLocal = k_Identity3x3;
}
else
{
Expand Down Expand Up @@ -366,7 +366,7 @@ real4 IntegrateGGXAndDisneyDiffuseFGD(real NdotV, real roughness, uint sampleCou
real3 V = real3(sqrt(1 - NdotV * NdotV), 0, NdotV);
real4 acc = real4(0.0, 0.0, 0.0, 0.0);

real3x3 localToWorld = k_identity3x3;
real3x3 localToWorld = k_Identity3x3;

for (uint i = 0; i < sampleCount; ++i)
{
Expand Down
1 change: 1 addition & 0 deletions com.unity.render-pipelines.core/ShaderLibrary/Macros.hlsl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
#define HALF_MIN 6.103515625e-5 // 2^-14, the same value for 10, 11 and 16-bit: https://www.khronos.org/opengl/wiki/Small_Float_Formats
#define HALF_MAX 65504.0
#define UINT_MAX 0xFFFFFFFFu
#define INT_MAX 0x7FFFFFFF


#ifdef SHADER_API_GLES
Expand Down
2 changes: 1 addition & 1 deletion com.unity.render-pipelines.high-definition/Editor/ShaderGraph/SharedCode.template.hlsl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
// Init to some default value to make the computer quiet (else it output 'divide by zero' warning even if value is not used).
// TODO: this is a really poor workaround, but the variable is used in a bunch of places
// to compute normals which are then passed on elsewhere to compute other values...
output.tangentToWorld = k_identity3x3;
output.tangentToWorld = k_Identity3x3;
output.positionSS = input.positionCS; // input.positionCS is SV_Position

$FragInputs.positionRWS: output.positionRWS = input.positionRWS;
Expand Down
16 changes: 9 additions & 7 deletions com.unity.render-pipelines.high-definition/Runtime/Lighting/LightLoop/LightCullUtils.hlsl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,31 +3,33 @@

// Used to index into our SFiniteLightBound (g_data) and
// LightVolumeData (_LightVolumeData) buffers.
int GenerateLightCullDataIndex(int lightIndex, uint numVisibleLights, uint eyeIndex)
uint GenerateLightCullDataIndex(uint lightIndex, uint numVisibleLights, uint eyeIndex)
{
lightIndex = min(lightIndex, numVisibleLights - 1); // Stay within bounds

// For monoscopic, there is just one set of light cull data structs.
// In stereo, all of the left eye structs are first, followed by the right eye structs.
const int perEyeBaseIndex = (int)eyeIndex * (int)numVisibleLights;
const uint perEyeBaseIndex = eyeIndex * numVisibleLights;
return (perEyeBaseIndex + lightIndex);
}

struct ScreenSpaceBoundsIndices
{
int min;
int max;
uint min;
uint max;
};

// The returned values are used to index into our AABB screen space bounding box buffer
// Usually named g_vBoundsBuffer. The two values represent the min/max indices.
ScreenSpaceBoundsIndices GenerateScreenSpaceBoundsIndices(int lightIndex, uint numVisibleLights, uint eyeIndex)
ScreenSpaceBoundsIndices GenerateScreenSpaceBoundsIndices(uint lightIndex, uint numVisibleLights, uint eyeIndex)
{
// In the monoscopic mode, there is one set of bounds (min,max -> 2 * g_iNrVisibLights)
// In stereo, there are two sets of bounds (leftMin, leftMax, rightMin, rightMax -> 4 * g_iNrVisibLights)
const int eyeRelativeBase = (int)eyeIndex * 2 * (int)numVisibleLights;
const uint eyeRelativeBase = eyeIndex * 2 * numVisibleLights;

ScreenSpaceBoundsIndices indices;
indices.min = eyeRelativeBase + lightIndex;
indices.max = eyeRelativeBase + lightIndex + (int)numVisibleLights;
indices.max = indices.min + numVisibleLights;

return indices;
}
Expand Down
72 changes: 39 additions & 33 deletions com.unity.render-pipelines.high-definition/Runtime/Lighting/LightLoop/LightLoop.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,8 +128,8 @@ struct SFiniteLightBound
public Vector3 boxAxisY; // Scaled by the extents (half-size)
public Vector3 boxAxisZ; // Scaled by the extents (half-size)
public Vector3 center; // Center of the bounds (box) in camera space
public Vector2 scaleXY; // Scale applied to the top of the box to turn it into a truncated pyramid
public float radius; // Circumscribed sphere for the bounds (box)
public float scaleXY; // Scale applied to the top of the box to turn it into a truncated pyramid (X = Y)
public float radius; // Circumscribed sphere for the bounds (box)
};

[GenerateHLSL]
Expand Down Expand Up @@ -564,8 +564,6 @@ public void Allocate()
Shader deferredTilePixelShader { get { return defaultResources.shaders.deferredTilePS; } }


static int s_GenAABBKernel;
static int s_GenAABBKernel_Oblique;
static int s_GenListPerTileKernel;
static int s_GenListPerTileKernel_Oblique;
static int s_GenListPerVoxelKernel;
Expand Down Expand Up @@ -782,9 +780,6 @@ void InitializeLightLoop(IBLFilterBSDF[] iBLFilterBSDFArray)
m_MaxLightsOnScreen = m_MaxDirectionalLightsOnScreen + m_MaxPunctualLightsOnScreen + m_MaxAreaLightsOnScreen + m_MaxEnvLightsOnScreen;
m_MaxPlanarReflectionOnScreen = lightLoopSettings.maxPlanarReflectionOnScreen;

s_GenAABBKernel = buildScreenAABBShader.FindKernel("ScreenBoundsAABB");
s_GenAABBKernel_Oblique = buildScreenAABBShader.FindKernel("ScreenBoundsAABB_Oblique");

// Cluster
{
s_ClearVoxelAtomicKernel = buildPerVoxelLightListShader.FindKernel("ClearAtomic");
Expand Down Expand Up @@ -1628,9 +1623,9 @@ void GetLightVolumeDataAndBound(LightCategory lightCategory, GPULightType gpuLig
fAltDx *= range; fAltDy *= range;

// Handle case of pyramid with this select (currently unused)
var altDist = Mathf.Sqrt(fAltDy * fAltDy + (true ? 1.0f : 2.0f) * fAltDx * fAltDx);
bound.radius = altDist > (0.5f * range) ? altDist : (0.5f * range); // will always pick fAltDist
bound.scaleXY = squeeze ? new Vector2(0.01f, 0.01f) : new Vector2(1.0f, 1.0f);
var altDist = Mathf.Sqrt(fAltDy * fAltDy + (true ? 1.0f : 2.0f) * fAltDx * fAltDx);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You did not add this code, but having true? is a bit weird here :D

bound.radius = altDist > (0.5f * range) ? altDist : (0.5f * range); // will always pick fAltDist
bound.scaleXY = squeeze ? 0.01f : 1.0f;

lightVolumeData.lightAxisX = vx;
lightVolumeData.lightAxisY = vy;
Expand All @@ -1642,16 +1637,19 @@ void GetLightVolumeDataAndBound(LightCategory lightCategory, GPULightType gpuLig
}
else if (gpuLightType == GPULightType.Point)
{
Vector3 vx = xAxisVS;
Vector3 vy = yAxisVS;
Vector3 vz = zAxisVS;
// Construct a view-space axis-aligned bounding cube around the bounding sphere.
// This allows us to utilize the same polygon clipping technique for all lights.
// Non-axis-aligned vectors may result in a larger screen-space AABB.
Vector3 vx = new Vector3(1, 0, 0);
Vector3 vy = new Vector3(0, 1, 0);
Vector3 vz = new Vector3(0, 0, 1);

bound.center = positionVS;
bound.boxAxisX = vx * range;
bound.boxAxisY = vy * range;
bound.boxAxisZ = vz * range;
bound.scaleXY.Set(1.0f, 1.0f);
bound.radius = range;
bound.scaleXY = 1.0f;
bound.radius = range;

// fill up ldata
lightVolumeData.lightAxisX = vx;
Expand All @@ -1672,7 +1670,7 @@ void GetLightVolumeDataAndBound(LightCategory lightCategory, GPULightType gpuLig
bound.boxAxisY = extents.y * yAxisVS;
bound.boxAxisZ = extents.z * zAxisVS;
bound.radius = extents.magnitude;
bound.scaleXY.Set(1.0f, 1.0f);
bound.scaleXY = 1.0f;

lightVolumeData.lightPos = centerVS;
lightVolumeData.lightAxisX = xAxisVS;
Expand All @@ -1692,7 +1690,7 @@ void GetLightVolumeDataAndBound(LightCategory lightCategory, GPULightType gpuLig
bound.boxAxisY = extents.y * yAxisVS;
bound.boxAxisZ = extents.z * zAxisVS;
bound.radius = extents.magnitude;
bound.scaleXY.Set(1.0f, 1.0f);
bound.scaleXY = 1.0f;

lightVolumeData.lightPos = centerVS;
lightVolumeData.lightAxisX = xAxisVS;
Expand All @@ -1712,7 +1710,7 @@ void GetLightVolumeDataAndBound(LightCategory lightCategory, GPULightType gpuLig
bound.boxAxisY = extents.y * yAxisVS;
bound.boxAxisZ = extents.z * zAxisVS;
bound.radius = extents.magnitude;
bound.scaleXY.Set(1.0f, 1.0f);
bound.scaleXY = 1.0f;

lightVolumeData.lightPos = centerVS;
lightVolumeData.lightAxisX = xAxisVS;
Expand Down Expand Up @@ -1891,8 +1889,8 @@ void GetEnvLightVolumeDataAndBound(HDProbe probe, LightVolumeType lightVolumeTyp
bound.boxAxisX = influenceRightVS * influenceExtents.x;
bound.boxAxisY = influenceUpVS * influenceExtents.x;
bound.boxAxisZ = influenceForwardVS * influenceExtents.x;
bound.scaleXY.Set(1.0f, 1.0f);
bound.radius = influenceExtents.x;
bound.scaleXY = 1.0f;
bound.radius = influenceExtents.x;
break;
}
case LightVolumeType.Box:
Expand All @@ -1901,8 +1899,8 @@ void GetEnvLightVolumeDataAndBound(HDProbe probe, LightVolumeType lightVolumeTyp
bound.boxAxisX = influenceExtents.x * influenceRightVS;
bound.boxAxisY = influenceExtents.y * influenceUpVS;
bound.boxAxisZ = influenceExtents.z * influenceForwardVS;
bound.scaleXY.Set(1.0f, 1.0f);
bound.radius = influenceExtents.magnitude;
bound.scaleXY = 1.0f;
bound.radius = influenceExtents.magnitude;

// The culling system culls pixels that are further
// than a threshold to the box influence extents.
Expand Down Expand Up @@ -1942,7 +1940,7 @@ void AddBoxVolumeDataAndBound(OrientedBBox obb, LightCategory category, LightFea
bound.boxAxisY = obb.extentY * upVS;
bound.boxAxisZ = obb.extentZ * forwardVS;
bound.radius = extents.magnitude;
bound.scaleXY.Set(1.0f, 1.0f);
bound.scaleXY = 1.0f;

// The culling system culls pixels that are further
// than a threshold to the box influence extents.
Expand Down Expand Up @@ -2771,19 +2769,27 @@ static void GenerateLightsScreenSpaceAABBs(in BuildGPULightListParameters parame
{
if (parameters.totalLightCount != 0)
{
var tileAndCluster = resources.tileAndClusterData;
using (new ProfilingScope(cmd, ProfilingSampler.Get(HDProfileId.GenerateLightAABBs)))
{
var tileAndCluster = resources.tileAndClusterData;

cmd.SetComputeIntParam(parameters.screenSpaceAABBShader, HDShaderIDs.g_isOrthographic, parameters.isOrthographic ? 1 : 0);

cmd.SetComputeIntParam(parameters.screenSpaceAABBShader, HDShaderIDs.g_isOrthographic, parameters.isOrthographic ? 1 : 0);
// With XR single-pass, we have one set of light bounds per view to iterate over (bounds are in view space for each view)
cmd.SetComputeIntParam(parameters.screenSpaceAABBShader, HDShaderIDs.g_iNrVisibLights, parameters.totalLightCount);
cmd.SetComputeBufferParam(parameters.screenSpaceAABBShader, parameters.screenSpaceAABBKernel, HDShaderIDs.g_data, tileAndCluster.convexBoundsBuffer);
cmd.SetComputeBufferParam(parameters.screenSpaceAABBShader, parameters.screenSpaceAABBKernel, HDShaderIDs.g_vBoundsBuffer, tileAndCluster.AABBBoundsBuffer);

// With XR single-pass, we have one set of light bounds per view to iterate over (bounds are in view space for each view)
cmd.SetComputeIntParam(parameters.screenSpaceAABBShader, HDShaderIDs.g_iNrVisibLights, parameters.totalLightCount);
cmd.SetComputeBufferParam(parameters.screenSpaceAABBShader, parameters.screenSpaceAABBKernel, HDShaderIDs.g_data, tileAndCluster.convexBoundsBuffer);
cmd.SetComputeBufferParam(parameters.screenSpaceAABBShader, parameters.screenSpaceAABBKernel, HDShaderIDs.g_vBoundsBuffer, tileAndCluster.AABBBoundsBuffer);
cmd.SetComputeMatrixArrayParam(parameters.screenSpaceAABBShader, HDShaderIDs.g_mProjectionArr, parameters.lightListProjHMatrices);
cmd.SetComputeMatrixArrayParam(parameters.screenSpaceAABBShader, HDShaderIDs.g_mInvProjectionArr, parameters.lightListInvProjHMatrices);

cmd.SetComputeMatrixArrayParam(parameters.screenSpaceAABBShader, HDShaderIDs.g_mProjectionArr, parameters.lightListProjHMatrices);
cmd.SetComputeMatrixArrayParam(parameters.screenSpaceAABBShader, HDShaderIDs.g_mInvProjectionArr, parameters.lightListInvProjHMatrices);
const int threadsPerLight = 4; // Shader: THREADS_PER_LIGHT (4)
const int threadsPerGroup = 64; // Shader: THREADS_PER_GROUP (64)

cmd.DispatchCompute(parameters.screenSpaceAABBShader, parameters.screenSpaceAABBKernel, (parameters.totalLightCount + 7) / 8, parameters.viewCount, 1);
int groupCount = HDUtils.DivRoundUp(parameters.totalLightCount * threadsPerLight, threadsPerGroup);

cmd.DispatchCompute(parameters.screenSpaceAABBShader, parameters.screenSpaceAABBKernel, groupCount, parameters.viewCount, 1);
}
}
}

Expand Down Expand Up @@ -3067,7 +3073,7 @@ BuildGPULightListParameters PrepareBuildGPULightListParameters(HDCamera hdCamera

// Screen space AABB
parameters.screenSpaceAABBShader = buildScreenAABBShader;
parameters.screenSpaceAABBKernel = isProjectionOblique ? s_GenAABBKernel_Oblique : s_GenAABBKernel;
parameters.screenSpaceAABBKernel = 0;
// camera to screen matrix (and it's inverse)
for (int viewIndex = 0; viewIndex < hdCamera.viewCount; ++viewIndex)
{
Expand Down
2 changes: 1 addition & 1 deletion com.unity.render-pipelines.high-definition/Runtime/Lighting/LightLoop/LightLoop.cs.hlsl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ struct SFiniteLightBound
float3 boxAxisY;
float3 boxAxisZ;
float3 center;
float2 scaleXY;
float scaleXY;
float radius;
};

Expand Down
Loading