Merge pull request #2550 from yuriks/pica-refactor2

Small VideoCore cleanups

src/citra_qt/debugger/graphics/graphics_cmdlists.cpp

@@ -72,7 +72,7 @@ QVariant GPUCommandListModel::data(const QModelIndex& index, int role) const {
     if (role == Qt::DisplayRole) {
         switch (index.column()) {
         case 0:
-            return QString::fromLatin1(Pica::Regs::GetCommandName(write.cmd_id).c_str());
+            return QString::fromLatin1(Pica::Regs::GetRegisterName(write.cmd_id));
         case 1:
             return QString("%1").arg(write.cmd_id, 3, 16, QLatin1Char('0'));
         case 2:

src/citra_qt/debugger/graphics/graphics_surface.cpp

@@ -17,7 +17,8 @@
 #include "core/hw/gpu.h"
 #include "core/memory.h"
 #include "video_core/pica_state.h"
-#include "video_core/regs.h"
+#include "video_core/regs_framebuffer.h"
+#include "video_core/regs_texturing.h"
 #include "video_core/texture/texture_decode.h"
 #include "video_core/utils.h"
 

src/video_core/clipper.cpp

@@ -15,7 +15,6 @@
 #include "video_core/pica_state.h"
 #include "video_core/pica_types.h"
 #include "video_core/rasterizer.h"
-#include "video_core/regs.h"
 #include "video_core/shader/shader.h"
 
 using Pica::Rasterizer::Vertex;

src/video_core/command_processor.cpp

@@ -21,6 +21,8 @@
 #include "video_core/primitive_assembly.h"
 #include "video_core/rasterizer_interface.h"
 #include "video_core/regs.h"
+#include "video_core/regs_pipeline.h"
+#include "video_core/regs_texturing.h"
 #include "video_core/renderer_base.h"
 #include "video_core/shader/shader.h"
 #include "video_core/vertex_loader.h"
@@ -49,19 +51,23 @@ MICROPROFILE_DEFINE(GPU_Drawing, "GPU", "Drawing", MP_RGB(50, 50, 240));
 static void WritePicaReg(u32 id, u32 value, u32 mask) {
     auto& regs = g_state.regs;
 
-    if (id >= regs.NumIds())
+    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[id];
+    u32 old_value = regs.reg_array[id];
 
     const u32 write_mask = expand_bits_to_bytes[mask];
 
-    regs[id] = (old_value & ~write_mask) | (value & write_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[id]});
+        DebugUtils::OnPicaRegWrite({(u16)id, (u16)mask, regs.reg_array[id]});
     }
 
     if (g_debug_context)

src/video_core/debug_utils/debug_utils.cpp

@@ -32,7 +32,9 @@
 #include "video_core/pica_state.h"
 #include "video_core/pica_types.h"
 #include "video_core/rasterizer_interface.h"
-#include "video_core/regs.h"
+#include "video_core/regs_rasterizer.h"
+#include "video_core/regs_shader.h"
+#include "video_core/regs_texturing.h"
 #include "video_core/renderer_base.h"
 #include "video_core/shader/shader.h"
 #include "video_core/texture/texture_decode.h"

src/video_core/debug_utils/debug_utils.h

@@ -17,7 +17,9 @@
 #include <vector>
 #include "common/common_types.h"
 #include "common/vector_math.h"
-#include "video_core/regs.h"
+#include "video_core/regs_rasterizer.h"
+#include "video_core/regs_shader.h"
+#include "video_core/regs_texturing.h"
 
 namespace CiTrace {
 class Recorder;

src/video_core/pica.cpp

@@ -5,7 +5,7 @@
 #include <cstring>
 #include "video_core/pica.h"
 #include "video_core/pica_state.h"
-#include "video_core/regs.h"
+#include "video_core/regs_pipeline.h"
 
 namespace Pica {
 

src/video_core/rasterizer.cpp

@@ -19,7 +19,9 @@
 #include "video_core/pica_state.h"
 #include "video_core/pica_types.h"
 #include "video_core/rasterizer.h"
-#include "video_core/regs.h"
+#include "video_core/regs_framebuffer.h"
+#include "video_core/regs_rasterizer.h"
+#include "video_core/regs_texturing.h"
 #include "video_core/shader/shader.h"
 #include "video_core/texture/texture_decode.h"
 #include "video_core/utils.h"

src/video_core/regs.cpp

@@ -2,8 +2,8 @@
 // Licensed under GPLv2 or any later version
 // Refer to the license.txt file included.
 
+#include <algorithm>
 #include <iterator>
-#include <unordered_map>
 #include <utility>
 
 #include "common/common_types.h"
@@ -474,19 +474,14 @@ static const std::pair<u16, const char*> register_names[] = {
     {0x2DD, "GPUREG_VSH_OPDESCS_DATA7"},
 };
 
-std::string Regs::GetCommandName(int index) {
+const char* Regs::GetRegisterName(u16 index) {
-    static std::unordered_map<u32, const char*> map;
+    auto found = std::lower_bound(std::begin(register_names), std::end(register_names), index,
-
+                                  [](auto p, auto i) { return p.first < i; });
-    if (map.empty()) {
+    if (found->first == index) {
-        map.insert(std::begin(register_names), std::end(register_names));
+        return found->second;
-    }
-
-    // Return empty string if no match is found
-    auto it = map.find(index);
-    if (it != map.end()) {
-        return it->second;
     } else {
-        return std::string();
+        // Return empty string if no match is found
+        return "";
     }
 }
 

src/video_core/regs.h

@@ -45,46 +45,31 @@ namespace Pica {
 #endif // _MSC_VER
 
 struct Regs {
-    INSERT_PADDING_WORDS(0x10);
+    static constexpr size_t NUM_REGS = 0x300;
-    u32 trigger_irq;
-    INSERT_PADDING_WORDS(0x2f);
-    RasterizerRegs rasterizer;
-    TexturingRegs texturing;
-    FramebufferRegs framebuffer;
-    LightingRegs lighting;
-    PipelineRegs pipeline;
-    ShaderRegs gs;
-    ShaderRegs vs;
-    INSERT_PADDING_WORDS(0x20);
 
-    // Map register indices to names readable by humans
+    union {
-    // Used for debugging purposes, so performance is not an issue here
+        struct {
-    static std::string GetCommandName(int index);
+            INSERT_PADDING_WORDS(0x10);
+            u32 trigger_irq;
+            INSERT_PADDING_WORDS(0x2f);
+            RasterizerRegs rasterizer;
+            TexturingRegs texturing;
+            FramebufferRegs framebuffer;
+            LightingRegs lighting;
+            PipelineRegs pipeline;
+            ShaderRegs gs;
+            ShaderRegs vs;
+            INSERT_PADDING_WORDS(0x20);
+        };
+        std::array<u32, NUM_REGS> reg_array;
+    };
 
-    static constexpr size_t NumIds() {
+    /// Map register indices to names readable by humans
-        return sizeof(Regs) / sizeof(u32);
+    static const char* GetRegisterName(u16 index);
-    }
-
-    const u32& operator[](int index) const {
-        const u32* content = reinterpret_cast<const u32*>(this);
-        return content[index];
-    }
-
-    u32& operator[](int index) {
-        u32* content = reinterpret_cast<u32*>(this);
-        return content[index];
-    }
-
-private:
-    /*
-    * Most physical addresses which Pica registers refer to are 8-byte aligned.
-    * This function should be used to get the address from a raw register value.
-    */
-    static inline u32 DecodeAddressRegister(u32 register_value) {
-        return register_value * 8;
-    }
 };
 
+static_assert(sizeof(Regs) == Regs::NUM_REGS * sizeof(u32), "Regs struct has wrong size");
+
 // TODO: MSVC does not support using offsetof() on non-static data members even though this
 //       is technically allowed since C++11. This macro should be enabled once MSVC adds
 //       support for that.
@@ -154,11 +139,4 @@ ASSERT_REG_POSITION(vs, 0x2b0);
 #undef ASSERT_REG_POSITION
 #endif // !defined(_MSC_VER)
 
-// The total number of registers is chosen arbitrarily, but let's make sure it's not some odd value
-// anyway.
-static_assert(sizeof(Regs) <= 0x300 * sizeof(u32),
-              "Register set structure larger than it should be");
-static_assert(sizeof(Regs) >= 0x300 * sizeof(u32),
-              "Register set structure smaller than it should be");
-
 } // namespace Pica

src/video_core/regs_lighting.h

@@ -34,7 +34,7 @@ struct LightingRegs {
     * configurations that require more LUTs, more cycles are required on HW to perform lighting
     * computations.
     */
-    enum class LightingConfig {
+    enum class LightingConfig : u32 {
         Config0 = 0, ///< Reflect Red, Distribution 0, Spotlight
         Config1 = 1, ///< Reflect Red, Fresnel, Spotlight
         Config2 = 2, ///< Reflect Red, Distribution 0/1
@@ -48,7 +48,7 @@ struct LightingRegs {
     };
 
     /// Selects which lighting components are affected by fresnel
-    enum class LightingFresnelSelector {
+    enum class LightingFresnelSelector : u32 {
         None = 0,           ///< Fresnel is disabled
         PrimaryAlpha = 1,   ///< Primary (diffuse) lighting alpha is affected by fresnel
         SecondaryAlpha = 2, ///< Secondary (specular) lighting alpha is affected by fresnel
@@ -58,7 +58,7 @@ struct LightingRegs {
     };
 
     /// Factor used to scale the output of a lighting LUT
-    enum class LightingScale {
+    enum class LightingScale : u32 {
         Scale1 = 0, ///< Scale is 1x
         Scale2 = 1, ///< Scale is 2x
         Scale4 = 2, ///< Scale is 4x
@@ -68,7 +68,7 @@ struct LightingRegs {
         Scale1_2 = 7, ///< Scale is 0.5x
     };
 
-    enum class LightingLutInput {
+    enum class LightingLutInput : u32 {
         NH = 0, // Cosine of the angle between the normal and half-angle vectors
         VH = 1, // Cosine of the angle between the view and half-angle vectors
         NV = 2, // Cosine of the angle between the normal and the view vector

src/video_core/regs_pipeline.h

@@ -14,7 +14,7 @@
 namespace Pica {
 
 struct PipelineRegs {
-    enum class VertexAttributeFormat : u64 {
+    enum class VertexAttributeFormat : u32 {
         BYTE = 0,
         UBYTE = 1,
         SHORT = 2,
@@ -31,34 +31,37 @@ struct PipelineRegs {
         // Descriptor for internal vertex attributes
         union {
             BitField<0, 2, VertexAttributeFormat> format0; // size of one element
-            BitField<2, 2, u64> size0;                     // number of elements minus 1
+            BitField<2, 2, u32> size0;                     // number of elements minus 1
             BitField<4, 2, VertexAttributeFormat> format1;
-            BitField<6, 2, u64> size1;
+            BitField<6, 2, u32> size1;
             BitField<8, 2, VertexAttributeFormat> format2;
-            BitField<10, 2, u64> size2;
+            BitField<10, 2, u32> size2;
             BitField<12, 2, VertexAttributeFormat> format3;
-            BitField<14, 2, u64> size3;
+            BitField<14, 2, u32> size3;
             BitField<16, 2, VertexAttributeFormat> format4;
-            BitField<18, 2, u64> size4;
+            BitField<18, 2, u32> size4;
             BitField<20, 2, VertexAttributeFormat> format5;
-            BitField<22, 2, u64> size5;
+            BitField<22, 2, u32> size5;
             BitField<24, 2, VertexAttributeFormat> format6;
-            BitField<26, 2, u64> size6;
+            BitField<26, 2, u32> size6;
             BitField<28, 2, VertexAttributeFormat> format7;
-            BitField<30, 2, u64> size7;
+            BitField<30, 2, u32> size7;
-            BitField<32, 2, VertexAttributeFormat> format8;
+        };
-            BitField<34, 2, u64> size8;
-            BitField<36, 2, VertexAttributeFormat> format9;
-            BitField<38, 2, u64> size9;
-            BitField<40, 2, VertexAttributeFormat> format10;
-            BitField<42, 2, u64> size10;
-            BitField<44, 2, VertexAttributeFormat> format11;
-            BitField<46, 2, u64> size11;
 
-            BitField<48, 12, u64> attribute_mask;
+        union {
+            BitField<0, 2, VertexAttributeFormat> format8;
+            BitField<2, 2, u32> size8;
+            BitField<4, 2, VertexAttributeFormat> format9;
+            BitField<6, 2, u32> size9;
+            BitField<8, 2, VertexAttributeFormat> format10;
+            BitField<10, 2, u32> size10;
+            BitField<12, 2, VertexAttributeFormat> format11;
+            BitField<14, 2, u32> size11;
+
+            BitField<16, 12, u32> attribute_mask;
 
             // number of total attributes minus 1
-            BitField<60, 4, u64> max_attribute_index;
+            BitField<28, 4, u32> max_attribute_index;
         };
 
         inline VertexAttributeFormat GetFormat(int n) const {
@@ -69,7 +72,7 @@ struct PipelineRegs {
         }
 
         inline int GetNumElements(int n) const {
-            u64 sizes[] = {size0, size1, size2, size3, size4,  size5,
+            u32 sizes[] = {size0, size1, size2, size3, size4,  size5,
                            size6, size7, size8, size9, size10, size11};
             return (int)sizes[n] + 1;
         }
@@ -99,27 +102,30 @@ struct PipelineRegs {
             u32 data_offset;
 
             union {
-                BitField<0, 4, u64> comp0;
+                BitField<0, 4, u32> comp0;
-                BitField<4, 4, u64> comp1;
+                BitField<4, 4, u32> comp1;
-                BitField<8, 4, u64> comp2;
+                BitField<8, 4, u32> comp2;
-                BitField<12, 4, u64> comp3;
+                BitField<12, 4, u32> comp3;
-                BitField<16, 4, u64> comp4;
+                BitField<16, 4, u32> comp4;
-                BitField<20, 4, u64> comp5;
+                BitField<20, 4, u32> comp5;
-                BitField<24, 4, u64> comp6;
+                BitField<24, 4, u32> comp6;
-                BitField<28, 4, u64> comp7;
+                BitField<28, 4, u32> comp7;
-                BitField<32, 4, u64> comp8;
+            };
-                BitField<36, 4, u64> comp9;
+
-                BitField<40, 4, u64> comp10;
+            union {
-                BitField<44, 4, u64> comp11;
+                BitField<0, 4, u32> comp8;
+                BitField<4, 4, u32> comp9;
+                BitField<8, 4, u32> comp10;
+                BitField<12, 4, u32> comp11;
 
                 // bytes for a single vertex in this loader
-                BitField<48, 8, u64> byte_count;
+                BitField<16, 8, u32> byte_count;
 
-                BitField<60, 4, u64> component_count;
+                BitField<28, 4, u32> component_count;
             };
 
             inline int GetComponent(int n) const {
-                u64 components[] = {comp0, comp1, comp2, comp3, comp4,  comp5,
+                u32 components[] = {comp0, comp1, comp2, comp3, comp4,  comp5,
                                     comp6, comp7, comp8, comp9, comp10, comp11};
                 return (int)components[n];
             }

src/video_core/renderer_opengl/gl_rasterizer.cpp

@@ -15,7 +15,9 @@
 #include "common/vector_math.h"
 #include "core/hw/gpu.h"
 #include "video_core/pica_state.h"
-#include "video_core/regs.h"
+#include "video_core/regs_framebuffer.h"
+#include "video_core/regs_rasterizer.h"
+#include "video_core/regs_texturing.h"
 #include "video_core/renderer_opengl/gl_rasterizer.h"
 #include "video_core/renderer_opengl/gl_shader_gen.h"
 #include "video_core/renderer_opengl/gl_shader_util.h"
@@ -26,18 +28,6 @@ MICROPROFILE_DEFINE(OpenGL_Drawing, "OpenGL", "Drawing", MP_RGB(128, 128, 192));
 MICROPROFILE_DEFINE(OpenGL_Blits, "OpenGL", "Blits", MP_RGB(100, 100, 255));
 MICROPROFILE_DEFINE(OpenGL_CacheManagement, "OpenGL", "Cache Mgmt", MP_RGB(100, 255, 100));
 
-static bool IsPassThroughTevStage(const Pica::TexturingRegs::TevStageConfig& stage) {
-    using TevStageConfig = Pica::TexturingRegs::TevStageConfig;
-
-    return (stage.color_op == TevStageConfig::Operation::Replace &&
-            stage.alpha_op == TevStageConfig::Operation::Replace &&
-            stage.color_source1 == TevStageConfig::Source::Previous &&
-            stage.alpha_source1 == TevStageConfig::Source::Previous &&
-            stage.color_modifier1 == TevStageConfig::ColorModifier::SourceColor &&
-            stage.alpha_modifier1 == TevStageConfig::AlphaModifier::SourceAlpha &&
-            stage.GetColorMultiplier() == 1 && stage.GetAlphaMultiplier() == 1);
-}
-
 RasterizerOpenGL::RasterizerOpenGL() : shader_dirty(true) {
     // Create sampler objects
     for (size_t i = 0; i < texture_samplers.size(); ++i) {

src/video_core/renderer_opengl/gl_rasterizer.h

@@ -19,7 +19,10 @@
 #include "video_core/pica_state.h"
 #include "video_core/pica_types.h"
 #include "video_core/rasterizer_interface.h"
-#include "video_core/regs.h"
+#include "video_core/regs_framebuffer.h"
+#include "video_core/regs_lighting.h"
+#include "video_core/regs_rasterizer.h"
+#include "video_core/regs_texturing.h"
 #include "video_core/renderer_opengl/gl_rasterizer_cache.h"
 #include "video_core/renderer_opengl/gl_resource_manager.h"
 #include "video_core/renderer_opengl/gl_state.h"

src/video_core/renderer_opengl/gl_rasterizer_cache.h

@@ -21,7 +21,8 @@
 #include "common/common_funcs.h"
 #include "common/common_types.h"
 #include "core/hw/gpu.h"
-#include "video_core/regs.h"
+#include "video_core/regs_framebuffer.h"
+#include "video_core/regs_texturing.h"
 #include "video_core/renderer_opengl/gl_resource_manager.h"
 
 namespace MathUtil {

src/video_core/renderer_opengl/gl_shader_gen.cpp

@@ -7,15 +7,19 @@
 #include "common/assert.h"
 #include "common/bit_field.h"
 #include "common/logging/log.h"
-#include "video_core/regs.h"
+#include "video_core/regs_framebuffer.h"
+#include "video_core/regs_lighting.h"
+#include "video_core/regs_rasterizer.h"
+#include "video_core/regs_texturing.h"
 #include "video_core/renderer_opengl/gl_rasterizer.h"
 #include "video_core/renderer_opengl/gl_shader_gen.h"
 #include "video_core/renderer_opengl/gl_shader_util.h"
 
-using Pica::Regs;
+using Pica::FramebufferRegs;
-using Pica::RasterizerRegs;
 using Pica::LightingRegs;
-using TevStageConfig = Pica::TexturingRegs::TevStageConfig;
+using Pica::RasterizerRegs;
+using Pica::TexturingRegs;
+using TevStageConfig = TexturingRegs::TevStageConfig;
 
 namespace GLShader {
 
@@ -48,10 +52,10 @@ static void AppendSource(std::string& out, const PicaShaderConfig& config,
     case Source::Texture0:
         // Only unit 0 respects the texturing type (according to 3DBrew)
         switch (state.texture0_type) {
-        case Pica::TexturingRegs::TextureConfig::Texture2D:
+        case TexturingRegs::TextureConfig::Texture2D:
             out += "texture(tex[0], texcoord[0])";
             break;
-        case Pica::TexturingRegs::TextureConfig::Projection2D:
+        case TexturingRegs::TextureConfig::Projection2D:
             out += "textureProj(tex[0], vec3(texcoord[0], texcoord0_w))";
             break;
         default:
@@ -278,8 +282,8 @@ static void AppendAlphaCombiner(std::string& out, TevStageConfig::Operation oper
 }
 
 /// Writes the if-statement condition used to evaluate alpha testing
-static void AppendAlphaTestCondition(std::string& out, Pica::FramebufferRegs::CompareFunc func) {
+static void AppendAlphaTestCondition(std::string& out, FramebufferRegs::CompareFunc func) {
-    using CompareFunc = Pica::FramebufferRegs::CompareFunc;
+    using CompareFunc = FramebufferRegs::CompareFunc;
     switch (func) {
     case CompareFunc::Never:
         out += "true";
@@ -309,7 +313,7 @@ static void AppendAlphaTestCondition(std::string& out, Pica::FramebufferRegs::Co
 /// Writes the code to emulate the specified TEV stage
 static void WriteTevStage(std::string& out, const PicaShaderConfig& config, unsigned index) {
     const auto stage =
-        static_cast<const Pica::TexturingRegs::TevStageConfig>(config.state.tev_stages[index]);
+        static_cast<const TexturingRegs::TevStageConfig>(config.state.tev_stages[index]);
     if (!IsPassThroughTevStage(stage)) {
         std::string index_name = std::to_string(index);
 
@@ -642,7 +646,7 @@ vec4 secondary_fragment_color = vec4(0.0);
 )";
 
     // Do not do any sort of processing if it's obvious we're not going to pass the alpha test
-    if (state.alpha_test_func == Pica::FramebufferRegs::CompareFunc::Never) {
+    if (state.alpha_test_func == FramebufferRegs::CompareFunc::Never) {
         out += "discard; }";
         return out;
     }
@@ -661,7 +665,7 @@ vec4 secondary_fragment_color = vec4(0.0);
 
     out += "float z_over_w = 1.0 - gl_FragCoord.z * 2.0;\n";
     out += "float depth = z_over_w * depth_scale + depth_offset;\n";
-    if (state.depthmap_enable == Pica::RasterizerRegs::DepthBuffering::WBuffering) {
+    if (state.depthmap_enable == RasterizerRegs::DepthBuffering::WBuffering) {
         out += "depth /= gl_FragCoord.w;\n";
     }
 
@@ -675,14 +679,14 @@ vec4 secondary_fragment_color = vec4(0.0);
     for (size_t index = 0; index < state.tev_stages.size(); ++index)
         WriteTevStage(out, config, (unsigned)index);
 
-    if (state.alpha_test_func != Pica::FramebufferRegs::CompareFunc::Always) {
+    if (state.alpha_test_func != FramebufferRegs::CompareFunc::Always) {
         out += "if (";
         AppendAlphaTestCondition(out, state.alpha_test_func);
         out += ") discard;\n";
     }
 
     // Append fog combiner
-    if (state.fog_mode == Pica::TexturingRegs::FogMode::Fog) {
+    if (state.fog_mode == TexturingRegs::FogMode::Fog) {
         // Get index into fog LUT
         if (state.fog_flip) {
             out += "float fog_index = (1.0 - depth) * 128.0;\n";

src/video_core/renderer_opengl/pica_to_gl.h

@@ -12,7 +12,9 @@
 #include "common/common_funcs.h"
 #include "common/common_types.h"
 #include "common/logging/log.h"
-#include "video_core/regs.h"
+#include "video_core/regs_framebuffer.h"
+#include "video_core/regs_lighting.h"
+#include "video_core/regs_texturing.h"
 
 using GLvec2 = std::array<GLfloat, 2>;
 using GLvec3 = std::array<GLfloat, 3>;

src/video_core/shader/shader.cpp

@@ -8,7 +8,8 @@
 #include "common/logging/log.h"
 #include "common/microprofile.h"
 #include "video_core/pica_state.h"
-#include "video_core/regs.h"
+#include "video_core/regs_rasterizer.h"
+#include "video_core/regs_shader.h"
 #include "video_core/shader/shader.h"
 #include "video_core/shader/shader_interpreter.h"
 #ifdef ARCHITECTURE_x86_64

src/video_core/shader/shader.h

@@ -13,7 +13,8 @@
 #include "common/common_types.h"
 #include "common/vector_math.h"
 #include "video_core/pica_types.h"
-#include "video_core/regs.h"
+#include "video_core/regs_rasterizer.h"
+#include "video_core/regs_shader.h"
 
 using nihstro::RegisterType;
 using nihstro::SourceRegister;