GHI-7125 : Tangent generation updates (#7252)

* Add a settings registry setting for choosing the default tangent generation method. Add another setting that will force scenes to fail to process if they are impacted by GHI-7125. Fix a bug where assets that return a Failure state from the TangentGenerateComponent will still successfully process.

Signed-off-by: Tommy Walton <waltont@amazon.com>

* Update with minor pr feedback

Signed-off-by: Tommy Walton <waltont@amazon.com>

* Update Gems/SceneProcessing/Code/Source/Generation/Components/TangentGenerator/TangentGenerateComponent.cpp

Use the AZ_STRING_FORMAT and AZ_STRING_ARG macros for printing a string_view

Co-authored-by: Chris Burel <burelc@amazon.com>
Signed-off-by: Tommy Walton <waltont@amazon.com>

Co-authored-by: Chris Burel <burelc@amazon.com>
Signed-off-by: Tommy Walton <waltont@amazon.com>
monroegm-disable-blank-issue-2
Tommy Walton 4 years ago committed by GitHub
parent d88fc40157
commit 122eef2680
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

@ -28,11 +28,15 @@
#include <SceneAPI/SceneData/GraphData/MeshVertexTangentData.h>
#include <AzCore/Math/Vector4.h>
#include <AzCore/Settings/SettingsRegistry.h>
#include <AzCore/std/smart_ptr/make_shared.h>
namespace AZ::SceneGenerationComponents
{
static constexpr AZStd::string_view DefaultTangentGenerationKey{ "/O3DE/SceneAPI/TangentGenerateComponent/DefaultGenerationMethod" };
static constexpr AZStd::string_view DebugBitangentFlipKey{ "/O3DE/SceneAPI/TangentGenerateComponent/DebugBitangentFlip" };
TangentGenerateComponent::TangentGenerateComponent()
{
BindToCall(&TangentGenerateComponent::GenerateTangentData);
@ -44,7 +48,7 @@ namespace AZ::SceneGenerationComponents
AZ::SerializeContext* serializeContext = azrtti_cast<AZ::SerializeContext*>(context);
if (serializeContext)
{
serializeContext->Class<TangentGenerateComponent, AZ::SceneAPI::SceneCore::GenerationComponent>()->Version(1);
serializeContext->Class<TangentGenerateComponent, AZ::SceneAPI::SceneCore::GenerationComponent>()->Version(2);
}
}
@ -66,8 +70,47 @@ namespace AZ::SceneGenerationComponents
return nullptr;
}
void TangentGenerateComponent::GetRegistrySettings(
AZ::SceneAPI::DataTypes::TangentGenerationMethod& defaultGenerationMethod, bool& debugBitangentFlip) const
{
if (auto settingsRegistry = AZ::SettingsRegistry::Get(); settingsRegistry != nullptr)
{
AZStd::string defaultTangentGenerationMethodString;
if (settingsRegistry->Get(defaultTangentGenerationMethodString, DefaultTangentGenerationKey))
{
const bool isCaseSensitive = false;
if (AZ::StringFunc::Equal(defaultTangentGenerationMethodString, "MikkT", isCaseSensitive))
{
defaultGenerationMethod = AZ::SceneAPI::DataTypes::TangentGenerationMethod::MikkT;
}
else
{
AZ_Warning(
AZ::SceneAPI::Utilities::WarningWindow,
AZ::StringFunc::Equal(defaultTangentGenerationMethodString, "FromSourceScene", isCaseSensitive),
"'" AZ_STRING_FORMAT "' is not a valid default tangent generation method. Check the value of %s in your settings registry, and change "
"it to 'FromSourceScene' or 'MikkT'",
defaultTangentGenerationMethodString.c_str(), AZ_STRING_ARG(DefaultTangentGenerationKey));
}
}
settingsRegistry->Get(debugBitangentFlip, DebugBitangentFlipKey);
}
}
AZ::SceneAPI::Events::ProcessingResult TangentGenerateComponent::GenerateTangentData(TangentGenerateContext& context)
{
// Get any tangent related settings from the settings registry
AZ::SceneAPI::DataTypes::TangentGenerationMethod defaultGenerationMethod =
AZ::SceneAPI::DataTypes::TangentGenerationMethod::FromSourceScene;
bool debugBitangentFlip = false;
GetRegistrySettings(defaultGenerationMethod, debugBitangentFlip);
// Get the generation setting for this scene
const AZ::SceneAPI::SceneData::TangentsRule* tangentsRule = GetTangentRule(context.GetScene());
const AZ::SceneAPI::DataTypes::TangentGenerationMethod generationMethod =
tangentsRule ? tangentsRule->GetGenerationMethod() : defaultGenerationMethod;
// Iterate over all graph content and filter out all meshes.
AZ::SceneAPI::Containers::SceneGraph& graph = context.GetScene().GetGraph();
AZ::SceneAPI::Containers::SceneGraph::ContentStorageData graphContent = graph.GetContentStorage();
@ -92,19 +135,31 @@ namespace AZ::SceneGenerationComponents
for (auto& [mesh, nodeIndex] : meshes)
{
// Generate tangents for the mesh (if this is desired or needed).
if (!GenerateTangentsForMesh(context.GetScene(), nodeIndex, mesh))
if (!GenerateTangentsForMesh(context.GetScene(), nodeIndex, mesh, generationMethod))
{
return AZ::SceneAPI::Events::ProcessingResult::Failure;
}
// Now that we have the tangents and bitangents, calculate the tangent w values for the ones that we imported from the scene file, as they only have xyz.
UpdateFbxTangentWValues(graph, nodeIndex, mesh);
// But only do this if we are getting tangents from the source scene, because MikkT will provide us with a correct tangent.w already
if (generationMethod == SceneAPI::DataTypes::TangentGenerationMethod::FromSourceScene)
{
if (!UpdateFbxTangentWValues(graph, nodeIndex, mesh, debugBitangentFlip))
{
return AZ::SceneAPI::Events::ProcessingResult::Failure;
}
}
}
return AZ::SceneAPI::Events::ProcessingResult::Success;
}
void TangentGenerateComponent::UpdateFbxTangentWValues(AZ::SceneAPI::Containers::SceneGraph& graph, const AZ::SceneAPI::Containers::SceneGraph::NodeIndex& nodeIndex, const AZ::SceneAPI::DataTypes::IMeshData* meshData)
bool TangentGenerateComponent::UpdateFbxTangentWValues(
AZ::SceneAPI::Containers::SceneGraph& graph,
const AZ::SceneAPI::Containers::SceneGraph::NodeIndex& nodeIndex,
const AZ::SceneAPI::DataTypes::IMeshData* meshData,
bool debugBitangentFlip)
{
// Iterate over all UV sets.
AZ::SceneAPI::DataTypes::IMeshVertexUVData* uvData = FindUvData(graph, nodeIndex, 0);
@ -145,6 +200,26 @@ namespace AZ::SceneGenerationComponents
tangent = fbxTangentData->GetTangent(i);
tangent.SetW(1.0f);
}
if (debugBitangentFlip)
{
AZ::Vector4 originalTangent = fbxTangentData->GetTangent(i);
if (originalTangent.GetW() > 0.0f)
{
// If the tangent has a positive w value, the fix for GHI-7125 is going to flip the bitangent
// compared to the original behavior. Report an error and fail to process as an indication
// that this asset will be impacted by GHI-7125
AZ_Error(
AZ::SceneAPI::Utilities::ErrorWindow, false,
"Tangent w is positive for at least one vertex in the mesh. This model will be impacted by GHI-7125. "
"See https://github.com/o3de/o3de/issues/7125 for details.");
return false;
}
}
// Update the tangent.w in the scene
fbxTangentData->SetTangent(i, tangent);
}
}
@ -152,6 +227,8 @@ namespace AZ::SceneGenerationComponents
// Find the next UV set.
uvData = FindUvData(graph, nodeIndex, ++uvSetIndex);
}
return true;
}
void TangentGenerateComponent::FindBlendShapes(
@ -173,7 +250,11 @@ namespace AZ::SceneGenerationComponents
}
}
bool TangentGenerateComponent::GenerateTangentsForMesh(AZ::SceneAPI::Containers::Scene& scene, const AZ::SceneAPI::Containers::SceneGraph::NodeIndex& nodeIndex, AZ::SceneAPI::DataTypes::IMeshData* meshData)
bool TangentGenerateComponent::GenerateTangentsForMesh(
AZ::SceneAPI::Containers::Scene& scene,
const AZ::SceneAPI::Containers::SceneGraph::NodeIndex& nodeIndex,
AZ::SceneAPI::DataTypes::IMeshData* meshData,
AZ::SceneAPI::DataTypes::TangentGenerationMethod ruleGenerationMethod)
{
AZ::SceneAPI::Containers::SceneGraph& graph = scene.GetGraph();
@ -186,7 +267,6 @@ namespace AZ::SceneGenerationComponents
}
const AZ::SceneAPI::SceneData::TangentsRule* tangentsRule = GetTangentRule(scene);
const AZ::SceneAPI::DataTypes::TangentGenerationMethod ruleGenerationMethod = tangentsRule ? tangentsRule->GetGenerationMethod() : AZ::SceneAPI::DataTypes::TangentGenerationMethod::FromSourceScene;
// Find all blend shape data under the mesh. We need to generate the tangent and bitangent for blend shape as well.
AZStd::vector<AZ::SceneData::GraphData::BlendShapeData*> blendShapes;

