Feat/gpu instancing/roads indirect #3203
Conversation
|
Windows and Mac build successfull in Unity Cloud! You can find a link to the downloadable artifact below. |
…traland/unity-explorer into feat/gpu-instancing/roads_indirect
# Conflicts: # Explorer/Assets/DCL/PerformanceAndDiagnostics/Diagnostics/ReportsHandling/ReportCategory.cs # Explorer/Assets/DCL/PluginSystem/DCL.Plugins.asmdef
…ct' into feat/gpu-instancing/roads_indirect
popuz
added
force-build
popuz
added
force-build
popuz
added
do not merge
force-build
| roadAssetsPrefabList.Add((await assetsProvisioner.ProvideMainAssetAsync(t, ct: ct)).Value); | ||
| { | ||
| var prefab = await assetsProvisioner.ProvideMainAssetAsync(t, ct: ct); | ||
| // prefab.Value.GetComponent<GPUInstancingPrefabData>().HideVisuals(); |
There was a problem hiding this comment.
What is this commented out line for?
| // uniform float4x4 modifierTransform; | ||
| // uniform float modifierRadius; | ||
| // uniform float modifierHeight; | ||
| // uniform float3 modifierAxis; |
There was a problem hiding this comment.
Is modifierAxis not redundant? Multiply (0, 1, 0, 1) with modifierTransform to get capsule's "up" direction.
| maxViewSize = max(max(vBoundsExtents.x * scale.x, vBoundsExtents.y * scale.y), vBoundsExtents.z * scale.z) / (dist * fCameraHalfAngle * 2); | ||
| } | ||
|
|
||
| uint4 EvaluateLODLevel(uint nLODCount, float4x4 mLODScreenSpaceSizes, float fShadowDistance, float fScreenSpaceSize, float fDistance) |
There was a problem hiding this comment.
Could use a comment explaining what and how is packed inside output and mLODScreenSpaceSizes.
There was a problem hiding this comment.
Also, couldn't they be structs with meaningful field names?
| #ifndef _DCL_CULLING_FUNCTIONS_ | ||
| #define _DCL_CULLING_FUNCTIONS_ | ||
|
|
||
| inline void CalculateBoundingBox(in float4x4 objectTransformMatrix, inout float4 BoundingBox[8], float4x4 matCamera_MVP, float3 vBoundsCenter, float3 vBoundsExtents) |
There was a problem hiding this comment.
Please put the BoundingBox at the end of the argument list to make clearer that it's the output.
| // Middle Point | ||
| float MaxDepth = 1 - hiZMap.SampleLevel(sampler_hiZMap, float2(((BoundingRect.z - BoundingRect.x) / 2.0) + BoundingRect.x + xOffset, ((BoundingRect.w - BoundingRect.y) / 2.0) + BoundingRect.y), LOD).r; | ||
| uint uintLOD = uint(LOD); | ||
| float2 scale = float2(max((uint(hiZTxtrSize.x) >> uintLOD), 1) / max(float(uint(hiZTxtrSize.z) >> uintLOD), 1), max((uint(hiZTxtrSize.y) >> uintLOD), 1) / max(float(uint(hiZTxtrSize.w) >> uintLOD), 1)); |
There was a problem hiding this comment.
Why float(uint()) instead of floor?
| Mathf.Abs(a.m30 - b.m30) < EPSILON && | ||
| Mathf.Abs(a.m31 - b.m31) < EPSILON && | ||
| Mathf.Abs(a.m32 - b.m32) < EPSILON && | ||
| Mathf.Abs(a.m33 - b.m33) < EPSILON; |
There was a problem hiding this comment.
Consider using Mathf.Approximately instead.
| var hash = 17; | ||
| hash = (hash * 23) + (int)(matrix.m03 / EPSILON); | ||
| hash = (hash * 23) + (int)(matrix.m13 / EPSILON); | ||
| hash = (hash * 23) + (int)(matrix.m23 / EPSILON); |
There was a problem hiding this comment.
Interesting, but also why only three components?
| public static class QuaternionExtensions | ||
| { | ||
| public static Quaternion SelfOrIdentity(this Quaternion quaternion) => | ||
| quaternion is { x: 0, y: 0, z: 0, w: 0 } ? Quaternion.identity : quaternion; |
There was a problem hiding this comment.
What? What is the use?
| --output=InspectCodeResult.xml \ | ||
| --verbosity=VERBOSE | ||
|
|
||
| echo "Проверка завершена. Результат также сохранён в InspectCodeResult.xml." |
There was a problem hiding this comment.
What is this? Does it have to be in Assets? Why is it in Russian?
There was a problem hiding this comment.
Are changes to submodules reviewed separately, or?
What does this PR change?
...
How to test the changes?
Our Code Review Standards
https://github.com/decentraland/unity-renderer/blob/master/docs/code-review-standards.md