birkeh/o3de
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix failed Atom rpi tests linux (#6319)

Browse Source 
- Remove trait AZ_TRAIT_DISABLE_FAILED_ATOM_RPI_TESTS
- Add template specialization for ConstantsData::SetConstant<Color> and ConstantsData::GetConstant<Color>
- Remove trait AZ_TRAIT_DISABLE_FAILED_ATOM_RPI_TESTS from source code
- Fix ConstantsData::SetConstantArray<bool> to pass unit test SetConstant_GetConstant_FalsePackedInGarbage_Bool on Linux, cause by an unwanted optimization (comments describe the situation). This will make this behavior consistent across platforms

Signed-off-by: Steve Pham <82231385+spham-amzn@users.noreply.github.com>
This commit is contained in:
Steve Pham
2021-12-10 14:54:08 -08:00
committed by GitHub
parent 3ae6b601a2
commit 4670003aaf
6 changed files with 47 additions and 23 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Code/Framework/AzTest/AzTest/Platform/Linux/AzTest_Traits_Linux.h
-2
View File
@@ -13,8 +13,6 @@
#define AZ_TRAIT_UNIT_TEST_ENTITY_ID_GEN_TEST_COUNT 10000
#define AZ_TRAIT_UNIT_TEST_DILLER_TRIGGER_EVENT_COUNT 100000
#define AZ_TRAIT_DISABLE_FAILED_ATOM_RPI_TESTS true
#define AZ_TRAIT_DISABLE_FAILED_MULTIPLAYER_GRIDMATE_TESTS true
#define AZ_TRAIT_DISABLE_FAILED_NATIVE_WINDOWS_TESTS true
#define AZ_TRAIT_DISABLE_FAILED_EMOTION_FX_TESTS true
Gems/Atom/RHI/Code/Include/Atom/RHI/ConstantsData.h
+7
View File
@@ -8,6 +8,7 @@
#pragma once
#include <Atom/RHI.Reflect/PipelineLayoutDescriptor.h>
#include <AzCore/Math/Color.h>
#include <AzCore/Math/Matrix3x3.h>
#include <AzCore/Math/Matrix3x4.h>
#include <AzCore/Math/Matrix4x4.h>
@@ -147,6 +148,9 @@ namespace AZ
template <>
bool ConstantsData::SetConstant<Vector4>(ShaderInputConstantIndex inputIndex, const Vector4& value);
template <>
bool ConstantsData::SetConstant<Color>(ShaderInputConstantIndex inputIndex, const Color& value);
template <>
bool ConstantsData::SetConstantArray<bool>(ShaderInputConstantIndex inputIndex, AZStd::array_view<bool> values);
@@ -171,6 +175,9 @@ namespace AZ
template <>
Vector4 ConstantsData::GetConstant<Vector4>(ShaderInputConstantIndex inputIndex) const;
template <>
Color ConstantsData::GetConstant<Color>(ShaderInputConstantIndex inputIndex) const;
template <typename T, uint32_t matrixSize>
bool ConstantsData::SetConstantMatrixRows(ShaderInputConstantIndex inputIndex, const T& value, uint32_t rowCount)
{
Gems/Atom/RHI/Code/Source/RHI/ConstantsData.cpp
+40 -1
View File
@@ -153,9 +153,21 @@ namespace AZ
{
bool isValidAll = true;
uint32_t offset = 0;
// Rather than doing the direct validation against values directly, we have to convert the array_view
// to a raw byte array, and then check against the bytes to determine whether or not to set the
// uint32 value to 1 (true) or 0 (false). Clang when building in non-debug builds was optimizing out
// the actual 1 and 0 values, so an expression like:
//
// const uint32_t fourByteValue = values[i] ? 1 : 0;
//
// when values[0] == 205, will instead set 'fourByteValue' is assigned to '205', instead of '1'.
// In debug builds, and other microsoft compilers (debug+release), this type of optimization doesnt
// occur and we get the expected results instead
const AZ::u8* byteValues = reinterpret_cast<const AZ::u8*>(values.data());
for (size_t i = 0; i < values.size(); i++)
{
const uint32_t fourByteValue = values[i] ? 1 : 0;
const uint32_t fourByteValue = byteValues[i] ? 1 : 0;
const bool isValid = SetConstantRaw(inputIndex, &fourByteValue, offset, elementSize);
isValidAll &= isValid;
@@ -273,6 +285,21 @@ namespace AZ
return false;
}
template <>
bool ConstantsData::SetConstant<Color>(ShaderInputConstantIndex inputIndex, const Color& value)
{
constexpr size_t sizeOfColor = sizeof(Color);
if (ValidateConstantAccess(inputIndex, ValidateConstantAccessExpect::Complete, 0, aznumeric_caster(sizeOfColor)))
{
const Interval interval = GetLayout()->GetInterval(inputIndex);
float* vectorValue = reinterpret_cast<float*>(&m_constantData[interval.m_min]);
value.StoreToFloat4(vectorValue);
return true;
}
return false;
}
bool ConstantsData::SetConstantMatrixRows(ShaderInputConstantIndex inputIndex, const Matrix3x3& value, uint32_t rowCount)
{
// See the packing comments in ConstantsData::SetConstant<Matrix3x3> for an explanation of why we only use
@@ -389,6 +416,18 @@ namespace AZ
return Vector4();
}
template <>
Color ConstantsData::GetConstant<Color>(ShaderInputConstantIndex inputIndex) const
{
constexpr size_t colorSize = sizeof(Color);
if (ValidateConstantAccess(inputIndex, ValidateConstantAccessExpect::Complete, 0, aznumeric_caster(colorSize)))
{
AZStd::array_view<uint8_t> constantBytes = GetConstantRaw(inputIndex);
return Color::CreateFromFloat4(reinterpret_cast<const float*>(constantBytes.data()));
}
return Color();
}
AZStd::array_view<uint8_t> ConstantsData::GetConstantRaw(ShaderInputConstantIndex inputIndex) const
{
const Interval interval = GetLayout()->GetInterval(inputIndex);
Gems/Atom/RPI/Code/Tests/Material/LuaMaterialFunctorTests.cpp
-4
View File
@@ -425,11 +425,7 @@ namespace UnitTest
EXPECT_EQ(Vector4(1.0f, 2.0f, 3.0f, 4.0f) / 4.0f, testData.GetMaterial()->GetRHIShaderResourceGroup()->GetData().GetConstant<Vector4>(testData.GetSrgConstantIndex()));
}
#if AZ_TRAIT_DISABLE_FAILED_ATOM_RPI_TESTS
TEST_F(LuaMaterialFunctorTests, DISABLED_LuaMaterialFunctor_RuntimeContext_GetMaterialProperty_SetShaderConstant_Color)
#else
TEST_F(LuaMaterialFunctorTests, LuaMaterialFunctor_RuntimeContext_GetMaterialProperty_SetShaderConstant_Color)
#endif // AZ_TRAIT_DISABLE_FAILED_ATOM_RPI_TESTS
{
using namespace AZ::RPI;
Gems/Atom/RPI/Code/Tests/Material/MaterialTests.cpp
-12
View File
@@ -211,21 +211,13 @@ namespace UnitTest
EXPECT_NE(materialInstance3, materialInstance4);
}
#if AZ_TRAIT_DISABLE_FAILED_ATOM_RPI_TESTS
TEST_F(MaterialTests, DISABLED_TestInitialValuesFromMaterial)
#else
TEST_F(MaterialTests, TestInitialValuesFromMaterial)
#endif // AZ_TRAIT_DISABLE_FAILED_ATOM_RPI_TESTS
{
Data::Instance<Material> material = Material::FindOrCreate(m_testMaterialAsset);
ValidateInitialValuesFromMaterial(material);
}
#if AZ_TRAIT_DISABLE_FAILED_ATOM_RPI_TESTS
TEST_F(MaterialTests, DISABLED_TestSetPropertyValue)
#else
TEST_F(MaterialTests, TestSetPropertyValue)
#endif // AZ_TRAIT_DISABLE_FAILED_ATOM_RPI_TESTS
{
Data::Instance<Material> material = Material::FindOrCreate(m_testMaterialAsset);
@@ -713,11 +705,7 @@ namespace UnitTest
AZ_TEST_STOP_ASSERTTEST(2);
}
#if AZ_TRAIT_DISABLE_FAILED_ATOM_RPI_TESTS
TEST_F(MaterialTests, DISABLED_Error_SetPropertyValue_WrongDataType)
#else
TEST_F(MaterialTests, Error_SetPropertyValue_WrongDataType)
#endif // AZ_TRAIT_DISABLE_FAILED_ATOM_RPI_TESTS
{
Data::Instance<Material> material = Material::FindOrCreate(m_testMaterialAsset);
Gems/Atom/RPI/Code/Tests/ShaderResourceGroup/ShaderResourceGroupConstantBufferTests.cpp
-4
View File
@@ -242,11 +242,7 @@ namespace UnitTest
ExpectEqual<uint32_t>({ 0 /*false*/, 1 /*true*/ }, resultInUint);
}
}
#if AZ_TRAIT_DISABLE_FAILED_ATOM_RPI_TESTS
TEST_F(ShaderResourceGroupConstantBufferTests, DISABLED_SetConstant_GetConstant_FalsePackedInGarbage_Bool)
#else
TEST_F(ShaderResourceGroupConstantBufferTests, SetConstant_GetConstant_FalsePackedInGarbage_Bool)
#endif // AZ_TRAIT_DISABLE_FAILED_ATOM_RPI_TESTS
{
using namespace AZ;