Implement graphics shader patching

Shader debugging now works for vertex/fragment shaders, too.
The base for ray tracing shaders is in place as well.
This now just requires shader table patching.

docs/own/ideas.md

@@ -1,6 +1,9 @@
 Moved from todo.md. Mostly ideas for experiments that might not even
 be possible or useful in the end.
 
+- add mode where hooking is disabled. Commands can still be inspected
+	but, like, just statically.
+
 For shader capture debugging:
 - can we support capturing images/samples/buffers/RayTracingAccelStructs
       and other types? Allowed in quite some high-level languages.
@@ -28,7 +31,7 @@ For shader capture debugging:
   don't hook anything. Instead just insert our own command buffers
   (before and after) into the submission stream where we write out timestamps
 
-- (low prio) add also full batch (e.g. vkQueueSubmit/vkQueueBindSparse) timings? 
+- (low prio) add also full batch (e.g. vkQueueSubmit/vkQueueBindSparse) timings?
   Would require custom cb recording on our side
 
 - When clicking on a flag, extension name or something, link to vulkan api spec

docs/own/todo.md

@@ -7,15 +7,12 @@ v0.3:
 - improve README, add more gifs/pics
 
 urgent, bugs:
-- [x] look into annoying lmm.cpp:138 match assert
 - [ ] fix image viewer layout
 - [ ] viewing texture in command viewer: show size of view (i.e. active mip level),
       not the texture itself. Can be confusing otherwise
 	- [ ] maybe show full image size on hover?
 	- [ ] also fix mip/layer selector that sometimes automatically resets itself
 		  (seen with slice 3D selector e.g. npt surfel lookup tex)
-- [ ] finish submissions in order, see CommandHookSubmission::finish
-      comment for accelStruct captures
 - [ ] immediately free HookedRecords that are not to be re-used in finish.
 
 - [ ] test more on laptop, intel gpu
@@ -58,9 +55,7 @@ urgent, bugs:
 	  (try to test with RDR2 again)
 
 new, workstack:
-- [ ] add mode where hooking is disabled. Commands can still be inspected
-	but, like, just statically.
-- [ ] add isStateCmd(const Command&) and remove remaining command dynamic casts
+- [ ] use isStateCmd(const Command&) to remove remaining command dynamic casts
 - [ ] handle imgui cursor-to-be-shown and clipboard
 	- [ ] make sure to pass it via interface
 	- [ ] with hooked overlay, we have to implement it ourselves
@@ -244,14 +239,14 @@ Freeze/selection changes:
 		  Might also happen when the window is minimized on some platforms?
 
 patch capture shader debugging:
-- [ ] add support for structs and arrays
-- [ ] improve filtering of variables so that it works well for glslang and slang
+- [x] add support for structs and arrays
+- [x] improve filtering of variables so that it works well for glslang and slang
 - [x] add support for dispatch thread ID selection
 	- [x] add branch to command patching
 	      branch based on root constant? or specialization constant?
 		  or just load information dynamically from the capture buffer?
 	- [x] re-add the selection widget (from debug emulation cpp)
-	- [ ] remove "allow outside of bounds exec"
+	- [x] remove "allow outside of bounds exec"
 	- [ ] instead allow a mode where just any thread that hits the point
 	      writes info (via atomics). Connected to idea below,
 		  implementation could be the same.
@@ -261,32 +256,42 @@ patch capture shader debugging:
       breakpoint was even hit. Clear it to 0 before recording dst command
 	- [x] when not hit: do not show variables in output, instead state
 	      that breakpoint is not hit.
-- [ ] add more general widget for selecting a stage of a pipe in the debugger
-- [ ] add support for vertex shader debugging
+- [x] add more general widget for selecting a stage of a pipe in the debugger
+- [x] add support for vertex shader debugging
 	- [x] branch based on vertexID
 	- [x] widget to select vertexID. See vertexViewer branch
-- [ ] additional shader debug selects
-	- [ ] Layer
-	- [ ] ViewIndex
-	- [ ] ViewportIndex
 - [ ] add support for pixel shader debugging
-	- [ ] branch based on pixel position
+	- [x] branch based on pixel position
 	- [ ] allow to select sample for msaa
-	- [ ] widget to select pixel position
+	- [x] widget to select pixel position
+	- [x] alternative positions: mouse cursor/last cliked mouse cursor
 	- [ ] allow to get there via a "debug this pixel" button
 	      in image viewer?
