From a8b2be5643cd5ee7aa47233fd2e7cbad61246a86 Mon Sep 17 00:00:00 2001
From: James Fulop <jameslynnfulop@gmail.com>
Date: Mon, 26 Sep 2016 01:59:00 -0400
Subject: [PATCH] minor comment/logic changes

---
 renderdoc/data/glsl/mesh.comp        |  8 +++-----
 renderdoc/driver/gl/gl_debug.cpp     | 16 +++-------------
 renderdoc/driver/vulkan/vk_debug.cpp | 10 +++-------
 3 files changed, 9 insertions(+), 25 deletions(-)

diff --git a/renderdoc/data/glsl/mesh.comp b/renderdoc/data/glsl/mesh.comp
index 2add44502..52764bd5c 100644
--- a/renderdoc/data/glsl/mesh.comp
+++ b/renderdoc/data/glsl/mesh.comp
@@ -79,11 +79,6 @@ bool TriangleRayIntersect(vec3 A, vec3 B, vec3 C,
 
 void trianglePath(uint threadID)
 {
-	// NOTE(james): when I turn on following early out the shader doesn't do anything.
-	// so I end up with a lot of duplicated work on smaller meshes
-
-	//if(threadID >= meshpick.numVerts)
-	//	return;
 	uint vertid = uint(mod(float(threadID), float(meshpick.numVerts)));
 	
 	uint vertid0 = 0;
@@ -137,6 +132,7 @@ void trianglePath(uint threadID)
 	vec4 pos0 = meshpick.use_indices != 0u ? vb.data[ib.data[vertid0]] : vb.data[vertid0];
 	vec4 pos1 = meshpick.use_indices != 0u ? vb.data[ib.data[vertid1]] : vb.data[vertid1];
 	vec4 pos2 = meshpick.use_indices != 0u ? vb.data[ib.data[vertid2]] : vb.data[vertid2];
+
 #ifdef VULKAN
 	if(meshpick.unproject == 1u)
     {
@@ -151,6 +147,8 @@ void trianglePath(uint threadID)
 									meshpick.rayPos, meshpick.rayDir, 
 									/*out*/ hitPosition);
 	
+    // ray hit a triangle, so return the vertex that was closest 
+    // to the triangle/ray intersection point
 	if (hit)
 	{
 		float dist0 = distance(pos0.xyz/pos0.w, hitPosition);
diff --git a/renderdoc/driver/gl/gl_debug.cpp b/renderdoc/driver/gl/gl_debug.cpp
index c2503c334..bade3f427 100644
--- a/renderdoc/driver/gl/gl_debug.cpp
+++ b/renderdoc/driver/gl/gl_debug.cpp
@@ -1007,6 +1007,7 @@ bool GLReplay::GetHistogram(ResourceId texid, uint32_t sliceFace, uint32_t mip,
   return true;
 }
 
+// TODO: VS_Out triangles doesn't pick correctly if you look back on the frustrum, (gl as well)
 uint32_t GLReplay::PickVertex(uint32_t eventID, const MeshDisplay &cfg, uint32_t x, uint32_t y)
 {
   WrappedOpenGL &gl = *m_pDriver;
@@ -1256,13 +1257,6 @@ uint32_t GLReplay::PickVertex(uint32_t eventID, const MeshDisplay &cfg, uint32_t
       // min with size of results buffer to protect against overflows
       for(uint32_t i = 1; i < RDCMIN((uint32_t)DebugRenderData::maxMeshPicks, numResults); i++)
       {
-        // We need to keep the picking order consistent in the face
-        // of random buffer appends, when multiple vertices have the
-        // identical position (e.g. if UVs or normals are different).
-        //
-        // We could do something to try and disambiguate, but it's
-        // never going to be intuitive, it's just going to flicker
-        // confusingly.
         float pickDistance = (pickResults[i].intersectionPoint - rayPos).Length();
         if(pickDistance < closestPickDistance)
         {
@@ -1270,11 +1264,9 @@ uint32_t GLReplay::PickVertex(uint32_t eventID, const MeshDisplay &cfg, uint32_t
         }
       }
 
-      uint32_t ret = closest->vertid;
-
       gl.glUnmapNamedBufferEXT(DebugData.pickResultBuf);
 
-      return ret;
+      return closest->vertid;
     }
     else
     {
@@ -1311,11 +1303,9 @@ uint32_t GLReplay::PickVertex(uint32_t eventID, const MeshDisplay &cfg, uint32_t
           closest = pickResults + i;
       }
 
-      uint32_t ret = closest->vertid;
-
       gl.glUnmapNamedBufferEXT(DebugData.pickResultBuf);
 
-      return ret;
+      return closest->vertid;
     }
   }
 
diff --git a/renderdoc/driver/vulkan/vk_debug.cpp b/renderdoc/driver/vulkan/vk_debug.cpp
index 947913721..bf6b4fe1a 100644
--- a/renderdoc/driver/vulkan/vk_debug.cpp
+++ b/renderdoc/driver/vulkan/vk_debug.cpp
@@ -3029,6 +3029,8 @@ FloatVector VulkanDebugManager::InterpretVertex(byte *data, uint32_t vert, const
   return ret;
 }
 
+// TODO: VS_Out triangles doesn't pick correctly if you look back on the frustrum
+// TODO: Point meshes don't pick correctly
 uint32_t VulkanDebugManager::PickVertex(uint32_t eventID, const MeshDisplay &cfg, uint32_t x,
                                         uint32_t y, uint32_t w, uint32_t h)
 {
@@ -3093,6 +3095,7 @@ uint32_t VulkanDebugManager::PickVertex(uint32_t eventID, const MeshDisplay &cfg
   ubo->use_indices = cfg.position.idxByteWidth ? 1U : 0U;
   ubo->numVerts = cfg.position.numVerts;
   bool isTriangleMesh = true;
+
   switch(cfg.position.topo)
   {
     case eTopology_TriangleList:
@@ -3356,13 +3359,6 @@ uint32_t VulkanDebugManager::PickVertex(uint32_t eventID, const MeshDisplay &cfg
       // min with size of results buffer to protect against overflows
       for(uint32_t i = 1; i < RDCMIN((uint32_t)maxMeshPicks, numResults); i++)
       {
-        // We need to keep the picking order consistent in the face
-        // of random buffer appends, when multiple vertices have the
-        // identical position (e.g. if UVs or normals are different).
-        //
-        // We could do something to try and disambiguate, but it's
-        // never going to be intuitive, it's just going to flicker
-        // confusingly.
         float pickDistance = (pickResults[i].intersectionPoint - rayPos).Length();
         if(pickDistance < closestPickDistance)
         {