From 9a4a7bc1564887b9e9772d365056db4f91501b52 Mon Sep 17 00:00:00 2001 From: lsemp3d <58790905+lsemp3d@users.noreply.github.com> Date: Thu, 22 Jul 2021 17:19:29 -0700 Subject: [PATCH 1/8] Script Canvas: Fixed Scope and Initial Value selection boxes Signed-off-by: lsemp3d <58790905+lsemp3d@users.noreply.github.com> --- .../ScriptCanvas/Variable/GraphVariable.cpp | 18 ++---------------- .../ScriptCanvas/Variable/GraphVariable.h | 16 ++++++++++++++++ 2 files changed, 18 insertions(+), 16 deletions(-) diff --git a/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Variable/GraphVariable.cpp b/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Variable/GraphVariable.cpp index 31bd5cbe41..cc9e60e765 100644 --- a/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Variable/GraphVariable.cpp +++ b/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Variable/GraphVariable.cpp @@ -186,20 +186,6 @@ namespace ScriptCanvas if (auto editContext = serializeContext->GetEditContext()) { - auto propertyChoices = [] { - AZStd::vector< AZStd::pair> choices; - choices.emplace_back(AZStd::make_pair(VariableFlags::InitialValueSource::Graph, s_InitialValueSourceNames[0])); - choices.emplace_back(AZStd::make_pair(VariableFlags::InitialValueSource::Component, s_InitialValueSourceNames[1])); - return choices; - }; - - auto scopeChoices = [] { - AZStd::vector< AZStd::pair> choices; - choices.emplace_back(AZStd::make_pair(VariableFlags::Scope::Graph, s_ScopeNames[0])); - choices.emplace_back(AZStd::make_pair(VariableFlags::Scope::Function, s_ScopeNames[1])); - return choices; - }; - editContext->Class("Variable", "Represents a Variable field within a Script Canvas Graph") ->ClassElement(AZ::Edit::ClassElements::EditorData, "") ->Attribute(AZ::Edit::Attributes::Visibility, &GraphVariable::GetVisibility) @@ -208,7 +194,7 @@ namespace ScriptCanvas ->Attribute(AZ::Edit::Attributes::DescriptionTextOverride, &GraphVariable::GetDescriptionOverride) ->DataElement(AZ::Edit::UIHandlers::ComboBox, &GraphVariable::m_InitialValueSource, "Initial Value Source", "Variables can get their values from within the graph or through component properties.") - ->Attribute(AZ::Edit::Attributes::GenericValueList, propertyChoices) + ->Attribute(AZ::Edit::Attributes::GenericValueList, &GraphVariable::GetPropertyChoices) ->Attribute(AZ::Edit::Attributes::ChangeNotify, &GraphVariable::OnInitialValueSourceChanged) ->Attribute(AZ::Edit::Attributes::ChangeNotify, AZ::Edit::PropertyRefreshLevels::AttributesAndValues) ->Attribute(AZ::Edit::Attributes::Visibility, &GraphVariable::GetInputControlVisibility) @@ -219,7 +205,7 @@ namespace ScriptCanvas ->DataElement(AZ::Edit::UIHandlers::ComboBox, &GraphVariable::m_scope, "Scope", "Controls the scope of this variable. i.e. If this is exposed as input to this script, or output from this script, or if the variable is just locally scoped.") ->Attribute(AZ::Edit::Attributes::Visibility, &GraphVariable::GetScopeControlVisibility) - ->Attribute(AZ::Edit::Attributes::GenericValueList, scopeChoices) + ->Attribute(AZ::Edit::Attributes::GenericValueList, &GraphVariable::GetScopeChoices) ->Attribute(AZ::Edit::Attributes::ChangeNotify, &GraphVariable::OnScopeTypedChanged) ->DataElement(AZ::Edit::UIHandlers::Default, &GraphVariable::m_networkProperties, "Network Properties", "Enables whether or not this value should be network synchronized") diff --git a/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Variable/GraphVariable.h b/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Variable/GraphVariable.h index 9ccc00ce14..fd15ac95ee 100644 --- a/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Variable/GraphVariable.h +++ b/Gems/ScriptCanvas/Code/Include/ScriptCanvas/Variable/GraphVariable.h @@ -182,6 +182,22 @@ namespace ScriptCanvas private: + AZStd::vector> GetPropertyChoices() const + { + AZStd::vector< AZStd::pair> choices; + choices.emplace_back(AZStd::make_pair(static_cast(VariableFlags::InitialValueSource::Graph), s_InitialValueSourceNames[0])); + choices.emplace_back(AZStd::make_pair(static_cast(VariableFlags::InitialValueSource::Component), s_InitialValueSourceNames[1])); + return choices; + } + + AZStd::vector> GetScopeChoices() const + { + AZStd::vector< AZStd::pair> choices; + choices.emplace_back(AZStd::make_pair(static_cast(VariableFlags::Scope::Graph), s_ScopeNames[0])); + choices.emplace_back(AZStd::make_pair(static_cast(VariableFlags::Scope::Function), s_ScopeNames[1])); + return choices; + } + bool IsInFunction() const; void OnScopeTypedChanged(); From d7627c05f64bddca17a8fa49d17dc4c0d733da5e Mon Sep 17 00:00:00 2001 From: onecent1101 Date: Wed, 7 Jul 2021 15:33:37 -0700 Subject: [PATCH 2/8] Fix autogen template RewindableObject construction for linux platform Signed-off-by: onecent1101 --- Gems/Multiplayer/Code/Source/AutoGen/AutoComponent_Header.jinja | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gems/Multiplayer/Code/Source/AutoGen/AutoComponent_Header.jinja b/Gems/Multiplayer/Code/Source/AutoGen/AutoComponent_Header.jinja index 2eb57498aa..c18faec74b 100644 --- a/Gems/Multiplayer/Code/Source/AutoGen/AutoComponent_Header.jinja +++ b/Gems/Multiplayer/Code/Source/AutoGen/AutoComponent_Header.jinja @@ -176,7 +176,7 @@ RewindableFixedVector<{{ Property.attrib['Type'] }}, {{ Property.attrib['Count'] AZStd::fixed_vector<{{ Property.attrib['Type'] }}, {{ Property.attrib['Count'] }}> m_{{ LowerFirst(Property.attrib['Name']) }}; {% endif %} {% elif Property.attrib['IsRewindable']|booleanTrue %} -Multiplayer::RewindableObject<{{ Property.attrib['Type'] }}, Multiplayer::RewindHistorySize> m_{{ LowerFirst(Property.attrib['Name']) }} = {{ Property.attrib['Init'] }}; +Multiplayer::RewindableObject<{{ Property.attrib['Type'] }}, Multiplayer::RewindHistorySize> m_{{ LowerFirst(Property.attrib['Name']) }} = { {{ Property.attrib['Init'] }} }; {% else %} {{ Property.attrib['Type'] }} m_{{ LowerFirst(Property.attrib['Name']) }} = {{ Property.attrib['Init'] }}; {% endif %} From 85c403ea06d870365429f2e1571d0bbea5f663d4 Mon Sep 17 00:00:00 2001 From: onecent1101 Date: Wed, 7 Jul 2021 15:57:10 -0700 Subject: [PATCH 3/8] Remove = Signed-off-by: onecent1101 --- Gems/Multiplayer/Code/Source/AutoGen/AutoComponent_Header.jinja | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gems/Multiplayer/Code/Source/AutoGen/AutoComponent_Header.jinja b/Gems/Multiplayer/Code/Source/AutoGen/AutoComponent_Header.jinja index c18faec74b..84fdc9ae54 100644 --- a/Gems/Multiplayer/Code/Source/AutoGen/AutoComponent_Header.jinja +++ b/Gems/Multiplayer/Code/Source/AutoGen/AutoComponent_Header.jinja @@ -176,7 +176,7 @@ RewindableFixedVector<{{ Property.attrib['Type'] }}, {{ Property.attrib['Count'] AZStd::fixed_vector<{{ Property.attrib['Type'] }}, {{ Property.attrib['Count'] }}> m_{{ LowerFirst(Property.attrib['Name']) }}; {% endif %} {% elif Property.attrib['IsRewindable']|booleanTrue %} -Multiplayer::RewindableObject<{{ Property.attrib['Type'] }}, Multiplayer::RewindHistorySize> m_{{ LowerFirst(Property.attrib['Name']) }} = { {{ Property.attrib['Init'] }} }; +Multiplayer::RewindableObject<{{ Property.attrib['Type'] }}, Multiplayer::RewindHistorySize> m_{{ LowerFirst(Property.attrib['Name']) }} { {{ Property.attrib['Init'] }} }; {% else %} {{ Property.attrib['Type'] }} m_{{ LowerFirst(Property.attrib['Name']) }} = {{ Property.attrib['Init'] }}; {% endif %} From 1e10ee61bac2ac56797ebbdd4d1b950689a5e5fc Mon Sep 17 00:00:00 2001 From: Chris Burel Date: Fri, 23 Jul 2021 10:13:53 -0700 Subject: [PATCH 4/8] [MeshOptimizer] Return the correct size from the position map when there are blendshapes (#2391) The previous version returned the incorrect size from the position map when blendshapes are present in the model. When there are blendshapes, the vertex welding is disabled, and nothing is inserted into the position map. However, the position map's size was being used to dictate how many elements to create in the skinning info. The skinning info tries to compensate for an incorrect max vertex index by resizing its underlying vector when adding an influence, but that was using the index as the new size of the vector, so it was off by one. This fixes both errors. Signed-off-by: Chris Burel --- .../MeshOptimizer/MeshBuilderSkinningInfo.h | 2 +- .../MeshOptimizer/MeshOptimizerComponent.cpp | 32 +++++++++++++++---- 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/Gems/SceneProcessing/Code/Source/Generation/Components/MeshOptimizer/MeshBuilderSkinningInfo.h b/Gems/SceneProcessing/Code/Source/Generation/Components/MeshOptimizer/MeshBuilderSkinningInfo.h index f5b716876e..dd3b715a21 100644 --- a/Gems/SceneProcessing/Code/Source/Generation/Components/MeshOptimizer/MeshBuilderSkinningInfo.h +++ b/Gems/SceneProcessing/Code/Source/Generation/Components/MeshOptimizer/MeshBuilderSkinningInfo.h @@ -35,7 +35,7 @@ namespace AZ::MeshBuilder MeshBuilderSkinningInfo(size_t numOrgVertices); - void AddInfluence(size_t orgVtxNr, const Influence& influence) { mInfluences.resize(AZStd::max(mInfluences.size(), orgVtxNr)); mInfluences.at(orgVtxNr).emplace_back(influence); } + void AddInfluence(size_t orgVtxNr, const Influence& influence) { mInfluences.resize(AZStd::max(mInfluences.size(), orgVtxNr + 1)); mInfluences.at(orgVtxNr).emplace_back(influence); } void RemoveInfluence(size_t orgVtxNr, size_t influenceNr) { mInfluences.at(orgVtxNr).erase(mInfluences.at(orgVtxNr).begin() + influenceNr); } const Influence& GetInfluence(size_t orgVtxNr, size_t influenceNr) const { return mInfluences.at(orgVtxNr).at(influenceNr); } size_t GetNumInfluences(size_t orgVtxNr) const { return mInfluences.at(orgVtxNr).size(); } diff --git a/Gems/SceneProcessing/Code/Source/Generation/Components/MeshOptimizer/MeshOptimizerComponent.cpp b/Gems/SceneProcessing/Code/Source/Generation/Components/MeshOptimizer/MeshOptimizerComponent.cpp index 35d6db6d03..6bc47476f8 100644 --- a/Gems/SceneProcessing/Code/Source/Generation/Components/MeshOptimizer/MeshOptimizerComponent.cpp +++ b/Gems/SceneProcessing/Code/Source/Generation/Components/MeshOptimizer/MeshOptimizerComponent.cpp @@ -105,7 +105,6 @@ namespace AZ::SceneGenerationComponents // Vector3 as a key into a unordered_map. template class Vector3Map - : private AZStd::unordered_map { public: Vector3Map(const MeshDataType* meshData, bool hasBlendShapes, float positionTolerance) @@ -116,9 +115,6 @@ namespace AZ::SceneGenerationComponents { } - using AZStd::unordered_map::reserve; - using AZStd::unordered_map::size; - AZ::u32 operator[](const AZ::u32 vertexIndex) { if (m_hasBlendShapes) @@ -130,7 +126,7 @@ namespace AZ::SceneGenerationComponents return m_meshData->GetUsedPointIndexForControlPoint(m_meshData->GetControlPointIndex(vertexIndex)); } - const auto& [iter, didInsert] = try_emplace(GetPositionForIndex(vertexIndex), m_currentOriginalVertexIndex); + const auto& [iter, didInsert] = m_map.try_emplace(GetPositionForIndex(vertexIndex), m_currentOriginalVertexIndex); if (didInsert) { ++m_currentOriginalVertexIndex; @@ -149,11 +145,32 @@ namespace AZ::SceneGenerationComponents return m_meshData->GetUsedPointIndexForControlPoint(m_meshData->GetControlPointIndex(vertexIndex)); } - auto iter = find(GetPositionForIndex(vertexIndex)); - AZSTD_CONTAINER_ASSERT(iter != end(), "Element with key is not present"); + auto iter = m_map.find(GetPositionForIndex(vertexIndex)); + AZSTD_CONTAINER_ASSERT(iter != m_map.end(), "Element with key is not present"); return iter->second; } + [[nodiscard]] size_t size() const + { + if (m_hasBlendShapes) + { + // Since blend shapes are present, the vertex welding is disabled, and the map will always be empty. + // Use the underlying mesh's vertex count instead. + return m_meshData->GetUsedControlPointCount(); + } + return m_map.size(); + } + + void reserve(size_t count) + { + if (m_hasBlendShapes) + { + // Since blend shapes are present, the vertex welding is disabled, and the map will always be empty. + return; + } + m_map.reserve(count); + } + private: AZ::Vector3 GetPositionForIndex(const AZ::u32 vertexIndex) const @@ -167,6 +184,7 @@ namespace AZ::SceneGenerationComponents ) * m_positionTolerance; } + AZStd::unordered_map m_map; const MeshDataType* m_meshData; bool m_hasBlendShapes; float m_positionTolerance; From 5b8b51924a49e125f379bd7b84938407c6e8dbbd Mon Sep 17 00:00:00 2001 From: Junbo Liang <68558268+junbo75@users.noreply.github.com> Date: Fri, 23 Jul 2021 10:29:03 -0700 Subject: [PATCH 5/8] [AWS][AppSec][Medium] Bucket logging not enabled in sample template (#2243) Add S3 bucket logging not enabled in sample template --- Gems/AWSCore/cdk/app.py | 13 ++- Gems/AWSCore/cdk/core/aws_core.py | 4 + Gems/AWSCore/cdk/core/core_stack.py | 26 ++++-- Gems/AWSCore/cdk/core_stack_properties.py | 25 ------ .../cdk/example/example_resources_stack.py | 90 ++++++++++++++++--- .../cdk/aws_metrics/aws_metrics_construct.py | 4 +- .../cdk/aws_metrics/aws_metrics_stack.py | 4 +- .../cdk/aws_metrics/data_lake_integration.py | 16 +++- 8 files changed, 126 insertions(+), 56 deletions(-) delete mode 100755 Gems/AWSCore/cdk/core_stack_properties.py diff --git a/Gems/AWSCore/cdk/app.py b/Gems/AWSCore/cdk/app.py index 17b7241227..ea038f8a51 100755 --- a/Gems/AWSCore/cdk/app.py +++ b/Gems/AWSCore/cdk/app.py @@ -37,7 +37,7 @@ env = core.Environment(account=ACCOUNT, region=REGION) app = core.App() -core = AWSCore( +core_construct = AWSCore( app, id_=f'{PROJECT_FEATURE_NAME}-Construct', project_name=PROJECT_NAME, @@ -46,20 +46,19 @@ core = AWSCore( ) # Below is the Core example stack which is provided for working with AWSCore ScriptCanvas examples. -# It also provided as an example how to reference properties across stacks in the same CDK applications -# Note: This will make the consuming stack a dependent stack on core -# CDK will deploy the dependent stack first and then the core stack +# It also provided as an example how to reference resources across stacks via stack outputs. # See https://docs.aws.amazon.com/cdk/latest/guide/resources.html#resource_stack -core_properties = core.properties -example = ExampleResources( +example_stack = ExampleResources( app, id_=f'{PROJECT_FEATURE_NAME}-Example-{env.region}', - props_=core_properties, project_name=f'{PROJECT_NAME}', feature_name=FEATURE_NAME, tags={Constants.O3DE_PROJECT_TAG_NAME: PROJECT_NAME, Constants.O3DE_FEATURE_TAG_NAME: FEATURE_NAME}, env=env ) +# +# Add the common stack as a dependency of the feature stack +example_stack.add_dependency(core_construct.common_stack) app.synth() diff --git a/Gems/AWSCore/cdk/core/aws_core.py b/Gems/AWSCore/cdk/core/aws_core.py index 49b5449443..53f1d0693b 100755 --- a/Gems/AWSCore/cdk/core/aws_core.py +++ b/Gems/AWSCore/cdk/core/aws_core.py @@ -42,3 +42,7 @@ class AWSCore(core.Construct): @property def properties(self): return self._feature_stack.properties + + @property + def common_stack(self): + return self._feature_stack diff --git a/Gems/AWSCore/cdk/core/core_stack.py b/Gems/AWSCore/cdk/core/core_stack.py index 24efca774f..fc1b4cf8d8 100755 --- a/Gems/AWSCore/cdk/core/core_stack.py +++ b/Gems/AWSCore/cdk/core/core_stack.py @@ -8,11 +8,11 @@ SPDX-License-Identifier: Apache-2.0 OR MIT from aws_cdk import ( core, aws_iam as iam, + aws_s3 as s3, aws_resourcegroups as resource_groups, ) from constants import Constants -from core_stack_properties import CoreStackProperties class CoreStack(core.Stack): @@ -60,6 +60,17 @@ class CoreStack(core.Stack): type='TAG_FILTERS_1_0') ) + # Create an S3 bucket for Amazon S3 server access logging + # See https://docs.aws.amazon.com/AmazonS3/latest/dev/security-best-practices.html + self._server_access_logs_bucket = s3.Bucket( + self, + f'{self._project_name}-{self._feature_name}-Access-Log-Bucket', + block_public_access=s3.BlockPublicAccess.BLOCK_ALL, + encryption=s3.BucketEncryption.S3_MANAGED, + access_control=s3.BucketAccessControl.LOG_DELIVERY_WRITE + ) + self._server_access_logs_bucket.grant_read(self._admin_group) + # Define exports # Export resource group self._resource_group_output = core.CfnOutput( @@ -83,9 +94,10 @@ class CoreStack(core.Stack): export_name=f"{self._project_name}:AdminGroup", value=self._admin_group.group_arn) - @property - def properties(self) -> CoreStackProperties: - _props = CoreStackProperties() - _props.user_group = self._user_group - _props.admin_group = self._admin_group - return _props + # Export access log bucket name + self._server_access_logs_bucket_output = core.CfnOutput( + self, + id=f'ServerAccessLogsBucketOutput', + description='Name of the S3 bucket for storing server access logs generated by the sample CDK application(s)', + export_name=f"{self._project_name}:ServerAccessLogsBucket", + value=self._server_access_logs_bucket.bucket_name) diff --git a/Gems/AWSCore/cdk/core_stack_properties.py b/Gems/AWSCore/cdk/core_stack_properties.py deleted file mode 100755 index 60ec697d53..0000000000 --- a/Gems/AWSCore/cdk/core_stack_properties.py +++ /dev/null @@ -1,25 +0,0 @@ -""" -Copyright (c) Contributors to the Open 3D Engine Project. -For complete copyright and license terms please see the LICENSE at the root of this distribution. - -SPDX-License-Identifier: Apache-2.0 OR MIT -""" - -from aws_cdk import ( - core, - aws_iam as iam -) - - -class CoreStackProperties(core.StackProps): - """ - Support for cross stack references in the application. - - Define any properties from the CoreStack other stacks in this application - may need to consume. - """ - # Common IAM group for users - user_group: iam.Group - - # Common IAM group for Admin users - admin_group: iam.Group diff --git a/Gems/AWSCore/cdk/example/example_resources_stack.py b/Gems/AWSCore/cdk/example/example_resources_stack.py index 7a63e1a727..23bc78d8fc 100755 --- a/Gems/AWSCore/cdk/example/example_resources_stack.py +++ b/Gems/AWSCore/cdk/example/example_resources_stack.py @@ -8,13 +8,13 @@ import os from aws_cdk import ( aws_lambda as lambda_, + aws_iam as iam, aws_s3 as s3, aws_s3_deployment as s3_deployment, aws_dynamodb as dynamo, core ) -from core_stack_properties import CoreStackProperties from .auth import AuthPolicy @@ -25,8 +25,7 @@ class ExampleResources(core.Stack): * A python 'echo' lambda * A small dynamodb table with the a primary 'id': str key """ - def __init__(self, scope: core.Construct, id_: str, project_name: str, feature_name: str, - props_: CoreStackProperties, **kwargs) -> None: + def __init__(self, scope: core.Construct, id_: str, project_name: str, feature_name: str, **kwargs) -> None: super().__init__(scope, id_, **kwargs, description=f'Contains resources for the AWSCore examples as part of the ' f'{project_name} project') @@ -42,17 +41,74 @@ class ExampleResources(core.Stack): self.__create_outputs() # Finally grant cross stack references - self.__grant_access(props=props_) + self.__grant_access() - def __grant_access(self, props: CoreStackProperties): - self._s3_bucket.grant_read(props.user_group) - self._s3_bucket.grant_read(props.admin_group) + def __grant_access(self): + user_group = iam.Group.from_group_arn( + self, + f'{self._project_name}-{self._feature_name}-ImportedUserGroup', + core.Fn.import_value(f'{self._project_name}:UserGroup') + ) + admin_group = iam.Group.from_group_arn( + self, + f'{self._project_name}-{self._feature_name}-ImportedAdminGroup', + core.Fn.import_value(f'{self._project_name}:AdminGroup') + ) - self._lambda.grant_invoke(props.user_group) - self._lambda.grant_invoke(props.admin_group) + # Provide the admin and user groups permissions to read the example S3 bucket. + # Cannot use the grant_read method defined by the Bucket structure since the method tries to add to + # the resource-based policy but the imported IAM groups (which are tokens from Fn.ImportValue) are + # not valid principals in S3 bucket policies. + # Check https://aws.amazon.com/premiumsupport/knowledge-center/s3-invalid-principal-in-policy-error/ + user_group.add_to_principal_policy( + iam.PolicyStatement( + actions=[ + "s3:GetBucket*", + "s3:GetObject*", + "s3:List*" + ], + effect=iam.Effect.ALLOW, + resources=[self._s3_bucket.bucket_arn, f'{self._s3_bucket.bucket_arn}/*'] + ) + ) + admin_group.add_to_principal_policy( + iam.PolicyStatement( + actions=[ + "s3:GetBucket*", + "s3:GetObject*", + "s3:List*" + ], + effect=iam.Effect.ALLOW, + resources=[self._s3_bucket.bucket_arn, f'{self._s3_bucket.bucket_arn}/*'] + ) + ) - self._table.grant_read_data(props.user_group) - self._table.grant_read_data(props.admin_group) + # Provide the admin and user groups permissions to invoke the example Lambda function. + # Cannot use the grant_invoke method defined by the Function structure since the method tries to add to + # the resource-based policy but the imported IAM groups (which are tokens from Fn.ImportValue) are + # not valid principals in Lambda function policies. + user_group.add_to_principal_policy( + iam.PolicyStatement( + actions=[ + "lambda:InvokeFunction" + ], + effect=iam.Effect.ALLOW, + resources=[self._lambda.function_arn] + ) + ) + admin_group.add_to_principal_policy( + iam.PolicyStatement( + actions=[ + "lambda:InvokeFunction" + ], + effect=iam.Effect.ALLOW, + resources=[self._lambda.function_arn] + ) + ) + + # Provide the admin and user groups permissions to read from the DynamoDB table. + self._table.grant_read_data(user_group) + self._table.grant_read_data(admin_group) def __create_s3_bucket(self) -> s3.Bucket: # Create a sample S3 bucket following S3 best practices @@ -60,11 +116,21 @@ class ExampleResources(core.Stack): # 1. Block all public access to the bucket # 2. Use SSE-S3 encryption. Explore encryption at rest options via # https://docs.aws.amazon.com/AmazonS3/latest/userguide/serv-side-encryption.html + # 3. Enable Amazon S3 server access logging + # https://docs.aws.amazon.com/AmazonS3/latest/userguide/ServerLogs.html + server_access_logs_bucket = s3.Bucket.from_bucket_name( + self, + f'{self._project_name}-{self._feature_name}-ImportedAccessLogsBucket', + core.Fn.import_value(f"{self._project_name}:ServerAccessLogsBucket") + ) + example_bucket = s3.Bucket( self, f'{self._project_name}-{self._feature_name}-Example-S3bucket', block_public_access=s3.BlockPublicAccess.BLOCK_ALL, - encryption=s3.BucketEncryption.S3_MANAGED + encryption=s3.BucketEncryption.S3_MANAGED, + server_access_logs_bucket=server_access_logs_bucket, + server_access_logs_prefix=f'{self._project_name}-{self._feature_name}-{self.region}-AccessLogs' ) s3_deployment.BucketDeployment( diff --git a/Gems/AWSMetrics/cdk/aws_metrics/aws_metrics_construct.py b/Gems/AWSMetrics/cdk/aws_metrics/aws_metrics_construct.py index c78fe06a82..d107c20b22 100755 --- a/Gems/AWSMetrics/cdk/aws_metrics/aws_metrics_construct.py +++ b/Gems/AWSMetrics/cdk/aws_metrics/aws_metrics_construct.py @@ -28,14 +28,14 @@ class AWSMetrics(core.Construct): # Check context variables to get enabled optional features optional_features = { - 'batch_processing': self.node.try_get_context("batch_processing") == 'true' + 'batch_processing': self.node.try_get_context("batch_processing") == 'true', + 'server_access_logs_bucket': self.node.try_get_context("server_access_logs_bucket") } # Deploy AWS Metrics Stack self._feature_stack = AWSMetricsStack( scope, stack_name, - stack_name=stack_name, application_name=application_name, description=f'Contains resources for the AWS Metrics Gem Feature stack as part of the {project_name} project', optional_features=optional_features, diff --git a/Gems/AWSMetrics/cdk/aws_metrics/aws_metrics_stack.py b/Gems/AWSMetrics/cdk/aws_metrics/aws_metrics_stack.py index aaea9b27a6..337a14a301 100755 --- a/Gems/AWSMetrics/cdk/aws_metrics/aws_metrics_stack.py +++ b/Gems/AWSMetrics/cdk/aws_metrics/aws_metrics_stack.py @@ -43,9 +43,11 @@ class AWSMetricsStack(core.Stack): ) batch_processing_enabled = optional_features.get('batch_processing', False) + server_access_logs_bucket = optional_features.get('server_access_logs_bucket') self._data_lake_integration = DataLakeIntegration( self, - application_name=application_name + application_name=application_name, + server_access_logs_bucket=server_access_logs_bucket ) if batch_processing_enabled else None self._batch_processing = BatchProcessing( diff --git a/Gems/AWSMetrics/cdk/aws_metrics/data_lake_integration.py b/Gems/AWSMetrics/cdk/aws_metrics/data_lake_integration.py index 3e0527b8e3..dff165ce56 100755 --- a/Gems/AWSMetrics/cdk/aws_metrics/data_lake_integration.py +++ b/Gems/AWSMetrics/cdk/aws_metrics/data_lake_integration.py @@ -19,9 +19,11 @@ class DataLakeIntegration: """ Create the AWS resources including the S3 bucket, Glue database, table and crawler for data lake integration """ - def __init__(self, stack: core.Construct, application_name: str) -> None: + def __init__(self, stack: core.Construct, application_name: str, + server_access_logs_bucket: str = None) -> None: self._stack = stack self._application_name = application_name + self._server_access_logs_bucket = server_access_logs_bucket self._create_analytics_bucket() self._create_events_database() @@ -34,6 +36,14 @@ class DataLakeIntegration: The bucket uses server-side encryption with a CMK managed by S3: https://docs.aws.amazon.com/AmazonS3/latest/userguide/UsingKMSEncryption.html """ + # Enable server access logging if the server access logs bucket is provided following S3 best practices. + # See https://docs.aws.amazon.com/AmazonS3/latest/dev/security-best-practices.html + server_access_logs_bucket = s3.Bucket.from_bucket_name( + self._stack, + f'{self._stack.stack_name}-ImportedAccessLogsBucket', + self._server_access_logs_bucket, + ) if self._server_access_logs_bucket else None + # Bucket name cannot contain uppercase characters # Do not specify the bucket name here since bucket name is required to be unique globally. If we set # a specific name here, only one customer can deploy the bucket successfully. @@ -46,7 +56,9 @@ class DataLakeIntegration: block_public_policy=True, ignore_public_acls=True, restrict_public_buckets=True - ) + ), + server_access_logs_bucket=server_access_logs_bucket, + server_access_logs_prefix=f'{self._stack.stack_name}-AccessLogs' if server_access_logs_bucket else None ) # For Amazon S3 buckets, you must delete all objects in the bucket for deletion to succeed. From c8b55af401182af6a47e0a10d4676b28934f205e Mon Sep 17 00:00:00 2001 From: Junbo Liang <68558268+junbo75@users.noreply.github.com> Date: Fri, 23 Jul 2021 11:33:11 -0700 Subject: [PATCH 6/8] [AWS][Metrics] Resource names are too long leading to CDK deployment failures (#2291) Sanitize resource names to prevent AWS resource failures. --- Gems/AWSMetrics/cdk/aws_metrics/auth.py | 6 ++- .../cdk/aws_metrics/aws_metrics_construct.py | 4 +- .../cdk/aws_metrics/aws_utils/__init__.py | 6 +++ .../aws_utils/resource_name_sanitizer.py | 45 +++++++++++++++++++ .../cdk/aws_metrics/batch_analytics.py | 13 ++++-- .../cdk/aws_metrics/batch_processing.py | 17 ++++--- Gems/AWSMetrics/cdk/aws_metrics/dashboard.py | 4 +- .../cdk/aws_metrics/data_ingestion.py | 8 ++-- .../cdk/aws_metrics/data_lake_integration.py | 8 +++- .../aws_metrics/real_time_data_processing.py | 13 ++++-- 10 files changed, 102 insertions(+), 22 deletions(-) create mode 100644 Gems/AWSMetrics/cdk/aws_metrics/aws_utils/__init__.py create mode 100644 Gems/AWSMetrics/cdk/aws_metrics/aws_utils/resource_name_sanitizer.py diff --git a/Gems/AWSMetrics/cdk/aws_metrics/auth.py b/Gems/AWSMetrics/cdk/aws_metrics/auth.py index 4b445c0c6d..95fbecd915 100755 --- a/Gems/AWSMetrics/cdk/aws_metrics/auth.py +++ b/Gems/AWSMetrics/cdk/aws_metrics/auth.py @@ -13,6 +13,7 @@ from aws_cdk import ( from .aws_metrics_stack import AWSMetricsStack from aws_metrics.policy_statements_builder.user_policy_statements_builder import UserPolicyStatementsBuilder from aws_metrics.policy_statements_builder.admin_policy_statements_builder import AdminPolicyStatementsBuilder +from .aws_utils import resource_name_sanitizer class AuthPolicy: @@ -58,12 +59,13 @@ class AuthPolicy: policy = iam.ManagedPolicy( self._stack, policy_id, - managed_policy_name=f'{self._stack.stack_name}-{role_name}Policy', + managed_policy_name=resource_name_sanitizer.sanitize_resource_name( + f'{self._stack.stack_name}-{role_name}Policy', 'iam_managed_policy'), statements=policy_statements) policy_output = core.CfnOutput( self._stack, id=f'{policy_id}Output', description=f'{role_name} policy arn to call service', - export_name=f"{self._application_name}:{policy_id}", + export_name=f'{self._application_name}:{policy_id}', value=policy.managed_policy_arn) diff --git a/Gems/AWSMetrics/cdk/aws_metrics/aws_metrics_construct.py b/Gems/AWSMetrics/cdk/aws_metrics/aws_metrics_construct.py index d107c20b22..70d85d1586 100755 --- a/Gems/AWSMetrics/cdk/aws_metrics/aws_metrics_construct.py +++ b/Gems/AWSMetrics/cdk/aws_metrics/aws_metrics_construct.py @@ -8,6 +8,7 @@ SPDX-License-Identifier: Apache-2.0 OR MIT from aws_cdk import core from .aws_metrics_stack import AWSMetricsStack from .auth import AuthPolicy +from .aws_utils import resource_name_sanitizer class AWSMetrics(core.Construct): @@ -23,7 +24,8 @@ class AWSMetrics(core.Construct): env: core.Environment) -> None: super().__init__(scope, id_) # Set-up any stack name(s) to be unique in account - stack_name = f'{project_name}-{feature_name}-{env.region}' + stack_name = resource_name_sanitizer.sanitize_resource_name( + f'{project_name}-{feature_name}-{env.region}', 'cloudformation_stack') application_name = f'{project_name}-{feature_name}' # Check context variables to get enabled optional features diff --git a/Gems/AWSMetrics/cdk/aws_metrics/aws_utils/__init__.py b/Gems/AWSMetrics/cdk/aws_metrics/aws_utils/__init__.py new file mode 100644 index 0000000000..50cbb262dd --- /dev/null +++ b/Gems/AWSMetrics/cdk/aws_metrics/aws_utils/__init__.py @@ -0,0 +1,6 @@ +""" +Copyright (c) Contributors to the Open 3D Engine Project. +For complete copyright and license terms please see the LICENSE at the root of this distribution. + +SPDX-License-Identifier: Apache-2.0 OR MIT +""" diff --git a/Gems/AWSMetrics/cdk/aws_metrics/aws_utils/resource_name_sanitizer.py b/Gems/AWSMetrics/cdk/aws_metrics/aws_utils/resource_name_sanitizer.py new file mode 100644 index 0000000000..45d7bb34cd --- /dev/null +++ b/Gems/AWSMetrics/cdk/aws_metrics/aws_utils/resource_name_sanitizer.py @@ -0,0 +1,45 @@ +""" +Copyright (c) Contributors to the Open 3D Engine Project. +For complete copyright and license terms please see the LICENSE at the root of this distribution. + +SPDX-License-Identifier: Apache-2.0 OR MIT +""" + +import hashlib + +MAX_RESOURCE_NAME_LENGTH_MAPPING = { + 'athena_work_group': 128, + 'athena_named_query': 128, + 'cloudformation_stack': 128, + 'cloudwatch_dashboard': 255, + 'cloudwatch_log_group': 512, + 'firehose_delivery_stream': 64, + 'iam_managed_policy': 144, + 'iam_role': 64, + 'kinesis_application': 128, + 'kinesis_stream': 128, + 'lambda_function': 64, + 's3_bucket': 63 +} + + +def sanitize_resource_name(resource_name: str, resource_type: str) -> str: + """ + Truncate the resource name if its length exceeds the limit. + This is the best effort for sanitizing resource names based on the AWS documents since each AWS service + has its unique restrictions. Customers can extend this function for validation or sanitization. + + :param resource_name: Original name of the resource. + :param resource_type: Type of the resource. + :return Sanitized resource name that can be deployed with AWS. + """ + result = resource_name + if not MAX_RESOURCE_NAME_LENGTH_MAPPING.get(resource_type): + return result + + if len(resource_name) > MAX_RESOURCE_NAME_LENGTH_MAPPING[resource_type]: + # PYTHONHASHSEED is set to "random" by default in Python 3.3 and up. Cannot use + # the built-in hash function here since it will give a different return value in each session + digest = "-%x" % (int(hashlib.md5(resource_name.encode('ascii', 'ignore')).hexdigest(), 16) & 0xffffffff) + result = resource_name[:MAX_RESOURCE_NAME_LENGTH_MAPPING[resource_type] - len(digest)] + digest + return result diff --git a/Gems/AWSMetrics/cdk/aws_metrics/batch_analytics.py b/Gems/AWSMetrics/cdk/aws_metrics/batch_analytics.py index 1b6bda7345..5be6562982 100755 --- a/Gems/AWSMetrics/cdk/aws_metrics/batch_analytics.py +++ b/Gems/AWSMetrics/cdk/aws_metrics/batch_analytics.py @@ -11,6 +11,7 @@ from aws_cdk import ( ) from . import aws_metrics_constants +from .aws_utils import resource_name_sanitizer class BatchAnalytics: @@ -37,7 +38,8 @@ class BatchAnalytics: self._athena_work_group = athena.CfnWorkGroup( self._stack, id='AthenaWorkGroup', - name=f'{self._stack.stack_name}-AthenaWorkGroup', + name=resource_name_sanitizer.sanitize_resource_name( + f'{self._stack.stack_name}-AthenaWorkGroup', 'athena_work_group'), recursive_delete_option=True, state='ENABLED', work_group_configuration=athena.CfnWorkGroup.WorkGroupConfigurationProperty( @@ -65,7 +67,8 @@ class BatchAnalytics: athena.CfnNamedQuery( self._stack, id='NamedQuery-CreatePartitionedEventsJson', - name=f'{self._stack.stack_name}-NamedQuery-CreatePartitionedEventsJson', + name=resource_name_sanitizer.sanitize_resource_name( + f'{self._stack.stack_name}-NamedQuery-CreatePartitionedEventsJson', 'athena_named_query'), database=self._events_database_name, query_string="CREATE TABLE events_json " "WITH (format='JSON',partitioned_by=ARRAY['application_id']) " @@ -78,7 +81,8 @@ class BatchAnalytics: athena.CfnNamedQuery( self._stack, id='NamedQuery-TotalEventsLastMonth', - name=f'{self._stack.stack_name}-NamedQuery-TotalEventsLastMonth', + name=resource_name_sanitizer.sanitize_resource_name( + f'{self._stack.stack_name}-NamedQuery-TotalEventsLastMonth', 'athena_named_query'), database=self._events_database_name, query_string="WITH detail AS " "(SELECT date_trunc('month', date(date_parse(CONCAT(year, '-', month, '-', day), '%Y-%m-%d'))) as event_month, * " @@ -93,7 +97,8 @@ class BatchAnalytics: athena.CfnNamedQuery( self._stack, id='NamedQuery-NewUsersLastMonth', - name=f'{self._stack.stack_name}-NamedQuery-NewUsersLastMonth', + name=resource_name_sanitizer.sanitize_resource_name( + f'{self._stack.stack_name}-NamedQuery-NewUsersLastMonth', 'athena_named_query'), database=self._events_database_name, query_string="WITH detail AS (" "SELECT date_trunc('month', date(date_parse(CONCAT(year, '-', month, '-', day), '%Y-%m-%d'))) as event_month, * " diff --git a/Gems/AWSMetrics/cdk/aws_metrics/batch_processing.py b/Gems/AWSMetrics/cdk/aws_metrics/batch_processing.py index 4b40d4393c..4dbb3b2120 100755 --- a/Gems/AWSMetrics/cdk/aws_metrics/batch_processing.py +++ b/Gems/AWSMetrics/cdk/aws_metrics/batch_processing.py @@ -16,6 +16,7 @@ from aws_cdk import ( import os from . import aws_metrics_constants +from .aws_utils import resource_name_sanitizer class BatchProcessing: @@ -42,7 +43,8 @@ class BatchProcessing: """ Generate the events processing lambda to filter the invalid metrics events. """ - events_processing_lambda_name = f'{self._stack.stack_name}-EventsProcessingLambda' + events_processing_lambda_name = resource_name_sanitizer.sanitize_resource_name( + f'{self._stack.stack_name}-EventsProcessingLambda', 'lambda_function') self._create_events_processing_lambda_role(events_processing_lambda_name) self._events_processing_lambda = lambda_.Function( @@ -89,7 +91,8 @@ class BatchProcessing: self._events_processing_lambda_role = iam.Role( self._stack, id='EventsProcessingLambdaRole', - role_name=f'{self._stack.stack_name}-EventsProcessingLambdaRole', + role_name=resource_name_sanitizer.sanitize_resource_name( + f'{self._stack.stack_name}-EventsProcessingLambdaRole', 'iam_role'), assumed_by=iam.ServicePrincipal( service='lambda.amazonaws.com' ), @@ -107,8 +110,10 @@ class BatchProcessing: self._events_firehose_delivery_stream = kinesisfirehose.CfnDeliveryStream( self._stack, - id=f'{self._stack.stack_name}-EventsFirehoseDeliveryStream', + id=f'EventsFirehoseDeliveryStream', delivery_stream_type='KinesisStreamAsSource', + delivery_stream_name=resource_name_sanitizer.sanitize_resource_name( + f'{self._stack.stack_name}-EventsFirehoseDeliveryStream', 'firehose_delivery_stream'), kinesis_stream_source_configuration=kinesisfirehose.CfnDeliveryStream.KinesisStreamSourceConfigurationProperty( kinesis_stream_arn=self._input_stream_arn, role_arn=self._firehose_delivery_stream_role.role_arn @@ -192,7 +197,8 @@ class BatchProcessing: self._firehose_delivery_stream_log_group = logs.LogGroup( self._stack, id='FirehoseLogGroup', - log_group_name=f'{self._stack.stack_name}-FirehoseLogGroup', + log_group_name=resource_name_sanitizer.sanitize_resource_name( + f'{self._stack.stack_name}-FirehoseLogGroup', 'cloudwatch_log_group'), removal_policy=core.RemovalPolicy.DESTROY, retention=logs.RetentionDays.ONE_MONTH ) @@ -299,7 +305,8 @@ class BatchProcessing: self._firehose_delivery_stream_role = iam.Role( self._stack, id='GameEventsFirehoseRole', - role_name=f'{self._stack.stack_name}-GameEventsFirehoseRole', + role_name=resource_name_sanitizer.sanitize_resource_name( + f'{self._stack.stack_name}-GameEventsFirehoseRole', 'iam_role'), assumed_by=iam.ServicePrincipal( service='firehose.amazonaws.com' ), diff --git a/Gems/AWSMetrics/cdk/aws_metrics/dashboard.py b/Gems/AWSMetrics/cdk/aws_metrics/dashboard.py index f86b374abf..32ff0d9c84 100755 --- a/Gems/AWSMetrics/cdk/aws_metrics/dashboard.py +++ b/Gems/AWSMetrics/cdk/aws_metrics/dashboard.py @@ -12,6 +12,7 @@ from aws_cdk import ( from . import aws_metrics_constants from .layout_widget_construct import LayoutWidget +from .aws_utils import resource_name_sanitizer class Dashboard: @@ -28,7 +29,8 @@ class Dashboard: events_processing_lambda_name: str = '', ) -> None: - self._dashboard_name = f"{stack.stack_name}-Dashboard" + self._dashboard_name = resource_name_sanitizer.sanitize_resource_name( + f'{stack.stack_name}-Dashboard', 'cloudwatch_dashboard') self._dashboard = cloudwatch.Dashboard( stack, id="DashBoard", diff --git a/Gems/AWSMetrics/cdk/aws_metrics/data_ingestion.py b/Gems/AWSMetrics/cdk/aws_metrics/data_ingestion.py index 6f31818bf4..a21e629c38 100755 --- a/Gems/AWSMetrics/cdk/aws_metrics/data_ingestion.py +++ b/Gems/AWSMetrics/cdk/aws_metrics/data_ingestion.py @@ -12,10 +12,11 @@ from aws_cdk import ( aws_kinesis as kinesis ) -from . import aws_metrics_constants - import json +from . import aws_metrics_constants +from .aws_utils import resource_name_sanitizer + class DataIngestion: """ @@ -29,7 +30,8 @@ class DataIngestion: self._input_stream = kinesis.Stream( self._stack, id='InputStream', - stream_name=f'{self._stack.stack_name}-InputStream', + stream_name=resource_name_sanitizer.sanitize_resource_name( + f'{self._stack.stack_name}-InputStream', 'kinesis_stream'), shard_count=1 ) diff --git a/Gems/AWSMetrics/cdk/aws_metrics/data_lake_integration.py b/Gems/AWSMetrics/cdk/aws_metrics/data_lake_integration.py index dff165ce56..aaaf03b1fa 100755 --- a/Gems/AWSMetrics/cdk/aws_metrics/data_lake_integration.py +++ b/Gems/AWSMetrics/cdk/aws_metrics/data_lake_integration.py @@ -13,6 +13,7 @@ from aws_cdk import ( ) from . import aws_metrics_constants +from .aws_utils import resource_name_sanitizer class DataLakeIntegration: @@ -49,7 +50,9 @@ class DataLakeIntegration: # a specific name here, only one customer can deploy the bucket successfully. self._analytics_bucket = s3.Bucket( self._stack, - id=f'{self._stack.stack_name}-AnalyticsBucket'.lower(), + id=f'AnalyticsBucket'.lower(), + bucket_name=resource_name_sanitizer.sanitize_resource_name( + f'{self._stack.stack_name}-AnalyticsBucket'.lower(), 's3_bucket'), encryption=s3.BucketEncryption.S3_MANAGED, block_public_access=s3.BlockPublicAccess( block_public_acls=True, @@ -297,7 +300,8 @@ class DataLakeIntegration: self._events_crawler_role = iam.Role( self._stack, id='EventsCrawlerRole', - role_name=f'{self._stack.stack_name}-EventsCrawlerRole', + role_name=resource_name_sanitizer.sanitize_resource_name( + f'{self._stack.stack_name}-EventsCrawlerRole', 'iam_role'), assumed_by=iam.ServicePrincipal( service='glue.amazonaws.com' ), diff --git a/Gems/AWSMetrics/cdk/aws_metrics/real_time_data_processing.py b/Gems/AWSMetrics/cdk/aws_metrics/real_time_data_processing.py index 9809bf0c9c..52fdcdc122 100755 --- a/Gems/AWSMetrics/cdk/aws_metrics/real_time_data_processing.py +++ b/Gems/AWSMetrics/cdk/aws_metrics/real_time_data_processing.py @@ -16,6 +16,7 @@ from aws_cdk import ( import os from . import aws_metrics_constants +from .aws_utils import resource_name_sanitizer class RealTimeDataProcessing: @@ -44,7 +45,8 @@ class RealTimeDataProcessing: self._analytics_application = analytics.CfnApplication( self._stack, 'AnalyticsApplication', - application_name=f'{self._stack.stack_name}-AnalyticsApplication', + application_name=resource_name_sanitizer.sanitize_resource_name( + f'{self._stack.stack_name}-AnalyticsApplication', 'kinesis_application'), inputs=[ analytics.CfnApplication.InputProperty( input_schema=analytics.CfnApplication.InputSchemaProperty( @@ -162,7 +164,8 @@ class RealTimeDataProcessing: kinesis_analytics_role = iam.Role( self._stack, id='AnalyticsApplicationRole', - role_name=f'{self._stack.stack_name}-AnalyticsApplicationRole', + role_name=resource_name_sanitizer.sanitize_resource_name( + f'{self._stack.stack_name}-AnalyticsApplicationRole', 'iam_role'), assumed_by=iam.ServicePrincipal( service='kinesisanalytics.amazonaws.com' ), @@ -178,7 +181,8 @@ class RealTimeDataProcessing: """ Generate the analytics processing lambda to send processed data to CloudWatch for visualization. """ - analytics_processing_function_name = f'{self._stack.stack_name}-AnalyticsProcessingLambdaName' + analytics_processing_function_name = resource_name_sanitizer.sanitize_resource_name( + f'{self._stack.stack_name}-AnalyticsProcessingLambdaName', 'lambda_function') self._analytics_processing_lambda_role = self._create_analytics_processing_lambda_role( analytics_processing_function_name ) @@ -246,7 +250,8 @@ class RealTimeDataProcessing: analytics_processing_lambda_role = iam.Role( self._stack, id='AnalyticsLambdaRole', - role_name=f'{self._stack.stack_name}-AnalyticsLambdaRole', + role_name=resource_name_sanitizer.sanitize_resource_name( + f'{self._stack.stack_name}-AnalyticsLambdaRole', 'iam_role'), assumed_by=iam.ServicePrincipal( service='lambda.amazonaws.com' ), From f3a5e9d9489f7c666c9163847184841a9768de07 Mon Sep 17 00:00:00 2001 From: Esteban Papp <81431996+amznestebanpapp@users.noreply.github.com> Date: Fri, 23 Jul 2021 15:50:27 -0700 Subject: [PATCH 7/8] changing type that was coming from a cry header (#2397) Signed-off-by: Esteban Papp <81431996+amznestebanpapp@users.noreply.github.com> --- .../Code/Source/Platform/iOS/MicrophoneSystemComponent_iOS.mm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gems/Microphone/Code/Source/Platform/iOS/MicrophoneSystemComponent_iOS.mm b/Gems/Microphone/Code/Source/Platform/iOS/MicrophoneSystemComponent_iOS.mm index a603aa0ab4..fe8580723a 100644 --- a/Gems/Microphone/Code/Source/Platform/iOS/MicrophoneSystemComponent_iOS.mm +++ b/Gems/Microphone/Code/Source/Platform/iOS/MicrophoneSystemComponent_iOS.mm @@ -238,7 +238,7 @@ public: void ProcessAudio(AudioBufferList* bufferList) { AudioBuffer sourceBuffer = bufferList->mBuffers[0]; - m_captureData->AddData((int16*)sourceBuffer.mData, sourceBuffer.mDataByteSize / 2, m_config.m_numChannels); + m_captureData->AddData((AZ::s16*)sourceBuffer.mData, sourceBuffer.mDataByteSize / 2, m_config.m_numChannels); } From 8f35def25bd3cf977e6eb06761df9ba6b67a7152 Mon Sep 17 00:00:00 2001 From: Benjamin Jillich <43751992+amzn-jillich@users.noreply.github.com> Date: Sun, 25 Jul 2021 23:39:02 -0700 Subject: [PATCH 8/8] AssImp skeleton import improvements (#2348) * Disabled to skip exporting a node in case there are child bones underneath, this broke motion extraction as we skipped the motion extraction node. * Added several helper methods for getting the local space bind pose transform, finding all bones, getting the first bone for a given node name and a recursive has child bones. * Unified the cloned get all bone methods. Signed-off-by: Benjamin Jillich --- .../Importers/AssImpBoneImporter.cpp | 87 ++++---------- .../Importers/AssImpImporterUtilities.cpp | 112 +++++++++++++++++- .../Importers/AssImpImporterUtilities.h | 15 ++- .../Importers/AssImpTransformImporter.cpp | 88 +------------- 4 files changed, 147 insertions(+), 155 deletions(-) diff --git a/Code/Tools/SceneAPI/SceneBuilder/Importers/AssImpBoneImporter.cpp b/Code/Tools/SceneAPI/SceneBuilder/Importers/AssImpBoneImporter.cpp index 92f84ad8fc..eeddf4a0b3 100644 --- a/Code/Tools/SceneAPI/SceneBuilder/Importers/AssImpBoneImporter.cpp +++ b/Code/Tools/SceneAPI/SceneBuilder/Importers/AssImpBoneImporter.cpp @@ -41,45 +41,6 @@ namespace AZ } } - void MakeBoneMap(const aiScene* scene, AZStd::unordered_map& boneLookup) - { - AZStd::queue queue; - AZStd::unordered_set nodesWithNoMesh; - - queue.push(scene->mRootNode); - - while (!queue.empty()) - { - const aiNode* currentNode = queue.front(); - queue.pop(); - - if (currentNode->mNumMeshes == 0) - { - nodesWithNoMesh.emplace(currentNode->mName.C_Str()); - } - - for (int childIndex = 0; childIndex < currentNode->mNumChildren; ++childIndex) - { - queue.push(currentNode->mChildren[childIndex]); - } - } - - for (unsigned int meshIndex = 0; meshIndex < scene->mNumMeshes; ++meshIndex) - { - const aiMesh* mesh = scene->mMeshes[meshIndex]; - - for (unsigned int boneIndex = 0; boneIndex < mesh->mNumBones; ++boneIndex) - { - const aiBone* bone = mesh->mBones[boneIndex]; - - if (nodesWithNoMesh.contains(bone->mName.C_Str())) - { - boneLookup.emplace(bone->mName.C_Str(), bone); - } - } - } - } - aiMatrix4x4 CalculateWorldTransform(const aiNode* currentNode) { aiMatrix4x4 transform = {}; @@ -106,37 +67,39 @@ namespace AZ return Events::ProcessingResult::Ignored; } - bool isBone = false; - + AZStd::unordered_multimap boneByNameMap; + FindAllBones(scene, boneByNameMap); + + bool isBone = FindFirstBoneByNodeName(currentNode, boneByNameMap); + if (!isBone) { - AZStd::unordered_map boneLookup; - MakeBoneMap(scene, boneLookup); - - isBone = boneLookup.contains(currentNode->mName.C_Str()); - - // If we have an animation, the bones will be listed in there - if (!isBone) + for(unsigned animIndex = 0; animIndex < scene->mNumAnimations; ++animIndex) { - for(unsigned animIndex = 0; animIndex < scene->mNumAnimations; ++animIndex) + aiAnimation* animation = scene->mAnimations[animIndex]; + + for (unsigned channelIndex = 0; channelIndex < animation->mNumChannels; ++channelIndex) { - aiAnimation* animation = scene->mAnimations[animIndex]; + aiNodeAnim* nodeAnim = animation->mChannels[channelIndex]; - for (unsigned channelIndex = 0; channelIndex < animation->mNumChannels; ++channelIndex) - { - aiNodeAnim* nodeAnim = animation->mChannels[channelIndex]; - - if (nodeAnim->mNodeName == currentNode->mName) - { - isBone = true; - break; - } - } - - if (isBone) + if (nodeAnim->mNodeName == currentNode->mName) { + isBone = true; break; } } + + if (isBone) + { + break; + } + } + + // In case any of the children, or children of children is a bone, make sure to not skip this node. + // Don't do this for the scene root itself, else wise all mesh nodes will be exported as bones and pollute the skeleton. + if (currentNode != scene->mRootNode && + RecursiveHasChildBone(currentNode, boneByNameMap)) + { + isBone = true; } } diff --git a/Code/Tools/SceneAPI/SceneBuilder/Importers/AssImpImporterUtilities.cpp b/Code/Tools/SceneAPI/SceneBuilder/Importers/AssImpImporterUtilities.cpp index 15bb65399c..81feff7d69 100644 --- a/Code/Tools/SceneAPI/SceneBuilder/Importers/AssImpImporterUtilities.cpp +++ b/Code/Tools/SceneAPI/SceneBuilder/Importers/AssImpImporterUtilities.cpp @@ -6,12 +6,13 @@ * */ -#include - -#include -#include - #include +#include +#include +#include +#include +#include +#include namespace AZ { @@ -85,6 +86,107 @@ namespace AZ return combinedTransform; } + + void FindAllBones(const aiScene* scene, AZStd::unordered_multimap& outBoneByNameMap) + { + outBoneByNameMap.clear(); + AZStd::queue queue; + AZStd::unordered_set nodesWithNoMesh; + + queue.push(scene->mRootNode); + + while (!queue.empty()) + { + const aiNode* currentNode = queue.front(); + queue.pop(); + + if (currentNode->mNumMeshes == 0) + { + nodesWithNoMesh.emplace(currentNode->mName.C_Str()); + } + + for (int childIndex = 0; childIndex < currentNode->mNumChildren; ++childIndex) + { + queue.push(currentNode->mChildren[childIndex]); + } + } + + for (unsigned meshIndex = 0; meshIndex < scene->mNumMeshes; ++meshIndex) + { + const aiMesh* mesh = scene->mMeshes[meshIndex]; + + for (unsigned boneIndex = 0; boneIndex < mesh->mNumBones; ++boneIndex) + { + const aiBone* bone = mesh->mBones[boneIndex]; + + if (nodesWithNoMesh.contains(bone->mName.C_Str())) + { + outBoneByNameMap.emplace(bone->mName.C_Str(), bone); + } + } + } + } + + DataTypes::MatrixType GetLocalSpaceBindPoseTransform(const aiScene* scene, const aiNode* node) + { + AZStd::unordered_multimap boneByNameMap; + FindAllBones(scene, boneByNameMap); + + const aiBone* bone = FindFirstBoneByNodeName(node, boneByNameMap); + if (bone) + { + const DataTypes::MatrixType inverseOffsetMatrix = AssImpSDKWrapper::AssImpTypeConverter::ToTransform(bone->mOffsetMatrix).GetInverseFull(); + + const aiBone* parentBone = FindFirstBoneByNodeName(node->mParent, boneByNameMap); + if (parentBone) + { + const DataTypes::MatrixType parentBoneOffsetMatrix = AssImpSDKWrapper::AssImpTypeConverter::ToTransform(parentBone->mOffsetMatrix); + return parentBoneOffsetMatrix * inverseOffsetMatrix; + } + else + { + return inverseOffsetMatrix; + } + } + + return AssImpSDKWrapper::AssImpTypeConverter::ToTransform(GetConcatenatedLocalTransform(node)); + } + + const aiBone* FindFirstBoneByNodeName(const aiNode* node, AZStd::unordered_multimap& boneByNameMap) + { + if (!node) + { + return nullptr; + } + + auto boneIterator = boneByNameMap.find(node->mName.C_Str()); + if (boneIterator != boneByNameMap.end()) + { + return boneIterator->second; + } + + return nullptr; + } + + bool RecursiveHasChildBone(const aiNode* node, const AZStd::unordered_multimap& boneByNameMap) + { + const bool isBone = boneByNameMap.contains(node->mName.C_Str()); + if (isBone) + { + return true; + } + + for (int childIndex = 0; childIndex < node->mNumChildren; ++childIndex) + { + const aiNode* childNode = node->mChildren[childIndex]; + if (RecursiveHasChildBone(childNode, boneByNameMap)) + { + return true; + } + } + + return false; + } } // namespace SceneBuilder } // namespace SceneAPI } // namespace AZ diff --git a/Code/Tools/SceneAPI/SceneBuilder/Importers/AssImpImporterUtilities.h b/Code/Tools/SceneAPI/SceneBuilder/Importers/AssImpImporterUtilities.h index 5a943f339c..a629fe52d8 100644 --- a/Code/Tools/SceneAPI/SceneBuilder/Importers/AssImpImporterUtilities.h +++ b/Code/Tools/SceneAPI/SceneBuilder/Importers/AssImpImporterUtilities.h @@ -9,13 +9,15 @@ #pragma once #include +#include #include +#include +struct aiBone; struct aiNode; struct aiScene; struct aiString; - namespace AZ::SceneAPI::SceneBuilder { inline constexpr char PivotNodeMarker[] = "_$AssimpFbx$_"; @@ -30,5 +32,16 @@ namespace AZ::SceneAPI::SceneBuilder // Gets the entire, combined local transform for a node taking pivot nodes into account. When pivot nodes are not used, this just returns the node's transform aiMatrix4x4 GetConcatenatedLocalTransform(const aiNode* currentNode); + + DataTypes::MatrixType GetLocalSpaceBindPoseTransform(const aiScene* scene, const aiNode* node); + + // Gather all bones from the scene. (Bone in AssImp corresponds to nodes that influence any of the vertices). + void FindAllBones(const aiScene* scene, AZStd::unordered_multimap& outBoneByNameMap); + + // Find the first bone with the name of the given node. + const aiBone* FindFirstBoneByNodeName(const aiNode* node, AZStd::unordered_multimap& boneByNameMap); + + // Check if the given node or any of its children, or children of children, is a bone by checking if the node name is part of the given map. + bool RecursiveHasChildBone(const aiNode* node, const AZStd::unordered_multimap& boneByNameMap); } // namespace AZ diff --git a/Code/Tools/SceneAPI/SceneBuilder/Importers/AssImpTransformImporter.cpp b/Code/Tools/SceneAPI/SceneBuilder/Importers/AssImpTransformImporter.cpp index eba1063a1e..134c408bae 100644 --- a/Code/Tools/SceneAPI/SceneBuilder/Importers/AssImpTransformImporter.cpp +++ b/Code/Tools/SceneAPI/SceneBuilder/Importers/AssImpTransformImporter.cpp @@ -42,45 +42,6 @@ namespace AZ serializeContext->Class()->Version(1); } } - - void GetAllBones(const aiScene* scene, AZStd::unordered_multimap& boneLookup) - { - AZStd::queue queue; - AZStd::unordered_set nodesWithNoMesh; - - queue.push(scene->mRootNode); - - while (!queue.empty()) - { - const aiNode* currentNode = queue.front(); - queue.pop(); - - if (currentNode->mNumMeshes == 0) - { - nodesWithNoMesh.emplace(currentNode->mName.C_Str()); - } - - for (int childIndex = 0; childIndex < currentNode->mNumChildren; ++childIndex) - { - queue.push(currentNode->mChildren[childIndex]); - } - } - - for (unsigned meshIndex = 0; meshIndex < scene->mNumMeshes; ++meshIndex) - { - const aiMesh* mesh = scene->mMeshes[meshIndex]; - - for (unsigned boneIndex = 0; boneIndex < mesh->mNumBones; ++boneIndex) - { - const aiBone* bone = mesh->mBones[boneIndex]; - - if (nodesWithNoMesh.contains(bone->mName.C_Str())) - { - boneLookup.emplace(bone->mName.C_Str(), bone); - } - } - } - } Events::ProcessingResult AssImpTransformImporter::ImportTransform(AssImpSceneNodeAppendedContext& context) { @@ -93,54 +54,7 @@ namespace AZ return Events::ProcessingResult::Ignored; } - AZStd::unordered_multimap boneLookup; - GetAllBones(scene, boneLookup); - - auto boneIterator = boneLookup.find(currentNode->mName.C_Str()); - const bool isBone = boneIterator != boneLookup.end(); - - DataTypes::MatrixType localTransform; - - if (isBone) - { - AZStd::vector offsets, inverseOffsets; - auto iteratingNode = currentNode; - - while (iteratingNode && boneLookup.count(iteratingNode->mName.C_Str())) - { - AZStd::string name = iteratingNode->mName.C_Str(); - - auto range = boneLookup.equal_range(name); - - if (range.first != range.second) - { - // There can be multiple offsetMatrices for a given bone, we're only interested in grabbing the first one - auto boneFirstOffsetMatrix = range.first->second->mOffsetMatrix; - auto azMat = AssImpSDKWrapper::AssImpTypeConverter::ToTransform(boneFirstOffsetMatrix); - offsets.push_back(azMat); - inverseOffsets.push_back(azMat.GetInverseFull()); - } - - iteratingNode = iteratingNode->mParent; - } - - if (inverseOffsets.size() == 1) - { - // If this is the root bone, just use the inverseOffset, otherwise the equation below just results in the identity matrix - localTransform = inverseOffsets[0]; - } - else - { - localTransform = offsets.at(1) // parent bone offset - * inverseOffsets.at(inverseOffsets.size() - 1) // Inverse of root bone offset - * offsets.at(offsets.size() - 1) // Root bone offset - * inverseOffsets.at(0); // Inverse of current node offset - } - } - else - { - localTransform = AssImpSDKWrapper::AssImpTypeConverter::ToTransform(GetConcatenatedLocalTransform(currentNode)); - } + DataTypes::MatrixType localTransform = GetLocalSpaceBindPoseTransform(scene, currentNode); // Don't bother adding a node with the identity matrix if (localTransform == DataTypes::MatrixType::Identity())