+- [ ] separate function arguments and local vars in UI
+- [ ] toggle via UI: also capture all local named SSA IDs
+- [ ] matrix decoration in captured output
+- [ ] show global variables in captured output? (entry point interface vars)
+	- [ ] also builtins? maybe in different tab/node?
 - [ ] ray tracing debugging
-	- [ ] branch in all shaders via LaunchID
+	- [x] branch in all shaders via LaunchID
 	- [ ] additionally: allow to branch via ahs/chs inputs
 	- [ ] hook shader tables
 	      create reverse mapping inside of pipeline
 		  then, when hooking the traceRayCommand, create own shader table
 		  on the fly where the hooked shaders are replaced?
-- [ ] separate function arguments and local vars in UI
-- [ ] toggle via UI: also capture all local named SSA IDs
-- [ ] matrix decoration in captured output
-- [ ] show global variables in captured output?
-	- [ ] also builtins? maybe in different tab/node?
+- [ ] fix CRASH potential in PatchJobData::pipe. Should not be
+      IntrusiveDerivedPtr, something else. Should keep it alive in
+	  a different way.
+- [ ] fix terrible pipeline-keepAlive ShaderPatch hack
+- [ ] additional shader debug selects (vertex/fragment)
+	- [ ] Layer
+	- [ ] ViewIndex
+	- [ ] ViewportIndex
+- [ ] improve patch compile time by passing in basePipelineHandle
+- [ ] improve patching speed: don't insert every instruction into spirv vector.
+      First gather everything (for every section etc) then do one
+	  patch-build pass.
+	  Current approach copies again and again, problematic for large shaders.
 
 spvm:
 - [x] Add OpSpecConstant* support

src/command/commands.hpp

@@ -472,6 +472,12 @@ struct StateCmdBase : Command {
 	void visit(CommandVisitor& v) const override { doVisit(v, *this); }
 };
 
