From 0bcd3c79bb4bbd90fb1f8d02adb26198690709fb Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Tue, 22 Nov 2022 16:54:05 -0800 Subject: [PATCH] VertexLoader: Eliminate use of DataReader DataReader is generally jank - it has a start and end pointer, but the end pointer is generally not used, and all of the vertex loaders mostly bypassed it anyways. Wrapper code (the vertex loaer test, as well as Fifo.cpp and OpcodeDecoding.cpp) still uses it, as does the software vertex loader (which is not a subclass of VertexLoader). These can probably be eliminated later. --- Source/Core/VideoCommon/OpcodeDecoding.cpp | 4 +--- Source/Core/VideoCommon/VertexLoader.cpp | 9 ++++----- Source/Core/VideoCommon/VertexLoader.h | 3 +-- Source/Core/VideoCommon/VertexLoaderARM64.cpp | 6 ++---- Source/Core/VideoCommon/VertexLoaderARM64.h | 3 +-- Source/Core/VideoCommon/VertexLoaderBase.cpp | 13 +++++-------- Source/Core/VideoCommon/VertexLoaderBase.h | 4 +--- Source/Core/VideoCommon/VertexLoaderManager.cpp | 10 ++++------ Source/Core/VideoCommon/VertexLoaderManager.h | 3 +-- Source/Core/VideoCommon/VertexLoaderUtils.h | 9 +++++---- Source/Core/VideoCommon/VertexLoaderX64.cpp | 7 +++---- Source/Core/VideoCommon/VertexLoaderX64.h | 2 +- Source/Core/VideoCommon/VertexLoader_Normal.cpp | 5 +---- Source/Core/VideoCommon/VertexLoader_Position.cpp | 13 +++---------- Source/Core/VideoCommon/VertexLoader_TextCoord.cpp | 12 ++---------- Source/UnitTests/VideoCommon/VertexLoaderTest.cpp | 2 +- 16 files changed, 36 insertions(+), 69 deletions(-) diff --git a/Source/Core/VideoCommon/OpcodeDecoding.cpp b/Source/Core/VideoCommon/OpcodeDecoding.cpp index fcc33add24..c1ac5c876c 100644 --- a/Source/Core/VideoCommon/OpcodeDecoding.cpp +++ b/Source/Core/VideoCommon/OpcodeDecoding.cpp @@ -127,10 +127,8 @@ public: // load vertices const u32 size = vertex_size * num_vertices; - // HACK - DataReader src{const_cast(vertex_data), const_cast(vertex_data) + size}; const u32 bytes = - VertexLoaderManager::RunVertices(vat, primitive, num_vertices, src); + VertexLoaderManager::RunVertices(vat, primitive, num_vertices, vertex_data); ASSERT(bytes == size); diff --git a/Source/Core/VideoCommon/VertexLoader.cpp b/Source/Core/VideoCommon/VertexLoader.cpp index 8176027b45..bc97ac041d 100644 --- a/Source/Core/VideoCommon/VertexLoader.cpp +++ b/Source/Core/VideoCommon/VertexLoader.cpp @@ -6,7 +6,6 @@ #include "Common/Assert.h" #include "Common/CommonTypes.h" -#include "VideoCommon/DataReader.h" #include "VideoCommon/VertexLoaderManager.h" #include "VideoCommon/VertexLoaderUtils.h" #include "VideoCommon/VertexLoader_Color.h" @@ -16,7 +15,7 @@ #include "VideoCommon/VideoCommon.h" // This pointer is used as the source/dst for all fixed function loader calls -u8* g_video_buffer_read_ptr; +const u8* g_video_buffer_read_ptr; u8* g_vertex_manager_write_ptr; static void PosMtx_ReadDirect_UByte(VertexLoader* loader) @@ -249,10 +248,10 @@ void VertexLoader::WriteCall(TPipelineFunction func) m_PipelineStages[m_numPipelineStages++] = func; } -int VertexLoader::RunVertices(DataReader src, DataReader dst, int count) +int VertexLoader::RunVertices(const u8* src, u8* dst, int count) { - g_vertex_manager_write_ptr = dst.GetPointer(); - g_video_buffer_read_ptr = src.GetPointer(); + g_vertex_manager_write_ptr = dst; + g_video_buffer_read_ptr = src; m_numLoadedVertices += count; m_skippedVertices = 0; diff --git a/Source/Core/VideoCommon/VertexLoader.h b/Source/Core/VideoCommon/VertexLoader.h index b3bb0b270a..d1417401a4 100644 --- a/Source/Core/VideoCommon/VertexLoader.h +++ b/Source/Core/VideoCommon/VertexLoader.h @@ -11,7 +11,6 @@ #include "Common/CommonTypes.h" #include "VideoCommon/VertexLoaderBase.h" -class DataReader; class VertexLoader; typedef void (*TPipelineFunction)(VertexLoader* loader); @@ -20,7 +19,7 @@ class VertexLoader : public VertexLoaderBase public: VertexLoader(const TVtxDesc& vtx_desc, const VAT& vtx_attr); - int RunVertices(DataReader src, DataReader dst, int count) override; + int RunVertices(const u8* src, u8* dst, int count) override; // They are used for the communication with the loader functions float m_posScale; float m_tcScale[8]; diff --git a/Source/Core/VideoCommon/VertexLoaderARM64.cpp b/Source/Core/VideoCommon/VertexLoaderARM64.cpp index 99ba5b3357..b65425c451 100644 --- a/Source/Core/VideoCommon/VertexLoaderARM64.cpp +++ b/Source/Core/VideoCommon/VertexLoaderARM64.cpp @@ -7,7 +7,6 @@ #include "Common/CommonTypes.h" #include "VideoCommon/CPMemory.h" -#include "VideoCommon/DataReader.h" #include "VideoCommon/VertexLoaderManager.h" using namespace Arm64Gen; @@ -517,9 +516,8 @@ void VertexLoaderARM64::GenerateVertexLoader() m_native_vtx_decl.stride = m_dst_ofs; } -int VertexLoaderARM64::RunVertices(DataReader src, DataReader dst, int count) +int VertexLoaderARM64::RunVertices(const u8* src, u8* dst, int count) { m_numLoadedVertices += count; - return ((int (*)(u8 * src, u8 * dst, int count)) region)(src.GetPointer(), dst.GetPointer(), - count - 1); + return ((int (*)(const u8* src, u8* dst, int count))region)(src, dst, count - 1); } diff --git a/Source/Core/VideoCommon/VertexLoaderARM64.h b/Source/Core/VideoCommon/VertexLoaderARM64.h index 83ebbc1a7a..05f89ce407 100644 --- a/Source/Core/VideoCommon/VertexLoaderARM64.h +++ b/Source/Core/VideoCommon/VertexLoaderARM64.h @@ -9,7 +9,6 @@ #include "Common/CommonTypes.h" #include "VideoCommon/VertexLoaderBase.h" -class DataReader; enum class VertexComponentFormat; enum class ComponentFormat; enum class ColorFormat; @@ -21,7 +20,7 @@ public: VertexLoaderARM64(const TVtxDesc& vtx_desc, const VAT& vtx_att); protected: - int RunVertices(DataReader src, DataReader dst, int count) override; + int RunVertices(const u8* src, u8* dst, int count) override; private: u32 m_src_ofs = 0; diff --git a/Source/Core/VideoCommon/VertexLoaderBase.cpp b/Source/Core/VideoCommon/VertexLoaderBase.cpp index bf4e364147..0ed8fc6439 100644 --- a/Source/Core/VideoCommon/VertexLoaderBase.cpp +++ b/Source/Core/VideoCommon/VertexLoaderBase.cpp @@ -17,7 +17,6 @@ #include "Common/Logging/Log.h" #include "Common/MsgHandler.h" -#include "VideoCommon/DataReader.h" #include "VideoCommon/VertexLoader.h" #include "VideoCommon/VertexLoader_Color.h" #include "VideoCommon/VertexLoader_Normal.h" @@ -57,15 +56,13 @@ public: b->m_vertex_size, b->m_native_components, b->m_native_vtx_decl.stride); } } - int RunVertices(DataReader src, DataReader dst, int count) override + int RunVertices(const u8* src, u8* dst, int count) override { buffer_a.resize(count * a->m_native_vtx_decl.stride + 4); buffer_b.resize(count * b->m_native_vtx_decl.stride + 4); - int count_a = - a->RunVertices(src, DataReader(buffer_a.data(), buffer_a.data() + buffer_a.size()), count); - int count_b = - b->RunVertices(src, DataReader(buffer_b.data(), buffer_b.data() + buffer_b.size()), count); + int count_a = a->RunVertices(src, buffer_a.data(), count); + int count_b = b->RunVertices(src, buffer_b.data(), count); if (count_a != count_b) { @@ -84,7 +81,7 @@ public: m_VtxDesc, m_VtxAttr); } - memcpy(dst.GetPointer(), buffer_a.data(), count_a * m_native_vtx_decl.stride); + memcpy(dst, buffer_a.data(), count_a * m_native_vtx_decl.stride); m_numLoadedVertices += count; return count_a; } @@ -162,7 +159,7 @@ std::unique_ptr VertexLoaderBase::CreateVertexLoader(const TVt { std::unique_ptr loader = nullptr; - //#define COMPARE_VERTEXLOADERS + // #define COMPARE_VERTEXLOADERS #if defined(_M_X86_64) loader = std::make_unique(vtx_desc, vtx_attr); diff --git a/Source/Core/VideoCommon/VertexLoaderBase.h b/Source/Core/VideoCommon/VertexLoaderBase.h index 11e9136251..26d6f3ff2e 100644 --- a/Source/Core/VideoCommon/VertexLoaderBase.h +++ b/Source/Core/VideoCommon/VertexLoaderBase.h @@ -12,8 +12,6 @@ #include "VideoCommon/CPMemory.h" #include "VideoCommon/NativeVertexFormat.h" -class DataReader; - class VertexLoaderUID { std::array vid{}; @@ -65,7 +63,7 @@ public: static std::unique_ptr CreateVertexLoader(const TVtxDesc& vtx_desc, const VAT& vtx_attr); virtual ~VertexLoaderBase() {} - virtual int RunVertices(DataReader src, DataReader dst, int count) = 0; + virtual int RunVertices(const u8* src, u8* dst, int count) = 0; // per loader public state PortableVertexDeclaration m_native_vtx_decl{}; diff --git a/Source/Core/VideoCommon/VertexLoaderManager.cpp b/Source/Core/VideoCommon/VertexLoaderManager.cpp index 5725a3738a..c37b4a4fa2 100644 --- a/Source/Core/VideoCommon/VertexLoaderManager.cpp +++ b/Source/Core/VideoCommon/VertexLoaderManager.cpp @@ -332,7 +332,7 @@ static void CheckCPConfiguration(int vtx_attr_group) } template -int RunVertices(int vtx_attr_group, OpcodeDecoder::Primitive primitive, int count, DataReader src) +int RunVertices(int vtx_attr_group, OpcodeDecoder::Primitive primitive, int count, const u8* src) { if (count == 0) [[unlikely]] return 0; @@ -341,8 +341,6 @@ int RunVertices(int vtx_attr_group, OpcodeDecoder::Primitive primitive, int coun VertexLoaderBase* loader = RefreshLoader(vtx_attr_group); int size = count * loader->m_vertex_size; - if ((int)src.size() < size) [[unlikely]] - return -1; if constexpr (!IsPreprocess) { @@ -371,7 +369,7 @@ int RunVertices(int vtx_attr_group, OpcodeDecoder::Primitive primitive, int coun DataReader dst = g_vertex_manager->PrepareForAdditionalData( primitive, count, loader->m_native_vtx_decl.stride, cullall); - count = loader->RunVertices(src, dst, count); + count = loader->RunVertices(src, dst.GetPointer(), count); g_vertex_manager->AddIndices(primitive, count); g_vertex_manager->FlushData(count, loader->m_native_vtx_decl.stride); @@ -383,9 +381,9 @@ int RunVertices(int vtx_attr_group, OpcodeDecoder::Primitive primitive, int coun } template int RunVertices(int vtx_attr_group, OpcodeDecoder::Primitive primitive, int count, - DataReader src); + const u8* src); template int RunVertices(int vtx_attr_group, OpcodeDecoder::Primitive primitive, int count, - DataReader src); + const u8* src); NativeVertexFormat* GetCurrentVertexFormat() { diff --git a/Source/Core/VideoCommon/VertexLoaderManager.h b/Source/Core/VideoCommon/VertexLoaderManager.h index 0396a6762a..b50beae0a5 100644 --- a/Source/Core/VideoCommon/VertexLoaderManager.h +++ b/Source/Core/VideoCommon/VertexLoaderManager.h @@ -12,7 +12,6 @@ #include "Common/EnumMap.h" #include "VideoCommon/CPMemory.h" -class DataReader; class NativeVertexFormat; struct PortableVertexDeclaration; @@ -43,7 +42,7 @@ NativeVertexFormat* GetUberVertexFormat(const PortableVertexDeclaration& decl); // Returns -1 if buf_size is insufficient, else the amount of bytes consumed template -int RunVertices(int vtx_attr_group, OpcodeDecoder::Primitive primitive, int count, DataReader src); +int RunVertices(int vtx_attr_group, OpcodeDecoder::Primitive primitive, int count, const u8* src); namespace detail { diff --git a/Source/Core/VideoCommon/VertexLoaderUtils.h b/Source/Core/VideoCommon/VertexLoaderUtils.h index 34107ed2b9..09a55986b2 100644 --- a/Source/Core/VideoCommon/VertexLoaderUtils.h +++ b/Source/Core/VideoCommon/VertexLoaderUtils.h @@ -7,8 +7,9 @@ #include "Common/CommonTypes.h" #include "Common/Inline.h" +#include "Common/Swap.h" -extern u8* g_video_buffer_read_ptr; +extern const u8* g_video_buffer_read_ptr; extern u8* g_vertex_manager_write_ptr; DOLPHIN_FORCE_INLINE void DataSkip(u32 skip) @@ -24,7 +25,7 @@ DOLPHIN_FORCE_INLINE void DataSkip() } template -DOLPHIN_FORCE_INLINE T DataPeek(int _uOffset, u8* bufp = g_video_buffer_read_ptr) +DOLPHIN_FORCE_INLINE T DataPeek(int _uOffset, const u8* bufp = g_video_buffer_read_ptr) { T result; std::memcpy(&result, &bufp[_uOffset], sizeof(T)); @@ -32,7 +33,7 @@ DOLPHIN_FORCE_INLINE T DataPeek(int _uOffset, u8* bufp = g_video_buffer_read_ptr } template -DOLPHIN_FORCE_INLINE T DataRead(u8** bufp = &g_video_buffer_read_ptr) +DOLPHIN_FORCE_INLINE T DataRead(const u8** bufp = &g_video_buffer_read_ptr) { auto const result = DataPeek(0, *bufp); *bufp += sizeof(T); @@ -47,7 +48,7 @@ DOLPHIN_FORCE_INLINE u32 DataReadU32Unswapped() return result; } -DOLPHIN_FORCE_INLINE u8* DataGetPosition() +DOLPHIN_FORCE_INLINE const u8* DataGetPosition() { return g_video_buffer_read_ptr; } diff --git a/Source/Core/VideoCommon/VertexLoaderX64.cpp b/Source/Core/VideoCommon/VertexLoaderX64.cpp index b1474fd2fa..aec42a41a6 100644 --- a/Source/Core/VideoCommon/VertexLoaderX64.cpp +++ b/Source/Core/VideoCommon/VertexLoaderX64.cpp @@ -16,7 +16,6 @@ #include "Common/x64ABI.h" #include "Common/x64Emitter.h" #include "VideoCommon/CPMemory.h" -#include "VideoCommon/DataReader.h" #include "VideoCommon/VertexLoaderManager.h" using namespace Gen; @@ -582,9 +581,9 @@ void VertexLoaderX64::GenerateVertexLoader() m_native_vtx_decl.stride = m_dst_ofs; } -int VertexLoaderX64::RunVertices(DataReader src, DataReader dst, int count) +int VertexLoaderX64::RunVertices(const u8* src, u8* dst, int count) { m_numLoadedVertices += count; - return ((int (*)(u8*, u8*, int, const void*))region)(src.GetPointer(), dst.GetPointer(), count, - memory_base_ptr); + return ((int (*)(const u8* src, u8* dst, int count, const void* base))region)(src, dst, count, + memory_base_ptr); } diff --git a/Source/Core/VideoCommon/VertexLoaderX64.h b/Source/Core/VideoCommon/VertexLoaderX64.h index 8650c70c27..377d47d296 100644 --- a/Source/Core/VideoCommon/VertexLoaderX64.h +++ b/Source/Core/VideoCommon/VertexLoaderX64.h @@ -18,7 +18,7 @@ public: VertexLoaderX64(const TVtxDesc& vtx_desc, const VAT& vtx_att); protected: - int RunVertices(DataReader src, DataReader dst, int count) override; + int RunVertices(const u8* src, u8* dst, int count) override; private: u32 m_src_ofs = 0; diff --git a/Source/Core/VideoCommon/VertexLoader_Normal.cpp b/Source/Core/VideoCommon/VertexLoader_Normal.cpp index 3b685e0ca4..c713e4c9c6 100644 --- a/Source/Core/VideoCommon/VertexLoader_Normal.cpp +++ b/Source/Core/VideoCommon/VertexLoader_Normal.cpp @@ -9,7 +9,6 @@ #include "Common/CommonTypes.h" #include "Common/EnumMap.h" -#include "VideoCommon/DataReader.h" #include "VideoCommon/VertexLoader.h" #include "VideoCommon/VertexLoaderManager.h" #include "VideoCommon/VertexLoaderUtils.h" @@ -43,7 +42,6 @@ template void ReadIndirect(VertexLoader* loader, const T* data) { static_assert(3 == N || 9 == N, "N is only sane as 3 or 9!"); - DataReader dst(g_vertex_manager_write_ptr, nullptr); for (u32 i = 0; i < N; ++i) { @@ -55,10 +53,9 @@ void ReadIndirect(VertexLoader* loader, const T* data) else if (i >= 6 && i < 9) VertexLoaderManager::binormal_cache[i - 6] = value; } - dst.Write(value); + DataWrite(value); } - g_vertex_manager_write_ptr = dst.GetPointer(); LOG_NORM(); } diff --git a/Source/Core/VideoCommon/VertexLoader_Position.cpp b/Source/Core/VideoCommon/VertexLoader_Position.cpp index 92583e0726..42ebf4985f 100644 --- a/Source/Core/VideoCommon/VertexLoader_Position.cpp +++ b/Source/Core/VideoCommon/VertexLoader_Position.cpp @@ -10,7 +10,6 @@ #include "Common/EnumMap.h" #include "Common/Swap.h" -#include "VideoCommon/DataReader.h" #include "VideoCommon/VertexLoader.h" #include "VideoCommon/VertexLoaderManager.h" #include "VideoCommon/VertexLoaderUtils.h" @@ -35,19 +34,15 @@ void Pos_ReadDirect(VertexLoader* loader) { static_assert(N <= 3, "N > 3 is not sane!"); const auto scale = loader->m_posScale; - DataReader dst(g_vertex_manager_write_ptr, nullptr); - DataReader src(g_video_buffer_read_ptr, nullptr); for (int i = 0; i < N; ++i) { - const float value = PosScale(src.Read(), scale); + const float value = PosScale(DataRead(), scale); if (loader->m_remaining < 3) VertexLoaderManager::position_cache[loader->m_remaining][i] = value; - dst.Write(value); + DataWrite(value); } - g_vertex_manager_write_ptr = dst.GetPointer(); - g_video_buffer_read_ptr = src.GetPointer(); LOG_VTX(); } @@ -63,17 +58,15 @@ void Pos_ReadIndex(VertexLoader* loader) reinterpret_cast(VertexLoaderManager::cached_arraybases[CPArray::Position] + (index * g_main_cp_state.array_strides[CPArray::Position])); const auto scale = loader->m_posScale; - DataReader dst(g_vertex_manager_write_ptr, nullptr); for (int i = 0; i < N; ++i) { const float value = PosScale(Common::FromBigEndian(data[i]), scale); if (loader->m_remaining < 3) VertexLoaderManager::position_cache[loader->m_remaining][i] = value; - dst.Write(value); + DataWrite(value); } - g_vertex_manager_write_ptr = dst.GetPointer(); LOG_VTX(); } diff --git a/Source/Core/VideoCommon/VertexLoader_TextCoord.cpp b/Source/Core/VideoCommon/VertexLoader_TextCoord.cpp index abe8d36fc4..fcb2982e6f 100644 --- a/Source/Core/VideoCommon/VertexLoader_TextCoord.cpp +++ b/Source/Core/VideoCommon/VertexLoader_TextCoord.cpp @@ -8,7 +8,6 @@ #include "Common/CommonTypes.h" #include "Common/Swap.h" -#include "VideoCommon/DataReader.h" #include "VideoCommon/VertexLoader.h" #include "VideoCommon/VertexLoaderManager.h" #include "VideoCommon/VertexLoaderUtils.h" @@ -36,14 +35,9 @@ template void TexCoord_ReadDirect(VertexLoader* loader) { const auto scale = loader->m_tcScale[loader->m_tcIndex]; - DataReader dst(g_vertex_manager_write_ptr, nullptr); - DataReader src(g_video_buffer_read_ptr, nullptr); for (int i = 0; i != N; ++i) - dst.Write(TCScale(src.Read(), scale)); - - g_vertex_manager_write_ptr = dst.GetPointer(); - g_video_buffer_read_ptr = src.GetPointer(); + DataWrite(TCScale(DataRead(), scale)); ++loader->m_tcIndex; } @@ -58,12 +52,10 @@ void TexCoord_ReadIndex(VertexLoader* loader) VertexLoaderManager::cached_arraybases[CPArray::TexCoord0 + loader->m_tcIndex] + (index * g_main_cp_state.array_strides[CPArray::TexCoord0 + loader->m_tcIndex])); const auto scale = loader->m_tcScale[loader->m_tcIndex]; - DataReader dst(g_vertex_manager_write_ptr, nullptr); for (int i = 0; i != N; ++i) - dst.Write(TCScale(Common::FromBigEndian(data[i]), scale)); + DataWrite(TCScale(Common::FromBigEndian(data[i]), scale)); - g_vertex_manager_write_ptr = dst.GetPointer(); ++loader->m_tcIndex; } diff --git a/Source/UnitTests/VideoCommon/VertexLoaderTest.cpp b/Source/UnitTests/VideoCommon/VertexLoaderTest.cpp index 5e3898ed28..0ad4252221 100644 --- a/Source/UnitTests/VideoCommon/VertexLoaderTest.cpp +++ b/Source/UnitTests/VideoCommon/VertexLoaderTest.cpp @@ -91,7 +91,7 @@ protected: if (expected_count == -1) expected_count = count; ResetPointers(); - int actual_count = m_loader->RunVertices(m_src, m_dst, count); + int actual_count = m_loader->RunVertices(m_src.GetPointer(), m_dst.GetPointer(), count); EXPECT_EQ(actual_count, expected_count); }