From 308c7c97cb0c16c0bc1bc5eb9616a3f07624298c Mon Sep 17 00:00:00 2001 From: Dan Hawson Date: Fri, 24 Nov 2023 13:22:53 +0000 Subject: [PATCH] Fix possible divide by zero explode edge case Only currently apparent with vulkan but could also exist with d3d in the right circumstances: With vulkan mesh drawing, the axis construction lines would disappear even when all explode params are zero. This is almost certainly down to normalizing a vertex offset that's coincident with 'exploderCentre', producing INFs/NaNs. Hlsl would return explodeDir to zero by multiplying with a zero 'exploderScale' but vulkan doesn't seem to do that... TLDR: Avoid dividing by zero. Also added some lines between MeshUBOData fields to better see padding requirements, since I wondered if I'd misaligned something. --- renderdoc/data/glsl/glsl_ubos.h | 4 ++++ renderdoc/data/glsl/mesh.vert | 11 +++++++++-- renderdoc/data/hlsl/mesh.hlsl | 11 +++++++++-- 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/renderdoc/data/glsl/glsl_ubos.h b/renderdoc/data/glsl/glsl_ubos.h index 0ee47fea97f..c8399f27205 100644 --- a/renderdoc/data/glsl/glsl_ubos.h +++ b/renderdoc/data/glsl/glsl_ubos.h @@ -35,15 +35,19 @@ BINDING(0) uniform MeshUBOData mat4 mvp; mat4 invProj; vec4 color; + int displayFormat; uint homogenousInput; vec2 pointSpriteSize; + uint rawoutput; uint flipY; float vtxExploderSNorm; float exploderScale; + vec3 exploderCentre; float padding; + uvec4 meshletColours[12]; } INST_NAME(Mesh); diff --git a/renderdoc/data/glsl/mesh.vert b/renderdoc/data/glsl/mesh.vert index 30120fdffc7..068c60608f0 100644 --- a/renderdoc/data/glsl/mesh.vert +++ b/renderdoc/data/glsl/mesh.vert @@ -43,10 +43,17 @@ void vtxExploder(in int vtxID, inout vec3 pos, inout vec3 secondary) { float nonLinearVtxExplodeScale = 4.0f * Mesh.exploderScale * Mesh.vtxExploderSNorm * Mesh.vtxExploderSNorm * Mesh.vtxExploderSNorm; - vec3 explodeDir = normalize(pos - Mesh.exploderCentre); + + // A vertex might be coincident with our 'exploderCentre' so that, when normalized, + // can give us INFs/NaNs that, even if multiplied by a zero 'exploderScale', can + // leave us with bad numbers (as seems to be the case with glsl/vulkan, but not hlsl). + // Still, we should make this case safe for when we have a non-zero 'exploderScale' - + vec3 offset = pos - Mesh.exploderCentre; + float offsetDistSquared = dot(offset, offset); + vec3 safeExplodeDir = offset * inversesqrt(max(offsetDistSquared, FLT_EPSILON)); float displacement = nonLinearVtxExplodeScale * ((float((vtxID >> 1) & 0xf) / 15.0f) * 1.5f - 0.5f); - pos += (explodeDir * displacement); + pos += (safeExplodeDir * displacement); if(Mesh.exploderScale > 0.0f) { diff --git a/renderdoc/data/hlsl/mesh.hlsl b/renderdoc/data/hlsl/mesh.hlsl index 4f14a75ce2a..37533b1aecd 100644 --- a/renderdoc/data/hlsl/mesh.hlsl +++ b/renderdoc/data/hlsl/mesh.hlsl @@ -44,11 +44,18 @@ void vtxExploder(in uint vtxID, inout float3 pos, inout float3 secondary) { float nonLinearVtxExplodeScale = 4.0f * exploderScale * vtxExploderSNorm * vtxExploderSNorm * vtxExploderSNorm; - float3 explodeDir = normalize(pos - exploderCentre); + + // A vertex might be coincident with our 'exploderCentre' so that, when normalized, + // can give us INFs/NaNs that, even if multiplied by a zero 'exploderScale', can + // leave us with bad numbers (as seems to be the case with glsl/vulkan, but not hlsl). + // Still, we should make this case safe for when we have a non-zero 'exploderScale' - + float3 offset = pos - exploderCentre; + float offsetDistSquared = dot(offset, offset); + float3 safeExplodeDir = offset * rsqrt(max(offsetDistSquared, FLT_EPSILON)); float displacement = nonLinearVtxExplodeScale * ((float((vtxID >> 1u) & 0xfu) / 15.0f) * 1.5f - 0.5f); - pos += (explodeDir * displacement); + pos += (safeExplodeDir * displacement); if(exploderScale > 0.0f) {