+inline bool isStateCmd(const Command& cmd) {
+	return cmd.category() == CommandCategory::dispatch ||
+		cmd.category() == CommandCategory::draw ||
+		cmd.category() == CommandCategory::traceRays;
+}
+
 struct DrawCmdBase : StateCmdBase {
 	const GraphicsState* state {};
 	PushConstantData pushConstants;

src/commandHook/record.cpp

@@ -464,8 +464,9 @@ void CommandHookRecord::hookRecordDst(Command& cmd, RecordInfo& info) {
 			dev.dispatch.CmdBindPipeline(cb, VK_PIPELINE_BIND_POINT_COMPUTE,
 				info.ops.useCapturePipe);
 		} else if(cmd.category() == CommandCategory::traceRays) {
-			dev.dispatch.CmdBindPipeline(cb, VK_PIPELINE_BIND_POINT_RAY_TRACING_KHR,
-				info.ops.useCapturePipe);
+			dlg_warn("TODO: implement capturePipe for RayTracing via shaderTable patching");
+			// dev.dispatch.CmdBindPipeline(cb, VK_PIPELINE_BIND_POINT_RAY_TRACING_KHR,
+			// 	info.ops.useCapturePipe);
 		} else {
 			dlg_error("Unexpected hooked command used with capturePipe");
 		}
@@ -1732,11 +1733,14 @@ void CommandHookRecord::afterDstOutsideRp(Command& bcmd, RecordInfo& info) {
 
 	if(info.ops.useCapturePipe) {
 		if(bcmd.category() == CommandCategory::draw) {
-			dlg_error("rebindGraphicsState not implemented");
+			dev.dispatch.CmdBindPipeline(cb, VK_PIPELINE_BIND_POINT_GRAPHICS,
+				static_cast<const DrawCmdBase&>(bcmd).boundPipe()->handle);
 		} else if(bcmd.category() == CommandCategory::dispatch) {
-			info.rebindComputeState = true;
+			dev.dispatch.CmdBindPipeline(cb, VK_PIPELINE_BIND_POINT_COMPUTE,
+				static_cast<const DispatchCmdBase&>(bcmd).boundPipe()->handle);
 		} else if(bcmd.category() == CommandCategory::traceRays) {
-			dlg_error("rebindRayTraceState not implemented");
+			dev.dispatch.CmdBindPipeline(cb, VK_PIPELINE_BIND_POINT_COMPUTE,
+				static_cast<const TraceRaysCmdBase&>(bcmd).boundPipe()->handle);
 		} else {
 			dlg_error("Unexpected hooked command used with capturePipe");
 		}

src/data/shaderTable.comp

@@ -1,5 +1,6 @@
 #version 460
 
+#extension GL_EXT_buffer_reference : require
 #extension GL_GOOGLE_include_directive : require
 
 layout(local_size_x = 64) in;
@@ -17,12 +18,12 @@ layout(buffer_reference, std430, buffer_reference_align = 16) buffer Mappings {
 };
 
 layout(push_constant) uniform PCR {
-	uint stride;
-	uint size;
-	uint count;
 	Src src;
 	Dst dst;
 	Mappings mappings;
+	uint stride;
+	uint size;
+	uint count;
 } pcr;
 
 void main() {

src/device.cpp

@@ -569,6 +569,10 @@ VkResult doCreateDevice(
 		fpPhdevFeatures2 && fpPhdevProps2 &&
 		((ini.vulkan12 && phdevProps.apiVersion >= VK_API_VERSION_1_2) ||
 			hasExt(supportedExts, VK_KHR_8BIT_STORAGE_EXTENSION_NAME));
+	auto hasDrawParamsApi =
+		fpPhdevFeatures2 && fpPhdevProps2 &&
+		((ini.vulkan11 && phdevProps.apiVersion >= VK_API_VERSION_1_1) ||
+			hasExt(supportedExts, VK_KHR_SHADER_DRAW_PARAMETERS_EXTENSION_NAME));
 	auto hasDeviceFaultApi = enableDeviceFault &&
 		fpPhdevFeatures2 && fpPhdevProps2 &&
 		hasExt(supportedExts, VK_EXT_DEVICE_FAULT_EXTENSION_NAME);
@@ -583,6 +587,7 @@ VkResult doCreateDevice(
 	auto hasAddressBindingReport = false;
 	bool hasStorage8 = false;
 	bool hasStorage16 = false;
+	bool hasDrawParams = false;
 
 	// find generally relevant feature structs in chain
 	VkPhysicalDeviceVulkan11Features features11 {};
@@ -598,6 +603,7 @@ VkResult doCreateDevice(
 	VkPhysicalDeviceAddressBindingReportFeaturesEXT* inABR = nullptr;
 	VkPhysicalDevice16BitStorageFeatures* inStorage16 {};
 	VkPhysicalDevice8BitStorageFeatures* inStorage8 {};
+	VkPhysicalDeviceShaderDrawParametersFeatures* inDrawParams {};
 
 	auto* link = static_cast<VkBaseOutStructure*>(pNext);
 	while(link) {
@@ -626,6 +632,8 @@ VkResult doCreateDevice(
 			inStorage8 = reinterpret_cast<VkPhysicalDevice8BitStorageFeatures*>(link);
 		} else if(link->sType == VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_16BIT_STORAGE_FEATURES) {
 			inStorage16 = reinterpret_cast<VkPhysicalDevice16BitStorageFeatures*>(link);
+		} else if(link->sType == VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_SHADER_DRAW_PARAMETERS_FEATURES) {
+			inDrawParams = reinterpret_cast<VkPhysicalDeviceShaderDrawParametersFeatures*>(link);
 		}
 
 		link = (static_cast<VkBaseOutStructure*>(link->pNext));
@@ -637,8 +645,8 @@ VkResult doCreateDevice(
 	VkPhysicalDeviceAddressBindingReportFeaturesEXT abrFeatures {};
 	VkPhysicalDevice16BitStorageFeatures storage16Features {};
 	VkPhysicalDevice8BitStorageFeatures storage8Features {};
-	if(hasTimelineSemaphoresApi || hasTransformFeedbackApi || hasDeviceFaultApi ||
-			has16BitStorageApi || has8BitStorageApi) {
+	VkPhysicalDeviceShaderDrawParametersFeatures drawParamsFeatures {};
+	if(fpPhdevFeatures2 && fpPhdevProps2) {
 		dlg_assert(fpPhdevFeatures2);
 		dlg_assert(fpPhdevProps2);
 
@@ -669,6 +677,12 @@ VkResult doCreateDevice(
 			features2.pNext = &storage16Features;
 		}
 
+		if(hasDrawParamsApi) {
+			drawParamsFeatures.sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_SHADER_DRAW_PARAMETERS_FEATURES;
+			drawParamsFeatures.pNext = features2.pNext;
+			features2.pNext = &drawParamsFeatures;
+		}
+
 		if(hasDeviceFaultApi) {
 			dfFeatures.sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_FAULT_FEATURES_EXT;
 			dfFeatures.pNext = features2.pNext;
@@ -749,6 +763,19 @@ VkResult doCreateDevice(
 			}
 		}
 
+		if(drawParamsFeatures.shaderDrawParameters) {
+			hasDrawParams = true;
+
+			if(inDrawParams) {
+				inDrawParams->shaderDrawParameters = true;
+			} else if(inVulkan11) {
+				inVulkan11->shaderDrawParameters = true;
+			} else {
+				drawParamsFeatures.pNext = const_cast<void*>(nci.pNext);
+				nci.pNext = &drawParamsFeatures;
+			}
+		}
+
 		if(dfFeatures.deviceFault) {
 			hasDeviceFault = true;
 
@@ -845,6 +872,7 @@ VkResult doCreateDevice(
 	dev.extDeviceFault = hasDeviceFault;
 	dev.storage8Bit = hasStorage8;
 	dev.storage16Bit = hasStorage16;
+	dev.shaderDrawParameters = hasDrawParams;
 
 	if(hasAddressBindingReport) {
 		dev.addressMap = std::make_unique<DeviceAddressMap>();

src/device.hpp

@@ -98,6 +98,7 @@ struct Device {
 	bool storage8Bit {};
 	bool storage16Bit {};
 	bool shaderStorageImageWriteWithoutFormat {};
+	bool shaderDrawParameters {};
 	bool extDeviceFault {}; // whether EXT_device_fault was enabled
 
 	// Only valid when EXT_device_address_binding_report enabled.

src/gui/command.cpp

@@ -1539,49 +1539,13 @@ void CommandViewer::displayCommand() {
 		}
 	}
 
-	// TODO: WIP
-	if(command_.back()->category() == CommandCategory::dispatch) {
-		auto* dcmd = deriveCast<const DispatchCmdBase*>(command_.back());
-		if(ImGui::Button("Debug shader")) {
-			if(dcmd->state->pipe) {
-				auto mod = copySpecializeSpirv(dcmd->state->pipe->stage);
-				shaderDebugger_.select(VK_SHADER_STAGE_COMPUTE_BIT,
-					std::move(mod), dcmd->state->pipe->stage.entryPoint);
-				view_ = IOView::shader;
-				doUpdateHook_ = true;
-				return;
-			}
-		}
-	} else if(auto* dcmd = dynamic_cast<const DrawCmdBase*>(command_.back()); dcmd) {
-		auto* pipe = dcmd->boundPipe();
-		if(pipe) {
-			dlg_assert(pipe->type == VK_PIPELINE_BIND_POINT_GRAPHICS);
-			auto& gpipe = static_cast<const GraphicsPipeline&>(*pipe);
-			const PipelineShaderStage* vertex {};
-			for(auto& stage : gpipe.stages) {
-				if(stage.stage == VK_SHADER_STAGE_VERTEX_BIT) {
-					vertex = &stage;
-					break;
-				}
-			}
-
-			// TODO: add support for mesh shader. Or at least show
-			//   message in UI?
-			dlg_assertm(vertex, "TODO Graphics pipeline without vertex shader");
-			if(vertex) {
-				if(ImGui::Button("Debug vertex shader")) {
-					if(dcmd->state->pipe) {
-						auto mod = copySpecializeSpirv(*vertex);
-						shaderDebugger_.select(vertex->stage, std::move(mod),
-							vertex->entryPoint);
-						view_ = IOView::shader;
-						doUpdateHook_ = true;
-						return;
-					}
-				}
-			}
-
-			// TODO: add fragment shader
+	if(isStateCmd(*command_.back())) {
+		auto* dcmd = deriveCast<const StateCmdBase*>(command_.back());
+		if(dcmd->boundPipe() && ImGui::Button("Debug shader")) {
+			shaderDebugger_.select(*dcmd->boundPipe());
+			view_ = IOView::shader;
+			doUpdateHook_ = true;
+			return;
 		}
 	}