Merge pull request #11432 from Pokechu22/generic-vertex-loader-test-error

VertexLoader: Fix loading tangent/binormal caches with NormalIndex3
This commit is contained in:
Pierre Bourdon 2023-01-17 23:30:29 +01:00 committed by GitHub
commit a7d1683149
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 130 additions and 27 deletions

View File

@ -13,11 +13,13 @@
#include <fmt/format.h> #include <fmt/format.h>
#include "Common/Assert.h" #include "Common/Assert.h"
#include "Common/BitUtils.h"
#include "Common/CommonTypes.h" #include "Common/CommonTypes.h"
#include "Common/Logging/Log.h" #include "Common/Logging/Log.h"
#include "Common/MsgHandler.h" #include "Common/MsgHandler.h"
#include "VideoCommon/VertexLoader.h" #include "VideoCommon/VertexLoader.h"
#include "VideoCommon/VertexLoaderManager.h"
#include "VideoCommon/VertexLoader_Color.h" #include "VideoCommon/VertexLoader_Color.h"
#include "VideoCommon/VertexLoader_Normal.h" #include "VideoCommon/VertexLoader_Normal.h"
#include "VideoCommon/VertexLoader_Position.h" #include "VideoCommon/VertexLoader_Position.h"
@ -61,25 +63,93 @@ public:
buffer_a.resize(count * a->m_native_vtx_decl.stride + 4); buffer_a.resize(count * a->m_native_vtx_decl.stride + 4);
buffer_b.resize(count * b->m_native_vtx_decl.stride + 4); buffer_b.resize(count * b->m_native_vtx_decl.stride + 4);
int count_a = a->RunVertices(src, buffer_a.data(), count); const std::array<u32, 3> old_position_matrix_index_cache =
int count_b = b->RunVertices(src, buffer_b.data(), count); VertexLoaderManager::position_matrix_index_cache;
const std::array<std::array<float, 4>, 3> old_position_cache =
VertexLoaderManager::position_cache;
const std::array<float, 4> old_tangent_cache = VertexLoaderManager::tangent_cache;
const std::array<float, 4> old_binormal_cache = VertexLoaderManager::binormal_cache;
if (count_a != count_b) const int count_a = a->RunVertices(src, buffer_a.data(), count);
{
ERROR_LOG_FMT( const std::array<u32, 3> a_position_matrix_index_cache =
VIDEO, VertexLoaderManager::position_matrix_index_cache;
const std::array<std::array<float, 4>, 3> a_position_cache =
VertexLoaderManager::position_cache;
const std::array<float, 4> a_tangent_cache = VertexLoaderManager::tangent_cache;
const std::array<float, 4> a_binormal_cache = VertexLoaderManager::binormal_cache;
// Reset state before running b
VertexLoaderManager::position_matrix_index_cache = old_position_matrix_index_cache;
VertexLoaderManager::position_cache = old_position_cache;
VertexLoaderManager::tangent_cache = old_tangent_cache;
VertexLoaderManager::binormal_cache = old_binormal_cache;
const int count_b = b->RunVertices(src, buffer_b.data(), count);
const std::array<u32, 3> b_position_matrix_index_cache =
VertexLoaderManager::position_matrix_index_cache;
const std::array<std::array<float, 4>, 3> b_position_cache =
VertexLoaderManager::position_cache;
const std::array<float, 4> b_tangent_cache = VertexLoaderManager::tangent_cache;
const std::array<float, 4> b_binormal_cache = VertexLoaderManager::binormal_cache;
ASSERT_MSG(VIDEO, count_a == count_b,
"The two vertex loaders have loaded a different amount of vertices (a: {}, b: {}).", "The two vertex loaders have loaded a different amount of vertices (a: {}, b: {}).",
count_a, count_b); count_a, count_b);
}
if (memcmp(buffer_a.data(), buffer_b.data(), ASSERT_MSG(VIDEO,
std::min(count_a, count_b) * m_native_vtx_decl.stride)) memcmp(buffer_a.data(), buffer_b.data(),
{ std::min(count_a, count_b) * m_native_vtx_decl.stride) == 0,
ERROR_LOG_FMT(VIDEO,
"The two vertex loaders have loaded different data. Configuration:" "The two vertex loaders have loaded different data. Configuration:"
"\nVertex desc:\n{}\n\nVertex attr:\n{}", "\nVertex desc:\n{}\n\nVertex attr:\n{}",
m_VtxDesc, m_VtxAttr); m_VtxDesc, m_VtxAttr);
ASSERT_MSG(VIDEO, a_position_matrix_index_cache == b_position_matrix_index_cache,
"Expected matching position matrix caches after loading (a: {}; b: {})",
fmt::join(a_position_matrix_index_cache, ", "),
fmt::join(b_position_matrix_index_cache, ", "));
// Some games (e.g. Donkey Kong Country Returns) have a few draws that contain NaN.
// Since NaN != NaN, we need to compare the bits instead.
const auto bit_equal = [](float a, float b) {
return Common::BitCast<u32>(a) == Common::BitCast<u32>(b);
};
// The last element is allowed to be garbage for SIMD overwrites.
// For XY, the last 2 are garbage.
const bool positions_match = [&] {
const size_t max_component = m_VtxAttr.g0.PosElements == CoordComponentCount::XYZ ? 3 : 2;
for (size_t vertex = 0; vertex < 3; vertex++)
{
if (!std::equal(a_position_cache[vertex].begin(),
a_position_cache[vertex].begin() + max_component,
b_position_cache[vertex].begin(), bit_equal))
{
return false;
} }
}
return true;
}();
ASSERT_MSG(VIDEO, positions_match,
"Expected matching position caches after loading (a: {} / {} / {}; b: {} / {} / {})",
fmt::join(a_position_cache[0], ", "), fmt::join(a_position_cache[1], ", "),
fmt::join(a_position_cache[2], ", "), fmt::join(b_position_cache[0], ", "),
fmt::join(b_position_cache[1], ", "), fmt::join(b_position_cache[2], ", "));
// The last element is allowed to be garbage for SIMD overwrites
ASSERT_MSG(VIDEO,
std::equal(a_tangent_cache.begin(), a_tangent_cache.begin() + 3,
b_tangent_cache.begin(), b_tangent_cache.begin() + 3, bit_equal),
"Expected matching tangent caches after loading (a: {}; b: {})",
fmt::join(a_tangent_cache, ", "), fmt::join(b_tangent_cache, ", "));
ASSERT_MSG(VIDEO,
std::equal(a_binormal_cache.begin(), a_binormal_cache.begin() + 3,
b_binormal_cache.begin(), b_binormal_cache.begin() + 3, bit_equal),
"Expected matching binormal caches after loading (a: {}; b: {})",
fmt::join(a_binormal_cache, ", "), fmt::join(b_binormal_cache, ", "));
memcpy(dst, 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; m_numLoadedVertices += count;

View File

@ -38,12 +38,13 @@ constexpr float FracAdjust(float val)
return val; return val;
} }
template <typename T, u32 N> template <typename T, u32 N, u32 Offset>
void ReadIndirect(VertexLoader* loader, const T* data) void ReadIndirect(VertexLoader* loader, const T* data)
{ {
static_assert(3 == N || 9 == N, "N is only sane as 3 or 9!"); static_assert(3 == N || 9 == N, "N is only sane as 3 or 9!");
static_assert(!(Offset != 0 && 9 == N), "N == 9 only makes sense if offset == 0");
for (u32 i = 0; i < N; ++i) for (u32 i = Offset; i < N + Offset; ++i)
{ {
const float value = FracAdjust(Common::FromBigEndian(data[i])); const float value = FracAdjust(Common::FromBigEndian(data[i]));
if (loader->m_remaining == 0) if (loader->m_remaining == 0)
@ -63,7 +64,7 @@ template <typename T, u32 N>
void Normal_ReadDirect(VertexLoader* loader) void Normal_ReadDirect(VertexLoader* loader)
{ {
const auto source = reinterpret_cast<const T*>(DataGetPosition()); const auto source = reinterpret_cast<const T*>(DataGetPosition());
ReadIndirect<T, N * 3>(loader, source); ReadIndirect<T, N * 3, 0>(loader, source);
DataSkip<N * 3 * sizeof(T)>(); DataSkip<N * 3 * sizeof(T)>();
} }
@ -73,10 +74,10 @@ void Normal_ReadIndex_Offset(VertexLoader* loader)
static_assert(std::is_unsigned_v<I>, "Only unsigned I is sane!"); static_assert(std::is_unsigned_v<I>, "Only unsigned I is sane!");
const auto index = DataRead<I>(); const auto index = DataRead<I>();
const auto data = reinterpret_cast<const T*>( const auto data =
VertexLoaderManager::cached_arraybases[CPArray::Normal] + reinterpret_cast<const T*>(VertexLoaderManager::cached_arraybases[CPArray::Normal] +
(index * g_main_cp_state.array_strides[CPArray::Normal]) + sizeof(T) * 3 * Offset); (index * g_main_cp_state.array_strides[CPArray::Normal]));
ReadIndirect<T, N * 3>(loader, data); ReadIndirect<T, N * 3, Offset * 3>(loader, data);
} }
template <typename I, typename T, u32 N> template <typename I, typename T, u32 N>

View File

@ -62,7 +62,7 @@ void Pos_ReadIndex(VertexLoader* loader)
for (int i = 0; i < N; ++i) for (int i = 0; i < N; ++i)
{ {
const float value = PosScale(Common::FromBigEndian(data[i]), scale); const float value = PosScale(Common::FromBigEndian(data[i]), scale);
if (loader->m_remaining < 3) if (loader->m_remaining < 3 && !loader->m_vertexSkip)
VertexLoaderManager::position_cache[loader->m_remaining][i] = value; VertexLoaderManager::position_cache[loader->m_remaining][i] = value;
DataWrite(value); DataWrite(value);
} }

View File

@ -4,6 +4,8 @@ add_custom_command(TARGET unittests POST_BUILD COMMAND ${CMAKE_CTEST_COMMAND})
string(APPEND CMAKE_RUNTIME_OUTPUT_DIRECTORY "/Tests") string(APPEND CMAKE_RUNTIME_OUTPUT_DIRECTORY "/Tests")
add_library(unittests_main OBJECT UnitTestsMain.cpp)
target_link_libraries(unittests_main PUBLIC fmt gtest)
# Since this is a Core dependency, it can't be linked as a normal library. # Since this is a Core dependency, it can't be linked as a normal library.
# Otherwise CMake inserts the library after core, but before other core # Otherwise CMake inserts the library after core, but before other core
# dependencies like videocommon which also use Host_ functions, which makes the # dependencies like videocommon which also use Host_ functions, which makes the
@ -16,7 +18,7 @@ macro(add_dolphin_test target)
$<TARGET_OBJECTS:unittests_stubhost> $<TARGET_OBJECTS:unittests_stubhost>
) )
set_target_properties(${target} PROPERTIES FOLDER Tests) set_target_properties(${target} PROPERTIES FOLDER Tests)
target_link_libraries(${target} PRIVATE core uicommon gtest_main) target_link_libraries(${target} PRIVATE core uicommon unittests_main)
add_dependencies(unittests ${target}) add_dependencies(unittests ${target})
add_test(NAME ${target} COMMAND ${target}) add_test(NAME ${target} COMMAND ${target})
endmacro() endmacro()

