From 92e04417d5e8dec25ea3ad3449f98ff28f206e08 Mon Sep 17 00:00:00 2001 From: baldurk Date: Thu, 23 Apr 2020 18:53:59 +0100 Subject: [PATCH] Add support for storage image load/store --- .../driver/shaders/spirv/spirv_debug.cpp | 73 +++++++-- renderdoc/driver/shaders/spirv/spirv_debug.h | 7 + .../shaders/spirv/spirv_debug_setup.cpp | 25 ++- renderdoc/driver/vulkan/vk_shaderdebug.cpp | 148 ++++++++++++++++++ util/test/demos/vk/vk_shader_debug_zoo.cpp | 28 +++- 5 files changed, 258 insertions(+), 23 deletions(-) diff --git a/renderdoc/driver/shaders/spirv/spirv_debug.cpp b/renderdoc/driver/shaders/spirv/spirv_debug.cpp index aa1e66e04..2110ad093 100644 --- a/renderdoc/driver/shaders/spirv/spirv_debug.cpp +++ b/renderdoc/driver/shaders/spirv/spirv_debug.cpp @@ -2191,6 +2191,65 @@ void ThreadState::StepNext(ShaderDebugState *state, const rdcarray SetDst(opdata.result, result); break; } + case Op::ImageRead: + { + OpImageRead read(it); + + ShaderVariable img = GetSrc(read.image); + ShaderVariable coord = GetSrc(read.coordinate); + + const DataType &resultType = debugger.GetType(opdata.resultType); + + // only the sample operand should be here + RDCASSERT((read.imageOperands.flags & ImageOperands::Sample) == read.imageOperands.flags); + + ShaderVariable result; + result.type = resultType.scalar().Type(); + + if(!debugger.GetAPIWrapper()->ReadTexel(img.GetBinding(), coord, + read.imageOperands.flags & ImageOperands::Sample + ? GetSrc(read.imageOperands.sample).value.uv[0] + : 0, + result)) + { + // sample failed. Pretend we got 0 columns back + result.value.uv[0] = 0; + result.value.uv[1] = 0; + result.value.uv[2] = 0; + + if(result.type == VarType::Float || result.type == VarType::Half) + result.value.fv[3] = 1.0f; + else if(result.type == VarType::Double) + result.value.dv[3] = 1.0; + else + result.value.uv[3] = 1; + } + + result.rows = 1; + result.columns = RDCMAX(1U, resultType.vector().count); + + SetDst(read.result, result); + break; + } + case Op::ImageWrite: + { + OpImageWrite write(it); + + ShaderVariable img = GetSrc(write.image); + ShaderVariable coord = GetSrc(write.coordinate); + ShaderVariable texel = GetSrc(write.texel); + + // only the sample operand should be here + RDCASSERT((write.imageOperands.flags & ImageOperands::Sample) == write.imageOperands.flags); + + debugger.GetAPIWrapper()->WriteTexel(img.GetBinding(), coord, + write.imageOperands.flags & ImageOperands::Sample + ? GetSrc(write.imageOperands.sample).value.uv[0] + : 0, + texel); + + break; + } ////////////////////////////////////////////////////////////////////////////// // @@ -2430,20 +2489,6 @@ void ThreadState::StepNext(ShaderDebugState *state, const rdcarray break; } - // TODO image load/store - case Op::ImageRead: - case Op::ImageWrite: - { - RDCERR("Image load/store not yet implemented."); - - ShaderVariable var("", 0U, 0U, 0U, 0U); - var.columns = 1; - - SetDst(opdata.result, var); - - break; - } - // TODO atomics case Op::ImageTexelPointer: case Op::AtomicLoad: diff --git a/renderdoc/driver/shaders/spirv/spirv_debug.h b/renderdoc/driver/shaders/spirv/spirv_debug.h index e5df419ba..399e6658e 100644 --- a/renderdoc/driver/shaders/spirv/spirv_debug.h +++ b/renderdoc/driver/shaders/spirv/spirv_debug.h @@ -57,6 +57,12 @@ public: virtual void ReadBufferValue(BindpointIndex bind, uint64_t offset, uint64_t byteSize, void *dst) = 0; virtual void WriteBufferValue(BindpointIndex bind, uint64_t offset, uint64_t byteSize, const void *src) = 0; + + virtual bool ReadTexel(BindpointIndex imageBind, const ShaderVariable &coord, uint32_t sample, + ShaderVariable &output) = 0; + virtual bool WriteTexel(BindpointIndex imageBind, const ShaderVariable &coord, uint32_t sample, + const ShaderVariable &value) = 0; + virtual void FillInputValue(ShaderVariable &var, ShaderBuiltin builtin, uint32_t location, uint32_t component) = 0; @@ -98,6 +104,7 @@ public: static const uint32_t TextureTypeVariableSlot = 8; static const uint32_t BufferPointerByteOffsetVariableSlot = 8; static const uint32_t BufferPointerTypeIdVariableSlot = 9; +static const uint32_t SSBOVariableSlot = 10; typedef ShaderVariable (*ExtInstImpl)(ThreadState &, uint32_t, const rdcarray &); diff --git a/renderdoc/driver/shaders/spirv/spirv_debug_setup.cpp b/renderdoc/driver/shaders/spirv/spirv_debug_setup.cpp index 0afac5aab..95939bb08 100644 --- a/renderdoc/driver/shaders/spirv/spirv_debug_setup.cpp +++ b/renderdoc/driver/shaders/spirv/spirv_debug_setup.cpp @@ -185,6 +185,7 @@ ShaderDebugTrace *Debugger::BeginDebug(DebugAPIWrapper *apiWrapper, const Shader case Capability::TessellationPointSize: case Capability::GeometryPointSize: case Capability::ImageGatherExtended: + case Capability::StorageImageMultisample: case Capability::UniformBufferArrayDynamicIndexing: case Capability::SampledImageArrayDynamicIndexing: case Capability::StorageBufferArrayDynamicIndexing: @@ -244,9 +245,6 @@ ShaderDebugTrace *Debugger::BeginDebug(DebugAPIWrapper *apiWrapper, const Shader // we plan to support these but needs additional testing/proving - // image storage - case Capability::StorageImageMultisample: - // all these are related to non-32-bit types case Capability::Float16Buffer: case Capability::Float16: @@ -563,6 +561,8 @@ ShaderDebugTrace *Debugger::BeginDebug(DebugAPIWrapper *apiWrapper, const Shader // TODO handle arrays var.SetBinding((int32_t)set, (int32_t)bind, 0U); + var.value.u64v[SSBOVariableSlot] = 1; + sourceVar.type = VarType::ReadWriteResource; sourceVar.rows = 1; sourceVar.columns = 1; @@ -671,8 +671,19 @@ ShaderDebugTrace *Debugger::BeginDebug(DebugAPIWrapper *apiWrapper, const Shader var.value.uv[TextureTypeVariableSlot] = texType; - global.readOnlyResources.push_back(var); - readOnlyIDs.push_back(v.id); + if(imageTypes[imgid].sampled == 2) + { + var.type = VarType::ReadWriteResource; + debugType = DebugVariableType::ReadWriteResource; + + global.readWriteResources.push_back(var); + readWriteIDs.push_back(v.id); + } + else + { + global.readOnlyResources.push_back(var); + readOnlyIDs.push_back(v.id); + } } else { @@ -979,7 +990,7 @@ ShaderVariable Debugger::MakeCompositePointer(const ShaderVariable &base, Id id, if(base.type == VarType::GPUPointer) leaf = (const ShaderVariable *)(uintptr_t)base.value.u64v[0]; - if(leaf->type == VarType::ReadWriteResource) + if(leaf->type == VarType::ReadWriteResource && leaf->value.u64v[SSBOVariableSlot]) { ShaderVariable ret = MakePointerVariable(id, leaf); @@ -1132,7 +1143,7 @@ ShaderVariable Debugger::ReadFromPointer(const ShaderVariable &ptr) const ShaderVariable ret; - if(inner->type == VarType::ReadWriteResource) + if(inner->type == VarType::ReadWriteResource && inner->value.u64v[SSBOVariableSlot]) { rdcspv::Id typeId = rdcspv::Id::fromWord(uint32_t(ptr.value.u64v[BufferPointerTypeIdVariableSlot])); diff --git a/renderdoc/driver/vulkan/vk_shaderdebug.cpp b/renderdoc/driver/vulkan/vk_shaderdebug.cpp index c8373f294..180847d0b 100644 --- a/renderdoc/driver/vulkan/vk_shaderdebug.cpp +++ b/renderdoc/driver/vulkan/vk_shaderdebug.cpp @@ -329,6 +329,56 @@ public: memcpy(data.data() + (size_t)offset, src, (size_t)byteSize); } + virtual bool ReadTexel(BindpointIndex imageBind, const ShaderVariable &coord, uint32_t sample, + ShaderVariable &output) override + { + ImageData &data = PopulateImage(imageBind); + + if(data.width == 0) + return false; + + if(coord.value.uv[0] > data.width || coord.value.uv[1] > data.height || + coord.value.uv[2] > data.depth) + { + m_pDriver->AddDebugMessage( + MessageCategory::Execution, MessageSeverity::High, MessageSource::RuntimeWarning, + StringFormat::Fmt( + "Out of bounds access to image, coord %u,%u,%u outside of dimensions %ux%ux%u", + coord.value.uv[0], coord.value.uv[1], coord.value.uv[2], data.width, data.height, + data.depth)); + return false; + } + + memcpy(output.value.uv, data.texel(coord.value.uv, sample), data.texelSize); + + return true; + } + + virtual bool WriteTexel(BindpointIndex imageBind, const ShaderVariable &coord, uint32_t sample, + const ShaderVariable &value) override + { + ImageData &data = PopulateImage(imageBind); + + if(data.width == 0) + return false; + + if(coord.value.uv[0] > data.width || coord.value.uv[1] > data.height || + coord.value.uv[2] > data.depth) + { + m_pDriver->AddDebugMessage( + MessageCategory::Execution, MessageSeverity::High, MessageSource::RuntimeWarning, + StringFormat::Fmt( + "Out of bounds access to image, coord %u,%u,%u outside of dimensions %ux%ux%u", + coord.value.uv[0], coord.value.uv[1], coord.value.uv[2], data.width, data.height, + data.depth)); + return false; + } + + memcpy(data.texel(coord.value.uv, sample), value.value.uv, data.texelSize); + + return true; + } + virtual void FillInputValue(ShaderVariable &var, ShaderBuiltin builtin, uint32_t location, uint32_t component) override { @@ -1077,6 +1127,28 @@ private: bytebuf pushData; std::map bufferCache; + + struct ImageData + { + uint32_t width = 0, height = 0, depth = 0; + uint32_t texelSize = 0, rowPitch = 0, slicePitch = 0, samplePitch = 0; + bytebuf bytes; + + byte *texel(const uint32_t *coord, uint32_t sample) + { + byte *ret = bytes.data(); + + ret += samplePitch * sample; + ret += slicePitch * coord[2]; + ret += rowPitch * coord[1]; + ret += texelSize * coord[0]; + + return ret; + } + }; + + std::map imageCache; + template const T &GetDescriptor(const rdcstr &access, BindpointIndex index, bool &valid) { @@ -1175,6 +1247,82 @@ private: return data; } + ImageData &PopulateImage(BindpointIndex bind) + { + auto insertIt = imageCache.insert(std::make_pair(bind, ImageData())); + ImageData &data = insertIt.first->second; + if(insertIt.second) + { + bool valid = true; + const VkDescriptorImageInfo &imgData = + GetDescriptor("performing image load/store", bind, valid); + if(valid) + { + // if the resources might be dirty from side-effects from the draw, replay back to right + // before it. + if(m_ResourcesDirty) + { + m_pDriver->ReplayLog(0, m_EventID, eReplay_WithoutDraw); + m_ResourcesDirty = false; + } + + const VulkanCreationInfo::ImageView &viewProps = + m_Creation.m_ImageView[GetResID(imgData.imageView)]; + const VulkanCreationInfo::Image &imageProps = m_Creation.m_Image[viewProps.image]; + + uint32_t mip = viewProps.range.baseMipLevel; + + data.width = RDCMAX(1U, imageProps.extent.width >> mip); + data.height = RDCMAX(1U, imageProps.extent.height >> mip); + if(imageProps.type == VK_IMAGE_TYPE_3D) + { + data.depth = RDCMAX(1U, imageProps.extent.depth >> mip); + } + else + { + data.depth = viewProps.range.layerCount; + if(data.depth == VK_REMAINING_ARRAY_LAYERS) + data.depth = imageProps.arrayLayers - viewProps.range.baseArrayLayer; + } + + data.texelSize = GetByteSize(1, 1, 1, imageProps.format, 0); + data.rowPitch = GetByteSize(data.width, 1, 1, imageProps.format, 0); + data.slicePitch = GetByteSize(data.width, data.height, 1, imageProps.format, 0); + data.samplePitch = GetByteSize(data.width, data.height, data.depth, imageProps.format, 0); + + const uint32_t numSlices = imageProps.type == VK_IMAGE_TYPE_3D ? 1 : data.depth; + const uint32_t numSamples = (uint32_t)imageProps.samples; + + data.bytes.reserve(data.samplePitch * numSamples); + + // defaults are fine - no interpretation. Maybe we could use the view's typecast? + const GetTextureDataParams params; + + for(uint32_t sample = 0; sample < numSamples; sample++) + { + for(uint32_t slice = 0; slice < numSlices; slice++) + { + bytebuf subBytes; + m_pDriver->GetReplay()->GetTextureData(viewProps.image, Subresource(mip, slice, sample), + params, subBytes); + + // fast path, swap into output if there's only one slice and one sample (common case) + if(numSlices == 1 && numSamples == 1) + { + subBytes.swap(data.bytes); + } + else + { + data.bytes.append(subBytes); + } + } + } + } + } + + return data; + } + VkPipeline MakePipe(const ShaderConstParameters ¶ms, bool uintTex, bool sintTex) { VkSpecializationMapEntry specMaps[sizeof(params) / sizeof(uint32_t)]; diff --git a/util/test/demos/vk/vk_shader_debug_zoo.cpp b/util/test/demos/vk/vk_shader_debug_zoo.cpp index 532ebd866..ab650f21c 100644 --- a/util/test/demos/vk/vk_shader_debug_zoo.cpp +++ b/util/test/demos/vk/vk_shader_debug_zoo.cpp @@ -141,10 +141,10 @@ layout(set = 0, binding = 5, std430) buffer storebuftype vec4 arr[]; } storebuf; -//layout(set = 0, binding = 6, rgba32f) uniform coherent image2D storeImage; +layout(set = 0, binding = 6, rgba32f) uniform coherent image2D storeImage; layout(set = 0, binding = 7) uniform samplerBuffer texBuffer; -//layout(set = 0, binding = 8, rgba32f) uniform coherent imageBuffer storeTexBuffer; +layout(set = 0, binding = 8, rgba32f) uniform coherent imageBuffer storeTexBuffer; layout(set = 0, binding = 9) uniform sampler shadowSampler; @@ -1080,6 +1080,22 @@ void main() Color.zw = textureQueryLod(linearSampledImage, vec2(1.0f, 1.0f)/inpos); break; } + case 131: + { + Color = vec4(vec2(imageSize(storeImage)), 0.0f, 1.0f); + break; + } + case 132: + { + Color = vec4(float(imageSize(storeTexBuffer)), 0.0f, 0.0f, 1.0f); + break; + } + case 133: + { + imageStore(storeImage, ivec2(zeroi+1,zeroi+3), vec4(3.1f, 4.1f, 5.9f, 2.6f)); + Color = imageLoad(storeImage, ivec2(zeroi+1,zeroi+3)); + break; + } default: break; } } @@ -1090,6 +1106,9 @@ void main() void main() { uint test = flatData.test; + int intval = int(flatData.intval); + uint zerou = flatData.intval - flatData.test - 7u; + int zeroi = int(zerou); Color = vec4(0,0,0,0); switch(test) @@ -1112,6 +1131,11 @@ void main() Color = storebuf.arr[flatData.intval - flatData.test]; break; } + case 3: + { + Color = imageLoad(storeImage, ivec2(zeroi+1,zeroi+3)); + break; + } default: break; } }