Extracted the attribute setup and draw commands into their own functions

src/video_core/command_processor.cpp

@@ -119,64 +119,16 @@ static void WriteUniformFloatReg(ShaderRegs& config, Shader::ShaderSetup& setup,
     }
 }
 
-static void WritePicaReg(u32 id, u32 value, u32 mask) {
+static void LoadDefaultVertexAttributes(u32 register_value) {
     auto& regs = g_state.regs;
 
-    if (id >= Regs::NUM_REGS) {
-        LOG_ERROR(HW_GPU,
-                  "Commandlist tried to write to invalid register 0x%03X (value: %08X, mask: %X)",
-                  id, value, mask);
-        return;
-    }
-
-    // TODO: Figure out how register masking acts on e.g. vs.uniform_setup.set_value
-    u32 old_value = regs.reg_array[id];
-
-    const u32 write_mask = expand_bits_to_bytes[mask];
-
-    regs.reg_array[id] = (old_value & ~write_mask) | (value & write_mask);
-
-    // Double check for is_pica_tracing to avoid call overhead
-    if (DebugUtils::IsPicaTracing()) {
-        DebugUtils::OnPicaRegWrite({(u16)id, (u16)mask, regs.reg_array[id]});
-    }
-
-    if (g_debug_context)
-        g_debug_context->OnEvent(DebugContext::Event::PicaCommandLoaded,
-                                 reinterpret_cast<void*>(&id));
-
-    switch (id) {
-    // Trigger IRQ
-    case PICA_REG_INDEX(trigger_irq):
-        Service::GSP::SignalInterrupt(Service::GSP::InterruptId::P3D);
-        break;
-
-    case PICA_REG_INDEX(pipeline.triangle_topology):
-        g_state.primitive_assembler.Reconfigure(regs.pipeline.triangle_topology);
-        break;
-
-    case PICA_REG_INDEX(pipeline.restart_primitive):
-        g_state.primitive_assembler.Reset();
-        break;
-
-    case PICA_REG_INDEX(pipeline.vs_default_attributes_setup.index):
-        g_state.immediate.current_attribute = 0;
-        g_state.immediate.reset_geometry_pipeline = true;
-        default_attr_counter = 0;
-        break;
-
-    // Load default vertex input attributes
-    case PICA_REG_INDEX_WORKAROUND(pipeline.vs_default_attributes_setup.set_value[0], 0x233):
-    case PICA_REG_INDEX_WORKAROUND(pipeline.vs_default_attributes_setup.set_value[1], 0x234):
-    case PICA_REG_INDEX_WORKAROUND(pipeline.vs_default_attributes_setup.set_value[2], 0x235): {
     // TODO: Does actual hardware indeed keep an intermediate buffer or does
     //       it directly write the values?
-        default_attr_write_buffer[default_attr_counter++] = value;
+    default_attr_write_buffer[default_attr_counter++] = register_value;
 
-        // Default attributes are written in a packed format such that four float24 values are
+    // Default attributes are written in a packed format such that four float24 values are encoded
-        // encoded in
+    // in three 32-bit numbers.
-        // three 32-bit numbers. We write to internal memory once a full such vector is
+    // We write to internal memory once a full such vector is written.
-        // written.
     if (default_attr_counter >= 3) {
         default_attr_counter = 0;
 
@@ -184,7 +136,7 @@ static void WritePicaReg(u32 id, u32 value, u32 mask) {
 
         if (setup.index >= 16) {
             LOG_ERROR(HW_GPU, "Invalid VS default attribute index %d", (int)setup.index);
-                break;
+            return;
         }
 
         Math::Vec4<float24> attribute;
@@ -249,34 +201,16 @@ static void WritePicaReg(u32 id, u32 value, u32 mask) {
                 // See: https://github.com/citra-emu/citra/pull/2866#issuecomment-327011550
                 VideoCore::g_renderer->Rasterizer()->DrawTriangles();
                 if (g_debug_context) {
-                        g_debug_context->OnEvent(DebugContext::Event::FinishedPrimitiveBatch,
+                    g_debug_context->OnEvent(DebugContext::Event::FinishedPrimitiveBatch, nullptr);
-                                                 nullptr);
                 }
             }
         }
     }
-        break;
+}
-    }
 
-    case PICA_REG_INDEX(pipeline.gpu_mode):
+static void Draw(u32 command_id) {
-        // This register likely just enables vertex processing and doesn't need any special handling
-        break;
-
-    case PICA_REG_INDEX_WORKAROUND(pipeline.command_buffer.trigger[0], 0x23c):
-    case PICA_REG_INDEX_WORKAROUND(pipeline.command_buffer.trigger[1], 0x23d): {
-        unsigned index =
-            static_cast<unsigned>(id - PICA_REG_INDEX(pipeline.command_buffer.trigger[0]));
-        u32* head_ptr = (u32*)Memory::GetPhysicalPointer(
-            regs.pipeline.command_buffer.GetPhysicalAddress(index));
-        g_state.cmd_list.head_ptr = g_state.cmd_list.current_ptr = head_ptr;
-        g_state.cmd_list.length = regs.pipeline.command_buffer.GetSize(index) / sizeof(u32);
-        break;
-    }
-
-    // It seems like these trigger vertex rendering
-    case PICA_REG_INDEX(pipeline.trigger_draw):
-    case PICA_REG_INDEX(pipeline.trigger_draw_indexed): {
     MICROPROFILE_SCOPE(GPU_Drawing);
+    auto& regs = g_state.regs;
 
 #if PICA_LOG_TEV
     DebugUtils::DumpTevStageConfig(regs.GetTevStages());
@@ -291,7 +225,7 @@ static void WritePicaReg(u32 id, u32 value, u32 mask) {
     VertexLoader loader(regs.pipeline);
 
     // Load vertices
-        bool is_indexed = (id == PICA_REG_INDEX(pipeline.trigger_draw_indexed));
+    bool is_indexed = (command_id == PICA_REG_INDEX(pipeline.trigger_draw_indexed));
 
     const auto& index_info = regs.pipeline.index_array;
     const u8* index_address_8 = Memory::GetPhysicalPointer(base_address + index_info.offset);
@@ -338,8 +272,8 @@ static void WritePicaReg(u32 id, u32 value, u32 mask) {
 
     for (unsigned int index = 0; index < regs.pipeline.num_vertices; ++index) {
         // Indexed rendering doesn't use the start offset
-            unsigned int vertex =
+        unsigned int vertex = is_indexed
-                is_indexed ? (index_u16 ? index_address_16[index] : index_address_8[index])
+                                  ? (index_u16 ? index_address_16[index] : index_address_8[index])
                                   : (index + regs.pipeline.vertex_offset);
 
         // -1 is a common special value used for primitive restart. Since it's unknown if
@@ -356,8 +290,7 @@ static void WritePicaReg(u32 id, u32 value, u32 mask) {
 
             if (g_debug_context && Pica::g_debug_context->recorder) {
                 int size = index_u16 ? 2 : 1;
-                    memory_accesses.AddAccess(base_address + index_info.offset + size * index,
+                memory_accesses.AddAccess(base_address + index_info.offset + size * index, size);
-                                              size);
             }
 
             for (unsigned int i = 0; i < VERTEX_CACHE_SIZE; ++i) {
@@ -402,10 +335,82 @@ static void WritePicaReg(u32 id, u32 value, u32 mask) {
     if (g_debug_context) {
         g_debug_context->OnEvent(DebugContext::Event::FinishedPrimitiveBatch, nullptr);
     }
+}
 
+static void WritePicaReg(u32 id, u32 value, u32 mask) {
+    auto& regs = g_state.regs;
+
+    if (id >= Regs::NUM_REGS) {
+        LOG_ERROR(HW_GPU,
+                  "Commandlist tried to write to invalid register 0x%03X (value: %08X, mask: %X)",
+                  id, value, mask);
+        return;
+    }
+
+    // TODO: Figure out how register masking acts on e.g. vs.uniform_setup.set_value
+    u32 old_value = regs.reg_array[id];
+
+    const u32 write_mask = expand_bits_to_bytes[mask];
+
+    regs.reg_array[id] = (old_value & ~write_mask) | (value & write_mask);
+
+    // Double check for is_pica_tracing to avoid call overhead
+    if (DebugUtils::IsPicaTracing()) {
+        DebugUtils::OnPicaRegWrite({(u16)id, (u16)mask, regs.reg_array[id]});
+    }
+
+    if (g_debug_context)
+        g_debug_context->OnEvent(DebugContext::Event::PicaCommandLoaded,
+                                 reinterpret_cast<void*>(&id));
+
+    switch (id) {
+    // Trigger IRQ
+    case PICA_REG_INDEX(trigger_irq):
+        Service::GSP::SignalInterrupt(Service::GSP::InterruptId::P3D);
+        break;
+
+    case PICA_REG_INDEX(pipeline.triangle_topology):
+        g_state.primitive_assembler.Reconfigure(regs.pipeline.triangle_topology);
+        break;
+
+    case PICA_REG_INDEX(pipeline.restart_primitive):
+        g_state.primitive_assembler.Reset();
+        break;
+
+    case PICA_REG_INDEX(pipeline.vs_default_attributes_setup.index):
+        g_state.immediate.current_attribute = 0;
+        g_state.immediate.reset_geometry_pipeline = true;
+        default_attr_counter = 0;
+        break;
+
+    // Load default vertex input attributes
+    case PICA_REG_INDEX_WORKAROUND(pipeline.vs_default_attributes_setup.set_value[0], 0x233):
+    case PICA_REG_INDEX_WORKAROUND(pipeline.vs_default_attributes_setup.set_value[1], 0x234):
+    case PICA_REG_INDEX_WORKAROUND(pipeline.vs_default_attributes_setup.set_value[2], 0x235):
+        LoadDefaultVertexAttributes(value);
+        break;
+
+    case PICA_REG_INDEX(pipeline.gpu_mode):
+        // This register likely just enables vertex processing and doesn't need any special handling
+        break;
+
+    case PICA_REG_INDEX_WORKAROUND(pipeline.command_buffer.trigger[0], 0x23c):
+    case PICA_REG_INDEX_WORKAROUND(pipeline.command_buffer.trigger[1], 0x23d): {
+        unsigned index =
+            static_cast<unsigned>(id - PICA_REG_INDEX(pipeline.command_buffer.trigger[0]));
+        u32* head_ptr = (u32*)Memory::GetPhysicalPointer(
+            regs.pipeline.command_buffer.GetPhysicalAddress(index));
+        g_state.cmd_list.head_ptr = g_state.cmd_list.current_ptr = head_ptr;
+        g_state.cmd_list.length = regs.pipeline.command_buffer.GetSize(index) / sizeof(u32);
         break;
     }
 
+    // It seems like these trigger vertex rendering
+    case PICA_REG_INDEX(pipeline.trigger_draw):
+    case PICA_REG_INDEX(pipeline.trigger_draw_indexed):
+        Draw(id);
+        break;
+
     case PICA_REG_INDEX(gs.bool_uniforms):
         WriteUniformBoolReg(g_state.gs, g_state.regs.gs.bool_uniforms.Value());
         break;