View File

@ -36,8 +36,8 @@
<ItemGroup> <ItemGroup>
<!--gtest is rather small, so just include it into the build here--> <!--gtest is rather small, so just include it into the build here-->
<ClCompile Include="$(ExternalsDir)gtest\src\gtest-all.cc" /> <ClCompile Include="$(ExternalsDir)gtest\src\gtest-all.cc" />
<ClCompile Include="$(ExternalsDir)gtest\src\gtest_main.cc" />
<!--Lump all of the tests (and supporting code) into one binary--> <!--Lump all of the tests (and supporting code) into one binary-->
<ClCompile Include="UnitTestsMain.cpp" />
<ClCompile Include="Common\BitFieldTest.cpp" /> <ClCompile Include="Common\BitFieldTest.cpp" />
<ClCompile Include="Common\BitSetTest.cpp" /> <ClCompile Include="Common\BitSetTest.cpp" />
<ClCompile Include="Common\BitUtilsTest.cpp" /> <ClCompile Include="Common\BitUtilsTest.cpp" />

View File

@ -0,0 +1,30 @@
// Copyright 2023 Dolphin Emulator Project
// SPDX-License-Identifier: GPL-2.0-or-later
// Based on gtest_main.cc
#include <cstdio>
#include <fmt/format.h>
#include "Common/MsgHandler.h"
#include "gtest/gtest.h"
namespace
{
bool TestMsgHandler(const char* caption, const char* text, bool yes_no, Common::MsgType style)
{
fmt::print(stderr, "{}\n", text);
ADD_FAILURE();
// Return yes to any question (we don't need Dolphin to break on asserts)
return true;
}
} // namespace
int main(int argc, char** argv)
{
fmt::print(stderr, "Running main() from UnitTestsMain.cpp\n");
Common::RegisterMsgAlertHandler(TestMsgHandler);
::testing::InitGoogleTest(&argc, argv);
return RUN_ALL_TESTS();
}