@ -58,9 +58,19 @@ namespace AZ::SceneGenerationComponents
void FindBlendShapes(
AZ::SceneAPI::Containers::SceneGraph& graph, const AZ::SceneAPI::Containers::SceneGraph::NodeIndex& nodeIndex,
AZStd::vector<AZ::SceneData::GraphData::BlendShapeData*>& outBlendShapes) const;
bool GenerateTangentsForMesh(AZ::SceneAPI::Containers::Scene& scene, const AZ::SceneAPI::Containers::SceneGraph::NodeIndex& nodeIndex, AZ::SceneAPI::DataTypes::IMeshData* meshData);
void UpdateFbxTangentWValues(AZ::SceneAPI::Containers::SceneGraph& graph, const AZ::SceneAPI::Containers::SceneGraph::NodeIndex& nodeIndex, const AZ::SceneAPI::DataTypes::IMeshData* meshData);
bool GenerateTangentsForMesh(
AZ::SceneAPI::Containers::Scene& scene,
const AZ::SceneAPI::Containers::SceneGraph::NodeIndex& nodeIndex,
AZ::SceneAPI::DataTypes::IMeshData* meshData,
AZ::SceneAPI::DataTypes::TangentGenerationMethod defaultGenerationMethod);
bool UpdateFbxTangentWValues(
AZ::SceneAPI::Containers::SceneGraph& graph,
const AZ::SceneAPI::Containers::SceneGraph::NodeIndex& nodeIndex,
const AZ::SceneAPI::DataTypes::IMeshData* meshData,
bool deubgBitangentFlip);
const AZ::SceneAPI::SceneData::TangentsRule* GetTangentRule(const AZ::SceneAPI::Containers::Scene& scene) const;
void GetRegistrySettings(
AZ::SceneAPI::DataTypes::TangentGenerationMethod& defaultTangentGenrationMethod, bool& debugBitangentFlip) const;
size_t CalcUvSetCount(AZ::SceneAPI::Containers::SceneGraph& graph, const AZ::SceneAPI::Containers::SceneGraph::NodeIndex& nodeIndex) const;
AZ::SceneAPI::DataTypes::IMeshVertexUVData* FindUvData(AZ::SceneAPI::Containers::SceneGraph& graph, const AZ::SceneAPI::Containers::SceneGraph::NodeIndex& nodeIndex, AZ::u64 uvSet) const;

@ -35,6 +35,6 @@ namespace AZ::SceneGenerationComponents
SceneEvents::ProcessingResultCombiner result;
TangentGenerateContext tangentGenerateContext(context.GetScene());
result += SceneEvents::Process<TangentGenerateContext>(tangentGenerateContext);
return SceneEvents::ProcessingResult::Success;
return result.GetResult();
}
} // namespace AZ::SceneGenerationComponents

@ -15,6 +15,11 @@
{
"Enable": true,
"DefaultMaterial": "Materials/Presets/PBR/default_grid.material"
},
"TangentGenerateComponent":
{
"DefaultGenerationMethod": "FromSourceScene",
"DebugBitangentFlip": false
}
}
}

Loading…
Cancel
Save