From 345208e0d9fa2b03f4902c272c2df34dde161e7b Mon Sep 17 00:00:00 2001 From: baldurk Date: Wed, 5 Sep 2018 12:50:37 +0100 Subject: [PATCH] Avoid atomic ops on vector members * This doesn't translate well to metalsl and is in general not very useful - we were only doing it to avoid packing issues which in some cases are moot anyway. E.g. the histogram buffer doesn't have to be std140 and packed like an array of vectors, it can be std430 and packed like an array of uints. --- renderdoc/data/glsl/histogram.comp | 8 ++++---- renderdoc/data/glsl/mesh.comp | 11 ++++++++--- renderdoc/driver/gl/gl_debug.cpp | 13 +++---------- renderdoc/driver/vulkan/vk_debug.cpp | 4 ++-- renderdoc/driver/vulkan/vk_replay.cpp | 4 +--- 5 files changed, 18 insertions(+), 22 deletions(-) diff --git a/renderdoc/data/glsl/histogram.comp b/renderdoc/data/glsl/histogram.comp index 84b9fbd9d..0aa629bf5 100644 --- a/renderdoc/data/glsl/histogram.comp +++ b/renderdoc/data/glsl/histogram.comp @@ -24,9 +24,9 @@ //#include "texsample.h" // while includes aren't supported in glslang, this will be added in code -layout(binding=0, std140) buffer minmaxresultdest +layout(binding=0, std430) buffer minmaxresultdest { - uvec4 result[HGRAM_NUM_BUCKETS]; + uint result[HGRAM_NUM_BUCKETS]; } dest; layout (local_size_x = HGRAM_TILES_PER_BLOCK, local_size_y = HGRAM_TILES_PER_BLOCK) in; @@ -183,8 +183,8 @@ void main() } #endif - if(bucketIdx >= 0 && bucketIdx < HGRAM_NUM_BUCKETS) - atomicAdd(dest.result[bucketIdx].x, 1U); + if(bucketIdx < HGRAM_NUM_BUCKETS) + atomicAdd(dest.result[bucketIdx], 1U); } } } diff --git a/renderdoc/data/glsl/mesh.comp b/renderdoc/data/glsl/mesh.comp index 79699835c..fcea71908 100644 --- a/renderdoc/data/glsl/mesh.comp +++ b/renderdoc/data/glsl/mesh.comp @@ -34,7 +34,12 @@ layout(binding = 2, std430) readonly buffer index_data layout(binding = 3, std140) buffer pickresult_buffer { - uvec4 counter; + uint counter; + // individual padding to prevent uint/uint3 packing woes + uint pad0; + uint pad1; + uint pad2; + uvec4 results[]; } pickresult; @@ -162,7 +167,7 @@ void trianglePath(uint threadID) float dist1 = distance(pos1.xyz/pos1.w, hitPosition); float dist2 = distance(pos2.xyz/pos2.w, hitPosition); - uint result_idx = atomicAdd(pickresult.counter.x, 1u); + uint result_idx = atomicAdd(pickresult.counter, 1u); uint meshVert = vertid0; if (dist1 < dist0 && dist1 < dist2) @@ -208,7 +213,7 @@ void defaultPath(uint threadID) float len = length(scr - meshpick.coords); if(len < 35.0f) { - uint result_idx = atomicAdd(pickresult.counter.x, 1u); + uint result_idx = atomicAdd(pickresult.counter, 1u); pickresult.results[result_idx] = uvec4(vertid, idx, floatBitsToUint(len), floatBitsToUint(wpos.z)); } } diff --git a/renderdoc/driver/gl/gl_debug.cpp b/renderdoc/driver/gl/gl_debug.cpp index afe950ba1..e00dfe036 100644 --- a/renderdoc/driver/gl/gl_debug.cpp +++ b/renderdoc/driver/gl/gl_debug.cpp @@ -500,7 +500,7 @@ void GLReplay::InitDebugData() drv.glNamedBufferDataEXT(DebugData.minmaxTileResult, byteSize, NULL, eGL_DYNAMIC_DRAW); drv.glNamedBufferDataEXT(DebugData.minmaxResult, sizeof(Vec4f) * 2, NULL, eGL_DYNAMIC_READ); - drv.glNamedBufferDataEXT(DebugData.histogramBuf, sizeof(uint32_t) * 4 * HGRAM_NUM_BUCKETS, NULL, + drv.glNamedBufferDataEXT(DebugData.histogramBuf, sizeof(uint32_t) * HGRAM_NUM_BUCKETS, NULL, eGL_DYNAMIC_READ); } @@ -1168,17 +1168,10 @@ bool GLReplay::GetHistogram(ResourceId texid, uint32_t sliceFace, uint32_t mip, GL.glMemoryBarrier(GL_SHADER_STORAGE_BARRIER_BIT); histogram.clear(); - histogram.resize(HGRAM_NUM_BUCKETS * 4); + histogram.resize(HGRAM_NUM_BUCKETS); GL.glBindBuffer(eGL_COPY_READ_BUFFER, DebugData.histogramBuf); - GL.glGetBufferSubData(eGL_COPY_READ_BUFFER, 0, sizeof(uint32_t) * 4 * HGRAM_NUM_BUCKETS, - &histogram[0]); - - // compress down from uvec4, then resize down - for(size_t i = 1; i < HGRAM_NUM_BUCKETS; i++) - histogram[i] = histogram[i * 4]; - - histogram.resize(HGRAM_NUM_BUCKETS); + GL.glGetBufferSubData(eGL_COPY_READ_BUFFER, 0, sizeof(uint32_t) * HGRAM_NUM_BUCKETS, &histogram[0]); if(maxlevel >= 0) GL.glTextureParameterivEXT(texname, target, eGL_TEXTURE_MAX_LEVEL, (GLint *)&maxlevel); diff --git a/renderdoc/driver/vulkan/vk_debug.cpp b/renderdoc/driver/vulkan/vk_debug.cpp index 772b9f6da..7bf2ea332 100644 --- a/renderdoc/driver/vulkan/vk_debug.cpp +++ b/renderdoc/driver/vulkan/vk_debug.cpp @@ -2399,9 +2399,9 @@ void VulkanReplay::HistogramMinMax::Init(WrappedVulkan *driver, VkDescriptorPool m_MinMaxResult.Create(driver, driver->GetDev(), sizeof(Vec4f) * 2, 1, GPUBuffer::eGPUBufferSSBO); m_MinMaxReadback.Create(driver, driver->GetDev(), sizeof(Vec4f) * 2, 1, GPUBuffer::eGPUBufferReadback); - m_HistogramBuf.Create(driver, driver->GetDev(), sizeof(uint32_t) * 4 * HGRAM_NUM_BUCKETS, 1, + m_HistogramBuf.Create(driver, driver->GetDev(), sizeof(uint32_t) * HGRAM_NUM_BUCKETS, 1, GPUBuffer::eGPUBufferSSBO); - m_HistogramReadback.Create(driver, driver->GetDev(), sizeof(uint32_t) * 4 * HGRAM_NUM_BUCKETS, 1, + m_HistogramReadback.Create(driver, driver->GetDev(), sizeof(uint32_t) * HGRAM_NUM_BUCKETS, 1, GPUBuffer::eGPUBufferReadback); // don't need to ring this, as we hard-sync for readback anyway diff --git a/renderdoc/driver/vulkan/vk_replay.cpp b/renderdoc/driver/vulkan/vk_replay.cpp index 3666c1c40..04c8398f2 100644 --- a/renderdoc/driver/vulkan/vk_replay.cpp +++ b/renderdoc/driver/vulkan/vk_replay.cpp @@ -2033,9 +2033,7 @@ bool VulkanReplay::GetHistogram(ResourceId texid, uint32_t sliceFace, uint32_t m uint32_t *buckets = (uint32_t *)m_Histogram.m_HistogramReadback.Map(NULL); - histogram.resize(HGRAM_NUM_BUCKETS); - for(size_t i = 0; i < HGRAM_NUM_BUCKETS; i++) - histogram[i] = buckets[i * 4]; + histogram.assign(buckets, buckets + HGRAM_NUM_BUCKETS); m_Histogram.m_HistogramReadback.Unmap();