graphics: fix vertex and instance IDs for unpatched indirect draws

src/graphics/guest_gpu/graphicsRun.cpp

@@ -856,9 +856,8 @@ void CommandProcessor::DrawIndirectMulti(uint32_t data_offset, uint32_t max_coun
 	// NUM_INSTANCES keeps the last draw's instance count.
 	m_num_instances_addr = args_addr + static_cast<uint64_t>(draw_count - 1) * stride_in_bytes +
 	                       offsetof(vk::DrawIndirectCommand, instanceCount);
-	// The GPU reads the records if the shaders allow; the VS prolog loads the patched SGPRs from
+	// The GPU reads the records if the shaders allow; the VS prolog removes Vulkan's bases
-	// Vulkan's bases. With no SGPR patched, Vulkan applies the offsets, which matches while they
+	// from the index VGPRs and copies them only to the user SGPRs the packet patches.
-	// are zero.
 	DrawIndirectArgs records {args_addr, draw_count, stride_in_bytes};
 	if (IndirectDrawOffsets(registers, records.offsets) && (!indexed || m_index_buffer_size != 0) &&
 	    (indexed ? executor.DrawIndex(m_submit_id, CurrentBuffer(),

src/graphics/host_gpu/renderer/pipeline/pipelineCache.cpp

@@ -689,6 +689,7 @@ void PipelineCache::GetGraphicsPrograms(const CommandBuffer& command, DrawRender
 	const bool mesh_active = vertex_info[0].logical_stage == ShaderType::Mesh;
 	if (!tess_active && !mesh_active) {
 		vertex_info[0].indirect_offsets = state.indirect_offsets;
+		vertex_info[0].indirect_draw    = state.indirect_draw;
 	}
 	if (object_primitive_id) {
 		// CxObjectIdMode: register source plus a 32-bit pipelined primitive-ID addition.

src/graphics/host_gpu/renderer/pipeline/pipelineCache.h

@@ -139,6 +139,7 @@ struct DrawRenderState {
 	ShaderPixelInputInfo                 ps_input_info;
 	// Input: the user-data slots a GPU-read indirect draw patches.
 	ShaderIndirectOffsets                indirect_offsets;
+	bool                                 indirect_draw = false;
 
 	[[nodiscard]] std::span<const ShaderVertexInputInfo> VertexStages() const {
 		return {vertex_info.data(), vertex_info[1].stage ? 3u : 1u};

src/graphics/host_gpu/renderer/renderDraw.cpp

@@ -52,7 +52,7 @@ std::pair<int32_t, uint32_t> ResolveDrawOffsets(uint32_t index_offset,
 	                                           const ShaderVertexInputInfo& vs_input_info) {
 	auto     vertex_offset   = static_cast<int32_t>(index_offset);
 	uint32_t instance_offset = 0;
-	if (!vs_input_info.fetch_embedded) {
+	if (!vs_input_info.has_dynamic_vertex_fetch) {
 		return {vertex_offset, instance_offset};
 	}
 
@@ -1251,16 +1251,26 @@ void RenderExecutor::ExecutePreparedDraw(uint64_t submit_id, CommandBuffer& buff
 }
 
 // Vulkan offsets for a prepared draw, or nullopt when its indirect records need the CPU: mesh
-// and quad-list draws for counts, tessellation draws for patched SGPRs. Indirect records carry
+// and quad-list draws for counts, tessellation draws for the index/SGPR prolog. Indirect records
-// their own offsets.
+// carry their own offsets.
 static std::optional<DrawEmitInfo> ResolveDrawEmit(const HW::UserConfig&        ucfg,
                                                    const ShaderVertexInputInfo& vs,
                                                    DrawOffsetSource             source,
                                                    int32_t                      first_vertex,
                                                    uint32_t                     first_instance,
                                                    const DrawIndirectArgs&      indirect) {
+	if ((indirect.address != 0 || source == DrawOffsetSource::IndirectArgs) &&
+	    ucfg.GetIndexOffset() != 0) {
+		static std::once_flag warning_once;
+		std::call_once(warning_once, [&] {
+			Log::WriteToConsoleAndLog(fmt::format(
+			    "GPU: indirect draw ignores GE_INDX_OFFSET=0x{:08x}; geometry may be incorrect.\n",
+			    ucfg.GetIndexOffset()));
+		});
+	}
 	if (indirect.address != 0) {
-		if (vs.stage.module->info.stage == ShaderType::Mesh ||
+		if (!vs.indirect_draw ||
+		    vs.stage.module->info.stage == ShaderType::Mesh ||
 		    ucfg.GetPrimType() == Prospero::PrimitiveType::kQuadListLegacy ||
 		    vs.indirect_offsets != indirect.offsets) {
 			return std::nullopt;
@@ -1269,7 +1279,7 @@ static std::optional<DrawEmitInfo> ResolveDrawEmit(const HW::UserConfig&
 	}
 	if (source == DrawOffsetSource::IndirectArgs) {
 		// Native fetches consume the patched SGPRs; rewritten fetches need Vulkan offsets.
-		return vs.fetch_embedded ? DrawEmitInfo {first_vertex, first_instance} : DrawEmitInfo {};
+		return vs.has_dynamic_vertex_fetch ? DrawEmitInfo {first_vertex, first_instance} : DrawEmitInfo {};
 	}
 	const auto [vertex_offset, instance_offset] = ResolveDrawOffsets(ucfg.GetIndexOffset(), vs);
 	return DrawEmitInfo {vertex_offset + first_vertex, instance_offset};
@@ -1326,7 +1336,8 @@ bool RenderExecutor::DrawIndex(uint64_t submit_id, CommandBuffer& buffer,
 
 	hw_check(buffer);
 
-	DrawRenderState state {.indirect_offsets = args.indirect.offsets};
+	DrawRenderState state {.indirect_offsets = args.indirect.offsets,
+	                       .indirect_draw    = args.indirect.address != 0};
 	if (!GetDrawTopology(ucfg, state.topology)) {
 		return true;
 	}
@@ -1431,7 +1442,8 @@ bool RenderExecutor::DrawAuto(uint64_t submit_id, CommandBuffer& buffer, const D
 	const DrawCallInfo draw {CommandBufferDebugOp::DrawIndexAuto,
 	                         args.vertex_count, args.instance_count, args.first_instance};
 
-	DrawRenderState state {.indirect_offsets = args.indirect.offsets};
+	DrawRenderState state {.indirect_offsets = args.indirect.offsets,
+	                       .indirect_draw    = args.indirect.address != 0};
 	if (!GetDrawTopology(ucfg, state.topology)) {
 		ResetBindings();
 		return true;

src/graphics/shader/recompiler/ShaderRecompiler.cpp

@@ -814,7 +814,7 @@ TranslateResult TranslateProgram(const Decoder::Program& decoded, const CompileO
 	const auto& cfg = *selected_cfg;
 	Frontend::EmbeddedFetchPlan embedded_fetch;
 	if ((options.stage == ShaderType::Vertex || options.stage == ShaderType::Local) &&
-	    options.input_info.vertex->fetch_embedded) {
+	    options.input_info.vertex->has_dynamic_vertex_fetch) {
 		embedded_fetch = DetectEmbeddedVertexFetch(
 		    decoded, *options.input_info.vertex, options.user_data_base,
 		    static_cast<uint32_t>(options.user_data.size()), options.wave_size);

src/graphics/shader/recompiler/frontend/translate/Translate.cpp

@@ -1390,11 +1390,12 @@ IR::Program TranslateProgram(const Decoder::Program& decoded, const CFG::Graph&
 			entry_ir.SetScalarReg(static_cast<IR::ScalarReg>(3),
 			                      entry_ir.BitwiseOr(vertex_count, IR::U32(IR::Value(1u << 28u))));
 			// Vulkan adds an indirect record's bases to the indices; the hardware passes them
-			// only in the user SGPRs the packet patches.
+			// only in the user SGPRs the packet patches. SH_NOP leaves those SGPRs unchanged;
+			// it does not make the record's bases zero.
 			const auto& offsets = options.input_info.vertex->indirect_offsets;
 			const auto  index   = [&](uint8_t slot, IR::StageInputKind kind,
 			                          IR::StageInputKind base_kind) {
-				if (offsets == ShaderIndirectOffsets {}) {
+				if (!options.input_info.vertex->indirect_draw) {
 					return builtin(kind);
 				}
 				const auto base = builtin(base_kind);

src/graphics/shader/shader.cpp

@@ -291,10 +291,9 @@ static bool ShaderGetStaticVertexInputInfo(uint64_t shader_addr, const HW::UserS
 	const auto& metadata = data.vertex;
 
 	if (metadata.vertex_attrib_reg >= 0) {
-		info.fetch_external   = false;
+		info.has_dynamic_vertex_fetch = true;
-		info.fetch_embedded   = true;
+		info.fetch_attrib_reg         = metadata.vertex_attrib_reg;
-		info.fetch_attrib_reg = metadata.vertex_attrib_reg;
+		info.fetch_buffer_reg         = metadata.vertex_buffer_reg;
-		info.fetch_buffer_reg = metadata.vertex_buffer_reg;
 
 		const auto* attrib = reinterpret_cast<const uint32_t*>(
 		    static_cast<uint64_t>(user_sgpr.value[metadata.vertex_attrib_reg]) |
@@ -413,7 +412,7 @@ static void ShaderGetStaticInputInfoCS(const HW::ComputeShaderInfo& regs,
 void BuildStageStaticKey(const ShaderVertexInputInfo& info, std::vector<uint32_t>& key) {
 	EXIT_IF(info.resources_num < 0 || info.resources_num > ShaderVertexInputInfo::RES_MAX);
 	key.clear();
-	key.push_back(static_cast<uint32_t>(info.fetch_embedded));
+	key.push_back(static_cast<uint32_t>(info.has_dynamic_vertex_fetch));
 	key.push_back(static_cast<uint32_t>(info.fetch_attrib_reg));
 	key.push_back(static_cast<uint32_t>(info.fetch_buffer_reg));
 	key.push_back(info.resources_num);
@@ -422,7 +421,8 @@ void BuildStageStaticKey(const ShaderVertexInputInfo& info, std::vector<uint32_t
 	key.push_back(info.lds_size_dwords);
 	key.push_back(info.pa_cl_vs_out_cntl);
 	key.push_back(info.user_clip_planes);
-	key.push_back(info.indirect_offsets.vertex | (uint32_t {info.indirect_offsets.instance} << 8u));
+	key.push_back(info.indirect_offsets.vertex | (uint32_t {info.indirect_offsets.instance} << 8u) |
+	              (uint32_t {info.indirect_draw} << 16u));
 	key.push_back(static_cast<uint32_t>(info.clip_space.enabled));
 	if (info.clip_space.enabled) {
 		for (const float value: info.clip_space.scale) {
@@ -693,9 +693,7 @@ void ShaderDbgDumpInputInfo(const ShaderVertexInputInfo& info) {
 
 	LOGF("ShaderDbgDumpInputInfo()\n");
 
-	LOGF("\t fetch_external = %s\n"
+	LOGF("\t has_dynamic_vertex_fetch = %s\n", info.has_dynamic_vertex_fetch ? "true" : "false");
-	     "\t fetch_embedded = %s\n",
-	     info.fetch_external ? "true" : "false", info.fetch_embedded ? "true" : "false");
 
 	for (int i = 0; i < info.resources_num; i++) {
 		LOGF("\t input %d\n", i);

src/graphics/shader/shader.h

@@ -125,8 +125,7 @@ struct ShaderTessellationInputInfo {
 };
 
 // GS user-data slots an indirect draw packet writes its record's base vertex and first instance
-// to. Like the hardware index VGPRs, the vertex shader's indices exclude both bases once either
+// to. A packet can leave both slots unused even when its record contains nonzero offsets.
-// slot is set.
 struct ShaderIndirectOffsets {
 	static constexpr uint8_t None = UINT8_MAX;
 
@@ -159,8 +158,10 @@ struct ShaderVertexInputInfo {
 	ShaderMeshInputInfo      mesh;
 	ShaderTessellationInputInfo tess;
 	ShaderIndirectOffsets   indirect_offsets;
-	bool                    fetch_external      = false;
+	// GPU-read records add Vulkan bases even when the packet patches no user SGPRs.
-	bool                    fetch_embedded      = false;
+	bool                    indirect_draw       = false;
+	// Attribute-table metadata enables dynamic fetch detection; loads are matched later.
+	bool                    has_dynamic_vertex_fetch = false;
 };
 
 struct ShaderComputeInputInfo: ShaderWorkgroupInputInfo {

tests/ShaderRecompilerComputeTests.cpp

@@ -19739,9 +19739,9 @@ public:
               "the draw's cull mode or front face did not reach the host GPU");
       registers.SetModeControl({});
 
-      // Indirect records come from the GPU copy; the CPU backing holds zeros. In each pair the
+      // Indirect records come from the GPU copy; the CPU backing holds zeros. Embedded
-      // first record draws nothing and the second covers the target, through vertex offset 7
+      // fetches use vertex offset 7. Native shaders receive raw vertex/instance IDs when
-      // for embedded fetches.
+      // the packet does not patch either offset SGPR, even with nonzero record offsets.
       {
         constexpr uint64_t vertices_address = depth_address + 0x38000;
         constexpr uint64_t args_address = vertices_address + 0x100;
@@ -19752,19 +19752,23 @@ public:
                     sizeof(positions));
         constexpr uint32_t stride = 32;
         const vk::DrawIndexedIndirectCommand indexed_records[]{
-            {3, 1, 0, 0, 5}, {3, 1, 0, 7, 5}, {0, 1, 0, 0, 0}, {3, 1, 0, 0, 0}};
+            {3, 1, 0, 0, 5}, {3, 1, 0, 7, 5}, {0, 1, 0, 0, 0}, {3, 1, 0, 0, 0},
+            {0, 1, 0, 0, 0}, {3, 1, 0, -7, 0}, {0, 1, 0, 0, 0}, {3, 1, 0, 0, 11}};
         const vk::DrawIndirectCommand auto_records[]{{3, 1, 0, 5}, {3, 1, 7, 5}, {0, 1, 0, 0},
-                                                     {3, 1, 0, 0}};
+                                                     {3, 1, 0, 0}, {0, 1, 0, 0}, {3, 1, 13, 0},
-        for (u32 i = 0; i < 4; i++) {
+                                                     {0, 1, 0, 0}, {3, 1, 0, 11}};
+        constexpr u32 records_per_kind = std::size(indexed_records);
+        static_assert(records_per_kind == std::size(auto_records));
+        for (u32 i = 0; i < records_per_kind; i++) {
           std::memcpy(reinterpret_cast<void *>(args_address + i * stride), &indexed_records[i],
                       sizeof(indexed_records[i]));
-          std::memcpy(reinterpret_cast<void *>(args_address + (4 + i) * stride), &auto_records[i],
+          std::memcpy(reinterpret_cast<void *>(args_address + (records_per_kind + i) * stride),
-                      sizeof(auto_records[i]));
+                      &auto_records[i], sizeof(auto_records[i]));
         }
         // Larger than a caching page, so the arguments are cached instead of streamed.
         (void)context.GetBufferCache().ObtainBuffer(
             vertices_address, BufferCache::CACHING_PAGESIZE * 2, false);
-        const std::array<u32, 8 * stride / 4> zeros{};
+        const std::array<u32, 2 * records_per_kind * stride / 4> zeros{};
         Require("GpuIndirectDraw", "CPU backing",
                 LibKernel::Memory::TryWriteBacking(args_address, zeros.data(), sizeof(zeros)),
                 "the CPU copy of the arguments could not be cleared");
@@ -19825,26 +19829,35 @@ public:
           }
         };
         check("indexed embedded fetch", fetch_vs, true, 0);
-        check("auto embedded fetch", fetch_vs, false, 4);
+        check("auto embedded fetch", fetch_vs, false, records_per_kind);
         check("indexed native fetch", vertex_address, true, 2);
-        check("auto native fetch", vertex_address, false, 6);
+        check("auto native fetch", vertex_address, false, records_per_kind + 2);
         // Pipelines created from only their key and stage modules draw the same.
         scheduler.Finish();
         Require("PipelineFromKey", "vertex input pipelines",
                 PipelineCacheTestAccess::RecreateGraphicsPipelines(context.GetPipelineCache()) > 0,
                 "no cached graphics pipeline named only compiled modules");
         check("recreated indexed embedded fetch", fetch_vs, true, 0);
-        check("recreated auto native fetch", vertex_address, false, 6);
+        check("recreated auto native fetch", vertex_address, false, records_per_kind + 2);
         // Patched user SGPRs 0/1 read the records' base vertex 7 and first instance 5, not the
         // table pointers they hold, and the index VGPRs exclude both bases.
         check("indexed patched offsets", indirect_vs, true, 0, {0, 1});
-        check("auto patched offsets", indirect_vs, false, 4, {0, 1});
+        check("auto patched offsets", indirect_vs, false, records_per_kind, {0, 1});
         // Slots 4/5 lie past USER_SGPR, so the tables survive; rewritten fetches keep Vulkan's
         // vertex offset.
         check("indexed patched embedded fetch", fetch_vs, true, 0, {4, 5});
         // Patching one offset still leaves both bases out of the index VGPRs.
-        check("auto instance-only patch", vertex_address, false, 5,
+        check("auto instance-only patch", vertex_address, false, records_per_kind + 1,
               {ShaderIndirectOffsets::None, 1});
+        // SH_NOP leaves the existing SGPR values intact; the native prolog adds these
+        // values once while the record's unrelated offsets are ignored. Check each base
+        // independently so a correct vertex offset cannot hide a broken instance offset.
+        shaders.SetGsUserSgpr(0, 7);
+        shaders.SetGsUserSgpr(1, 5);
+        check("indexed unpatched vertex offset", indirect_vs, true, 4);
+        check("auto unpatched vertex offset", indirect_vs, false, records_per_kind + 4);
+        check("indexed unpatched instance offset", indirect_vs, true, 6);
+        check("auto unpatched instance offset", indirect_vs, false, records_per_kind + 6);
       }
 
       // A fourth VS invocation reads beyond the descriptor instead of reconstructing the corner.
@@ -47399,7 +47412,7 @@ void CheckEmbeddedFetchVertexOffset() {
     vertex.tess = {.input_control_points = 3, .output_control_points = 3,
                    .ls_stride = 124, .hs_stride = 128, .domain = 1,
                    .partitioning = 2, .output_topology = 2};
-    vertex.fetch_embedded = true;
+    vertex.has_dynamic_vertex_fetch = true;
     vertex.fetch_buffer_reg = 0;
     vertex.fetch_attrib_reg = 2;
     vertex.resources_num = 1;
@@ -47439,7 +47452,7 @@ void CheckEmbeddedFetchVertexOffset() {
   const auto Resolve = [](const CompiledShader &result,
                           u32 index_offset) {
     ShaderVertexInputInfo vertex;
-    vertex.fetch_embedded = true;
+    vertex.has_dynamic_vertex_fetch = true;
     ShaderModule module;
     module.info.user_data_base = result.program.user_data_base;
     module.info.info.vertex_offset_sgpr = result.program.info.vertex_offset_sgpr;
@@ -47450,7 +47463,7 @@ void CheckEmbeddedFetchVertexOffset() {
 
   const auto ResolveInstance = [](const CompiledShader &result) {
     ShaderVertexInputInfo vertex;
-    vertex.fetch_embedded = true;
+    vertex.has_dynamic_vertex_fetch = true;
     ShaderModule module;
     module.info.user_data_base = result.program.user_data_base;
     module.info.info.instance_offset_sgpr =
@@ -50344,7 +50357,7 @@ void CheckEmbeddedFetchLaneSpill() {
 
   std::array<u32, 11> user_data{};
   ShaderVertexInputInfo vertex;
-  vertex.fetch_embedded = true;
+  vertex.has_dynamic_vertex_fetch = true;
   vertex.fetch_buffer_reg = 0;
   vertex.resources_num = 1;
   vertex.resources[0].fields[3] =

tests/shaderCfgTests.cpp

@@ -1938,7 +1938,7 @@ void TestVertexBufferGrouping() {
   Check(input.fetch_attrib_reg == 2, "a draw reread the vertex metadata of a mapped shader");
   ShaderMapUserData(regs.es_regs.data_addr, mapped);
   (void)PrepareProgram(regs, HW::Context{}, HW::UserConfig{}, input);
-  Check(!input.fetch_embedded, "remapping a shader kept its previous vertex metadata");
+  Check(!input.has_dynamic_vertex_fetch, "remapping a shader kept its previous vertex metadata");
 }
 
 void TestNggVertexEntryState() {
@@ -11575,7 +11575,7 @@ void TestEmbeddedFetchPreservesSharedScalarLoad() {
                              reinterpret_cast<uint64_t>(attributes.data())};
   std::memcpy(user_data.data() + 12, tables, sizeof(tables));
   ShaderVertexInputInfo input{};
-  input.fetch_embedded = true;
+  input.has_dynamic_vertex_fetch = true;
   input.fetch_buffer_reg = 12;
   input.fetch_attrib_reg = 14;
   input.resources_num = 1;