From fbf09f27c72e0713b4dd0f3fd970aba6e629a92c Mon Sep 17 00:00:00 2001 From: lsemp3d <58790905+lsemp3d@users.noreply.github.com> Date: Thu, 30 Sep 2021 17:15:06 -0700 Subject: [PATCH 1/5] Marked test_VariableManager_UnpinVariableType_Works as xfail, the test needs to be reworked Signed-off-by: lsemp3d <58790905+lsemp3d@users.noreply.github.com> --- .../Gem/PythonTests/scripting/TestSuite_Periodic.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/AutomatedTesting/Gem/PythonTests/scripting/TestSuite_Periodic.py b/AutomatedTesting/Gem/PythonTests/scripting/TestSuite_Periodic.py index 0862e03a79..b2001e6825 100755 --- a/AutomatedTesting/Gem/PythonTests/scripting/TestSuite_Periodic.py +++ b/AutomatedTesting/Gem/PythonTests/scripting/TestSuite_Periodic.py @@ -122,6 +122,7 @@ class TestAutomation(TestAutomationBase): from . import Debugger_HappyPath_TargetMultipleEntities as test_module self._run_test(request, workspace, editor, test_module) + @pytest.mark.xfail(reason="Test fails to find expected lines, it needs to be fixed.") def test_EditMenu_Default_UndoRedo(self, request, workspace, editor, launcher_platform, project): from . import EditMenu_Default_UndoRedo as test_module self._run_test(request, workspace, editor, test_module) @@ -181,6 +182,7 @@ class TestAutomation(TestAutomationBase): from . import NodePalette_SearchText_Deletion as test_module self._run_test(request, workspace, editor, test_module) + @pytest.mark.xfail(reason="Test fails to find expected lines, it needs to be fixed.") def test_VariableManager_UnpinVariableType_Works(self, request, workspace, editor, launcher_platform): from . import VariableManager_UnpinVariableType_Works as test_module self._run_test(request, workspace, editor, test_module) From 093f321b681f477f8d26e22cd2e0ea8dc6f313f9 Mon Sep 17 00:00:00 2001 From: Yaakuro Date: Mon, 4 Oct 2021 23:34:55 +0200 Subject: [PATCH 2/5] Fix issue that gives wrong results for running debugger (#4434) * Fix issue that gives wrong results for running debugger Signed-off-by: Yaakuro * Remove redundant part. Signed-off-by: Yaakuro --- .../Platform/Common/UnixLike/AzCore/Debug/Trace_UnixLike.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Code/Framework/AzCore/Platform/Common/UnixLike/AzCore/Debug/Trace_UnixLike.cpp b/Code/Framework/AzCore/Platform/Common/UnixLike/AzCore/Debug/Trace_UnixLike.cpp index e9c0234ffa..f4d7db802b 100644 --- a/Code/Framework/AzCore/Platform/Common/UnixLike/AzCore/Debug/Trace_UnixLike.cpp +++ b/Code/Framework/AzCore/Platform/Common/UnixLike/AzCore/Debug/Trace_UnixLike.cpp @@ -38,7 +38,7 @@ namespace AZ { return false; } - for (size_t i = tracerPidOffset; i < numRead; ++i) + for (size_t i = tracerPidOffset + tracerPidString.length(); i < numRead; ++i) { if (!::isspace(processStatusView[i])) { From 7052ccfa525fe8c3dc56addc82ca78ecf3f4dd32 Mon Sep 17 00:00:00 2001 From: Ken Pruiksma Date: Mon, 4 Oct 2021 16:35:22 -0500 Subject: [PATCH 3/5] Terrain detail and macro texture support (#4403) * Terrain feature processor improvements regarding material, mesh, and lod - Now using a material with pbr lighting for terrain. Removed some of the old shader code that wasn't needed anymore - Move heightmap image declaration to the material so it's not needed in the object SRG anymore - Added prototype code (commented out) for smoothing the terrain with a b-spline weighting function - Mesh data is all made with a proper mesh asset now. - Added basic LOD support (no continuous LOD yet, but it does pop between lod levels) - Moved RenderCommon to be accessible publicly. It contains stencil refs that should be public. Signed-off-by: Ken Pruiksma * Adding more material options to terrain shader Signed-off-by: Ken Pruiksma * Fixing terrain's per object srg because of changes in the default per object srg. Signed-off-by: Ken Pruiksma * Added more features to the terrain material. Shader & Material reloads are now handled. Signed-off-by: Ken Pruiksma * Added stub for terrain render max area setting in the renderer component. Added detail texture tiling setable from the material Signed-off-by: Ken Pruiksma * Missing change to material type from last commit Signed-off-by: Ken Pruiksma * Adding macro material in material type and support detail roughness. Signed-off-by: Ken Pruiksma * Fixing merge issues in terrain material type Signed-off-by: Ken Pruiksma * Adding more features to the terrain material - Macro color and normals - Detail layer fade out - Reoriented detail normals Signed-off-by: Ken Pruiksma * Remove line that unintentionally was added back during a rebase & merge. Signed-off-by: Ken Pruiksma * Fixing previously deleted unused static that came back after a merge. Fixed an issue with macro normals in the terrain shader. Signed-off-by: Ken Pruiksma --- .../Terrain/DefaultPbrTerrain.material | 23 +- .../Materials/Terrain/PbrTerrain.materialtype | 306 +++++++++++++++++- .../Shaders/Terrain/TerrainCommon.azsli | 56 +++- .../Terrain/TerrainPBR_ForwardPass.azsl | 49 ++- .../TerrainWorldRendererComponent.cpp | 47 ++- .../TerrainWorldRendererComponent.h | 19 +- .../EditorTerrainWorldRendererComponent.cpp | 8 +- .../TerrainFeatureProcessor.cpp | 53 +-- .../TerrainRenderer/TerrainFeatureProcessor.h | 12 +- 9 files changed, 519 insertions(+), 54 deletions(-) diff --git a/Gems/Terrain/Assets/Materials/Terrain/DefaultPbrTerrain.material b/Gems/Terrain/Assets/Materials/Terrain/DefaultPbrTerrain.material index 095fbba21a..c0de5969b2 100644 --- a/Gems/Terrain/Assets/Materials/Terrain/DefaultPbrTerrain.material +++ b/Gems/Terrain/Assets/Materials/Terrain/DefaultPbrTerrain.material @@ -4,8 +4,27 @@ "parentMaterial": "", "propertyLayoutVersion": 1, "properties": { - "settings": { - "baseColor": [ 0.78, 0.59, 0.48 ] + "macroColor": { + "useTexture": false + }, + "macroNormal": { + "useTexture": false + }, + "baseColor": { + "color": [ 0.18, 0.18, 0.18 ], + "useTexture": false + }, + "normal": + { + "useTexture": false + }, + "roughness": + { + "useTexture": false + }, + "specularF0": + { + "useTexture": false } } } diff --git a/Gems/Terrain/Assets/Materials/Terrain/PbrTerrain.materialtype b/Gems/Terrain/Assets/Materials/Terrain/PbrTerrain.materialtype index bf6cbbb850..191fa7a731 100644 --- a/Gems/Terrain/Assets/Materials/Terrain/PbrTerrain.materialtype +++ b/Gems/Terrain/Assets/Materials/Terrain/PbrTerrain.materialtype @@ -100,25 +100,269 @@ } }, { - "id": "baseColor", - "displayName": "Base Color", + "id": "detailTextureMultiplier", + "displayName": "Detail Texture UV Multiplier", + "description": "How many times to repeat the detail texture per sector", + "type": "Float", + "defaultValue": 8.0, + "connection": { + "type": "ShaderInput", + "id": "m_detailTextureMultiplier" + } + }, + { + "id": "detailFadeDistance", + "displayName": "Detail Fade Distance (meters)", + "description": "The distance in meters that the detail texture starts to fade", + "type": "Float", + "defaultValue": 100.0, + "connection": { + "type": "ShaderInput", + "id": "m_detailFadeDistance" + } + }, + { + "id": "detailFadeLength", + "displayName": "Detail Fade Length", + "description": "How far beyond Detail Fade Distance where the detail texture no longer applies.", + "type": "Float", + "defaultValue": 50.0, + "connection": { + "type": "ShaderInput", + "id": "m_detailFadeLength" + } + } + ], + "macroColor": [ + { + "id": "textureMap", + "displayName": "Texture", + "description": "Macro color texture map", + "type": "Image", + "connection": { + "type": "ShaderInput", + "id": "m_macroColorMap" + } + }, + { + "id": "useTexture", + "displayName": "Use Texture", + "description": "Whether to use the texture.", + "type": "Bool", + "defaultValue": true + } + + ], + "macroNormal": [ + { + "id": "textureMap", + "displayName": "Texture", + "description": "Macro normal texture map", + "type": "Image", + "connection": { + "type": "ShaderInput", + "id": "m_macroNormalMap" + } + }, + { + "id": "useTexture", + "displayName": "Use Texture", + "description": "Whether to use the texture.", + "type": "Bool", + "defaultValue": true + }, + { + "id": "flipX", + "displayName": "Flip X Channel", + "description": "Flip tangent direction for this normal map.", + "type": "Bool", + "defaultValue": false, + "connection": { + "type": "ShaderInput", + "id": "m_flipMacroNormalX" + } + }, + { + "id": "flipY", + "displayName": "Flip Y Channel", + "description": "Flip bitangent direction for this normal map.", + "type": "Bool", + "defaultValue": false, + "connection": { + "type": "ShaderInput", + "id": "m_flipMacroNormalY" + } + } + ], + "baseColor": [ + { + "id": "color", + "displayName": "Color", + "description": "Color is displayed as sRGB but the values are stored as linear color.", "type": "Color", - "defaultValue": [ 0.18, 0.18, 0.18 ], + "defaultValue": [ 1.0, 1.0, 1.0 ], "connection": { "type": "ShaderInput", "id": "m_baseColor" } }, { - "id": "roughness", - "displayName": "Roughness", + "id": "factor", + "displayName": "Factor", + "description": "Strength factor for scaling the base color values. Zero (0.0) is black, white (1.0) is full color.", "type": "Float", "defaultValue": 1.0, "min": 0.0, "max": 1.0, "connection": { "type": "ShaderInput", - "id": "m_roughness" + "id": "m_baseColorFactor" + } + }, + { + "id": "textureMap", + "displayName": "Texture", + "description": "Base color texture map", + "type": "Image", + "connection": { + "type": "ShaderInput", + "id": "m_baseColorMap" + } + }, + { + "id": "useTexture", + "displayName": "Use Texture", + "description": "Whether to use the texture.", + "type": "Bool", + "defaultValue": true + }, + { + "id": "textureBlendMode", + "displayName": "Texture Blend Mode", + "description": "Selects the equation to use when combining Color, Factor, and Texture.", + "type": "Enum", + "enumValues": [ "Multiply", "LinearLight", "Lerp", "Overlay" ], + "defaultValue": "Overlay", + "connection": { + "type": "ShaderOption", + "id": "o_baseColorTextureBlendMode" + } + } + ], + "normal": [ + { + "id": "textureMap", + "displayName": "Texture", + "description": "Texture for defining surface normal direction.", + "type": "Image", + "connection": { + "type": "ShaderInput", + "id": "m_normalMap" + } + }, + { + "id": "useTexture", + "displayName": "Use Texture", + "description": "Whether to use the texture, or just rely on vertex normals.", + "type": "Bool", + "defaultValue": true + }, + { + "id": "flipX", + "displayName": "Flip X Channel", + "description": "Flip tangent direction for this normal map.", + "type": "Bool", + "defaultValue": false, + "connection": { + "type": "ShaderInput", + "id": "m_flipNormalX" + } + }, + { + "id": "flipY", + "displayName": "Flip Y Channel", + "description": "Flip bitangent direction for this normal map.", + "type": "Bool", + "defaultValue": false, + "connection": { + "type": "ShaderInput", + "id": "m_flipNormalY" + } + }, + { + "id": "factor", + "displayName": "Factor", + "description": "Strength factor for scaling the values", + "type": "Float", + "defaultValue": 1.0, + "min": 0.0, + "softMax": 2.0, + "connection": { + "type": "ShaderInput", + "id": "m_normalFactor" + } + } + ], + "roughness": [ + { + "id": "textureMap", + "displayName": "Texture", + "description": "Texture for defining surface roughness.", + "type": "Image", + "connection": { + "type": "ShaderInput", + "id": "m_roughnessMap" + } + }, + { + "id": "useTexture", + "description": "Whether to use the texture, or just default to the Factor value.", + "type": "Bool", + "defaultValue": true + }, + { + "id": "factor", + "displayName": "Factor", + "description": "Controls the roughness value", + "type": "Float", + "defaultValue": 1.0, + "min": 0.0, + "max": 1.0, + "connection": { + "type": "ShaderInput", + "id": "m_roughnessFactor" + } + } + ], + "specularF0": [ + { + "id": "textureMap", + "displayName": "Texture", + "description": "Texture for defining surface reflectance.", + "type": "Image", + "connection": { + "type": "ShaderInput", + "id": "m_specularF0Map" + } + }, + { + "id": "useTexture", + "displayName": "Use Texture", + "description": "Whether to use the texture, or just default to the Factor value.", + "type": "Bool", + "defaultValue": true + }, + { + "id": "factor", + "displayName": "Factor", + "description": "The default IOR is 1.5, which gives you 0.04 (4% of light reflected at 0 degree angle for dielectric materials). F0 values lie in the range 0-0.08, so that is why the default F0 slider is set on 0.5.", + "type": "Float", + "defaultValue": 0.5, + "min": 0.0, + "max": 1.0, + "connection": { + "type": "ShaderInput", + "id": "m_specularF0Factor" } } ] @@ -136,5 +380,55 @@ } ], "functors": [ + { + "type": "UseTexture", + "args": { + "textureProperty": "macroColor.textureMap", + "useTextureProperty": "macroColor.useTexture", + "shaderOption": "o_macroColor_useTexture" + } + }, + { + "type": "UseTexture", + "args": { + "textureProperty": "macroNormal.textureMap", + "useTextureProperty": "macroNormal.useTexture", + "shaderOption": "o_macroNormal_useTexture" + } + }, + { + "type": "UseTexture", + "args": { + "textureProperty": "baseColor.textureMap", + "useTextureProperty": "baseColor.useTexture", + "dependentProperties": ["baseColor.textureBlendMode"], + "shaderOption": "o_baseColor_useTexture" + } + }, + { + "type": "UseTexture", + "args": { + "textureProperty": "specularF0.textureMap", + "useTextureProperty": "specularF0.useTexture", + "shaderOption": "o_specularF0_useTexture" + } + }, + { + "type": "UseTexture", + "args": { + "textureProperty": "normal.textureMap", + "useTextureProperty": "normal.useTexture", + "dependentProperties": ["normal.factor", "normal.flipX", "normal.flipY"], + "shaderOption": "o_normal_useTexture" + } + }, + { + "type": "UseTexture", + "args": { + "textureProperty": "roughness.textureMap", + "useTextureProperty": "roughness.useTexture", + "shaderOption": "o_roughness_useTexture" + } + } ] } diff --git a/Gems/Terrain/Assets/Shaders/Terrain/TerrainCommon.azsli b/Gems/Terrain/Assets/Shaders/Terrain/TerrainCommon.azsli index 9df7b35ce8..95d1a73bbd 100644 --- a/Gems/Terrain/Assets/Shaders/Terrain/TerrainCommon.azsli +++ b/Gems/Terrain/Assets/Shaders/Terrain/TerrainCommon.azsli @@ -7,6 +7,12 @@ #pragma once +#include <../Materials/Types/MaterialInputs/BaseColorInput.azsli> +#include <../Materials/Types/MaterialInputs/RoughnessInput.azsli> +#include <../Materials/Types/MaterialInputs/MetallicInput.azsli> +#include <../Materials/Types/MaterialInputs/SpecularInput.azsli> +#include <../Materials/Types/MaterialInputs/NormalInput.azsli> + ShaderResourceGroup ObjectSrg : SRG_PerObject { row_major float3x4 m_modelToWorld; @@ -70,7 +76,10 @@ ShaderResourceGroup ObjectSrg : SRG_PerObject ShaderResourceGroup TerrainMaterialSrg : SRG_PerMaterial { - Texture2D m_heightmapImage; + Texture2D m_heightmapImage; + float m_detailTextureMultiplier; + float m_detailFadeDistance; + float m_detailFadeLength; Sampler HeightmapSampler { @@ -82,11 +91,52 @@ ShaderResourceGroup TerrainMaterialSrg : SRG_PerMaterial AddressW = Clamp; }; + Sampler m_sampler + { + AddressU = Wrap; + AddressV = Wrap; + MinFilter = Linear; + MagFilter = Linear; + MipFilter = Linear; + MaxAnisotropy = 16; + }; + + // Macro Color + Texture2D m_macroColorMap; + + // Macro normal + Texture2D m_macroNormalMap; + bool m_flipMacroNormalX; + bool m_flipMacroNormalY; + + // Base Color float3 m_baseColor; - float m_roughness; + float m_baseColorFactor; + Texture2D m_baseColorMap; + + // Normal + Texture2D m_normalMap; + bool m_flipNormalX; + bool m_flipNormalY; + float m_normalFactor; + + // Roughness + Texture2D m_roughnessMap; + float m_roughnessFactor; + + // Specular + Texture2D m_specularF0Map; + float m_specularF0Factor; } option bool o_useTerrainSmoothing = false; +option bool o_macroColor_useTexture = true; +option bool o_macroNormal_useTexture = true; +option bool o_baseColor_useTexture = true; +option bool o_specularF0_useTexture = true; +option bool o_normal_useTexture = true; +option bool o_roughness_useTexture = true; +option TextureBlendMode o_baseColorTextureBlendMode = TextureBlendMode::Multiply; struct VertexInput { @@ -98,7 +148,7 @@ struct VertexInput // This function samples a 4x4 neighborhood around the uv. Normally this would take 16 samples, but by taking // advantage of bilinear filtering this can be done with 9 taps on the edges between pixels. The cost is further // reduced by dropping the diagonals. -float SampleBSpline5Tap(Texture2D texture, SamplerState textureSampler, float2 uv, float2 textureSize, float2 rcpTextureSize) +float SampleBSpline5Tap(Texture2D texture, SamplerState textureSampler, float2 uv, float2 textureSize, float2 rcpTextureSize) { // Think of sample locations in the 4x4 neighborhood as having a top left coordinate of 0,0 and // a bottom right coordinate of 3,3. diff --git a/Gems/Terrain/Assets/Shaders/Terrain/TerrainPBR_ForwardPass.azsl b/Gems/Terrain/Assets/Shaders/Terrain/TerrainPBR_ForwardPass.azsl index d77b944498..df2a801969 100644 --- a/Gems/Terrain/Assets/Shaders/Terrain/TerrainPBR_ForwardPass.azsl +++ b/Gems/Terrain/Assets/Shaders/Terrain/TerrainPBR_ForwardPass.azsl @@ -71,18 +71,49 @@ ForwardPassOutput TerrainPBR_MainPassPS(VSOutput IN) Surface surface; - // Position, Normal, Roughness + // Position surface.position = IN.m_worldPosition.xyz; - surface.normal = normalize(IN.m_normal); - surface.roughnessLinear = TerrainMaterialSrg::m_roughness; + float viewDistance = length(ViewSrg::m_worldPosition - surface.position); + float detailFactor = saturate((viewDistance - TerrainMaterialSrg::m_detailFadeDistance) / max(TerrainMaterialSrg::m_detailFadeLength, EPSILON)); + + ObjectSrg::TerrainData terrainData = ObjectSrg::m_terrainData; + float2 origUv = lerp(terrainData.m_uvMin, terrainData.m_uvMax, IN.m_uv); + origUv.y = 1.0 - origUv.y; + float2 detailUv = IN.m_uv * TerrainMaterialSrg::m_detailTextureMultiplier; + + // ------- Normal ------- + float3 macroNormal = IN.m_normal; + if (o_macroNormal_useTexture) + { + macroNormal = GetNormalInputTS(TerrainMaterialSrg::m_macroNormalMap, TerrainMaterialSrg::m_sampler, + origUv, TerrainMaterialSrg::m_flipMacroNormalX, TerrainMaterialSrg::m_flipMacroNormalY, CreateIdentity3x3(), true, 1.0); + } + + float3 detailNormal = GetNormalInputTS(TerrainMaterialSrg::m_normalMap, TerrainMaterialSrg::m_sampler, + detailUv, TerrainMaterialSrg::m_flipNormalX, TerrainMaterialSrg::m_flipNormalY, CreateIdentity3x3(), o_normal_useTexture, TerrainMaterialSrg::m_normalFactor); + + detailNormal = ReorientTangentSpaceNormal(macroNormal, detailNormal); + surface.normal = lerp(detailNormal, macroNormal, detailFactor); + surface.normal = normalize(surface.normal); + + // ------- Macro Color ------- + float3 macroColor = GetBaseColorInput(TerrainMaterialSrg::m_macroColorMap, TerrainMaterialSrg::m_sampler, origUv, TerrainMaterialSrg::m_baseColor.rgb, o_baseColor_useTexture); + + // ------- Base Color ------- + float3 detailColor = GetBaseColorInput(TerrainMaterialSrg::m_baseColorMap, TerrainMaterialSrg::m_sampler, detailUv, TerrainMaterialSrg::m_baseColor.rgb, o_baseColor_useTexture); + float3 blendedColor = BlendBaseColor(lerp(detailColor, TerrainMaterialSrg::m_baseColor.rgb, detailFactor), macroColor, TerrainMaterialSrg::m_baseColorFactor, o_baseColorTextureBlendMode, o_baseColor_useTexture); + + // ------- Specular ------- + float specularF0Factor = GetSpecularInput(TerrainMaterialSrg::m_specularF0Map, TerrainMaterialSrg::m_sampler, detailUv, TerrainMaterialSrg::m_specularF0Factor, o_specularF0_useTexture); + specularF0Factor = lerp(specularF0Factor, 0.5, detailFactor); + surface.SetAlbedoAndSpecularF0(blendedColor, specularF0Factor, 0.0); + + // ------- Roughness ------- + surface.roughnessLinear = GetRoughnessInput(TerrainMaterialSrg::m_roughnessMap, TerrainMaterialSrg::m_sampler, detailUv, TerrainMaterialSrg::m_roughnessFactor, 0.0, 1.0, o_roughness_useTexture); + surface.roughnessLinear = lerp(surface.roughnessLinear, 1.0, detailFactor); surface.CalculateRoughnessA(); - // Albedo, SpecularF0 - const float specularF0Factor = 0.5f; - float3 color = TerrainMaterialSrg::m_baseColor; - surface.SetAlbedoAndSpecularF0(color, specularF0Factor, 0.0); - - // Clear Coat, Transmission + // Clear Coat, Transmission (Not used for terrain) surface.clearCoat.InitializeToZero(); surface.transmission.InitializeToZero(); diff --git a/Gems/Terrain/Code/Source/Components/TerrainWorldRendererComponent.cpp b/Gems/Terrain/Code/Source/Components/TerrainWorldRendererComponent.cpp index 3bf34fa019..f3eed41717 100644 --- a/Gems/Terrain/Code/Source/Components/TerrainWorldRendererComponent.cpp +++ b/Gems/Terrain/Code/Source/Components/TerrainWorldRendererComponent.cpp @@ -28,16 +28,27 @@ namespace Terrain AZ::SerializeContext* serialize = azrtti_cast(context); if (serialize) { - serialize->Class()->Version(1); + serialize->Class() + ->Version(1) + ->Field("WorldSize", &TerrainWorldRendererConfig::m_worldSize) + ; - AZ::EditContext* edit = serialize->GetEditContext(); - if (edit) + AZ::EditContext* editContext = serialize->GetEditContext(); + if (editContext) { - edit->Class("Terrain World Renderer Component", "Enables terrain rendering") + editContext->Class("Terrain World Renderer Component", "Enables terrain rendering") ->ClassElement(AZ::Edit::ClassElements::EditorData, "") - ->Attribute(AZ::Edit::Attributes::AppearsInAddComponentMenu, AZStd::vector({ AZ_CRC_CE("Level") })) - ->Attribute(AZ::Edit::Attributes::Visibility, AZ::Edit::PropertyVisibility::ShowChildrenOnly) - ->Attribute(AZ::Edit::Attributes::AutoExpand, true); + ->Attribute(AZ::Edit::Attributes::AppearsInAddComponentMenu, AZStd::vector({ AZ_CRC_CE("Level") })) + ->Attribute(AZ::Edit::Attributes::Visibility, AZ::Edit::PropertyVisibility::ShowChildrenOnly) + ->Attribute(AZ::Edit::Attributes::AutoExpand, true) + ->DataElement(AZ::Edit::UIHandlers::ComboBox, &TerrainWorldRendererConfig::m_worldSize, "Rendered world size", "The maximum amount of terrain that's rendered") + ->EnumAttribute(TerrainWorldRendererConfig::WorldSize::_512Meters, "512 Meters") + ->EnumAttribute(TerrainWorldRendererConfig::WorldSize::_1024Meters, "1 Kilometer") + ->EnumAttribute(TerrainWorldRendererConfig::WorldSize::_2048Meters, "2 Kilometers") + ->EnumAttribute(TerrainWorldRendererConfig::WorldSize::_4096Meters, "4 Kilometers") + ->EnumAttribute(TerrainWorldRendererConfig::WorldSize::_8192Meters, "8 Kilometers") + ->EnumAttribute(TerrainWorldRendererConfig::WorldSize::_16384Meters, "16 Kilometers") + ; } } } @@ -72,6 +83,28 @@ namespace Terrain TerrainWorldRendererComponent::TerrainWorldRendererComponent(const TerrainWorldRendererConfig& configuration) : m_configuration(configuration) { + switch (configuration.m_worldSize) + { + case TerrainWorldRendererConfig::WorldSize::_512Meters: + m_terrainFeatureProcessor->SetWorldSize(AZ::Vector2(512.0f, 512.0f)); + break; + case TerrainWorldRendererConfig::WorldSize::_1024Meters: + m_terrainFeatureProcessor->SetWorldSize(AZ::Vector2(1024.0f, 1024.0f)); + break; + case TerrainWorldRendererConfig::WorldSize::_2048Meters: + m_terrainFeatureProcessor->SetWorldSize(AZ::Vector2(2048.0f, 2048.0f)); + break; + case TerrainWorldRendererConfig::WorldSize::_4096Meters: + m_terrainFeatureProcessor->SetWorldSize(AZ::Vector2(4096.0f, 4096.0f)); + break; + case TerrainWorldRendererConfig::WorldSize::_8192Meters: + m_terrainFeatureProcessor->SetWorldSize(AZ::Vector2(8192.0f, 8192.0f)); + break; + case TerrainWorldRendererConfig::WorldSize::_16384Meters: + m_terrainFeatureProcessor->SetWorldSize(AZ::Vector2(16384.0f, 16384.0f)); + break; + + } } TerrainWorldRendererComponent::~TerrainWorldRendererComponent() diff --git a/Gems/Terrain/Code/Source/Components/TerrainWorldRendererComponent.h b/Gems/Terrain/Code/Source/Components/TerrainWorldRendererComponent.h index a1e9498be8..cd2a4ea5aa 100644 --- a/Gems/Terrain/Code/Source/Components/TerrainWorldRendererComponent.h +++ b/Gems/Terrain/Code/Source/Components/TerrainWorldRendererComponent.h @@ -27,13 +27,28 @@ namespace Terrain { class TerrainFeatureProcessor; - class TerrainWorldRendererConfig + struct TerrainWorldRendererConfig final : public AZ::ComponentConfig { - public: AZ_CLASS_ALLOCATOR(TerrainWorldRendererConfig, AZ::SystemAllocator, 0); AZ_RTTI(TerrainWorldRendererConfig, "{08C5863C-092D-4A69-8226-4978E4F6E343}", AZ::ComponentConfig); static void Reflect(AZ::ReflectContext* context); + + enum class WorldSize : uint8_t + { + Unknown, + + _512Meters, + _1024Meters, + _2048Meters, + _4096Meters, + _8192Meters, + _16384Meters, + + WorldSizeCount, + }; + + WorldSize m_worldSize; }; diff --git a/Gems/Terrain/Code/Source/EditorComponents/EditorTerrainWorldRendererComponent.cpp b/Gems/Terrain/Code/Source/EditorComponents/EditorTerrainWorldRendererComponent.cpp index 2d42585fa5..3dd8d1e911 100644 --- a/Gems/Terrain/Code/Source/EditorComponents/EditorTerrainWorldRendererComponent.cpp +++ b/Gems/Terrain/Code/Source/EditorComponents/EditorTerrainWorldRendererComponent.cpp @@ -29,10 +29,10 @@ namespace Terrain editContext->Class( "Terrain World Renderer", "") ->ClassElement(AZ::Edit::ClassElements::EditorData, "") - ->Attribute(AZ::Edit::Attributes::Category, "Terrain") - ->Attribute(AZ::Edit::Attributes::Icon, "Editor/Icons/Components/TerrainWorldRenderer.svg") - ->Attribute(AZ::Edit::Attributes::ViewportIcon, "Editor/Icons/Components/Viewport/TerrainWorldRenderer.svg") - ->Attribute(AZ::Edit::Attributes::AppearsInAddComponentMenu, AZStd::vector({ AZ_CRC_CE("Level") })) + ->Attribute(AZ::Edit::Attributes::Category, "Terrain") + ->Attribute(AZ::Edit::Attributes::Icon, "Editor/Icons/Components/TerrainWorldRenderer.svg") + ->Attribute(AZ::Edit::Attributes::ViewportIcon, "Editor/Icons/Components/Viewport/TerrainWorldRenderer.svg") + ->Attribute(AZ::Edit::Attributes::AppearsInAddComponentMenu, AZStd::vector({ AZ_CRC_CE("Level") })) ; } } diff --git a/Gems/Terrain/Code/Source/TerrainRenderer/TerrainFeatureProcessor.cpp b/Gems/Terrain/Code/Source/TerrainRenderer/TerrainFeatureProcessor.cpp index 3e5f9a0608..59984a1b92 100644 --- a/Gems/Terrain/Code/Source/TerrainRenderer/TerrainFeatureProcessor.cpp +++ b/Gems/Terrain/Code/Source/TerrainRenderer/TerrainFeatureProcessor.cpp @@ -76,26 +76,24 @@ namespace Terrain void TerrainFeatureProcessor::Initialize() { - { - // Load the terrain material asynchronously - const AZStd::string materialFilePath = "Materials/Terrain/DefaultPbrTerrain.azmaterial"; - m_materialAssetLoader = AZStd::make_unique(); - *m_materialAssetLoader = AZ::RPI::AssetUtils::AsyncAssetLoader::Create(materialFilePath, 0u, - [&](AZ::Data::Asset assetData, bool success) -> void + // Load the terrain material asynchronously + const AZStd::string materialFilePath = "Materials/Terrain/DefaultPbrTerrain.azmaterial"; + m_materialAssetLoader = AZStd::make_unique(); + *m_materialAssetLoader = AZ::RPI::AssetUtils::AsyncAssetLoader::Create(materialFilePath, 0u, + [&](AZ::Data::Asset assetData, bool success) -> void + { + const AZ::Data::Asset& materialAsset = static_cast>(assetData); + if (success) { - const AZ::Data::Asset& materialAsset = static_cast>(assetData); - if (success) + m_materialInstance = AZ::RPI::Material::FindOrCreate(assetData); + AZ::RPI::MaterialReloadNotificationBus::Handler::BusConnect(materialAsset->GetId()); + if (!materialAsset->GetObjectSrgLayout()) { - m_materialInstance = AZ::RPI::Material::FindOrCreate(assetData); - if (!materialAsset->GetObjectSrgLayout()) - { - AZ_Error("TerrainFeatureProcessor", false, "No per-object ShaderResourceGroup found on terrain material."); - } + AZ_Error("TerrainFeatureProcessor", false, "No per-object ShaderResourceGroup found on terrain material."); } } - ); - } - + } + ); if (!InitializePatchModel()) { AZ_Error(TerrainFPName, false, "Failed to create Terrain render buffers!"); @@ -105,10 +103,9 @@ namespace Terrain void TerrainFeatureProcessor::Deactivate() { - DisableSceneNotification(); - m_patchModel = {}; m_areaData = {}; + AZ::RPI::MaterialReloadNotificationBus::Handler::BusDisconnect(); } void TerrainFeatureProcessor::Render(const AZ::RPI::FeatureProcessor::RenderPacket& packet) @@ -291,7 +288,7 @@ namespace Terrain AZ::Vector2 sectorCenterXY = AZ::Vector2(sectorData.m_aabb.GetCenter().GetX(), sectorData.m_aabb.GetCenter().GetY()); float sectorDistance = sectorCenterXY.GetDistance(cameraPositionXY); - float lodForCamera = ceilf(AZ::GetMax(0.0f, log2f(sectorDistance / (GridMeters * 4.0f)))); + float lodForCamera = floorf(AZ::GetMax(0.0f, log2f(sectorDistance / (GridMeters * 4.0f)))); lodChoice = AZ::GetMin(lodChoice, aznumeric_cast(lodForCamera)); } } @@ -317,6 +314,7 @@ namespace Terrain uint16_t gridVertices = gridSize + 1; // For m_gridSize quads, (m_gridSize + 1) vertices are needed. size_t size = gridVertices * gridVertices; + size *= size; patchdata.m_positions.reserve(size); patchdata.m_uvs.reserve(size); @@ -432,4 +430,21 @@ namespace Terrain return success; } + + void TerrainFeatureProcessor::OnMaterialReinitialized([[maybe_unused]] const AZ::Data::Instance& material) + { + for (auto& sectorData : m_sectorData) + { + for (auto& drawPacket : sectorData.m_drawPackets) + { + drawPacket.Update(*GetParentScene()); + } + } + } + + void TerrainFeatureProcessor::SetWorldSize([[maybe_unused]] AZ::Vector2 sizeInMeters) + { + // This will control the max rendering size. Actual terrain size can be much + // larger but this will limit how much is rendered. + } } diff --git a/Gems/Terrain/Code/Source/TerrainRenderer/TerrainFeatureProcessor.h b/Gems/Terrain/Code/Source/TerrainRenderer/TerrainFeatureProcessor.h index 160d3113c2..32f88565c2 100644 --- a/Gems/Terrain/Code/Source/TerrainRenderer/TerrainFeatureProcessor.h +++ b/Gems/Terrain/Code/Source/TerrainRenderer/TerrainFeatureProcessor.h @@ -27,6 +27,7 @@ #include #include #include +#include namespace AZ::RPI { @@ -41,6 +42,7 @@ namespace Terrain { class TerrainFeatureProcessor final : public AZ::RPI::FeatureProcessor + , private AZ::RPI::MaterialReloadNotificationBus::Handler { public: AZ_RTTI(TerrainFeatureProcessor, "{D7DAC1F9-4A9F-4D3C-80AE-99579BF8AB1C}", AZ::RPI::FeatureProcessor); @@ -52,12 +54,18 @@ namespace Terrain TerrainFeatureProcessor() = default; ~TerrainFeatureProcessor() = default; - ////////////////////////////////////////////////////////////////////////// - // AZ::Component interface implementation + // AZ::Component overrides... void Activate() override; void Deactivate() override; + + // AZ::RPI::FeatureProcessor overrides... void Render(const AZ::RPI::FeatureProcessor::RenderPacket& packet) override; + // AZ::RPI::MaterialReloadNotificationBus::Handler overrides... + void OnMaterialReinitialized(const AZ::Data::Instance& material) override; + + void SetWorldSize(AZ::Vector2 sizeInMeters); + void UpdateTerrainData(const AZ::Transform& transform, const AZ::Aabb& worldBounds, float sampleSpacing, uint32_t width, uint32_t height, const AZStd::vector& heightData); From 0a8170f52a8006107450d57d4e7f173482252191 Mon Sep 17 00:00:00 2001 From: lumberyard-employee-dm <56135373+lumberyard-employee-dm@users.noreply.github.com> Date: Mon, 4 Oct 2021 16:36:54 -0500 Subject: [PATCH 4/5] Added IsDirectory function to SystemFile (#4454) * Added IsDirectory function to SystemFile This takes the implementation in LocalFileIO and uses it for SystemFile and then just has LocalFileIO call the SystemFile implementation Signed-off-by: lumberyard-employee-dm <56135373+lumberyard-employee-dm@users.noreply.github.com> * Fixed logic to detect the WinApi FILE_ATTRIBUTE_DIRECTORY attribute Updated the FileIO.cpp test to use AZ::IO::Path and removed direct uses of AZStd::string Signed-off-by: lumberyard-employee-dm <56135373+lumberyard-employee-dm@users.noreply.github.com> * Adding googletest printers for string and Path classes Signed-off-by: lumberyard-employee-dm <56135373+lumberyard-employee-dm@users.noreply.github.com> * Updated the SystemFile_WinAPI functions to use AZStd::to_wstring This makes the the SystemFile function convert from UTF-8 to UTF-16 Signed-off-by: lumberyard-employee-dm <56135373+lumberyard-employee-dm@users.noreply.github.com> --- .../Framework/AzCore/AzCore/IO/SystemFile.cpp | 6 + Code/Framework/AzCore/AzCore/IO/SystemFile.h | 2 + .../Android/AzCore/IO/SystemFile_Android.cpp | 15 + .../AzCore/IO/SystemFile_UnixLikeDefault.cpp | 10 + .../WinAPI/AzCore/IO/SystemFile_WinAPI.cpp | 173 ++++----- .../AzFramework/IO/LocalFileIO.cpp | 8 + .../AzFramework/IO/LocalFileIO_Android.cpp | 20 -- .../AzFramework/IO/LocalFileIO_UnixLike.cpp | 13 - .../AzFramework/IO/LocalFileIO_WinAPI.cpp | 16 - Code/Framework/AzFramework/Tests/FileIO.cpp | 335 ++++++++---------- Code/Framework/AzTest/AzTest/Printers.cpp | 27 ++ Code/Framework/AzTest/AzTest/Printers.h | 40 +++ Code/Framework/AzTest/AzTest/Printers.inl | 34 ++ Code/Framework/AzTest/AzTest/Utils.h | 2 +- .../AzTest/AzTest/aztest_files.cmake | 3 + 15 files changed, 350 insertions(+), 354 deletions(-) create mode 100644 Code/Framework/AzTest/AzTest/Printers.cpp create mode 100644 Code/Framework/AzTest/AzTest/Printers.h create mode 100644 Code/Framework/AzTest/AzTest/Printers.inl diff --git a/Code/Framework/AzCore/AzCore/IO/SystemFile.cpp b/Code/Framework/AzCore/AzCore/IO/SystemFile.cpp index 651abb89fe..ac12a59609 100644 --- a/Code/Framework/AzCore/AzCore/IO/SystemFile.cpp +++ b/Code/Framework/AzCore/AzCore/IO/SystemFile.cpp @@ -35,6 +35,7 @@ namespace Platform SystemFile::SizeType Length(FileHandleType handle, const SystemFile* systemFile); bool Exists(const char* fileName); + bool IsDirectory(const char* filePath); void FindFiles(const char* filter, SystemFile::FindFileCB cb); AZ::u64 ModificationTime(const char* fileName); SystemFile::SizeType Length(const char* fileName); @@ -235,6 +236,11 @@ bool SystemFile::Exists(const char* fileName) return Platform::Exists(fileName); } +bool SystemFile::IsDirectory(const char* filePath) +{ + return Platform::IsDirectory(filePath); +} + void SystemFile::FindFiles(const char* filter, FindFileCB cb) { Platform::FindFiles(filter, cb); diff --git a/Code/Framework/AzCore/AzCore/IO/SystemFile.h b/Code/Framework/AzCore/AzCore/IO/SystemFile.h index 551ce89ce7..39e44f86da 100644 --- a/Code/Framework/AzCore/AzCore/IO/SystemFile.h +++ b/Code/Framework/AzCore/AzCore/IO/SystemFile.h @@ -99,6 +99,8 @@ namespace AZ // Utility functions /// Check if a file or directory exists. static bool Exists(const char* path); + /// Check if path is a directory + static bool IsDirectory(const char* path); /// FindFiles typedef AZStd::function FindFileCB; static void FindFiles(const char* filter, FindFileCB cb); diff --git a/Code/Framework/AzCore/Platform/Android/AzCore/IO/SystemFile_Android.cpp b/Code/Framework/AzCore/Platform/Android/AzCore/IO/SystemFile_Android.cpp index 001cd57ac5..b9a6cfef9a 100644 --- a/Code/Framework/AzCore/Platform/Android/AzCore/IO/SystemFile_Android.cpp +++ b/Code/Framework/AzCore/Platform/Android/AzCore/IO/SystemFile_Android.cpp @@ -368,6 +368,21 @@ namespace Platform return access(fileName, F_OK) == 0; } } + + bool IsDirectory(const char* filePath) + { + if (AZ::Android::Utils::IsApkPath(filePath)) + { + return AZ::Android::APKFileHandler::IsDirectory(AZ::Android::Utils::StripApkPrefix(filePath).c_str()); + } + + struct stat result; + if (stat(filePath, &result) == 0) + { + return S_ISDIR(result.st_mode); + } + return false; + } } // namespace AZ::IO::Platform } // namespace AZ::IO diff --git a/Code/Framework/AzCore/Platform/Common/UnixLikeDefault/AzCore/IO/SystemFile_UnixLikeDefault.cpp b/Code/Framework/AzCore/Platform/Common/UnixLikeDefault/AzCore/IO/SystemFile_UnixLikeDefault.cpp index 3ca7a3fc76..88c47ed142 100644 --- a/Code/Framework/AzCore/Platform/Common/UnixLikeDefault/AzCore/IO/SystemFile_UnixLikeDefault.cpp +++ b/Code/Framework/AzCore/Platform/Common/UnixLikeDefault/AzCore/IO/SystemFile_UnixLikeDefault.cpp @@ -249,6 +249,16 @@ namespace Platform { return access(fileName, F_OK) == 0; } + + bool IsDirectory(const char* filePath) + { + struct stat result; + if (stat(filePath, &result) == 0) + { + return S_ISDIR(result.st_mode); + } + return false; + } } } // namespace AZ::IO diff --git a/Code/Framework/AzCore/Platform/Common/WinAPI/AzCore/IO/SystemFile_WinAPI.cpp b/Code/Framework/AzCore/Platform/Common/WinAPI/AzCore/IO/SystemFile_WinAPI.cpp index 15d779017d..0fe32dcd4a 100644 --- a/Code/Framework/AzCore/Platform/Common/WinAPI/AzCore/IO/SystemFile_WinAPI.cpp +++ b/Code/Framework/AzCore/Platform/Common/WinAPI/AzCore/IO/SystemFile_WinAPI.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include @@ -18,7 +19,7 @@ namespace AZ::IO { - + using FixedMaxPathWString = AZStd::fixed_wstring; namespace { //========================================================================= @@ -28,16 +29,9 @@ namespace //========================================================================= DWORD GetAttributes(const char* fileName) { - wchar_t fileNameW[AZ_MAX_PATH_LEN]; - size_t numCharsConverted; - if (mbstowcs_s(&numCharsConverted, fileNameW, fileName, AZ_ARRAY_SIZE(fileNameW) - 1) == 0) - { - return GetFileAttributesW(fileNameW); - } - else - { - return INVALID_FILE_ATTRIBUTES; - } + FixedMaxPathWString fileNameW; + AZStd::to_wstring(fileNameW, fileName); + return GetFileAttributesW(fileNameW.c_str()); } //========================================================================= @@ -47,16 +41,9 @@ namespace //========================================================================= BOOL SetAttributes(const char* fileName, DWORD fileAttributes) { - wchar_t fileNameW[AZ_MAX_PATH_LEN]; - size_t numCharsConverted; - if (mbstowcs_s(&numCharsConverted, fileNameW, fileName, AZ_ARRAY_SIZE(fileNameW) - 1) == 0) - { - return SetFileAttributesW(fileNameW, fileAttributes); - } - else - { - return FALSE; - } + FixedMaxPathWString fileNameW; + AZStd::to_wstring(fileNameW, fileName); + return SetFileAttributesW(fileNameW.c_str(), fileAttributes); } //========================================================================= @@ -68,9 +55,9 @@ namespace // * GetLastError() on Windows-like platforms // * errno on Unix platforms //========================================================================= - bool CreateDirRecursive(wchar_t* dirPath) + bool CreateDirRecursive(AZ::IO::FixedMaxPathWString& dirPath) { - if (CreateDirectoryW(dirPath, nullptr)) + if (CreateDirectoryW(dirPath.c_str(), nullptr)) { return true; // Created without error } @@ -78,28 +65,24 @@ namespace if (error == ERROR_PATH_NOT_FOUND) { // try to create our parent hierarchy - for (size_t i = wcslen(dirPath); i > 0; --i) + if (size_t i = dirPath.find_last_of(LR"(/\)"); i != FixedMaxPathWString::npos) { - if (dirPath[i] == L'/' || dirPath[i] == L'\\') + wchar_t delimiter = dirPath[i]; + dirPath[i] = 0; // null-terminate at the previous slash + const bool ret = CreateDirRecursive(dirPath); + dirPath[i] = delimiter; // restore slash + if (ret) { - wchar_t delimiter = dirPath[i]; - dirPath[i] = 0; // null-terminate at the previous slash - bool ret = CreateDirRecursive(dirPath); - dirPath[i] = delimiter; // restore slash - if (ret) - { - // now that our parent is created, try to create again - return CreateDirectoryW(dirPath, nullptr) != 0; - } - return false; + // now that our parent is created, try to create again + return CreateDirectoryW(dirPath.c_str(), nullptr) != 0; } } // if we reach here then there was no parent folder to create, so we failed for other reasons } else if (error == ERROR_ALREADY_EXISTS) { - DWORD attributes = GetFileAttributesW(dirPath); - return (attributes & FILE_ATTRIBUTE_DIRECTORY) != 0; + DWORD attributes = GetFileAttributesW(dirPath.c_str()); + return attributes != INVALID_FILE_ATTRIBUTES && (attributes & FILE_ATTRIBUTE_DIRECTORY) != 0; } return false; } @@ -152,13 +135,10 @@ bool SystemFile::PlatformOpen(int mode, int platformFlags) CreatePath(m_fileName.c_str()); } - wchar_t fileNameW[AZ_MAX_PATH_LEN]; - size_t numCharsConverted; + AZ::IO::FixedMaxPathWString fileNameW; + AZStd::to_wstring(fileNameW, m_fileName); m_handle = INVALID_HANDLE_VALUE; - if (mbstowcs_s(&numCharsConverted, fileNameW, m_fileName.c_str(), AZ_ARRAY_SIZE(fileNameW) - 1) == 0) - { - m_handle = CreateFileW(fileNameW, dwDesiredAccess, dwShareMode, 0, dwCreationDisposition, dwFlagsAndAttributes, 0); - } + m_handle = CreateFileW(fileNameW.c_str(), dwDesiredAccess, dwShareMode, 0, dwCreationDisposition, dwFlagsAndAttributes, 0); if (m_handle == INVALID_HANDLE_VALUE) { @@ -350,6 +330,12 @@ namespace Platform return GetAttributes(fileName) != INVALID_FILE_ATTRIBUTES; } + bool IsDirectory(const char* filePath) + { + DWORD attributes = GetAttributes(filePath); + return attributes != INVALID_FILE_ATTRIBUTES && (attributes & FILE_ATTRIBUTE_DIRECTORY) != 0; + } + void FindFiles(const char* filter, SystemFile::FindFileCB cb) { @@ -357,35 +343,26 @@ namespace Platform HANDLE hFile; int lastError; - wchar_t filterW[AZ_MAX_PATH_LEN]; - size_t numCharsConverted; + AZ::IO::FixedMaxPathWString filterW; + AZStd::to_wstring(filterW, filter); hFile = INVALID_HANDLE_VALUE; - if (mbstowcs_s(&numCharsConverted, filterW, filter, AZ_ARRAY_SIZE(filterW) - 1) == 0) - { - hFile = FindFirstFile(filterW, &fd); - } + hFile = FindFirstFileW(filterW.c_str(), &fd); if (hFile != INVALID_HANDLE_VALUE) { const char* fileName; - char fileNameA[AZ_MAX_PATH_LEN]; - fileName = NULL; - if (wcstombs_s(&numCharsConverted, fileNameA, fd.cFileName, AZ_ARRAY_SIZE(fileNameA) - 1) == 0) - { - fileName = fileNameA; - } + AZ::IO::FixedMaxPathString fileNameUtf8; + AZStd::to_string(fileNameUtf8, fd.cFileName); + fileName = fileNameUtf8.c_str(); cb(fileName, (fd.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) == 0); // List all the other files in the directory. while (FindNextFileW(hFile, &fd) != 0) { - fileName = NULL; - if (wcstombs_s(&numCharsConverted, fileNameA, fd.cFileName, AZ_ARRAY_SIZE(fileNameA) - 1) == 0) - { - fileName = fileNameA; - } + AZStd::to_string(fileNameUtf8, fd.cFileName); + fileName = fileNameUtf8.c_str(); cb(fileName, (fd.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) == 0); } @@ -411,12 +388,9 @@ namespace Platform { HANDLE handle = nullptr; - wchar_t fileNameW[AZ_MAX_PATH_LEN]; - size_t numCharsConverted; - if (mbstowcs_s(&numCharsConverted, fileNameW, fileName, AZ_ARRAY_SIZE(fileNameW) - 1) == 0) - { - handle = CreateFileW(fileNameW, 0, FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, OPEN_EXISTING, 0, NULL); - } + AZ::IO::FixedMaxPathWString fileNameW; + AZStd::to_wstring(fileNameW, fileName); + handle = CreateFileW(fileNameW.c_str(), 0, FILE_SHARE_READ | FILE_SHARE_WRITE, nullptr, OPEN_EXISTING, 0, nullptr); if (handle == INVALID_HANDLE_VALUE) { @@ -448,12 +422,9 @@ namespace Platform WIN32_FILE_ATTRIBUTE_DATA data = { 0 }; BOOL result = FALSE; - wchar_t fileNameW[AZ_MAX_PATH_LEN]; - size_t numCharsConverted; - if (mbstowcs_s(&numCharsConverted, fileNameW, fileName, AZ_ARRAY_SIZE(fileNameW) - 1) == 0) - { - result = GetFileAttributesExW(fileNameW, GetFileExInfoStandard, &data); - } + AZ::IO::FixedMaxPathWString fileNameW; + AZStd::to_wstring(fileNameW, fileName); + result = GetFileAttributesExW(fileNameW.c_str(), GetFileExInfoStandard, &data); if (result) { @@ -473,18 +444,11 @@ namespace Platform bool Delete(const char* fileName) { - wchar_t fileNameW[AZ_MAX_PATH_LEN]; - size_t numCharsConverted; - if (mbstowcs_s(&numCharsConverted, fileNameW, fileName, AZ_ARRAY_SIZE(fileNameW) - 1) == 0) - { - if (DeleteFileW(fileNameW) == 0) - { - EBUS_EVENT(FileIOEventBus, OnError, nullptr, fileName, (int)GetLastError()); - return false; - } - } - else + AZ::IO::FixedMaxPathWString fileNameW; + AZStd::to_wstring(fileNameW, fileName); + if (DeleteFileW(fileNameW.c_str()) == 0) { + EBUS_EVENT(FileIOEventBus, OnError, nullptr, fileName, (int)GetLastError()); return false; } @@ -493,20 +457,13 @@ namespace Platform bool Rename(const char* sourceFileName, const char* targetFileName, bool overwrite) { - wchar_t sourceFileNameW[AZ_MAX_PATH_LEN]; - wchar_t targetFileNameW[AZ_MAX_PATH_LEN]; - size_t numCharsConverted; - if (mbstowcs_s(&numCharsConverted, sourceFileNameW, sourceFileName, AZ_ARRAY_SIZE(sourceFileNameW) - 1) == 0 && - mbstowcs_s(&numCharsConverted, targetFileNameW, targetFileName, AZ_ARRAY_SIZE(targetFileNameW) - 1) == 0) - { - if (MoveFileExW(sourceFileNameW, targetFileNameW, overwrite ? MOVEFILE_REPLACE_EXISTING : 0) == 0) - { - EBUS_EVENT(FileIOEventBus, OnError, nullptr, sourceFileName, (int)GetLastError()); - return false; - } - } - else + AZ::IO::FixedMaxPathWString sourceFileNameW; + AZStd::to_wstring(sourceFileNameW, sourceFileName); + AZ::IO::FixedMaxPathWString targetFileNameW; + AZStd::to_wstring(targetFileNameW, targetFileName); + if (MoveFileExW(sourceFileNameW.c_str(), targetFileNameW.c_str(), overwrite ? MOVEFILE_REPLACE_EXISTING : 0) == 0) { + EBUS_EVENT(FileIOEventBus, OnError, nullptr, sourceFileName, (int)GetLastError()); return false; } @@ -543,17 +500,14 @@ namespace Platform { if (dirName) { - wchar_t dirPath[AZ_MAX_PATH_LEN]; - size_t numCharsConverted; - if (mbstowcs_s(&numCharsConverted, dirPath, dirName, AZ_ARRAY_SIZE(dirPath) - 1) == 0) + AZ::IO::FixedMaxPathWString dirNameW; + AZStd::to_wstring(dirNameW, dirName); + bool success = CreateDirRecursive(dirNameW); + if (!success) { - bool success = CreateDirRecursive(dirPath); - if (!success) - { - EBUS_EVENT(FileIOEventBus, OnError, nullptr, dirName, (int)GetLastError()); - } - return success; + EBUS_EVENT(FileIOEventBus, OnError, nullptr, dirName, (int)GetLastError()); } + return success; } return false; } @@ -562,12 +516,9 @@ namespace Platform { if (dirName) { - wchar_t dirNameW[AZ_MAX_PATH_LEN]; - size_t numCharsConverted; - if (mbstowcs_s(&numCharsConverted, dirNameW, dirName, AZ_ARRAY_SIZE(dirNameW) - 1) == 0) - { - return RemoveDirectory(dirNameW) != 0; - } + AZ::IO::FixedMaxPathWString dirNameW; + AZStd::to_wstring(dirNameW, dirName); + return RemoveDirectory(dirNameW.c_str()) != 0; } return false; diff --git a/Code/Framework/AzFramework/AzFramework/IO/LocalFileIO.cpp b/Code/Framework/AzFramework/AzFramework/IO/LocalFileIO.cpp index 4072c17ea0..49e16dcb90 100644 --- a/Code/Framework/AzFramework/AzFramework/IO/LocalFileIO.cpp +++ b/Code/Framework/AzFramework/AzFramework/IO/LocalFileIO.cpp @@ -281,6 +281,14 @@ namespace AZ return SystemFile::Exists(resolvedPath); } + bool LocalFileIO::IsDirectory(const char* filePath) + { + char resolvedPath[AZ_MAX_PATH_LEN]; + ResolvePath(filePath, resolvedPath, AZ_MAX_PATH_LEN); + + return SystemFile::IsDirectory(resolvedPath); + } + void LocalFileIO::CheckInvalidWrite([[maybe_unused]] const char* path) { #if defined(AZ_ENABLE_TRACING) diff --git a/Code/Framework/AzFramework/Platform/Android/AzFramework/IO/LocalFileIO_Android.cpp b/Code/Framework/AzFramework/Platform/Android/AzFramework/IO/LocalFileIO_Android.cpp index b88c7cf25b..b454755cd4 100644 --- a/Code/Framework/AzFramework/Platform/Android/AzFramework/IO/LocalFileIO_Android.cpp +++ b/Code/Framework/AzFramework/Platform/Android/AzFramework/IO/LocalFileIO_Android.cpp @@ -40,26 +40,6 @@ namespace AZ { namespace IO { - bool LocalFileIO::IsDirectory(const char* filePath) - { - ANDROID_IO_PROFILE_SECTION_ARGS("IsDir:%s", filePath); - - char resolvedPath[AZ_MAX_PATH_LEN]; - ResolvePath(filePath, resolvedPath, AZ_MAX_PATH_LEN); - - if (AZ::Android::Utils::IsApkPath(resolvedPath)) - { - return AZ::Android::APKFileHandler::IsDirectory(AZ::Android::Utils::StripApkPrefix(resolvedPath).c_str()); - } - - struct stat result; - if (stat(resolvedPath, &result) == 0) - { - return S_ISDIR(result.st_mode); - } - return false; - } - Result LocalFileIO::Copy(const char* sourceFilePath, const char* destinationFilePath) { char resolvedSourcePath[AZ_MAX_PATH_LEN]; diff --git a/Code/Framework/AzFramework/Platform/Common/UnixLike/AzFramework/IO/LocalFileIO_UnixLike.cpp b/Code/Framework/AzFramework/Platform/Common/UnixLike/AzFramework/IO/LocalFileIO_UnixLike.cpp index c9cbdfbe7d..844464681a 100644 --- a/Code/Framework/AzFramework/Platform/Common/UnixLike/AzFramework/IO/LocalFileIO_UnixLike.cpp +++ b/Code/Framework/AzFramework/Platform/Common/UnixLike/AzFramework/IO/LocalFileIO_UnixLike.cpp @@ -17,19 +17,6 @@ namespace AZ { namespace IO { - bool LocalFileIO::IsDirectory(const char* filePath) - { - char resolvedPath[AZ_MAX_PATH_LEN] = {0}; - ResolvePath(filePath, resolvedPath, AZ_MAX_PATH_LEN); - - struct stat result; - if (stat(resolvedPath, &result) == 0) - { - return S_ISDIR(result.st_mode); - } - return false; - } - Result LocalFileIO::Copy(const char* sourceFilePath, const char* destinationFilePath) { char resolvedSourceFilePath[AZ_MAX_PATH_LEN] = {0}; diff --git a/Code/Framework/AzFramework/Platform/Common/WinAPI/AzFramework/IO/LocalFileIO_WinAPI.cpp b/Code/Framework/AzFramework/Platform/Common/WinAPI/AzFramework/IO/LocalFileIO_WinAPI.cpp index 5a56a360a8..64787d4951 100644 --- a/Code/Framework/AzFramework/Platform/Common/WinAPI/AzFramework/IO/LocalFileIO_WinAPI.cpp +++ b/Code/Framework/AzFramework/Platform/Common/WinAPI/AzFramework/IO/LocalFileIO_WinAPI.cpp @@ -15,22 +15,6 @@ namespace AZ { namespace IO { - bool LocalFileIO::IsDirectory(const char* filePath) - { - char resolvedPath[AZ_MAX_PATH_LEN]; - ResolvePath(filePath, resolvedPath, AZ_MAX_PATH_LEN); - - wchar_t resolvedPathW[AZ_MAX_PATH_LEN]; - AZStd::to_wstring(resolvedPathW, AZ_MAX_PATH_LEN, resolvedPath); - DWORD fileAttributes = GetFileAttributesW(resolvedPathW); - if (fileAttributes == INVALID_FILE_ATTRIBUTES) - { - return false; - } - - return (fileAttributes & FILE_ATTRIBUTE_DIRECTORY) != 0; - } - Result LocalFileIO::FindFiles(const char* filePath, const char* filter, FindFilesCallbackType callback) { char resolvedPath[AZ_MAX_PATH_LEN]; diff --git a/Code/Framework/AzFramework/Tests/FileIO.cpp b/Code/Framework/AzFramework/Tests/FileIO.cpp index fb95512968..dbee109978 100644 --- a/Code/Framework/AzFramework/Tests/FileIO.cpp +++ b/Code/Framework/AzFramework/Tests/FileIO.cpp @@ -14,7 +14,6 @@ #include #include #include -#include #include #include #include @@ -30,21 +29,6 @@ using namespace AZ; using namespace AZ::IO; using namespace AZ::Debug; -namespace PathUtil -{ - AZStd::string AddSlash(const AZStd::string& path) - { - if (path.empty() || path[path.length() - 1] == '/') - { - return path; - } - if (path[path.length() - 1] == '\\') - { - return path.substr(0, path.length() - 1) + "/"; - } - return path + "/"; - } -} namespace UnitTest { @@ -161,15 +145,16 @@ namespace UnitTest : public ScopedAllocatorSetupFixture { public: - AZStd::string m_root; - AZStd::string folderName; - AZStd::string deepFolder; - AZStd::string extraFolder; + AZ::Test::ScopedAutoTempDirectory m_tempDir; + AZ::IO::Path m_root; + AZ::IO::Path m_folderName; + AZ::IO::Path m_deepFolder; + AZ::IO::Path m_extraFolder; - AZStd::string fileRoot; - AZStd::string file01Name; - AZStd::string file02Name; - AZStd::string file03Name; + AZ::IO::Path m_fileRoot; + AZ::IO::Path m_file01Name; + AZ::IO::Path m_file02Name; + AZ::IO::Path m_file03Name; int m_randomFolderKey = 0; FolderFixture() @@ -179,43 +164,13 @@ namespace UnitTest void ChooseRandomFolder() { - char currentDir[AZ_MAX_PATH_LEN]; - AZ::Utils::GetExecutableDirectory(currentDir, AZ_MAX_PATH_LEN); - - folderName = currentDir; - folderName.append("/temp"); - m_root = folderName; - if (folderName.size() > 0) - { - folderName = PathUtil::AddSlash(folderName); - } - - AZStd::string tempName = AZStd::string::format("tmp%08x", m_randomFolderKey); - folderName.append(tempName.c_str()); - folderName = PathUtil::AddSlash(folderName); - AZStd::replace(folderName.begin(), folderName.end(), '\\', '/'); - - // Make sure the drive letter is capitalized - if (folderName.size() > 2) - { - if (folderName[1] == ':') - { - folderName[0] = static_cast(toupper(folderName[0])); - } - } - - deepFolder = folderName; - deepFolder.append("test"); - - deepFolder = PathUtil::AddSlash(deepFolder); - deepFolder.append("subdir"); - - extraFolder = deepFolder; - extraFolder = PathUtil::AddSlash(extraFolder); - extraFolder.append("subdir2"); + m_root = m_tempDir.GetDirectory(); + m_folderName = m_root / AZStd::string::format("tmp%08x", m_randomFolderKey); + m_deepFolder = m_folderName / "test" / "subdir"; + m_extraFolder = m_deepFolder / "subdir2"; // make a couple files there, and in the root: - fileRoot = PathUtil::AddSlash(extraFolder); + m_fileRoot = m_extraFolder; } void SetUp() override @@ -229,37 +184,33 @@ namespace UnitTest { ChooseRandomFolder(); ++m_randomFolderKey; - } while (local.IsDirectory(fileRoot.c_str())); + } while (local.IsDirectory(m_fileRoot.c_str())); - file01Name = fileRoot + "file01.txt"; - file02Name = fileRoot + "file02.asdf"; - file03Name = fileRoot + "test123.wha"; + m_file01Name = m_fileRoot / "file01.txt"; + m_file02Name = m_fileRoot / "file02.asdf"; + m_file03Name = m_fileRoot / "test123.wha"; } void TearDown() override { - if ((!folderName.empty())&&(strstr(folderName.c_str(), "/temp") != nullptr)) - { - // cleanup! - LocalFileIO local; - local.DestroyPath(folderName.c_str()); - } } void CreateTestFiles() { + constexpr auto openMode = SystemFile::OpenMode::SF_OPEN_WRITE_ONLY + | SystemFile::OpenMode::SF_OPEN_CREATE + | SystemFile::OpenMode::SF_OPEN_CREATE_NEW; + constexpr AZStd::string_view testContent("this is just a test"); + LocalFileIO local; - AZ_TEST_ASSERT(local.CreatePath(fileRoot.c_str())); - AZ_TEST_ASSERT(local.IsDirectory(fileRoot.c_str())); - for (const AZStd::string& filename : { file01Name, file02Name, file03Name }) + AZ_TEST_ASSERT(local.CreatePath(m_fileRoot.c_str())); + AZ_TEST_ASSERT(local.IsDirectory(m_fileRoot.c_str())); + for (const AZ::IO::Path& filename : { m_file01Name, m_file02Name, m_file03Name }) { -#ifdef AZ_COMPILER_MSVC - FILE* tempFile; - fopen_s(&tempFile, filename.c_str(), "wb"); -#else - FILE* tempFile = fopen(filename.c_str(), "wb"); -#endif - fwrite("this is just a test", 1, 19, tempFile); - fclose(tempFile); + SystemFile tempFile; + tempFile.Open(filename.c_str(), openMode); + + tempFile.Write(testContent.data(), testContent.size()); + tempFile.Close(); } } }; @@ -272,28 +223,23 @@ namespace UnitTest { LocalFileIO local; - AZ_TEST_ASSERT(!local.Exists(folderName.c_str())); + AZ_TEST_ASSERT(!local.Exists(m_folderName.c_str())); - AZStd::string longPathCreateTest = folderName; - longPathCreateTest.append("one"); - longPathCreateTest = PathUtil::AddSlash(longPathCreateTest); - longPathCreateTest.append("two"); - longPathCreateTest = PathUtil::AddSlash(longPathCreateTest); - longPathCreateTest.append("three"); + AZ::IO::Path longPathCreateTest = m_folderName / "one" / "two" / "three"; AZ_TEST_ASSERT(!local.Exists(longPathCreateTest.c_str())); AZ_TEST_ASSERT(!local.IsDirectory(longPathCreateTest.c_str())); AZ_TEST_ASSERT(local.CreatePath(longPathCreateTest.c_str())); AZ_TEST_ASSERT(local.IsDirectory(longPathCreateTest.c_str())); - AZ_TEST_ASSERT(!local.Exists(deepFolder.c_str())); - AZ_TEST_ASSERT(!local.IsDirectory(deepFolder.c_str())); - AZ_TEST_ASSERT(local.CreatePath(deepFolder.c_str())); - AZ_TEST_ASSERT(local.IsDirectory(deepFolder.c_str())); + AZ_TEST_ASSERT(!local.Exists(m_deepFolder.c_str())); + AZ_TEST_ASSERT(!local.IsDirectory(m_deepFolder.c_str())); + AZ_TEST_ASSERT(local.CreatePath(m_deepFolder.c_str())); + AZ_TEST_ASSERT(local.IsDirectory(m_deepFolder.c_str())); - AZ_TEST_ASSERT(local.Exists(deepFolder.c_str())); - AZ_TEST_ASSERT(local.CreatePath(deepFolder.c_str())); - AZ_TEST_ASSERT(local.Exists(deepFolder.c_str())); + AZ_TEST_ASSERT(local.Exists(m_deepFolder.c_str())); + AZ_TEST_ASSERT(local.CreatePath(m_deepFolder.c_str())); + AZ_TEST_ASSERT(local.Exists(m_deepFolder.c_str())); } }; @@ -310,16 +256,19 @@ namespace UnitTest { LocalFileIO local; - AZ_TEST_ASSERT(!local.Exists(fileRoot.c_str())); - AZ_TEST_ASSERT(!local.IsDirectory(fileRoot.c_str())); - AZ_TEST_ASSERT(local.CreatePath(fileRoot.c_str())); - AZ_TEST_ASSERT(local.IsDirectory(fileRoot.c_str())); + AZ_TEST_ASSERT(!local.Exists(m_fileRoot.c_str())); + AZ_TEST_ASSERT(!local.IsDirectory(m_fileRoot.c_str())); + AZ_TEST_ASSERT(local.CreatePath(m_fileRoot.c_str())); + AZ_TEST_ASSERT(local.IsDirectory(m_fileRoot.c_str())); - FILE* tempFile = nullptr; - azfopen(&tempFile, file01Name.c_str(), "wb"); - - fwrite("this is just a test", 1, 19, tempFile); - fclose(tempFile); + constexpr auto openMode = SystemFile::OpenMode::SF_OPEN_WRITE_ONLY + | SystemFile::OpenMode::SF_OPEN_CREATE + | SystemFile::OpenMode::SF_OPEN_CREATE_NEW; + SystemFile tempFile; + tempFile.Open(m_file01Name.c_str(), openMode); + constexpr AZStd::string_view testContent("this is just a test"); + tempFile.Write(testContent.data(), testContent.size()); + tempFile.Close(); AZ::IO::HandleType fileHandle = AZ::IO::InvalidHandle; AZ_TEST_ASSERT(!local.Open("", AZ::IO::OpenMode::ModeWrite, fileHandle)); @@ -327,12 +276,12 @@ namespace UnitTest // test size without opening: AZ::u64 fs = 0; - AZ_TEST_ASSERT(local.Size(file01Name.c_str(), fs)); + AZ_TEST_ASSERT(local.Size(m_file01Name.c_str(), fs)); AZ_TEST_ASSERT(fs == 19); fileHandle = AZ::IO::InvalidHandle; - AZ::u64 modTimeA = local.ModificationTime(file01Name.c_str()); + AZ::u64 modTimeA = local.ModificationTime(m_file01Name.c_str()); AZ_TEST_ASSERT(modTimeA != 0); // test invalid handle ops: @@ -344,14 +293,14 @@ namespace UnitTest AZ_TEST_ASSERT(!local.Read(fileHandle, nullptr, 0, false)); AZ_TEST_ASSERT(!local.Tell(fileHandle, fs)); - AZ_TEST_ASSERT(!local.Exists((file01Name + "notexist").c_str())); + AZ_TEST_ASSERT(!local.Exists((m_file01Name.Native() + "notexist").c_str())); - AZ_TEST_ASSERT(local.Exists(file01Name.c_str())); - AZ_TEST_ASSERT(!local.IsReadOnly(file01Name.c_str())); - AZ_TEST_ASSERT(!local.IsDirectory(file01Name.c_str())); + AZ_TEST_ASSERT(local.Exists(m_file01Name.c_str())); + AZ_TEST_ASSERT(!local.IsReadOnly(m_file01Name.c_str())); + AZ_TEST_ASSERT(!local.IsDirectory(m_file01Name.c_str())); // test reads and seeks. - AZ_TEST_ASSERT(local.Open(file01Name.c_str(), AZ::IO::OpenMode::ModeRead, fileHandle)); + AZ_TEST_ASSERT(local.Open(m_file01Name.c_str(), AZ::IO::OpenMode::ModeRead, fileHandle)); AZ_TEST_ASSERT(fileHandle != AZ::IO::InvalidHandle); // use this again later... @@ -368,7 +317,7 @@ namespace UnitTest // test size without opening, after its already open: fs = 0; - AZ_TEST_ASSERT(local.Size(file01Name.c_str(), fs)); + AZ_TEST_ASSERT(local.Size(m_file01Name.c_str(), fs)); AZ_TEST_ASSERT(fs == 19); AZ::u64 offs = 0; @@ -442,22 +391,22 @@ namespace UnitTest #if AZ_TRAIT_AZFRAMEWORKTEST_PERFORM_CHMOD_TEST #if AZ_TRAIT_USE_WINDOWS_FILE_API - _chmod(file01Name.c_str(), _S_IREAD); + _chmod(m_file01Name.c_str(), _S_IREAD); #else - chmod(file01Name.c_str(), S_IRUSR | S_IRGRP | S_IROTH); + chmod(m_file01Name.c_str(), S_IRUSR | S_IRGRP | S_IROTH); #endif - AZ_TEST_ASSERT(local.IsReadOnly(file01Name.c_str())); + AZ_TEST_ASSERT(local.IsReadOnly(m_file01Name.c_str())); #if AZ_TRAIT_USE_WINDOWS_FILE_API - _chmod(file01Name.c_str(), _S_IREAD | _S_IWRITE); + _chmod(m_file01Name.c_str(), _S_IREAD | _S_IWRITE); #else - chmod(file01Name.c_str(), S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH); + chmod(m_file01Name.c_str(), S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH); #endif #endif - AZ_TEST_ASSERT(!local.IsReadOnly(file01Name.c_str())); + AZ_TEST_ASSERT(!local.IsReadOnly(m_file01Name.c_str())); } }; @@ -474,14 +423,14 @@ namespace UnitTest { LocalFileIO local; - AZ_TEST_ASSERT(local.CreatePath(fileRoot.c_str())); - AZ_TEST_ASSERT(local.IsDirectory(fileRoot.c_str())); + AZ_TEST_ASSERT(local.CreatePath(m_fileRoot.c_str())); + AZ_TEST_ASSERT(local.IsDirectory(m_fileRoot.c_str())); { #ifdef AZ_COMPILER_MSVC FILE* tempFile; - fopen_s(&tempFile, file01Name.c_str(), "wb"); + fopen_s(&tempFile, m_file01Name.c_str(), "wb"); #else - FILE* tempFile = fopen(file01Name.c_str(), "wb"); + FILE* tempFile = fopen(m_file01Name.c_str(), "wb"); #endif fwrite("this is just a test", 1, 19, tempFile); fclose(tempFile); @@ -489,47 +438,47 @@ namespace UnitTest // make sure attributes are copied (such as modtime) even if they're copied: AZStd::this_thread::sleep_for(AZStd::chrono::milliseconds(1500)); - AZ_TEST_ASSERT(local.Copy(file01Name.c_str(), file02Name.c_str())); + AZ_TEST_ASSERT(local.Copy(m_file01Name.c_str(), m_file02Name.c_str())); AZStd::this_thread::sleep_for(AZStd::chrono::milliseconds(1500)); - AZ_TEST_ASSERT(local.Copy(file01Name.c_str(), file03Name.c_str())); + AZ_TEST_ASSERT(local.Copy(m_file01Name.c_str(), m_file03Name.c_str())); - AZ_TEST_ASSERT(local.Exists(file01Name.c_str())); - AZ_TEST_ASSERT(local.Exists(file02Name.c_str())); - AZ_TEST_ASSERT(local.Exists(file03Name.c_str())); - AZ_TEST_ASSERT(!local.DestroyPath(file01Name.c_str())); // you may not destroy files. - AZ_TEST_ASSERT(!local.DestroyPath(file02Name.c_str())); - AZ_TEST_ASSERT(!local.DestroyPath(file03Name.c_str())); - AZ_TEST_ASSERT(local.Exists(file01Name.c_str())); - AZ_TEST_ASSERT(local.Exists(file02Name.c_str())); - AZ_TEST_ASSERT(local.Exists(file03Name.c_str())); + AZ_TEST_ASSERT(local.Exists(m_file01Name.c_str())); + AZ_TEST_ASSERT(local.Exists(m_file02Name.c_str())); + AZ_TEST_ASSERT(local.Exists(m_file03Name.c_str())); + AZ_TEST_ASSERT(!local.DestroyPath(m_file01Name.c_str())); // you may not destroy files. + AZ_TEST_ASSERT(!local.DestroyPath(m_file02Name.c_str())); + AZ_TEST_ASSERT(!local.DestroyPath(m_file03Name.c_str())); + AZ_TEST_ASSERT(local.Exists(m_file01Name.c_str())); + AZ_TEST_ASSERT(local.Exists(m_file02Name.c_str())); + AZ_TEST_ASSERT(local.Exists(m_file03Name.c_str())); AZ::u64 f1s = 0; AZ::u64 f2s = 0; AZ::u64 f3s = 0; - AZ_TEST_ASSERT(local.Size(file01Name.c_str(), f1s)); - AZ_TEST_ASSERT(local.Size(file02Name.c_str(), f2s)); - AZ_TEST_ASSERT(local.Size(file03Name.c_str(), f3s)); + AZ_TEST_ASSERT(local.Size(m_file01Name.c_str(), f1s)); + AZ_TEST_ASSERT(local.Size(m_file02Name.c_str(), f2s)); + AZ_TEST_ASSERT(local.Size(m_file03Name.c_str(), f3s)); AZ_TEST_ASSERT(f1s == f2s); AZ_TEST_ASSERT(f1s == f3s); // Copying over top other files is allowed SystemFile file; - EXPECT_TRUE(file.Open(file01Name.c_str(), SystemFile::SF_OPEN_WRITE_ONLY)); + EXPECT_TRUE(file.Open(m_file01Name.c_str(), SystemFile::SF_OPEN_WRITE_ONLY)); file.Write("this is just a test that is longer", 34); file.Close(); // make sure attributes are copied (such as modtime) even if they're copied: AZStd::this_thread::sleep_for(AZStd::chrono::milliseconds(1500)); - EXPECT_TRUE(local.Copy(file01Name.c_str(), file02Name.c_str())); + EXPECT_TRUE(local.Copy(m_file01Name.c_str(), m_file02Name.c_str())); f1s = 0; f2s = 0; f3s = 0; - EXPECT_TRUE(local.Size(file01Name.c_str(), f1s)); - EXPECT_TRUE(local.Size(file02Name.c_str(), f2s)); - EXPECT_TRUE(local.Size(file03Name.c_str(), f3s)); + EXPECT_TRUE(local.Size(m_file01Name.c_str(), f1s)); + EXPECT_TRUE(local.Size(m_file02Name.c_str(), f2s)); + EXPECT_TRUE(local.Size(m_file03Name.c_str(), f3s)); EXPECT_EQ(f1s, f2s); EXPECT_NE(f1s, f3s); } @@ -552,37 +501,37 @@ namespace UnitTest AZ::u64 modTimeC = 0; AZ::u64 modTimeD = 0; - modTimeC = local.ModificationTime(file02Name.c_str()); - modTimeD = local.ModificationTime(file03Name.c_str()); + modTimeC = local.ModificationTime(m_file02Name.c_str()); + modTimeD = local.ModificationTime(m_file03Name.c_str()); // make sure modtimes are in ascending order (at least) AZ_TEST_ASSERT(modTimeD >= modTimeC); // now touch some of the files. This is also how we test append mode, and write mode. AZ::IO::HandleType fileHandle = AZ::IO::InvalidHandle; - AZ_TEST_ASSERT(local.Open(file02Name.c_str(), AZ::IO::OpenMode::ModeAppend | AZ::IO::OpenMode::ModeBinary, fileHandle)); + AZ_TEST_ASSERT(local.Open(m_file02Name.c_str(), AZ::IO::OpenMode::ModeAppend | AZ::IO::OpenMode::ModeBinary, fileHandle)); AZ_TEST_ASSERT(fileHandle != AZ::IO::InvalidHandle); AZ_TEST_ASSERT(local.Write(fileHandle, "more", 4)); AZ_TEST_ASSERT(local.Close(fileHandle)); AZStd::this_thread::sleep_for(AZStd::chrono::milliseconds(1500)); // No-append-mode - AZ_TEST_ASSERT(local.Open(file03Name.c_str(), AZ::IO::OpenMode::ModeWrite | AZ::IO::OpenMode::ModeBinary, fileHandle)); + AZ_TEST_ASSERT(local.Open(m_file03Name.c_str(), AZ::IO::OpenMode::ModeWrite | AZ::IO::OpenMode::ModeBinary, fileHandle)); AZ_TEST_ASSERT(fileHandle != AZ::IO::InvalidHandle); AZ_TEST_ASSERT(local.Write(fileHandle, "more", 4)); AZ_TEST_ASSERT(local.Close(fileHandle)); - modTimeC = local.ModificationTime(file02Name.c_str()); - modTimeD = local.ModificationTime(file03Name.c_str()); + modTimeC = local.ModificationTime(m_file02Name.c_str()); + modTimeD = local.ModificationTime(m_file03Name.c_str()); AZ_TEST_ASSERT(modTimeD > modTimeC); AZ::u64 f1s = 0; AZ::u64 f2s = 0; AZ::u64 f3s = 0; - AZ_TEST_ASSERT(local.Size(file01Name.c_str(), f1s)); - AZ_TEST_ASSERT(local.Size(file02Name.c_str(), f2s)); - AZ_TEST_ASSERT(local.Size(file03Name.c_str(), f3s)); + AZ_TEST_ASSERT(local.Size(m_file01Name.c_str(), f1s)); + AZ_TEST_ASSERT(local.Size(m_file02Name.c_str(), f2s)); + AZ_TEST_ASSERT(local.Size(m_file03Name.c_str(), f3s)); AZ_TEST_ASSERT(f2s == f1s + 4); AZ_TEST_ASSERT(f3s == 4); } @@ -603,8 +552,8 @@ namespace UnitTest CreateTestFiles(); - AZStd::vector resultFiles; - bool foundOK = local.FindFiles(fileRoot.c_str(), "*", + AZStd::vector resultFiles; + bool foundOK = local.FindFiles(m_fileRoot.c_str(), "*", [&](const char* filePath) -> bool { resultFiles.push_back(filePath); @@ -616,7 +565,7 @@ namespace UnitTest resultFiles.clear(); - foundOK = local.FindFiles(fileRoot.c_str(), "*", + foundOK = local.FindFiles(m_fileRoot.c_str(), "*", [&](const char* filePath) -> bool { resultFiles.push_back(filePath); @@ -627,7 +576,7 @@ namespace UnitTest AZ_TEST_ASSERT(resultFiles.size() == 3); // note: following tests accumulate more files without clearing resultfiles. - foundOK = local.FindFiles(fileRoot.c_str(), "*.txt", + foundOK = local.FindFiles(m_fileRoot.c_str(), "*.txt", [&](const char* filePath) -> bool { resultFiles.push_back(filePath); @@ -637,7 +586,7 @@ namespace UnitTest AZ_TEST_ASSERT(foundOK); AZ_TEST_ASSERT(resultFiles.size() == 4); - foundOK = local.FindFiles(fileRoot.c_str(), "file*.asdf", + foundOK = local.FindFiles(m_fileRoot.c_str(), "file*.asdf", [&](const char* filePath) -> bool { resultFiles.push_back(filePath); @@ -647,7 +596,7 @@ namespace UnitTest AZ_TEST_ASSERT(foundOK); AZ_TEST_ASSERT(resultFiles.size() == 5); - foundOK = local.FindFiles(fileRoot.c_str(), "asaf.asdf", + foundOK = local.FindFiles(m_fileRoot.c_str(), "asaf.asdf", [&](const char* filePath) -> bool { resultFiles.push_back(filePath); @@ -660,7 +609,7 @@ namespace UnitTest resultFiles.clear(); // test to make sure directories show up: - foundOK = local.FindFiles(deepFolder.c_str(), "*", + foundOK = local.FindFiles(m_deepFolder.c_str(), "*", [&](const char* filePath) -> bool { resultFiles.push_back(filePath); @@ -668,11 +617,11 @@ namespace UnitTest }); // canonicalize the name in the same way that find does. - //AZStd::replace() extraFolder.replace('\\', '/'); FIXME PPATEL + //AZStd::replace() m_extraFolder.replace('\\', '/'); FIXME PPATEL AZ_TEST_ASSERT(foundOK); AZ_TEST_ASSERT(resultFiles.size() == 1); - AZ_TEST_ASSERT(resultFiles[0] == extraFolder); + AZ_TEST_ASSERT(resultFiles[0] == m_extraFolder); resultFiles.clear(); foundOK = local.FindFiles("o:137787621!@#$%^&&**())_+[])_", "asaf.asdf", [&](const char* filePath) -> bool @@ -684,13 +633,13 @@ namespace UnitTest AZ_TEST_ASSERT(!foundOK); AZ_TEST_ASSERT(resultFiles.size() == 0); - AZStd::string file04Name = fileRoot + "test.wha"; + AZ::IO::Path file04Name = m_fileRoot / "test.wha"; // test rename - AZ_TEST_ASSERT(local.Rename(file03Name.c_str(), file04Name.c_str())); - AZ_TEST_ASSERT(!local.Rename(file03Name.c_str(), file04Name.c_str())); + AZ_TEST_ASSERT(local.Rename(m_file03Name.c_str(), file04Name.c_str())); + AZ_TEST_ASSERT(!local.Rename(m_file03Name.c_str(), file04Name.c_str())); AZ_TEST_ASSERT(local.Rename(file04Name.c_str(), file04Name.c_str())); // this is valid and ok AZ_TEST_ASSERT(local.Exists(file04Name.c_str())); - AZ_TEST_ASSERT(!local.Exists(file03Name.c_str())); + AZ_TEST_ASSERT(!local.Exists(m_file03Name.c_str())); AZ_TEST_ASSERT(!local.IsDirectory(file04Name.c_str())); AZ::u64 f3s = 0; @@ -698,8 +647,8 @@ namespace UnitTest AZ_TEST_ASSERT(f3s == 19); // deep destroy directory: - AZ_TEST_ASSERT(local.DestroyPath(folderName.c_str())); - AZ_TEST_ASSERT(!local.Exists(folderName.c_str())); + AZ_TEST_ASSERT(local.DestroyPath(m_folderName.c_str())); + AZ_TEST_ASSERT(!local.Exists(m_folderName.c_str())); } }; @@ -715,7 +664,7 @@ namespace UnitTest AZ::IO::LocalFileIO local; // test aliases - local.SetAlias("@test@", folderName.c_str()); + local.SetAlias("@test@", m_folderName.c_str()); const char* testDest1 = local.GetAlias("@test@"); AZ_TEST_ASSERT(testDest1 != nullptr); const char* testDest2 = local.GetAlias("@NOPE@"); @@ -725,18 +674,18 @@ namespace UnitTest // test resolving const char* aliasTestPath = "@test@\\some\\path\\somefile.txt"; - char aliasResolvedPath[AZ_MAX_PATH_LEN]; - bool resolveDidWork = local.ResolvePath(aliasTestPath, aliasResolvedPath, AZ_MAX_PATH_LEN); + char aliasResolvedPath[AZ::IO::MaxPathLength]; + bool resolveDidWork = local.ResolvePath(aliasTestPath, aliasResolvedPath, AZ::IO::MaxPathLength); AZ_TEST_ASSERT(resolveDidWork); - AZStd::string expectedResolvedPath = folderName + "some/path/somefile.txt"; + AZ::IO::Path expectedResolvedPath = m_folderName / "some/path/somefile.txt"; AZ_TEST_ASSERT(aliasResolvedPath == expectedResolvedPath); // more resolve path tests with invalid inputs const char* testPath = nullptr; char* testResolvedPath = nullptr; - resolveDidWork = local.ResolvePath(testPath, aliasResolvedPath, AZ_MAX_PATH_LEN); + resolveDidWork = local.ResolvePath(testPath, aliasResolvedPath, AZ::IO::MaxPathLength); AZ_TEST_ASSERT(!resolveDidWork); - resolveDidWork = local.ResolvePath(aliasTestPath, testResolvedPath, AZ_MAX_PATH_LEN); + resolveDidWork = local.ResolvePath(aliasTestPath, testResolvedPath, AZ::IO::MaxPathLength); AZ_TEST_ASSERT(!resolveDidWork); resolveDidWork = local.ResolvePath(aliasTestPath, aliasResolvedPath, 0); AZ_TEST_ASSERT(!resolveDidWork); @@ -751,7 +700,7 @@ namespace UnitTest // Test that sending in a too small output path fails, // if the output buffer is too small to hold the resolved path - size_t SMALLER_THAN_FINAL_RESOLVED_PATH = expectedResolvedPath.length() - 1; + size_t SMALLER_THAN_FINAL_RESOLVED_PATH = expectedResolvedPath.Native().length() - 1; AZ_TEST_START_TRACE_SUPPRESSION; resolveDidWork = local.ResolvePath(aliasTestPath, aliasResolvedPath, SMALLER_THAN_FINAL_RESOLVED_PATH); AZ_TEST_STOP_TRACE_SUPPRESSION(1); @@ -766,22 +715,23 @@ namespace UnitTest TEST_F(AliasTest, ResolvePath_PathViewOverload_Succeeds) { AZ::IO::LocalFileIO local; - local.SetAlias("@test@", folderName.c_str()); + local.SetAlias("@test@", m_folderName.c_str()); AZ::IO::PathView aliasTestPath = "@test@\\some\\path\\somefile.txt"; AZ::IO::FixedMaxPath aliasResolvedPath; ASSERT_TRUE(local.ResolvePath(aliasResolvedPath, aliasTestPath)); - const auto expectedResolvedPath = AZ::IO::FixedMaxPathString::format("%ssome/path/somefile.txt", folderName.c_str()); - EXPECT_STREQ(expectedResolvedPath.c_str(), aliasResolvedPath.c_str()); + AZ::IO::Path expectedResolvedPath = m_folderName / "some" / "path" / "somefile.txt"; + + EXPECT_EQ(expectedResolvedPath, aliasResolvedPath); AZStd::optional optionalResolvedPath = local.ResolvePath(aliasTestPath); ASSERT_TRUE(optionalResolvedPath); - EXPECT_STREQ(expectedResolvedPath.c_str(), optionalResolvedPath->c_str()); + EXPECT_EQ(expectedResolvedPath, optionalResolvedPath.value()); } TEST_F(AliasTest, ResolvePath_PathViewOverloadWithEmptyPath_Fails) { AZ::IO::LocalFileIO local; - local.SetAlias("@test@", folderName.c_str()); + local.SetAlias("@test@", m_folderName.c_str()); AZ::IO::FixedMaxPath aliasResolvedPath; EXPECT_FALSE(local.ResolvePath(aliasResolvedPath, {})); } @@ -860,24 +810,23 @@ namespace UnitTest { LocalFileIO localFileIO; AZ::IO::FileIOBase::SetInstance(&localFileIO); - AZStd::string path; - AzFramework::StringFunc::Path::GetFullPath(file01Name.c_str(), path); + AZ::IO::Path path = m_file01Name.ParentPath(); AZ_TEST_ASSERT(localFileIO.CreatePath(path.c_str())); - AzFramework::StringFunc::Path::GetFullPath(file02Name.c_str(), path); + path = m_file01Name.ParentPath(); AZ_TEST_ASSERT(localFileIO.CreatePath(path.c_str())); AZ::IO::HandleType fileHandle = AZ::IO::InvalidHandle; - localFileIO.Open(file01Name.c_str(), OpenMode::ModeWrite | OpenMode::ModeText, fileHandle); + localFileIO.Open(m_file01Name.c_str(), OpenMode::ModeWrite | OpenMode::ModeText, fileHandle); localFileIO.Write(fileHandle, "DummyFile", 9); localFileIO.Close(fileHandle); AZ::IO::HandleType fileHandle1 = AZ::IO::InvalidHandle; - localFileIO.Open(file02Name.c_str(), OpenMode::ModeWrite | OpenMode::ModeText, fileHandle1); + localFileIO.Open(m_file02Name.c_str(), OpenMode::ModeWrite | OpenMode::ModeText, fileHandle1); localFileIO.Write(fileHandle1, "TestFile", 8); localFileIO.Close(fileHandle1); fileHandle1 = AZ::IO::InvalidHandle; - localFileIO.Open(file02Name.c_str(), OpenMode::ModeRead | OpenMode::ModeText, fileHandle1); + localFileIO.Open(m_file02Name.c_str(), OpenMode::ModeRead | OpenMode::ModeText, fileHandle1); static const size_t testStringLen = 256; char testString[testStringLen] = { 0 }; localFileIO.Read(fileHandle1, testString, testStringLen); @@ -885,50 +834,50 @@ namespace UnitTest AZ_TEST_ASSERT(strncmp(testString, "TestFile", 8) == 0); // try swapping files when none of the files are in use - AZ_TEST_ASSERT(AZ::IO::SmartMove(file01Name.c_str(), file02Name.c_str())); + AZ_TEST_ASSERT(AZ::IO::SmartMove(m_file01Name.c_str(), m_file02Name.c_str())); fileHandle1 = AZ::IO::InvalidHandle; - localFileIO.Open(file02Name.c_str(), OpenMode::ModeRead | OpenMode::ModeText, fileHandle1); + localFileIO.Open(m_file02Name.c_str(), OpenMode::ModeRead | OpenMode::ModeText, fileHandle1); testString[0] = '\0'; localFileIO.Read(fileHandle1, testString, testStringLen); localFileIO.Close(fileHandle1); AZ_TEST_ASSERT(strncmp(testString, "DummyFile", 9) == 0); //try swapping files when source file is not present, this should fail - AZ_TEST_ASSERT(!AZ::IO::SmartMove(file01Name.c_str(), file02Name.c_str())); + AZ_TEST_ASSERT(!AZ::IO::SmartMove(m_file01Name.c_str(), m_file02Name.c_str())); fileHandle = AZ::IO::InvalidHandle; - localFileIO.Open(file01Name.c_str(), OpenMode::ModeWrite | OpenMode::ModeText, fileHandle); + localFileIO.Open(m_file01Name.c_str(), OpenMode::ModeWrite | OpenMode::ModeText, fileHandle); localFileIO.Write(fileHandle, "TestFile", 8); localFileIO.Close(fileHandle); #if AZ_TRAIT_AZFRAMEWORKTEST_MOVE_WHILE_OPEN fileHandle1 = AZ::IO::InvalidHandle; - localFileIO.Open(file02Name.c_str(), OpenMode::ModeRead | OpenMode::ModeText, fileHandle1); + localFileIO.Open(m_file02Name.c_str(), OpenMode::ModeRead | OpenMode::ModeText, fileHandle1); testString[0] = '\0'; localFileIO.Read(fileHandle1, testString, testStringLen); // try swapping files when the destination file is open for read only, // since window is unable to move files that are open for read, this will fail. - AZ_TEST_ASSERT(!AZ::IO::SmartMove(file01Name.c_str(), file02Name.c_str())); + AZ_TEST_ASSERT(!AZ::IO::SmartMove(m_file01Name.c_str(), m_file02Name.c_str())); localFileIO.Close(fileHandle1); #endif fileHandle = AZ::IO::InvalidHandle; - localFileIO.Open(file01Name.c_str(), OpenMode::ModeRead | OpenMode::ModeText, fileHandle); + localFileIO.Open(m_file01Name.c_str(), OpenMode::ModeRead | OpenMode::ModeText, fileHandle); // try swapping files when the source file is open for read only - AZ_TEST_ASSERT(AZ::IO::SmartMove(file01Name.c_str(), file02Name.c_str())); + AZ_TEST_ASSERT(AZ::IO::SmartMove(m_file01Name.c_str(), m_file02Name.c_str())); localFileIO.Close(fileHandle); fileHandle1 = AZ::IO::InvalidHandle; - localFileIO.Open(file02Name.c_str(), OpenMode::ModeRead | OpenMode::ModeText, fileHandle1); + localFileIO.Open(m_file02Name.c_str(), OpenMode::ModeRead | OpenMode::ModeText, fileHandle1); testString[0] = '\0'; localFileIO.Read(fileHandle1, testString, testStringLen); AZ_TEST_ASSERT(strncmp(testString, "TestFile", 8) == 0); localFileIO.Close(fileHandle1); - localFileIO.Remove(file01Name.c_str()); - localFileIO.Remove(file02Name.c_str()); + localFileIO.Remove(m_file01Name.c_str()); + localFileIO.Remove(m_file02Name.c_str()); localFileIO.DestroyPath(m_root.c_str()); AZ::IO::FileIOBase::SetInstance(nullptr); diff --git a/Code/Framework/AzTest/AzTest/Printers.cpp b/Code/Framework/AzTest/AzTest/Printers.cpp new file mode 100644 index 0000000000..dc639d87d1 --- /dev/null +++ b/Code/Framework/AzTest/AzTest/Printers.cpp @@ -0,0 +1,27 @@ +/* + * 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 + * + */ +#include +#include + +namespace AZ::IO +{ + void PrintTo(const AZ::IO::PathView& path, ::std::ostream* os) + { + *os << "path: " << AZ::IO::Path(path.Native(), AZ::IO::PosixPathSeparator).MakePreferred().c_str(); + } + + void PrintTo(const AZ::IO::Path& path, ::std::ostream* os) + { + *os << "path: " << AZ::IO::Path(path.Native(), AZ::IO::PosixPathSeparator).MakePreferred().c_str(); + } + + void PrintTo(const AZ::IO::FixedMaxPath& path, ::std::ostream* os) + { + *os << "path: " << AZ::IO::FixedMaxPath(path.Native(), AZ::IO::PosixPathSeparator).MakePreferred().c_str(); + } +} diff --git a/Code/Framework/AzTest/AzTest/Printers.h b/Code/Framework/AzTest/AzTest/Printers.h new file mode 100644 index 0000000000..3615f7b576 --- /dev/null +++ b/Code/Framework/AzTest/AzTest/Printers.h @@ -0,0 +1,40 @@ +/* + * 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 + * + */ +#pragma once + +#include +#include + +namespace AZStd +{ + template + class basic_string; + + template + class basic_string_view; + + template + class basic_fixed_string; + + template + void PrintTo(const AZStd::basic_string& value, ::std::ostream* os); + template + void PrintTo(const AZStd::basic_string_view& value, ::std::ostream* os); + template + void PrintTo(const AZStd::basic_fixed_string& value, ::std::ostream* os); +} + +namespace AZ::IO +{ + // Add Googletest printers for the AZ::IO::Path classes + void PrintTo(const AZ::IO::PathView& path, ::std::ostream* os); + void PrintTo(const AZ::IO::Path& path, ::std::ostream* os); + void PrintTo(const AZ::IO::FixedMaxPath& path, ::std::ostream* os); +} + +#include diff --git a/Code/Framework/AzTest/AzTest/Printers.inl b/Code/Framework/AzTest/AzTest/Printers.inl new file mode 100644 index 0000000000..265918e48f --- /dev/null +++ b/Code/Framework/AzTest/AzTest/Printers.inl @@ -0,0 +1,34 @@ +/* + * 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 + * + */ +#pragma once + +#include +#include +#include +#include + +namespace AZStd +{ + template + void PrintTo(const AZStd::basic_string& value, ::std::ostream* os) + { + *os << value.c_str(); + } + + template + void PrintTo(const AZStd::basic_string_view& value, ::std::ostream* os) + { + *os << ::std::string_view(value.data(), value.size()); + } + + template + void PrintTo(const AZStd::basic_fixed_string& value, ::std::ostream* os) + { + *os << value.c_str(); + } +} diff --git a/Code/Framework/AzTest/AzTest/Utils.h b/Code/Framework/AzTest/AzTest/Utils.h index fc3dbdd13c..2778e163b3 100644 --- a/Code/Framework/AzTest/AzTest/Utils.h +++ b/Code/Framework/AzTest/AzTest/Utils.h @@ -9,8 +9,8 @@ #include #include #include -#include #include +#include namespace AZ { namespace Test diff --git a/Code/Framework/AzTest/AzTest/aztest_files.cmake b/Code/Framework/AzTest/AzTest/aztest_files.cmake index 9a9b085337..0e89a5dfa2 100644 --- a/Code/Framework/AzTest/AzTest/aztest_files.cmake +++ b/Code/Framework/AzTest/AzTest/aztest_files.cmake @@ -11,6 +11,9 @@ set(FILES AzTest.cpp ColorizedOutput.cpp Platform.h + Printers.h + Printers.inl + Printers.cpp Utils.h Utils.cpp GemTestEnvironment.cpp From 1e4faca332e0fc5eb1f512c6807ba0876728ea36 Mon Sep 17 00:00:00 2001 From: Scott Romero <24445312+AMZN-ScottR@users.noreply.github.com> Date: Mon, 4 Oct 2021 14:49:16 -0700 Subject: [PATCH 5/5] [development] Revived the statistical profiler (#4378) - Removed unused RunningStatisticsManager.cpp - Updated stats profiler proxy to use budgets - Fixed and re-enabled stats profiler tests - Enabled StatisticalProfilerProxySystemComponent in runtime Signed-off-by: AMZN-ScottR 24445312+AMZN-ScottR@users.noreply.github.com --- Code/Framework/AzCore/AzCore/AzCoreModule.cpp | 9 ++ Code/Framework/AzCore/AzCore/Debug/Budget.cpp | 8 +- Code/Framework/AzCore/AzCore/Debug/Profiler.h | 6 +- .../Statistics/RunningStatisticsManager.cpp | 106 ------------------ .../AzCore/Statistics/StatisticalProfiler.h | 4 +- .../Statistics/StatisticalProfilerProxy.h | 64 ++++++----- .../AzCore/Tests/StatisticalProfiler.cpp | 95 ++++++++-------- .../AzCore/Tests/azcoretests_files.cmake | 1 + 8 files changed, 107 insertions(+), 186 deletions(-) delete mode 100644 Code/Framework/AzCore/AzCore/Statistics/RunningStatisticsManager.cpp diff --git a/Code/Framework/AzCore/AzCore/AzCoreModule.cpp b/Code/Framework/AzCore/AzCore/AzCoreModule.cpp index ec45335f95..67123ed826 100644 --- a/Code/Framework/AzCore/AzCore/AzCoreModule.cpp +++ b/Code/Framework/AzCore/AzCore/AzCoreModule.cpp @@ -23,6 +23,7 @@ #include #include #include +#include namespace AZ { @@ -44,6 +45,10 @@ namespace AZ EventSchedulerSystemComponent::CreateDescriptor(), TaskGraphSystemComponent::CreateDescriptor(), +#if !defined(_RELEASE) + Statistics::StatisticalProfilerProxySystemComponent::CreateDescriptor(), +#endif + #if !defined(AZCORE_EXCLUDE_LUA) ScriptSystemComponent::CreateDescriptor(), #endif // #if !defined(AZCORE_EXCLUDE_LUA) @@ -58,6 +63,10 @@ namespace AZ azrtti_typeid(), azrtti_typeid(), azrtti_typeid(), + +#if !defined(_RELEASE) + azrtti_typeid(), +#endif }; } } diff --git a/Code/Framework/AzCore/AzCore/Debug/Budget.cpp b/Code/Framework/AzCore/AzCore/Debug/Budget.cpp index f9fe7e462a..4a6f3ed114 100644 --- a/Code/Framework/AzCore/AzCore/Debug/Budget.cpp +++ b/Code/Framework/AzCore/AzCore/Debug/Budget.cpp @@ -11,6 +11,7 @@ #include #include #include +#include AZ_DEFINE_BUDGET(Animation); AZ_DEFINE_BUDGET(Audio); @@ -30,8 +31,7 @@ namespace AZ::Debug }; Budget::Budget(const char* name) - : m_name{ name } - , m_crc{ Crc32(name) } + : Budget( name, Crc32(name) ) { } @@ -40,6 +40,10 @@ namespace AZ::Debug , m_crc{ crc } { m_impl = aznew BudgetImpl; + if (auto statsProfiler = Interface::Get(); statsProfiler) + { + statsProfiler->RegisterProfilerId(m_crc); + } } Budget::~Budget() diff --git a/Code/Framework/AzCore/AzCore/Debug/Profiler.h b/Code/Framework/AzCore/AzCore/Debug/Profiler.h index 56103e8314..6e86de2e93 100644 --- a/Code/Framework/AzCore/AzCore/Debug/Profiler.h +++ b/Code/Framework/AzCore/AzCore/Debug/Profiler.h @@ -8,6 +8,7 @@ #pragma once #include +#include #ifdef USE_PIX #include @@ -44,7 +45,10 @@ #define AZ_PROFILE_INTERVAL_START(...) #define AZ_PROFILE_INTERVAL_START_COLORED(...) #define AZ_PROFILE_INTERVAL_END(...) -#define AZ_PROFILE_INTERVAL_SCOPED(...) +#define AZ_PROFILE_INTERVAL_SCOPED(budget, scopeNameId, ...) \ + static constexpr AZ::Crc32 AZ_JOIN(blockId, __LINE__)(scopeNameId); \ + AZ::Statistics::StatisticalProfilerProxy::TimedScope AZ_JOIN(scope, __LINE__)(AZ_CRC_CE(#budget), AZ_JOIN(blockId, __LINE__)); + #endif #ifndef AZ_PROFILE_DATAPOINT diff --git a/Code/Framework/AzCore/AzCore/Statistics/RunningStatisticsManager.cpp b/Code/Framework/AzCore/AzCore/Statistics/RunningStatisticsManager.cpp deleted file mode 100644 index 52085d98e7..0000000000 --- a/Code/Framework/AzCore/AzCore/Statistics/RunningStatisticsManager.cpp +++ /dev/null @@ -1,106 +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 - * - */ - -#include "RunningStatisticsManager.h" - -namespace AzFramework -{ - namespace Statistics - { - bool RunningStatisticsManager::ContainsStatistic(const AZStd::string& name) - { - auto iterator = m_statisticsNamesToIndexMap.find(name); - return iterator != m_statisticsNamesToIndexMap.end(); - } - - bool RunningStatisticsManager::AddStatistic(const AZStd::string& name, const AZStd::string& units) - { - if (ContainsStatistic(name)) - { - return false; - } - AddStatisticValidated(name, units); - return true; - } - - void RunningStatisticsManager::RemoveStatistic(const AZStd::string& name) - { - auto iterator = m_statisticsNamesToIndexMap.find(name); - if (iterator == m_statisticsNamesToIndexMap.end()) - { - return; - } - AZ::u32 itemIndex = iterator->second; - m_statistics.erase(m_statistics.begin() + itemIndex); - m_statisticsNamesToIndexMap.erase(iterator); - //Update the indices in m_statisticsNamesToIndexMap. - while (itemIndex < m_statistics.size()) - { - const AZStd::string& statName = m_statistics[itemIndex].GetName(); - m_statisticsNamesToIndexMap[statName] = itemIndex; - ++itemIndex; - } - } - - void RunningStatisticsManager::ResetStatistic(const AZStd::string& name) - { - NamedRunningStatistic* stat = GetStatistic(name); - if (!stat) - { - return; - } - stat->Reset(); - } - - void RunningStatisticsManager::ResetAllStatistics() - { - for (NamedRunningStatistic& stat : m_statistics) - { - stat.Reset(); - } - } - - void RunningStatisticsManager::PushSampleForStatistic(const AZStd::string& name, double value) - { - NamedRunningStatistic* stat = GetStatistic(name); - if (!stat) - { - return; - } - stat->PushSample(value); - } - - NamedRunningStatistic* RunningStatisticsManager::GetStatistic(const AZStd::string& name, AZ::u32* indexOut) - { - auto iterator = m_statisticsNamesToIndexMap.find(name); - if (iterator == m_statisticsNamesToIndexMap.end()) - { - return nullptr; - } - const AZ::u32 index = iterator->second; - if (indexOut) - { - *indexOut = index; - } - return &m_statistics[index]; - } - - const AZStd::vector& RunningStatisticsManager::GetAllStatistics() const - { - return m_statistics; - } - - void RunningStatisticsManager::AddStatisticValidated(const AZStd::string& name, const AZStd::string& units) - { - m_statistics.emplace_back(NamedRunningStatistic(name, units)); - const AZ::u32 itemIndex = static_cast(m_statistics.size() - 1); - m_statisticsNamesToIndexMap[name] = itemIndex; - } - - }//namespace Statistics -}//namespace AzFramework diff --git a/Code/Framework/AzCore/AzCore/Statistics/StatisticalProfiler.h b/Code/Framework/AzCore/AzCore/Statistics/StatisticalProfiler.h index 681635cd1c..876898f8ba 100644 --- a/Code/Framework/AzCore/AzCore/Statistics/StatisticalProfiler.h +++ b/Code/Framework/AzCore/AzCore/Statistics/StatisticalProfiler.h @@ -8,7 +8,6 @@ #pragma once #include //Just to get AZ::NullMutex -#include #include #include #include @@ -37,8 +36,7 @@ namespace AZ //! are some things to consider when working with the StatisticalProfilerProxy: //! The StatisticalProfilerProxy OWNS an array of StatisticalProfiler. //! You can "manage" one of those StatisticalProfiler by getting a reference to it and - //! add Running statistics etc. See The TerrainProfilers mentioned above to see concrete use - //! cases on how to work with the StatisticalProfilerProxy. + //! add Running statistics etc. template class StatisticalProfiler { diff --git a/Code/Framework/AzCore/AzCore/Statistics/StatisticalProfilerProxy.h b/Code/Framework/AzCore/AzCore/Statistics/StatisticalProfilerProxy.h index 33d835076f..278b2ecc98 100644 --- a/Code/Framework/AzCore/AzCore/Statistics/StatisticalProfilerProxy.h +++ b/Code/Framework/AzCore/AzCore/Statistics/StatisticalProfilerProxy.h @@ -7,28 +7,12 @@ */ #pragma once -#include -#include -#include -#include -#include #include #include -#include +#include +#include -#if defined(AZ_STATISTICAL_PROFILING_ENABLED) - -#if defined(AZ_PROFILE_SCOPE) -#undef AZ_PROFILE_SCOPE -#endif // #if defined(AZ_PROFILE_SCOPE) - -#define AZ_PROFILE_SCOPE(profiler, scopeNameId) \ - static const AZStd::string AZ_JOIN(blockName, __LINE__)(scopeNameId); \ - AZ::Statistics::StatisticalProfilerProxy::TimedScope AZ_JOIN(scope, __LINE__)(profiler, AZ_JOIN(blockName, __LINE__)); - -#endif //#if defined(AZ_STATISTICAL_PROFILING_ENABLED) - namespace AZ::Statistics { using StatisticalProfilerId = uint32_t; @@ -65,7 +49,7 @@ namespace AZ::Statistics public: AZ_TYPE_INFO(StatisticalProfilerProxy, "{1103D0EB-1C32-4854-B9D9-40A2D65BDBD2}"); - using StatIdType = AZStd::string; + using StatIdType = AZ::Crc32; using StatisticalProfilerType = StatisticalProfiler; //! A Convenience class used to measure time performance of scopes of code @@ -94,6 +78,7 @@ namespace AZ::Statistics } m_startTime = AZStd::chrono::high_resolution_clock::now(); } + ~TimedScope() { if (!m_profilerProxy) @@ -122,7 +107,6 @@ namespace AZ::Statistics StatisticalProfilerProxy() { - // TODO:BUDGETS Query available budgets at registration time and create an associated profiler per type AZ::Interface::Register(this); } @@ -135,30 +119,54 @@ namespace AZ::Statistics StatisticalProfilerProxy(StatisticalProfilerProxy&&) = delete; StatisticalProfilerProxy& operator=(StatisticalProfilerProxy&&) = delete; + void RegisterProfilerId(StatisticalProfilerId id) + { + m_profilers.try_emplace(id, ProfilerInfo()); + } + bool IsProfilerActive(StatisticalProfilerId id) const { - return m_activeProfilersFlag[static_cast(id)]; + auto iter = m_profilers.find(id); + return (iter != m_profilers.end()) ? iter->second.m_enabled : false; } StatisticalProfilerType& GetProfiler(StatisticalProfilerId id) { - return m_profilers[static_cast(id)]; + auto iter = m_profilers.try_emplace(id, ProfilerInfo()).first; + return iter->second.m_profiler; } - void ActivateProfiler(StatisticalProfilerId id, bool activate) + void ActivateProfiler(StatisticalProfilerId id, bool activate, bool autoCreate = true) { - m_activeProfilersFlag[static_cast(id)] = activate; + if (autoCreate) + { + auto iter = m_profilers.try_emplace(id, ProfilerInfo()).first; + iter->second.m_enabled = activate; + } + else if (auto iter = m_profilers.find(id); iter != m_profilers.end()) + { + iter->second.m_enabled = activate; + } } void PushSample(StatisticalProfilerId id, const StatIdType& statId, double value) { - m_profilers[static_cast(id)].PushSample(statId, value); + if (auto iter = m_profilers.find(id); iter != m_profilers.end()) + { + iter->second.m_profiler.PushSample(statId, value); + } } private: - // TODO:BUDGETS the number of bits allocated here must be based on the number of budgets available at profiler registration time - AZStd::bitset<128> m_activeProfilersFlag; - AZStd::vector m_profilers; + struct ProfilerInfo + { + StatisticalProfilerType m_profiler; + bool m_enabled{ false }; + }; + + using ProfilerMap = AZStd::unordered_map; + + ProfilerMap m_profilers; }; // class StatisticalProfilerProxy }; // namespace AZ::Statistics diff --git a/Code/Framework/AzCore/Tests/StatisticalProfiler.cpp b/Code/Framework/AzCore/Tests/StatisticalProfiler.cpp index 04e70d92a5..fc5fe66c4b 100644 --- a/Code/Framework/AzCore/Tests/StatisticalProfiler.cpp +++ b/Code/Framework/AzCore/Tests/StatisticalProfiler.cpp @@ -30,6 +30,8 @@ namespace UnitTest { + constexpr AZ::u32 ProfilerProxyGroup = AZ_CRC_CE("StatisticalProfilerProxyTests"); + class StatisticalProfilerTest : public AllocatorsFixture { @@ -98,10 +100,10 @@ namespace UnitTest AZ::Statistics::StatisticalProfiler profiler; - const AZ::Crc32 statIdPerformance = AZ_CRC("PerformanceResult", 0xc1f29a10); + constexpr AZ::Crc32 statIdPerformance = AZ_CRC_CE("PerformanceResult"); const AZStd::string statNamePerformance("PerformanceResult"); - const AZ::Crc32 statIdBlock = AZ_CRC("Block", 0x831b9722); + constexpr AZ::Crc32 statIdBlock = AZ_CRC_CE("Block"); const AZStd::string statNameBlock("Block"); ASSERT_TRUE(profiler.GetStatsManager().AddStatistic(statIdPerformance, statNamePerformance, "us") != nullptr); @@ -175,10 +177,10 @@ namespace UnitTest AZ::Statistics::StatisticalProfiler profiler; - const AZ::Crc32 statIdPerformance = AZ_CRC("PerformanceResult", 0xc1f29a10); + constexpr AZ::Crc32 statIdPerformance = AZ_CRC_CE("PerformanceResult"); const AZStd::string statNamePerformance("PerformanceResult"); - const AZ::Crc32 statIdBlock = AZ_CRC("Block", 0x831b9722); + constexpr AZ::Crc32 statIdBlock = AZ_CRC_CE("Block"); const AZStd::string statNameBlock("Block"); ASSERT_TRUE(profiler.GetStatsManager().AddStatistic(statIdPerformance, statNamePerformance, "us") != nullptr); @@ -317,26 +319,26 @@ namespace UnitTest AZ::Statistics::StatisticalProfilerProxy::TimedScope::ClearCachedProxy(); AZ::Statistics::StatisticalProfilerProxy profilerProxy; AZ::Statistics::StatisticalProfilerProxy* proxy = AZ::Interface::Get(); - AZ::Statistics::StatisticalProfilerProxy::StatisticalProfilerType& profiler = proxy->GetProfiler(AZ::Debug::ProfileCategory::Terrain); + AZ::Statistics::StatisticalProfilerProxy::StatisticalProfilerType& profiler = proxy->GetProfiler(ProfilerProxyGroup); - const AZ::Statistics::StatisticalProfilerProxy::StatIdType statIdPerformance = "PerformanceResult"; + const AZ::Statistics::StatisticalProfilerProxy::StatIdType statIdPerformance("PerformanceResult"); const AZStd::string statNamePerformance("PerformanceResult"); - const AZ::Statistics::StatisticalProfilerProxy::StatIdType statIdBlock = "Block"; + const AZ::Statistics::StatisticalProfilerProxy::StatIdType statIdBlock("Block"); const AZStd::string statNameBlock("Block"); ASSERT_TRUE(profiler.GetStatsManager().AddStatistic(statIdPerformance, statNamePerformance, "us") != nullptr); ASSERT_TRUE(profiler.GetStatsManager().AddStatistic(statIdBlock, statNameBlock, "us") != nullptr); - proxy->ActivateProfiler(AZ::Debug::ProfileCategory::Terrain, true); + proxy->ActivateProfiler(ProfilerProxyGroup, true); const int iter_count = 10; { - CODE_PROFILER_PROXY_PUSH_TIME(AZ::Debug::ProfileCategory::Terrain, statIdPerformance) + CODE_PROFILER_PROXY_PUSH_TIME(ProfilerProxyGroup, statIdPerformance) int counter = 0; for (int i = 0; i < iter_count; i++) { - CODE_PROFILER_PROXY_PUSH_TIME(AZ::Debug::ProfileCategory::Terrain, statIdBlock) + CODE_PROFILER_PROXY_PUSH_TIME(ProfilerProxyGroup, statIdBlock) counter++; } } @@ -348,7 +350,7 @@ namespace UnitTest EXPECT_EQ(profiler.GetStatistic(statIdBlock)->GetNumSamples(), iter_count); //Clean Up - proxy->ActivateProfiler(AZ::Debug::ProfileCategory::Terrain, false); + proxy->ActivateProfiler(ProfilerProxyGroup, false); #undef CODE_PROFILER_PROXY_PUSH_TIME @@ -362,12 +364,12 @@ namespace UnitTest const AZ::Statistics::StatisticalProfilerProxy::StatIdType simple_thread1("simple_thread1"); const AZ::Statistics::StatisticalProfilerProxy::StatIdType simple_thread1_loop("simple_thread1_loop"); - CODE_PROFILER_PROXY_PUSH_TIME(AZ::Debug::ProfileCategory::Terrain, simple_thread1); + CODE_PROFILER_PROXY_PUSH_TIME(ProfilerProxyGroup, simple_thread1); static int counter = 0; for (int i = 0; i < loop_cnt; i++) { - CODE_PROFILER_PROXY_PUSH_TIME(AZ::Debug::ProfileCategory::Terrain, simple_thread1_loop); + CODE_PROFILER_PROXY_PUSH_TIME(ProfilerProxyGroup, simple_thread1_loop); counter++; } } @@ -377,12 +379,12 @@ namespace UnitTest const AZ::Statistics::StatisticalProfilerProxy::StatIdType simple_thread2("simple_thread2"); const AZ::Statistics::StatisticalProfilerProxy::StatIdType simple_thread2_loop("simple_thread2_loop"); - CODE_PROFILER_PROXY_PUSH_TIME(AZ::Debug::ProfileCategory::Terrain, simple_thread2); + CODE_PROFILER_PROXY_PUSH_TIME(ProfilerProxyGroup, simple_thread2); static int counter = 0; for (int i = 0; i < loop_cnt; i++) { - CODE_PROFILER_PROXY_PUSH_TIME(AZ::Debug::ProfileCategory::Terrain, simple_thread2_loop); + CODE_PROFILER_PROXY_PUSH_TIME(ProfilerProxyGroup, simple_thread2_loop); counter++; } } @@ -392,12 +394,13 @@ namespace UnitTest const AZ::Statistics::StatisticalProfilerProxy::StatIdType simple_thread3("simple_thread3"); const AZ::Statistics::StatisticalProfilerProxy::StatIdType simple_thread3_loop("simple_thread3_loop"); - CODE_PROFILER_PROXY_PUSH_TIME(AZ::Debug::ProfileCategory::Terrain, simple_thread3); + CODE_PROFILER_PROXY_PUSH_TIME(ProfilerProxyGroup, simple_thread3); static int counter = 0; for (int i = 0; i < loop_cnt; i++) { - CODE_PROFILER_PROXY_PUSH_TIME(AZ::Debug::ProfileCategory::Terrain, simple_thread3_loop); + CODE_PROFILER_PROXY_PUSH_TIME(ProfilerProxyGroup, simple_thread3_loop); + counter++; } } @@ -408,21 +411,21 @@ namespace UnitTest AZ::Statistics::StatisticalProfilerProxy::TimedScope::ClearCachedProxy(); AZ::Statistics::StatisticalProfilerProxy profilerProxy; AZ::Statistics::StatisticalProfilerProxy* proxy = AZ::Interface::Get(); - AZ::Statistics::StatisticalProfilerProxy::StatisticalProfilerType& profiler = proxy->GetProfiler(AZ::Debug::ProfileCategory::Terrain); + AZ::Statistics::StatisticalProfilerProxy::StatisticalProfilerType& profiler = proxy->GetProfiler(ProfilerProxyGroup); - const AZ::Statistics::StatisticalProfilerProxy::StatIdType statIdThread1 = "simple_thread1"; + const AZ::Statistics::StatisticalProfilerProxy::StatIdType statIdThread1("simple_thread1"); const AZStd::string statNameThread1("simple_thread1"); - const AZ::Statistics::StatisticalProfilerProxy::StatIdType statIdThread1Loop = "simple_thread1_loop"; + const AZ::Statistics::StatisticalProfilerProxy::StatIdType statIdThread1Loop("simple_thread1_loop"); const AZStd::string statNameThread1Loop("simple_thread1_loop"); - const AZ::Statistics::StatisticalProfilerProxy::StatIdType statIdThread2 = "simple_thread2"; + const AZ::Statistics::StatisticalProfilerProxy::StatIdType statIdThread2("simple_thread2"); const AZStd::string statNameThread2("simple_thread2"); - const AZ::Statistics::StatisticalProfilerProxy::StatIdType statIdThread2Loop = "simple_thread2_loop"; + const AZ::Statistics::StatisticalProfilerProxy::StatIdType statIdThread2Loop("simple_thread2_loop"); const AZStd::string statNameThread2Loop("simple_thread2_loop"); - const AZ::Statistics::StatisticalProfilerProxy::StatIdType statIdThread3 = "simple_thread3"; + const AZ::Statistics::StatisticalProfilerProxy::StatIdType statIdThread3("simple_thread3"); const AZStd::string statNameThread3("simple_thread3"); - const AZ::Statistics::StatisticalProfilerProxy::StatIdType statIdThread3Loop = "simple_thread3_loop"; + const AZ::Statistics::StatisticalProfilerProxy::StatIdType statIdThread3Loop("simple_thread3_loop"); const AZStd::string statNameThread3Loop("simple_thread3_loop"); ASSERT_TRUE(profiler.GetStatsManager().AddStatistic(statIdThread1, statNameThread1, "us")); @@ -432,7 +435,7 @@ namespace UnitTest ASSERT_TRUE(profiler.GetStatsManager().AddStatistic(statIdThread3, statNameThread3, "us")); ASSERT_TRUE(profiler.GetStatsManager().AddStatistic(statIdThread3Loop, statNameThread3Loop, "us")); - proxy->ActivateProfiler(AZ::Debug::ProfileCategory::Terrain, true); + proxy->ActivateProfiler(ProfilerProxyGroup, true); //Let's kickoff the threads to see how much contention affects the profiler's performance. const int iter_count = 10; @@ -459,7 +462,7 @@ namespace UnitTest EXPECT_EQ(profiler.GetStatistic(statIdThread3Loop)->GetNumSamples(), iter_count); //Clean Up - proxy->ActivateProfiler(AZ::Debug::ProfileCategory::Terrain, false); + proxy->ActivateProfiler(ProfilerProxyGroup, false); } /** Trace message handler to track messages during tests @@ -566,10 +569,10 @@ namespace UnitTest AZ::Statistics::StatisticalProfiler profiler; - const AZ::Crc32 statIdPerformance = AZ_CRC("PerformanceResult", 0xc1f29a10); + constexpr AZ::Crc32 statIdPerformance = AZ_CRC_CE("PerformanceResult"); const AZStd::string statNamePerformance("PerformanceResult"); - const AZ::Crc32 statIdBlock = AZ_CRC("Block", 0x831b9722); + constexpr AZ::Crc32 statIdBlock = AZ_CRC_CE("Block"); const AZStd::string statNameBlock("Block"); ASSERT_TRUE(profiler.GetStatsManager().AddStatistic(statIdPerformance, statNamePerformance, "us") != nullptr); @@ -647,10 +650,10 @@ namespace UnitTest AZ::Statistics::StatisticalProfiler profiler; - const AZ::Crc32 statIdPerformance = AZ_CRC("PerformanceResult", 0xc1f29a10); + constexpr AZ::Crc32 statIdPerformance = AZ_CRC_CE("PerformanceResult"); const AZStd::string statNamePerformance("PerformanceResult"); - const AZ::Crc32 statIdBlock = AZ_CRC("Block", 0x831b9722); + constexpr AZ::Crc32 statIdBlock = AZ_CRC_CE("Block"); const AZStd::string statNameBlock("Block"); ASSERT_TRUE(profiler.GetStatsManager().AddStatistic(statIdPerformance, statNamePerformance, "us") != nullptr); @@ -745,26 +748,26 @@ namespace UnitTest AZ::Statistics::StatisticalProfilerProxy::TimedScope::ClearCachedProxy(); AZ::Statistics::StatisticalProfilerProxy profilerProxy; AZ::Statistics::StatisticalProfilerProxy* proxy = AZ::Interface::Get(); - AZ::Statistics::StatisticalProfilerProxy::StatisticalProfilerType& profiler = proxy->GetProfiler(AZ::Debug::ProfileCategory::Terrain); + AZ::Statistics::StatisticalProfilerProxy::StatisticalProfilerType& profiler = proxy->GetProfiler(ProfilerProxyGroup); - const AZ::Statistics::StatisticalProfilerProxy::StatIdType statIdPerformance = "PerformanceResult"; + const AZ::Statistics::StatisticalProfilerProxy::StatIdType statIdPerformance("PerformanceResult"); const AZStd::string statNamePerformance("PerformanceResult"); - const AZ::Statistics::StatisticalProfilerProxy::StatIdType statIdBlock = "Block"; + const AZ::Statistics::StatisticalProfilerProxy::StatIdType statIdBlock("Block"); const AZStd::string statNameBlock("Block"); ASSERT_TRUE(profiler.GetStatsManager().AddStatistic(statIdPerformance, statNamePerformance, "us") != nullptr); ASSERT_TRUE(profiler.GetStatsManager().AddStatistic(statIdBlock, statNameBlock, "us") != nullptr); - proxy->ActivateProfiler(AZ::Debug::ProfileCategory::Terrain, true); + proxy->ActivateProfiler(ProfilerProxyGroup, true); const int iter_count = 1000000; { - CODE_PROFILER_PROXY_PUSH_TIME(AZ::Debug::ProfileCategory::Terrain, statIdPerformance) + CODE_PROFILER_PROXY_PUSH_TIME(ProfilerProxyGroup, statIdPerformance) int counter = 0; for (int i = 0; i < iter_count; i++) { - CODE_PROFILER_PROXY_PUSH_TIME(AZ::Debug::ProfileCategory::Terrain, statIdBlock) + CODE_PROFILER_PROXY_PUSH_TIME(ProfilerProxyGroup, statIdBlock) counter++; } } @@ -778,7 +781,7 @@ namespace UnitTest profiler.LogAndResetStats("StatisticalProfilerProxy"); //Clean Up - proxy->ActivateProfiler(AZ::Debug::ProfileCategory::Terrain, false); + proxy->ActivateProfiler(ProfilerProxyGroup, false); } #undef CODE_PROFILER_PROXY_PUSH_TIME @@ -788,21 +791,21 @@ namespace UnitTest AZ::Statistics::StatisticalProfilerProxy::TimedScope::ClearCachedProxy(); AZ::Statistics::StatisticalProfilerProxy profilerProxy; AZ::Statistics::StatisticalProfilerProxy* proxy = AZ::Interface::Get(); - AZ::Statistics::StatisticalProfilerProxy::StatisticalProfilerType& profiler = proxy->GetProfiler(AZ::Debug::ProfileCategory::Terrain); + AZ::Statistics::StatisticalProfilerProxy::StatisticalProfilerType& profiler = proxy->GetProfiler(ProfilerProxyGroup); - const AZ::Statistics::StatisticalProfilerProxy::StatIdType statIdThread1 = "simple_thread1"; + const AZ::Statistics::StatisticalProfilerProxy::StatIdType statIdThread1("simple_thread1"); const AZStd::string statNameThread1("simple_thread1"); - const AZ::Statistics::StatisticalProfilerProxy::StatIdType statIdThread1Loop = "simple_thread1_loop"; + const AZ::Statistics::StatisticalProfilerProxy::StatIdType statIdThread1Loop("simple_thread1_loop"); const AZStd::string statNameThread1Loop("simple_thread1_loop"); - const AZ::Statistics::StatisticalProfilerProxy::StatIdType statIdThread2 = "simple_thread2"; + const AZ::Statistics::StatisticalProfilerProxy::StatIdType statIdThread2("simple_thread2"); const AZStd::string statNameThread2("simple_thread2"); - const AZ::Statistics::StatisticalProfilerProxy::StatIdType statIdThread2Loop = "simple_thread2_loop"; + const AZ::Statistics::StatisticalProfilerProxy::StatIdType statIdThread2Loop("simple_thread2_loop"); const AZStd::string statNameThread2Loop("simple_thread2_loop"); - const AZ::Statistics::StatisticalProfilerProxy::StatIdType statIdThread3 = "simple_thread3"; + const AZ::Statistics::StatisticalProfilerProxy::StatIdType statIdThread3("simple_thread3"); const AZStd::string statNameThread3("simple_thread3"); - const AZ::Statistics::StatisticalProfilerProxy::StatIdType statIdThread3Loop = "simple_thread3_loop"; + const AZ::Statistics::StatisticalProfilerProxy::StatIdType statIdThread3Loop("simple_thread3_loop"); const AZStd::string statNameThread3Loop("simple_thread3_loop"); ASSERT_TRUE(profiler.GetStatsManager().AddStatistic(statIdThread1, statNameThread1, "us")); @@ -812,7 +815,7 @@ namespace UnitTest ASSERT_TRUE(profiler.GetStatsManager().AddStatistic(statIdThread3, statNameThread3, "us")); ASSERT_TRUE(profiler.GetStatsManager().AddStatistic(statIdThread3Loop, statNameThread3Loop, "us")); - proxy->ActivateProfiler(AZ::Debug::ProfileCategory::Terrain, true); + proxy->ActivateProfiler(ProfilerProxyGroup, true); //Let's kickoff the threads to see how much contention affects the profiler's performance. const int iter_count = 1000000; @@ -841,7 +844,7 @@ namespace UnitTest profiler.LogAndResetStats("3_Threads_StatisticalProfilerProxy"); //Clean Up - proxy->ActivateProfiler(AZ::Debug::ProfileCategory::Terrain, false); + proxy->ActivateProfiler(ProfilerProxyGroup, false); } }//namespace UnitTest diff --git a/Code/Framework/AzCore/Tests/azcoretests_files.cmake b/Code/Framework/AzCore/Tests/azcoretests_files.cmake index a111af0353..e155594aa0 100644 --- a/Code/Framework/AzCore/Tests/azcoretests_files.cmake +++ b/Code/Framework/AzCore/Tests/azcoretests_files.cmake @@ -61,6 +61,7 @@ set(FILES Slice.cpp State.cpp Statistics.cpp + StatisticalProfiler.cpp StreamerTests.cpp StringFunc.cpp SystemFile.cpp