From 6322dd5397d2a91c86bed2f2b8b1c378da549c03 Mon Sep 17 00:00:00 2001 From: Chris Galvan Date: Thu, 3 Feb 2022 12:29:18 -0600 Subject: [PATCH 1/5] Updated the image gradient component to use a streaming image asset Signed-off-by: Chris Galvan --- .../BuilderSettings/ImageProcessingDefines.h | 14 --- .../Code/Source/ImageBuilderComponent.cpp | 5 +- .../Source/ImageProcessingSystemComponent.cpp | 5 +- .../Include/Atom/RPI.Reflect/Image/Image.h | 14 +++ .../RPI/Code/Source/RPI.Public/RPIUtils.cpp | 12 +-- Gems/GradientSignal/Code/CMakeLists.txt | 3 + .../Components/ImageGradientComponent.h | 18 +++- .../Code/Include/GradientSignal/ImageAsset.h | 3 +- .../Components/ImageGradientComponent.cpp | 92 ++++++++++++++++++- .../GradientSignal/Code/Source/ImageAsset.cpp | 27 +++--- 10 files changed, 150 insertions(+), 43 deletions(-) diff --git a/Gems/Atom/Asset/ImageProcessingAtom/Code/Source/BuilderSettings/ImageProcessingDefines.h b/Gems/Atom/Asset/ImageProcessingAtom/Code/Source/BuilderSettings/ImageProcessingDefines.h index 7c35a0634e..dcd95c7fb6 100644 --- a/Gems/Atom/Asset/ImageProcessingAtom/Code/Source/BuilderSettings/ImageProcessingDefines.h +++ b/Gems/Atom/Asset/ImageProcessingAtom/Code/Source/BuilderSettings/ImageProcessingDefines.h @@ -49,20 +49,6 @@ namespace ImageProcessingAtom static const unsigned int s_MinReduceLevel = 0; static const unsigned int s_MaxReduceLevel = 5; - static const int s_TotalSupportedImageExtensions = 10; - static const char* s_SupportedImageExtensions[s_TotalSupportedImageExtensions] = { - "*.tif", - "*.tiff", - "*.png", - "*.bmp", - "*.jpg", - "*.jpeg", - "*.tga", - "*.gif", - "*.dds", - "*.exr" - }; - enum class RGBWeight : AZ::u32 { uniform, // uniform weights (1.0, 1.0, 1.0) (default) diff --git a/Gems/Atom/Asset/ImageProcessingAtom/Code/Source/ImageBuilderComponent.cpp b/Gems/Atom/Asset/ImageProcessingAtom/Code/Source/ImageBuilderComponent.cpp index 55a6c1ea8f..8c2e61ff6d 100644 --- a/Gems/Atom/Asset/ImageProcessingAtom/Code/Source/ImageBuilderComponent.cpp +++ b/Gems/Atom/Asset/ImageProcessingAtom/Code/Source/ImageBuilderComponent.cpp @@ -27,6 +27,7 @@ #include #include +#include #include namespace ImageProcessingAtom @@ -66,9 +67,9 @@ namespace ImageProcessingAtom AssetBuilderSDK::AssetBuilderDesc builderDescriptor; builderDescriptor.m_name = "Atom Image Builder"; - for (int i = 0; i < s_TotalSupportedImageExtensions; i++) + for (int i = 0; i < AZ::RPI::s_TotalSupportedImageExtensions; i++) { - builderDescriptor.m_patterns.push_back(AssetBuilderSDK::AssetBuilderPattern(s_SupportedImageExtensions[i], AssetBuilderSDK::AssetBuilderPattern::PatternType::Wildcard)); + builderDescriptor.m_patterns.push_back(AssetBuilderSDK::AssetBuilderPattern(AZ::RPI::s_SupportedImageExtensions[i], AssetBuilderSDK::AssetBuilderPattern::PatternType::Wildcard)); } builderDescriptor.m_busId = azrtti_typeid(); diff --git a/Gems/Atom/Asset/ImageProcessingAtom/Code/Source/ImageProcessingSystemComponent.cpp b/Gems/Atom/Asset/ImageProcessingAtom/Code/Source/ImageProcessingSystemComponent.cpp index 665d08b3ae..f0d881c416 100644 --- a/Gems/Atom/Asset/ImageProcessingAtom/Code/Source/ImageProcessingSystemComponent.cpp +++ b/Gems/Atom/Asset/ImageProcessingAtom/Code/Source/ImageProcessingSystemComponent.cpp @@ -7,6 +7,7 @@ */ +#include #include #include @@ -223,9 +224,9 @@ namespace ImageProcessingAtom { AZStd::string targetExtension = entry->GetExtension(); - for (int i = 0; i < s_TotalSupportedImageExtensions; i++) + for (int i = 0; i < AZ::RPI::s_TotalSupportedImageExtensions; i++) { - if (AZStd::wildcard_match(s_SupportedImageExtensions[i], targetExtension.c_str())) + if (AZStd::wildcard_match(AZ::RPI::s_SupportedImageExtensions[i], targetExtension.c_str())) { return true; } diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Image/Image.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Image/Image.h index 185666626c..66c91e92cf 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Image/Image.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Image/Image.h @@ -22,6 +22,20 @@ namespace AZ { class ImageAsset; + static const int s_TotalSupportedImageExtensions = 10; + static const char* s_SupportedImageExtensions[s_TotalSupportedImageExtensions] = { + "*.tif", + "*.tiff", + "*.png", + "*.bmp", + "*.jpg", + "*.jpeg", + "*.tga", + "*.gif", + "*.dds", + "*.exr" + }; + //! A base class for images providing access to common image information. class Image : public Data::InstanceData diff --git a/Gems/Atom/RPI/Code/Source/RPI.Public/RPIUtils.cpp b/Gems/Atom/RPI/Code/Source/RPI.Public/RPIUtils.cpp index e13db253f3..9ff26f0d3a 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Public/RPIUtils.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Public/RPIUtils.cpp @@ -133,7 +133,8 @@ namespace AZ case AZ::RHI::Format::R16G16_UNORM: case AZ::RHI::Format::R16G16B16A16_UNORM: { - return mem[index] / static_cast(std::numeric_limits::max()); + auto actualMem = reinterpret_cast(mem); + return actualMem[index] / static_cast(std::numeric_limits::max()); } case AZ::RHI::Format::R16_SNORM: case AZ::RHI::Format::R16G16_SNORM: @@ -480,14 +481,13 @@ namespace AZ const AZ::RHI::ImageDescriptor imageDescriptor = imageAsset->GetImageDescriptor(); auto width = imageDescriptor.m_size.m_width; const uint32_t numComponents = AZ::RHI::GetFormatComponentCount(imageDescriptor.m_format); - const uint32_t pixelSize = AZ::RHI::GetFormatSize(imageDescriptor.m_format) / numComponents; size_t outValuesIndex = 0; for (uint32_t y = topLeft.second; y < bottomRight.second; ++y) { for (uint32_t x = topLeft.first; x < bottomRight.first; ++x) { - size_t imageDataIndex = (y * width + x) * pixelSize + componentIndex; + size_t imageDataIndex = (y * width + x) * numComponents + componentIndex; auto& outValue = outValues[outValuesIndex++]; outValue = Internal::RetrieveFloatValue(imageData.data(), imageDataIndex, imageDescriptor.m_format); @@ -513,14 +513,13 @@ namespace AZ const AZ::RHI::ImageDescriptor imageDescriptor = imageAsset->GetImageDescriptor(); auto width = imageDescriptor.m_size.m_width; const uint32_t numComponents = AZ::RHI::GetFormatComponentCount(imageDescriptor.m_format); - const uint32_t pixelSize = AZ::RHI::GetFormatSize(imageDescriptor.m_format) / numComponents; size_t outValuesIndex = 0; for (uint32_t y = topLeft.second; y < bottomRight.second; ++y) { for (uint32_t x = topLeft.first; x < bottomRight.first; ++x) { - size_t imageDataIndex = (y * width + x) * pixelSize + componentIndex; + size_t imageDataIndex = (y * width + x) * numComponents + componentIndex; auto& outValue = outValues[outValuesIndex++]; outValue = Internal::RetrieveUintValue(imageData.data(), imageDataIndex, imageDescriptor.m_format); @@ -546,14 +545,13 @@ namespace AZ const AZ::RHI::ImageDescriptor imageDescriptor = imageAsset->GetImageDescriptor(); auto width = imageDescriptor.m_size.m_width; const uint32_t numComponents = AZ::RHI::GetFormatComponentCount(imageDescriptor.m_format); - const uint32_t pixelSize = AZ::RHI::GetFormatSize(imageDescriptor.m_format) / numComponents; size_t outValuesIndex = 0; for (uint32_t y = topLeft.second; y < bottomRight.second; ++y) { for (uint32_t x = topLeft.first; x < bottomRight.first; ++x) { - size_t imageDataIndex = (y * width + x) * pixelSize + componentIndex; + size_t imageDataIndex = (y * width + x) * numComponents + componentIndex; auto& outValue = outValues[outValuesIndex++]; outValue = Internal::RetrieveIntValue(imageData.data(), imageDataIndex, imageDescriptor.m_format); diff --git a/Gems/GradientSignal/Code/CMakeLists.txt b/Gems/GradientSignal/Code/CMakeLists.txt index d2d8f8c696..febf7d9cd4 100644 --- a/Gems/GradientSignal/Code/CMakeLists.txt +++ b/Gems/GradientSignal/Code/CMakeLists.txt @@ -17,9 +17,12 @@ ly_add_target( PUBLIC Include BUILD_DEPENDENCIES + PRIVATE + AZ::AtomCore PUBLIC AZ::AzCore AZ::AzFramework + Gem::Atom_RPI.Public Gem::SurfaceData Gem::ImageProcessingAtom.Headers Gem::LmbrCentral diff --git a/Gems/GradientSignal/Code/Include/GradientSignal/Components/ImageGradientComponent.h b/Gems/GradientSignal/Code/Include/GradientSignal/Components/ImageGradientComponent.h index 044e9d91b1..0cbd8bd4c7 100644 --- a/Gems/GradientSignal/Code/Include/GradientSignal/Components/ImageGradientComponent.h +++ b/Gems/GradientSignal/Code/Include/GradientSignal/Components/ImageGradientComponent.h @@ -8,8 +8,11 @@ #pragma once +#include #include #include +#include +#include #include #include #include @@ -25,6 +28,19 @@ namespace LmbrCentral namespace GradientSignal { + // Custom JSON serializer for ImageGradientConfig to handle version conversion + class JsonImageGradientConfigSerializer + : public AZ::BaseJsonSerializer + { + public: + AZ_RTTI(GradientSignal::JsonImageGradientConfigSerializer, "{C5B982C8-2E81-45C3-8932-B6F54B28F493}", AZ::BaseJsonSerializer); + AZ_CLASS_ALLOCATOR_DECL; + + AZ::JsonSerializationResult::Result Load( + void* outputValue, const AZ::Uuid& outputValueTypeId, const rapidjson::Value& inputValue, + AZ::JsonDeserializerContext& context) override; + }; + class ImageGradientConfig : public AZ::ComponentConfig { @@ -32,7 +48,7 @@ namespace GradientSignal AZ_CLASS_ALLOCATOR(ImageGradientConfig, AZ::SystemAllocator, 0); AZ_RTTI(ImageGradientConfig, "{1BDB5DA4-A4A8-452B-BE6D-6BD451D4E7CD}", AZ::ComponentConfig); static void Reflect(AZ::ReflectContext* context); - AZ::Data::Asset m_imageAsset = { AZ::Data::AssetLoadBehavior::QueueLoad }; + AZ::Data::Asset m_imageAsset = { AZ::Data::AssetLoadBehavior::QueueLoad }; float m_tilingX = 1.0f; float m_tilingY = 1.0f; }; diff --git a/Gems/GradientSignal/Code/Include/GradientSignal/ImageAsset.h b/Gems/GradientSignal/Code/Include/GradientSignal/ImageAsset.h index 4ffab0b4b4..811d74082d 100644 --- a/Gems/GradientSignal/Code/Include/GradientSignal/ImageAsset.h +++ b/Gems/GradientSignal/Code/Include/GradientSignal/ImageAsset.h @@ -12,6 +12,7 @@ #include #include #include +#include namespace AZ { @@ -61,6 +62,6 @@ namespace GradientSignal } }; - float GetValueFromImageAsset(const AZ::Data::Asset& imageAsset, const AZ::Vector3& uvw, float tilingX, float tilingY, float defaultValue); + float GetValueFromImageAsset(const AZ::Data::Asset& imageAsset, const AZ::Vector3& uvw, float tilingX, float tilingY, float defaultValue); } // namespace GradientSignal diff --git a/Gems/GradientSignal/Code/Source/Components/ImageGradientComponent.cpp b/Gems/GradientSignal/Code/Source/Components/ImageGradientComponent.cpp index 3639d5c6df..e3f128ec95 100644 --- a/Gems/GradientSignal/Code/Source/Components/ImageGradientComponent.cpp +++ b/Gems/GradientSignal/Code/Source/Components/ImageGradientComponent.cpp @@ -7,6 +7,7 @@ */ #include +#include #include #include #include @@ -18,13 +19,100 @@ namespace GradientSignal { + AZ::JsonSerializationResult::Result JsonImageGradientConfigSerializer::Load( + void* outputValue, [[maybe_unused]] const AZ::Uuid& outputValueTypeId, + const rapidjson::Value& inputValue, AZ::JsonDeserializerContext& context) + { + namespace JSR = AZ::JsonSerializationResult; + + auto configInstance = reinterpret_cast(outputValue); + AZ_Assert(configInstance, "Output value for JsonImageGradientConfigSerializer can't be null."); + + JSR::ResultCode result(JSR::Tasks::ReadField); + + rapidjson::Value::ConstMemberIterator itr = inputValue.FindMember("ImageAsset"); + if (itr != inputValue.MemberEnd()) + { + // Version 1 stored a custom GradientSignal::ImageAsset as the image asset. + // In Version 2, we changed the image asset to use the generic AZ::RPI::StreamingImageAsset, + // so they are both AZ::Data::Asset but reference different types. + // Using the assetHint, which will be something like "my_test_image.gradimage", + // we need to find the valid streaming image asset product from the same source, + // which will be something like "my_test_image.png.streamingimage" + AZStd::string assetHint; + AZ::Data::AssetId fixedAssetId; + auto it = itr->value.FindMember("assetHint"); + if (it != itr->value.MemberEnd()) + { + AZ::ScopedContextPath subPath(context, "assetHint"); + result.Combine(ContinueLoading(&assetHint, azrtti_typeid(), it->value, context)); + + if (assetHint.ends_with(".gradimage")) + { + // We don't know what image format the original source was, so we need to loop through + // all the supported image extensions to check if they have a valid corresponding + // streaming image asset + for (int i = 0; i < AZ::RPI::s_TotalSupportedImageExtensions; i++) + { + AZStd::string imageExtension(AZ::RPI::s_SupportedImageExtensions[i]); + + // The image extensions are stored with a wildcard (e.g. *.png) so we need to strip that off first + AZ::StringFunc::Replace(imageExtension, "*", ""); + + // Form potential streaming image path (e.g. my_test_image.png.streamingimage) + AZStd::string potentialStreamingImagePath(assetHint); + AZ::StringFunc::Replace(potentialStreamingImagePath, ".gradimage", ""); + potentialStreamingImagePath += imageExtension + ".streamingimage"; + + // Check if there is a valid streaming image asset for this path + AZ::Data::AssetCatalogRequestBus::BroadcastResult(fixedAssetId, &AZ::Data::AssetCatalogRequestBus::Events::GetAssetIdByPath, potentialStreamingImagePath.c_str(), azrtti_typeid>(), false); + if (fixedAssetId.IsValid()) + { + break; + } + } + } + } + + // Replace the old gradimage with new AssetId for streaming image asset (if we needed to replace) + if (fixedAssetId.IsValid()) + { + configInstance->m_imageAsset = AZ::Data::AssetManager::Instance().GetAsset(fixedAssetId, AZ::Data::AssetLoadBehavior::QueueLoad); + } + // Otherwise, this was already a streaming image asset, so just load it like normal + else + { + result.Combine(ContinueLoadingFromJsonObjectField( + &configInstance->m_imageAsset, azrtti_typeidm_imageAsset)>(), inputValue, "ImageAsset", context)); + } + } + + result.Combine(ContinueLoadingFromJsonObjectField( + &configInstance->m_tilingX, azrtti_typeidm_tilingX)>(), inputValue, "TilingX", context)); + + result.Combine(ContinueLoadingFromJsonObjectField( + &configInstance->m_tilingY, azrtti_typeidm_tilingY)>(), inputValue, "TilingY", context)); + + return context.Report(result, + result.GetProcessing() != JSR::Processing::Halted ? + "Successfully loaded ImageGradientConfig information." : + "Failed to load ImageGradientConfig information."); + } + + AZ_CLASS_ALLOCATOR_IMPL(JsonImageGradientConfigSerializer, AZ::SystemAllocator, 0); + void ImageGradientConfig::Reflect(AZ::ReflectContext* context) { + if (auto jsonContext = azrtti_cast(context)) + { + jsonContext->Serializer()->HandlesType(); + } + AZ::SerializeContext* serialize = azrtti_cast(context); if (serialize) { serialize->Class() - ->Version(1) + ->Version(2) ->Field("ImageAsset", &ImageGradientConfig::m_imageAsset) ->Field("TilingX", &ImageGradientConfig::m_tilingX) ->Field("TilingY", &ImageGradientConfig::m_tilingY) @@ -265,7 +353,7 @@ namespace GradientSignal { AZStd::unique_lock imageLock(m_imageMutex); - m_configuration.m_imageAsset = AZ::Data::AssetManager::Instance().FindOrCreateAsset(assetId, azrtti_typeid(), m_configuration.m_imageAsset.GetAutoLoadBehavior()); + m_configuration.m_imageAsset = AZ::Data::AssetManager::Instance().FindOrCreateAsset(assetId, azrtti_typeid(), m_configuration.m_imageAsset.GetAutoLoadBehavior()); } SetupDependencies(); diff --git a/Gems/GradientSignal/Code/Source/ImageAsset.cpp b/Gems/GradientSignal/Code/Source/ImageAsset.cpp index 7e5667a5be..6ccd9e0a2a 100644 --- a/Gems/GradientSignal/Code/Source/ImageAsset.cpp +++ b/Gems/GradientSignal/Code/Source/ImageAsset.cpp @@ -15,6 +15,7 @@ #include #include +#include #include #include @@ -152,17 +153,15 @@ namespace GradientSignal return true; } - float GetValueFromImageAsset(const AZ::Data::Asset& imageAsset, const AZ::Vector3& uvw, float tilingX, float tilingY, float defaultValue) + float GetValueFromImageAsset(const AZ::Data::Asset& imageAsset, const AZ::Vector3& uvw, float tilingX, float tilingY, float defaultValue) { if (imageAsset.IsReady()) { - const auto& image = imageAsset.Get(); - AZStd::size_t imageSize = image->m_imageWidth * image->m_imageHeight * - static_cast(image->m_bytesPerPixel); - - if (image->m_imageWidth > 0 && - image->m_imageHeight > 0 && - image->m_imageData.size() == imageSize) + auto imageDescriptor = imageAsset->GetImageDescriptor(); + auto width = imageDescriptor.m_size.m_width; + auto height = imageDescriptor.m_size.m_height; + + if (width > 0 && height > 0) { // When "rasterizing" from uvs, a range of 0-1 has slightly different meanings depending on the sampler state. // For repeating states (Unbounded/None, Repeat), a uv value of 1 should wrap around back to our 0th pixel. @@ -185,8 +184,8 @@ namespace GradientSignal // A 16x16 pixel image and tilingX = tilingY = 1 maps the uv range of 0-1 to 0-16 pixels. // A 16x16 pixel image and tilingX = tilingY = 1.5 maps the uv range of 0-1 to 0-24 pixels. - const AZ::Vector3 tiledDimensions((image->m_imageWidth * tilingX), - (image->m_imageHeight * tilingY), + const AZ::Vector3 tiledDimensions((width * tilingX), + (height * tilingY), 0.0f); // Convert from uv space back to pixel space @@ -195,13 +194,13 @@ namespace GradientSignal // UVs outside the 0-1 range are treated as infinitely tiling, so that we behave the same as the // other gradient generators. As mentioned above, if clamping is desired, we expect it to be applied // outside of this function. - size_t x = static_cast(pixelLookup.GetX()) % image->m_imageWidth; - size_t y = static_cast(pixelLookup.GetY()) % image->m_imageHeight; + auto x = aznumeric_cast(pixelLookup.GetX()) % width; + auto y = aznumeric_cast(pixelLookup.GetY()) % height; // Flip the y because images are stored in reverse of our world axes - size_t index = ((image->m_imageHeight - 1) - y) * image->m_imageWidth + x; + y = (height - 1) - y; - return RetrieveValue(image->m_imageData.data(), index, image->m_imageFormat); + return AZ::RPI::GetSubImagePixelValue(imageAsset, x, y); } } From 4e15cef2fca68d34b77256da8fc68d474ac97b03 Mon Sep 17 00:00:00 2001 From: Chris Galvan Date: Thu, 3 Feb 2022 13:49:54 -0600 Subject: [PATCH 2/5] Removed unused methods to fix compile error on linux Signed-off-by: Chris Galvan --- .../GradientSignal/Code/Source/ImageAsset.cpp | 75 ------------------- 1 file changed, 75 deletions(-) diff --git a/Gems/GradientSignal/Code/Source/ImageAsset.cpp b/Gems/GradientSignal/Code/Source/ImageAsset.cpp index 6ccd9e0a2a..0f91a3ea08 100644 --- a/Gems/GradientSignal/Code/Source/ImageAsset.cpp +++ b/Gems/GradientSignal/Code/Source/ImageAsset.cpp @@ -19,81 +19,6 @@ #include #include -namespace -{ - template - float RetrieveValue(const AZ::u8* mem, size_t index) - { - AZ_Assert(false, "Unimplemented!"); - return 0.0f; - } - - template <> - float RetrieveValue([[maybe_unused]] const AZ::u8* mem, [[maybe_unused]] size_t index) - { - return 0.0f; - } - - template <> - float RetrieveValue(const AZ::u8* mem, size_t index) - { - return mem[index] / static_cast(std::numeric_limits::max()); - } - - template <> - float RetrieveValue(const AZ::u8* mem, size_t index) - { - // 16 bits per channel - auto actualMem = reinterpret_cast(mem); - actualMem += index; - - return *actualMem / static_cast(std::numeric_limits::max()); - } - - template <> - float RetrieveValue(const AZ::u8* mem, size_t index) - { - // 32 bits per channel - auto actualMem = reinterpret_cast(mem); - actualMem += index; - - return *actualMem / static_cast(std::numeric_limits::max()); - } - - template <> - float RetrieveValue(const AZ::u8* mem, size_t index) - { - // 32 bits per channel - auto actualMem = reinterpret_cast(mem); - actualMem += index; - - return *actualMem; - } - - float RetrieveValue(const AZ::u8* mem, size_t index, ImageProcessingAtom::EPixelFormat format) - { - using namespace ImageProcessingAtom; - - switch (format) - { - case ePixelFormat_R8: - return RetrieveValue(mem, index); - - case ePixelFormat_R16: - return RetrieveValue(mem, index); - - case ePixelFormat_R32: - return RetrieveValue(mem, index); - - case ePixelFormat_R32F: - return RetrieveValue(mem, index); - - default: - return RetrieveValue(mem, index); - } - } -} - namespace GradientSignal { void ImageAsset::Reflect(AZ::ReflectContext* context) From 22f018c310b67c4a8c7fc54cde1feb5149454672 Mon Sep 17 00:00:00 2001 From: Chris Galvan Date: Thu, 3 Feb 2022 15:05:01 -0600 Subject: [PATCH 3/5] Moved the ImageProcessingDefines to the Include folder and added it to the header only target Signed-off-by: Chris Galvan --- .../Atom/ImageProcessing}/ImageProcessingDefines.h | 14 ++++++++++++++ .../Source/BuilderSettings/BuilderSettingManager.h | 2 +- .../Code/Source/BuilderSettings/CubemapSettings.h | 2 +- .../Code/Source/BuilderSettings/MipmapSettings.h | 2 +- .../Code/Source/BuilderSettings/PlatformSettings.h | 2 +- .../Code/Source/BuilderSettings/PresetSettings.h | 2 +- .../Code/Source/BuilderSettings/TextureSettings.h | 2 +- .../Code/Source/Compressors/Compressor.h | 2 +- .../Code/Source/ImageBuilderComponent.cpp | 5 ++--- .../Code/Source/ImageProcessingSystemComponent.cpp | 5 ++--- .../Code/Source/Processing/ImageConvert.h | 2 +- .../Code/Source/Processing/ImageConvertJob.h | 2 +- .../Code/Tests/ImageProcessing_Test.cpp | 2 +- .../Code/imageprocessing_files.cmake | 2 +- .../Code/imageprocessingatom_headers_files.cmake | 1 + .../Code/Include/Atom/RPI.Reflect/Image/Image.h | 14 -------------- .../Source/Components/ImageGradientComponent.cpp | 6 +++--- 17 files changed, 33 insertions(+), 34 deletions(-) rename Gems/Atom/Asset/ImageProcessingAtom/Code/{Source/BuilderSettings => Include/Atom/ImageProcessing}/ImageProcessingDefines.h (92%) diff --git a/Gems/Atom/Asset/ImageProcessingAtom/Code/Source/BuilderSettings/ImageProcessingDefines.h b/Gems/Atom/Asset/ImageProcessingAtom/Code/Include/Atom/ImageProcessing/ImageProcessingDefines.h similarity index 92% rename from Gems/Atom/Asset/ImageProcessingAtom/Code/Source/BuilderSettings/ImageProcessingDefines.h rename to Gems/Atom/Asset/ImageProcessingAtom/Code/Include/Atom/ImageProcessing/ImageProcessingDefines.h index dcd95c7fb6..38f6e4a488 100644 --- a/Gems/Atom/Asset/ImageProcessingAtom/Code/Source/BuilderSettings/ImageProcessingDefines.h +++ b/Gems/Atom/Asset/ImageProcessingAtom/Code/Include/Atom/ImageProcessing/ImageProcessingDefines.h @@ -49,6 +49,20 @@ namespace ImageProcessingAtom static const unsigned int s_MinReduceLevel = 0; static const unsigned int s_MaxReduceLevel = 5; + static const char* s_SupportedImageExtensions[] = { + "*.tif", + "*.tiff", + "*.png", + "*.bmp", + "*.jpg", + "*.jpeg", + "*.tga", + "*.gif", + "*.dds", + "*.exr" + }; + static constexpr int s_TotalSupportedImageExtensions = AZ_ARRAY_SIZE(s_SupportedImageExtensions); + enum class RGBWeight : AZ::u32 { uniform, // uniform weights (1.0, 1.0, 1.0) (default) diff --git a/Gems/Atom/Asset/ImageProcessingAtom/Code/Source/BuilderSettings/BuilderSettingManager.h b/Gems/Atom/Asset/ImageProcessingAtom/Code/Source/BuilderSettings/BuilderSettingManager.h index 443b91bc07..65fc0aa2a7 100644 --- a/Gems/Atom/Asset/ImageProcessingAtom/Code/Source/BuilderSettings/BuilderSettingManager.h +++ b/Gems/Atom/Asset/ImageProcessingAtom/Code/Source/BuilderSettings/BuilderSettingManager.h @@ -8,12 +8,12 @@ #pragma once -#include #include #include #include #include #include +#include #include #include diff --git a/Gems/Atom/Asset/ImageProcessingAtom/Code/Source/BuilderSettings/CubemapSettings.h b/Gems/Atom/Asset/ImageProcessingAtom/Code/Source/BuilderSettings/CubemapSettings.h index 77f804b646..fbfc95ac46 100644 --- a/Gems/Atom/Asset/ImageProcessingAtom/Code/Source/BuilderSettings/CubemapSettings.h +++ b/Gems/Atom/Asset/ImageProcessingAtom/Code/Source/BuilderSettings/CubemapSettings.h @@ -11,7 +11,7 @@ #include #include #include -#include +#include #include namespace ImageProcessingAtom diff --git a/Gems/Atom/Asset/ImageProcessingAtom/Code/Source/BuilderSettings/MipmapSettings.h b/Gems/Atom/Asset/ImageProcessingAtom/Code/Source/BuilderSettings/MipmapSettings.h index c9abc82b09..c5fa48e10f 100644 --- a/Gems/Atom/Asset/ImageProcessingAtom/Code/Source/BuilderSettings/MipmapSettings.h +++ b/Gems/Atom/Asset/ImageProcessingAtom/Code/Source/BuilderSettings/MipmapSettings.h @@ -8,7 +8,7 @@ #pragma once -#include +#include #include #include #include diff --git a/Gems/Atom/Asset/ImageProcessingAtom/Code/Source/BuilderSettings/PlatformSettings.h b/Gems/Atom/Asset/ImageProcessingAtom/Code/Source/BuilderSettings/PlatformSettings.h index 10f76db1dd..014b2c7cfb 100644 --- a/Gems/Atom/Asset/ImageProcessingAtom/Code/Source/BuilderSettings/PlatformSettings.h +++ b/Gems/Atom/Asset/ImageProcessingAtom/Code/Source/BuilderSettings/PlatformSettings.h @@ -10,7 +10,7 @@ #include #include -#include +#include #include namespace ImageProcessingAtom diff --git a/Gems/Atom/Asset/ImageProcessingAtom/Code/Source/BuilderSettings/PresetSettings.h b/Gems/Atom/Asset/ImageProcessingAtom/Code/Source/BuilderSettings/PresetSettings.h index 348fff989d..c0228cef79 100644 --- a/Gems/Atom/Asset/ImageProcessingAtom/Code/Source/BuilderSettings/PresetSettings.h +++ b/Gems/Atom/Asset/ImageProcessingAtom/Code/Source/BuilderSettings/PresetSettings.h @@ -11,9 +11,9 @@ #include #include -#include #include #include +#include #include namespace ImageProcessingAtom diff --git a/Gems/Atom/Asset/ImageProcessingAtom/Code/Source/BuilderSettings/TextureSettings.h b/Gems/Atom/Asset/ImageProcessingAtom/Code/Source/BuilderSettings/TextureSettings.h index 24a2f07bfb..1767b5d9e8 100644 --- a/Gems/Atom/Asset/ImageProcessingAtom/Code/Source/BuilderSettings/TextureSettings.h +++ b/Gems/Atom/Asset/ImageProcessingAtom/Code/Source/BuilderSettings/TextureSettings.h @@ -8,7 +8,7 @@ #pragma once -#include +#include #include #include #include diff --git a/Gems/Atom/Asset/ImageProcessingAtom/Code/Source/Compressors/Compressor.h b/Gems/Atom/Asset/ImageProcessingAtom/Code/Source/Compressors/Compressor.h index 6920ae0cc1..ae4217764d 100644 --- a/Gems/Atom/Asset/ImageProcessingAtom/Code/Source/Compressors/Compressor.h +++ b/Gems/Atom/Asset/ImageProcessingAtom/Code/Source/Compressors/Compressor.h @@ -8,7 +8,7 @@ #pragma once -#include +#include #include #include diff --git a/Gems/Atom/Asset/ImageProcessingAtom/Code/Source/ImageBuilderComponent.cpp b/Gems/Atom/Asset/ImageProcessingAtom/Code/Source/ImageBuilderComponent.cpp index 8c2e61ff6d..55a6c1ea8f 100644 --- a/Gems/Atom/Asset/ImageProcessingAtom/Code/Source/ImageBuilderComponent.cpp +++ b/Gems/Atom/Asset/ImageProcessingAtom/Code/Source/ImageBuilderComponent.cpp @@ -27,7 +27,6 @@ #include #include -#include #include namespace ImageProcessingAtom @@ -67,9 +66,9 @@ namespace ImageProcessingAtom AssetBuilderSDK::AssetBuilderDesc builderDescriptor; builderDescriptor.m_name = "Atom Image Builder"; - for (int i = 0; i < AZ::RPI::s_TotalSupportedImageExtensions; i++) + for (int i = 0; i < s_TotalSupportedImageExtensions; i++) { - builderDescriptor.m_patterns.push_back(AssetBuilderSDK::AssetBuilderPattern(AZ::RPI::s_SupportedImageExtensions[i], AssetBuilderSDK::AssetBuilderPattern::PatternType::Wildcard)); + builderDescriptor.m_patterns.push_back(AssetBuilderSDK::AssetBuilderPattern(s_SupportedImageExtensions[i], AssetBuilderSDK::AssetBuilderPattern::PatternType::Wildcard)); } builderDescriptor.m_busId = azrtti_typeid(); diff --git a/Gems/Atom/Asset/ImageProcessingAtom/Code/Source/ImageProcessingSystemComponent.cpp b/Gems/Atom/Asset/ImageProcessingAtom/Code/Source/ImageProcessingSystemComponent.cpp index f0d881c416..665d08b3ae 100644 --- a/Gems/Atom/Asset/ImageProcessingAtom/Code/Source/ImageProcessingSystemComponent.cpp +++ b/Gems/Atom/Asset/ImageProcessingAtom/Code/Source/ImageProcessingSystemComponent.cpp @@ -7,7 +7,6 @@ */ -#include #include #include @@ -224,9 +223,9 @@ namespace ImageProcessingAtom { AZStd::string targetExtension = entry->GetExtension(); - for (int i = 0; i < AZ::RPI::s_TotalSupportedImageExtensions; i++) + for (int i = 0; i < s_TotalSupportedImageExtensions; i++) { - if (AZStd::wildcard_match(AZ::RPI::s_SupportedImageExtensions[i], targetExtension.c_str())) + if (AZStd::wildcard_match(s_SupportedImageExtensions[i], targetExtension.c_str())) { return true; } diff --git a/Gems/Atom/Asset/ImageProcessingAtom/Code/Source/Processing/ImageConvert.h b/Gems/Atom/Asset/ImageProcessingAtom/Code/Source/Processing/ImageConvert.h index ba3d21191f..a59c3471b3 100644 --- a/Gems/Atom/Asset/ImageProcessingAtom/Code/Source/Processing/ImageConvert.h +++ b/Gems/Atom/Asset/ImageProcessingAtom/Code/Source/Processing/ImageConvert.h @@ -10,10 +10,10 @@ #include -#include #include #include #include +#include #include #include diff --git a/Gems/Atom/Asset/ImageProcessingAtom/Code/Source/Processing/ImageConvertJob.h b/Gems/Atom/Asset/ImageProcessingAtom/Code/Source/Processing/ImageConvertJob.h index ac15d47806..2a5aef9902 100644 --- a/Gems/Atom/Asset/ImageProcessingAtom/Code/Source/Processing/ImageConvertJob.h +++ b/Gems/Atom/Asset/ImageProcessingAtom/Code/Source/Processing/ImageConvertJob.h @@ -8,10 +8,10 @@ #pragma once -#include #include #include #include +#include #include namespace ImageProcessingAtom diff --git a/Gems/Atom/Asset/ImageProcessingAtom/Code/Tests/ImageProcessing_Test.cpp b/Gems/Atom/Asset/ImageProcessingAtom/Code/Tests/ImageProcessing_Test.cpp index 91b0952a06..f73446653f 100644 --- a/Gems/Atom/Asset/ImageProcessingAtom/Code/Tests/ImageProcessing_Test.cpp +++ b/Gems/Atom/Asset/ImageProcessingAtom/Code/Tests/ImageProcessing_Test.cpp @@ -32,6 +32,7 @@ #include #include +#include #include #include #include @@ -45,7 +46,6 @@ #include #include -#include #include #include diff --git a/Gems/Atom/Asset/ImageProcessingAtom/Code/imageprocessing_files.cmake b/Gems/Atom/Asset/ImageProcessingAtom/Code/imageprocessing_files.cmake index a6fe09bfa6..3f7cb0a79d 100644 --- a/Gems/Atom/Asset/ImageProcessingAtom/Code/imageprocessing_files.cmake +++ b/Gems/Atom/Asset/ImageProcessingAtom/Code/imageprocessing_files.cmake @@ -13,6 +13,7 @@ set(FILES Include/Atom/ImageProcessing/ImageProcessingEditorBus.h Include/Atom/ImageProcessing/PixelFormats.h Include/Atom/ImageProcessing/ImageObject.h + Include/Atom/ImageProcessing/ImageProcessingDefines.h ../Assets/Editor/Resources.qrc ../Assets/Editor/Backward.png ../Assets/Editor/Forward.png @@ -28,7 +29,6 @@ set(FILES Source/BuilderSettings/BuilderSettings.h Source/BuilderSettings/CubemapSettings.cpp Source/BuilderSettings/CubemapSettings.h - Source/BuilderSettings/ImageProcessingDefines.h Source/BuilderSettings/MipmapSettings.cpp Source/BuilderSettings/MipmapSettings.h Source/BuilderSettings/PlatformSettings.h diff --git a/Gems/Atom/Asset/ImageProcessingAtom/Code/imageprocessingatom_headers_files.cmake b/Gems/Atom/Asset/ImageProcessingAtom/Code/imageprocessingatom_headers_files.cmake index c2c5a11c4c..51864a8442 100644 --- a/Gems/Atom/Asset/ImageProcessingAtom/Code/imageprocessingatom_headers_files.cmake +++ b/Gems/Atom/Asset/ImageProcessingAtom/Code/imageprocessingatom_headers_files.cmake @@ -7,4 +7,5 @@ # set(FILES + Include/Atom/ImageProcessing/ImageProcessingDefines.h ) diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Image/Image.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Image/Image.h index 66c91e92cf..185666626c 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Image/Image.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Image/Image.h @@ -22,20 +22,6 @@ namespace AZ { class ImageAsset; - static const int s_TotalSupportedImageExtensions = 10; - static const char* s_SupportedImageExtensions[s_TotalSupportedImageExtensions] = { - "*.tif", - "*.tiff", - "*.png", - "*.bmp", - "*.jpg", - "*.jpeg", - "*.tga", - "*.gif", - "*.dds", - "*.exr" - }; - //! A base class for images providing access to common image information. class Image : public Data::InstanceData diff --git a/Gems/GradientSignal/Code/Source/Components/ImageGradientComponent.cpp b/Gems/GradientSignal/Code/Source/Components/ImageGradientComponent.cpp index e3f128ec95..c06792c1f1 100644 --- a/Gems/GradientSignal/Code/Source/Components/ImageGradientComponent.cpp +++ b/Gems/GradientSignal/Code/Source/Components/ImageGradientComponent.cpp @@ -7,7 +7,7 @@ */ #include -#include +#include #include #include #include @@ -52,9 +52,9 @@ namespace GradientSignal // We don't know what image format the original source was, so we need to loop through // all the supported image extensions to check if they have a valid corresponding // streaming image asset - for (int i = 0; i < AZ::RPI::s_TotalSupportedImageExtensions; i++) + for (int i = 0; i < ImageProcessingAtom::s_TotalSupportedImageExtensions; i++) { - AZStd::string imageExtension(AZ::RPI::s_SupportedImageExtensions[i]); + AZStd::string imageExtension(ImageProcessingAtom::s_SupportedImageExtensions[i]); // The image extensions are stored with a wildcard (e.g. *.png) so we need to strip that off first AZ::StringFunc::Replace(imageExtension, "*", ""); From 2c54b48a9e726906aa6b39780a1ea68af1dcc2ac Mon Sep 17 00:00:00 2001 From: Chris Galvan Date: Thu, 3 Feb 2022 16:20:55 -0600 Subject: [PATCH 4/5] Modified image gradient config json serializer to be able to detect version difference earlier Signed-off-by: Chris Galvan --- .../Components/ImageGradientComponent.cpp | 108 +++++++++--------- 1 file changed, 54 insertions(+), 54 deletions(-) diff --git a/Gems/GradientSignal/Code/Source/Components/ImageGradientComponent.cpp b/Gems/GradientSignal/Code/Source/Components/ImageGradientComponent.cpp index c06792c1f1..468b96e5ad 100644 --- a/Gems/GradientSignal/Code/Source/Components/ImageGradientComponent.cpp +++ b/Gems/GradientSignal/Code/Source/Components/ImageGradientComponent.cpp @@ -23,6 +23,16 @@ namespace GradientSignal void* outputValue, [[maybe_unused]] const AZ::Uuid& outputValueTypeId, const rapidjson::Value& inputValue, AZ::JsonDeserializerContext& context) { + // We can distinguish between version 1 and 2 by the presence of the "ImageAsset" field, + // which is only in version 1. + // For version 2, we don't need to do any special processing, so just let the base class + // load the JSON if we don't find the "ImageAsset" field. + rapidjson::Value::ConstMemberIterator itr = inputValue.FindMember("ImageAsset"); + if (itr == inputValue.MemberEnd()) + { + return AZ::BaseJsonSerializer::Load(outputValue, outputValueTypeId, inputValue, context); + } + namespace JSR = AZ::JsonSerializationResult; auto configInstance = reinterpret_cast(outputValue); @@ -30,68 +40,58 @@ namespace GradientSignal JSR::ResultCode result(JSR::Tasks::ReadField); - rapidjson::Value::ConstMemberIterator itr = inputValue.FindMember("ImageAsset"); - if (itr != inputValue.MemberEnd()) + result.Combine(ContinueLoadingFromJsonObjectField( + &configInstance->m_tilingX, azrtti_typeidm_tilingX)>(), inputValue, "TilingX", context)); + + result.Combine(ContinueLoadingFromJsonObjectField( + &configInstance->m_tilingY, azrtti_typeidm_tilingY)>(), inputValue, "TilingY", context)); + + // Version 1 stored a custom GradientSignal::ImageAsset as the image asset. + // In Version 2, we changed the image asset to use the generic AZ::RPI::StreamingImageAsset, + // so they are both AZ::Data::Asset but reference different types. + // Using the assetHint, which will be something like "my_test_image.gradimage", + // we need to find the valid streaming image asset product from the same source, + // which will be something like "my_test_image.png.streamingimage" + AZStd::string assetHint; + AZ::Data::AssetId fixedAssetId; + auto it = itr->value.FindMember("assetHint"); + if (it != itr->value.MemberEnd()) { - // Version 1 stored a custom GradientSignal::ImageAsset as the image asset. - // In Version 2, we changed the image asset to use the generic AZ::RPI::StreamingImageAsset, - // so they are both AZ::Data::Asset but reference different types. - // Using the assetHint, which will be something like "my_test_image.gradimage", - // we need to find the valid streaming image asset product from the same source, - // which will be something like "my_test_image.png.streamingimage" - AZStd::string assetHint; - AZ::Data::AssetId fixedAssetId; - auto it = itr->value.FindMember("assetHint"); - if (it != itr->value.MemberEnd()) - { - AZ::ScopedContextPath subPath(context, "assetHint"); - result.Combine(ContinueLoading(&assetHint, azrtti_typeid(), it->value, context)); + AZ::ScopedContextPath subPath(context, "assetHint"); + result.Combine(ContinueLoading(&assetHint, azrtti_typeid(), it->value, context)); - if (assetHint.ends_with(".gradimage")) + if (assetHint.ends_with(".gradimage")) + { + // We don't know what image format the original source was, so we need to loop through + // all the supported image extensions to check if they have a valid corresponding + // streaming image asset + for (auto& supportedImageExtension : ImageProcessingAtom::s_SupportedImageExtensions) { - // We don't know what image format the original source was, so we need to loop through - // all the supported image extensions to check if they have a valid corresponding - // streaming image asset - for (int i = 0; i < ImageProcessingAtom::s_TotalSupportedImageExtensions; i++) + AZStd::string imageExtension(supportedImageExtension); + + // The image extensions are stored with a wildcard (e.g. *.png) so we need to strip that off first + AZ::StringFunc::Replace(imageExtension, "*", ""); + + // Form potential streaming image path (e.g. my_test_image.png.streamingimage) + AZStd::string potentialStreamingImagePath(assetHint); + AZ::StringFunc::Replace(potentialStreamingImagePath, ".gradimage", ""); + potentialStreamingImagePath += imageExtension + ".streamingimage"; + + // Check if there is a valid streaming image asset for this path + AZ::Data::AssetCatalogRequestBus::BroadcastResult(fixedAssetId, &AZ::Data::AssetCatalogRequestBus::Events::GetAssetIdByPath, potentialStreamingImagePath.c_str(), azrtti_typeid>(), false); + if (fixedAssetId.IsValid()) { - AZStd::string imageExtension(ImageProcessingAtom::s_SupportedImageExtensions[i]); - - // The image extensions are stored with a wildcard (e.g. *.png) so we need to strip that off first - AZ::StringFunc::Replace(imageExtension, "*", ""); - - // Form potential streaming image path (e.g. my_test_image.png.streamingimage) - AZStd::string potentialStreamingImagePath(assetHint); - AZ::StringFunc::Replace(potentialStreamingImagePath, ".gradimage", ""); - potentialStreamingImagePath += imageExtension + ".streamingimage"; - - // Check if there is a valid streaming image asset for this path - AZ::Data::AssetCatalogRequestBus::BroadcastResult(fixedAssetId, &AZ::Data::AssetCatalogRequestBus::Events::GetAssetIdByPath, potentialStreamingImagePath.c_str(), azrtti_typeid>(), false); - if (fixedAssetId.IsValid()) - { - break; - } + break; } } } - - // Replace the old gradimage with new AssetId for streaming image asset (if we needed to replace) - if (fixedAssetId.IsValid()) - { - configInstance->m_imageAsset = AZ::Data::AssetManager::Instance().GetAsset(fixedAssetId, AZ::Data::AssetLoadBehavior::QueueLoad); - } - // Otherwise, this was already a streaming image asset, so just load it like normal - else - { - result.Combine(ContinueLoadingFromJsonObjectField( - &configInstance->m_imageAsset, azrtti_typeidm_imageAsset)>(), inputValue, "ImageAsset", context)); - } } - result.Combine(ContinueLoadingFromJsonObjectField( - &configInstance->m_tilingX, azrtti_typeidm_tilingX)>(), inputValue, "TilingX", context)); - - result.Combine(ContinueLoadingFromJsonObjectField( - &configInstance->m_tilingY, azrtti_typeidm_tilingY)>(), inputValue, "TilingY", context)); + // Replace the old gradimage with new AssetId for streaming image asset + if (fixedAssetId.IsValid()) + { + configInstance->m_imageAsset = AZ::Data::AssetManager::Instance().GetAsset(fixedAssetId, AZ::Data::AssetLoadBehavior::QueueLoad); + } return context.Report(result, result.GetProcessing() != JSR::Processing::Halted ? @@ -113,9 +113,9 @@ namespace GradientSignal { serialize->Class() ->Version(2) - ->Field("ImageAsset", &ImageGradientConfig::m_imageAsset) ->Field("TilingX", &ImageGradientConfig::m_tilingX) ->Field("TilingY", &ImageGradientConfig::m_tilingY) + ->Field("StreamingImageAsset", &ImageGradientConfig::m_imageAsset) ; AZ::EditContext* edit = serialize->GetEditContext(); From e66d55189e175864c40c2d1d776c615ffd11c0ed Mon Sep 17 00:00:00 2001 From: Chris Galvan Date: Mon, 7 Feb 2022 12:06:11 -0600 Subject: [PATCH 5/5] Updated the gradient signal unit tests for the image gradient now using a StreamingImageAsset Signed-off-by: Chris Galvan --- .../Code/Tests/GradientSignalImageTests.cpp | 5 +- .../Code/Tests/GradientSignalTestFixtures.cpp | 15 +- .../Code/Tests/GradientSignalTestFixtures.h | 3 +- .../Code/Tests/GradientSignalTestHelpers.cpp | 178 ++++++++++++++++++ .../Code/Tests/GradientSignalTestHelpers.h | 64 +++++++ .../Code/Tests/GradientSignalTestMocks.cpp | 74 -------- .../Code/Tests/GradientSignalTestMocks.h | 47 ----- .../gradientsignal_shared_tests_files.cmake | 1 - 8 files changed, 257 insertions(+), 130 deletions(-) delete mode 100644 Gems/GradientSignal/Code/Tests/GradientSignalTestMocks.cpp diff --git a/Gems/GradientSignal/Code/Tests/GradientSignalImageTests.cpp b/Gems/GradientSignal/Code/Tests/GradientSignalImageTests.cpp index 8ec190000d..c6656d589e 100644 --- a/Gems/GradientSignal/Code/Tests/GradientSignalImageTests.cpp +++ b/Gems/GradientSignal/Code/Tests/GradientSignalImageTests.cpp @@ -8,6 +8,7 @@ #include +#include #include #include @@ -88,7 +89,7 @@ namespace UnitTest // Create the Image Gradient Component. GradientSignal::ImageGradientConfig config; - config.m_imageAsset = ImageAssetMockAssetHandler::CreateSpecificPixelImageAsset( + config.m_imageAsset = UnitTest::CreateSpecificPixelImageAsset( test.m_imageSize, test.m_imageSize, static_cast(test.m_pixel.GetX()), static_cast(test.m_pixel.GetY())); config.m_tilingX = test.m_tiling; config.m_tilingY = test.m_tiling; @@ -379,7 +380,7 @@ namespace UnitTest // Create an ImageGradient with a 3x3 asset with the center pixel set. GradientSignal::ImageGradientConfig gradientConfig; - gradientConfig.m_imageAsset = ImageAssetMockAssetHandler::CreateSpecificPixelImageAsset(3, 3, 1, 1); + gradientConfig.m_imageAsset = UnitTest::CreateSpecificPixelImageAsset(3, 3, 1, 1); entity->CreateComponent(gradientConfig); // Create the test GradientTransform diff --git a/Gems/GradientSignal/Code/Tests/GradientSignalTestFixtures.cpp b/Gems/GradientSignal/Code/Tests/GradientSignalTestFixtures.cpp index 402438ee88..d07d2aae50 100644 --- a/Gems/GradientSignal/Code/Tests/GradientSignalTestFixtures.cpp +++ b/Gems/GradientSignal/Code/Tests/GradientSignalTestFixtures.cpp @@ -8,7 +8,10 @@ #include +#include +#include +#include #include #include #include @@ -70,14 +73,16 @@ namespace UnitTest void GradientSignalBaseFixture::SetupCoreSystems() { - m_mockHandler = new UnitTest::ImageAssetMockAssetHandler(); - AZ::Data::AssetManager::Instance().RegisterHandler(m_mockHandler, azrtti_typeid()); + // Using the AZ::RPI::MakeAssetHandler will both create the asset handlers, + // and register them with the AssetManager + m_assetHandlers.emplace_back(AZ::RPI::MakeAssetHandler()); + m_assetHandlers.emplace_back(AZ::RPI::MakeAssetHandler()); } void GradientSignalBaseFixture::TearDownCoreSystems() { - AZ::Data::AssetManager::Instance().UnregisterHandler(m_mockHandler); - delete m_mockHandler; // delete after removing from the asset manager + // This will delete the asset handlers, which will unregister themselves on deletion + m_assetHandlers.clear(); AzFramework::LegacyAssetEventBus::ClearQueuedEvents(); } @@ -147,7 +152,7 @@ namespace UnitTest GradientSignal::ImageGradientConfig config; const uint32_t imageSize = 4096; const int32_t imageSeed = 12345; - config.m_imageAsset = ImageAssetMockAssetHandler::CreateImageAsset(imageSize, imageSize, imageSeed); + config.m_imageAsset = UnitTest::CreateImageAsset(imageSize, imageSize, imageSeed); config.m_tilingX = 1.0f; config.m_tilingY = 1.0f; entity->CreateComponent(config); diff --git a/Gems/GradientSignal/Code/Tests/GradientSignalTestFixtures.h b/Gems/GradientSignal/Code/Tests/GradientSignalTestFixtures.h index 5fda88ea27..1c703964e7 100644 --- a/Gems/GradientSignal/Code/Tests/GradientSignalTestFixtures.h +++ b/Gems/GradientSignal/Code/Tests/GradientSignalTestFixtures.h @@ -9,6 +9,7 @@ #include #include +#include #include namespace UnitTest @@ -89,7 +90,7 @@ namespace UnitTest AZStd::unique_ptr BuildTestSurfaceMaskGradient(float shapeHalfBounds); AZStd::unique_ptr BuildTestSurfaceSlopeGradient(float shapeHalfBounds); - UnitTest::ImageAssetMockAssetHandler* m_mockHandler = nullptr; + AZ::RPI::AssetHandlerPtrList m_assetHandlers; }; struct GradientSignalTest diff --git a/Gems/GradientSignal/Code/Tests/GradientSignalTestHelpers.cpp b/Gems/GradientSignal/Code/Tests/GradientSignalTestHelpers.cpp index 46cc3475e8..560aa09f48 100644 --- a/Gems/GradientSignal/Code/Tests/GradientSignalTestHelpers.cpp +++ b/Gems/GradientSignal/Code/Tests/GradientSignalTestHelpers.cpp @@ -8,11 +8,189 @@ #include +#include +#include #include #include namespace UnitTest { + AZ::RHI::ImageSubresourceLayout BuildSubImageLayout(AZ::u32 width, AZ::u32 height, AZ::u32 pixelSize) + { + AZ::RHI::ImageSubresourceLayout layout; + layout.m_size = AZ::RHI::Size{ width, height, 1 }; + layout.m_rowCount = width; + layout.m_bytesPerRow = width * pixelSize; + layout.m_bytesPerImage = width * height * pixelSize; + return layout; + } + + AZStd::vector BuildBasicImageData(AZ::u32 width, AZ::u32 height, AZ::u32 pixelSize, AZ::s32 seed) + { + const size_t imageSize = width * height * pixelSize; + + AZStd::vector image; + image.reserve(imageSize); + + size_t value = 0; + AZStd::hash_combine(value, seed); + + for (AZ::u32 x = 0; x < width; ++x) + { + for (AZ::u32 y = 0; y < height; ++y) + { + AZStd::hash_combine(value, x); + AZStd::hash_combine(value, y); + image.push_back(static_cast(value)); + } + } + + EXPECT_EQ(image.size(), imageSize); + return image; + } + + AZ::Data::Asset BuildBasicMipChainAsset(AZ::u16 mipLevels, AZ::u16 arraySize, AZ::u32 width, AZ::u32 height, AZ::u32 pixelSize, AZ::s32 seed) + { + using namespace AZ; + + RPI::ImageMipChainAssetCreator assetCreator; + + assetCreator.Begin(Data::AssetId(AZ::Uuid::CreateRandom()), mipLevels, arraySize); + + RHI::ImageSubresourceLayout layout = BuildSubImageLayout(width, height, pixelSize); + + assetCreator.BeginMip(layout); + + for (AZ::u32 arrayIndex = 0; arrayIndex < arraySize; ++arrayIndex) + { + AZStd::vector data = BuildBasicImageData(width, height, pixelSize, seed); + assetCreator.AddSubImage(data.data(), data.size()); + } + + assetCreator.EndMip(); + + Data::Asset asset; + EXPECT_TRUE(assetCreator.End(asset)); + EXPECT_TRUE(asset.IsReady()); + EXPECT_NE(asset.Get(), nullptr); + + return asset; + } + + AZStd::vector BuildSpecificPixelImageData(AZ::u32 width, AZ::u32 height, AZ::u32 pixelSize, AZ::u32 pixelX, AZ::u32 pixelY) + { + const size_t imageSize = width * height * pixelSize; + + AZStd::vector image; + image.reserve(imageSize); + + const AZ::u8 pixelValue = 255; + + // Image data should be stored inverted on the y axis relative to our engine, so loop backwards through y. + for (int y = static_cast(height) - 1; y >= 0; --y) + { + for (AZ::u32 x = 0; x < width; ++x) + { + if ((x == static_cast(pixelX)) && (y == static_cast(pixelY))) + { + image.push_back(pixelValue); + } + else + { + image.push_back(0); + } + } + } + + EXPECT_EQ(image.size(), imageSize); + return image; + } + + AZ::Data::Asset BuildSpecificPixelMipChainAsset(AZ::u16 mipLevels, AZ::u16 arraySize, AZ::u32 width, AZ::u32 height, AZ::u32 pixelSize, AZ::u32 pixelX, AZ::u32 pixelY) + { + using namespace AZ; + + RPI::ImageMipChainAssetCreator assetCreator; + + assetCreator.Begin(Data::AssetId(AZ::Uuid::CreateRandom()), mipLevels, arraySize); + + RHI::ImageSubresourceLayout layout = BuildSubImageLayout(width, height, pixelSize); + + assetCreator.BeginMip(layout); + + for (AZ::u32 arrayIndex = 0; arrayIndex < arraySize; ++arrayIndex) + { + AZStd::vector data = BuildSpecificPixelImageData(width, height, pixelSize, pixelX, pixelY); + assetCreator.AddSubImage(data.data(), data.size()); + } + + assetCreator.EndMip(); + + Data::Asset asset; + EXPECT_TRUE(assetCreator.End(asset)); + EXPECT_TRUE(asset.IsReady()); + EXPECT_NE(asset.Get(), nullptr); + + return asset; + } + + AZ::Data::Asset CreateImageAsset(AZ::u32 width, AZ::u32 height, AZ::s32 seed) + { + auto randomAssetId = AZ::Data::AssetId(AZ::Uuid::CreateRandom()); + auto imageAsset = AZ::Data::AssetManager::Instance().CreateAsset( + randomAssetId, AZ::Data::AssetLoadBehavior::Default); + + const AZ::u32 arraySize = 1; + const AZ::u32 mipCountTotal = 1; + const auto format = AZ::RHI::Format::R8_UNORM; + const AZ::u32 pixelSize = AZ::RHI::GetFormatComponentCount(format); + + AZ::Data::Asset mipChain = BuildBasicMipChainAsset(mipCountTotal, arraySize, width, height, pixelSize, seed); + + AZ::RPI::StreamingImageAssetCreator assetCreator; + assetCreator.Begin(randomAssetId); + + AZ::RHI::ImageDescriptor imageDesc = AZ::RHI::ImageDescriptor::Create2DArray(AZ::RHI::ImageBindFlags::ShaderRead, width, height, arraySize, format); + imageDesc.m_mipLevels = static_cast(mipCountTotal); + + assetCreator.SetImageDescriptor(imageDesc); + assetCreator.AddMipChainAsset(*mipChain.Get()); + + EXPECT_TRUE(assetCreator.End(imageAsset)); + EXPECT_TRUE(imageAsset.IsReady()); + EXPECT_NE(imageAsset.Get(), nullptr); + + return imageAsset; + } + + AZ::Data::Asset CreateSpecificPixelImageAsset(AZ::u32 width, AZ::u32 height, AZ::u32 pixelX, AZ::u32 pixelY) + { + auto randomAssetId = AZ::Data::AssetId(AZ::Uuid::CreateRandom()); + auto imageAsset = AZ::Data::AssetManager::Instance().CreateAsset( + randomAssetId, AZ::Data::AssetLoadBehavior::Default); + + const AZ::u32 arraySize = 1; + const AZ::u32 mipCountTotal = 1; + const auto format = AZ::RHI::Format::R8_UNORM; + const AZ::u32 pixelSize = AZ::RHI::GetFormatComponentCount(format); + + AZ::Data::Asset mipChain = BuildSpecificPixelMipChainAsset(mipCountTotal, arraySize, width, height, pixelSize, pixelX, pixelY); + + AZ::RPI::StreamingImageAssetCreator assetCreator; + assetCreator.Begin(randomAssetId); + + AZ::RHI::ImageDescriptor imageDesc = AZ::RHI::ImageDescriptor::Create2DArray(AZ::RHI::ImageBindFlags::ShaderRead, width, height, arraySize, format); + imageDesc.m_mipLevels = static_cast(mipCountTotal); + + assetCreator.SetImageDescriptor(imageDesc); + assetCreator.AddMipChainAsset(*mipChain.Get()); + + EXPECT_TRUE(assetCreator.End(imageAsset)); + EXPECT_TRUE(imageAsset.IsReady()); + EXPECT_NE(imageAsset.Get(), nullptr); + return imageAsset; + } + void GradientSignalTestHelpers::CompareGetValueAndGetValues(AZ::EntityId gradientEntityId, float shapeHalfBounds) { // Create a gradient sampler and run through a series of points to see if they match expectations. diff --git a/Gems/GradientSignal/Code/Tests/GradientSignalTestHelpers.h b/Gems/GradientSignal/Code/Tests/GradientSignalTestHelpers.h index 8a175939ee..5a13167975 100644 --- a/Gems/GradientSignal/Code/Tests/GradientSignalTestHelpers.h +++ b/Gems/GradientSignal/Code/Tests/GradientSignalTestHelpers.h @@ -12,8 +12,72 @@ #include #include +#include +#include +#include + namespace UnitTest { + //! Helper method to build a AZ::RHI::ImageSubresourceLayout + //! @param width The width of the image + //! @param height The height of the image + //! @param pixelSize Number of bytes per pixel + //! @return The AZ::RHI::ImageSubresourceLayout that has been filled out appropriately + AZ::RHI::ImageSubresourceLayout BuildSubImageLayout(AZ::u32 width, AZ::u32 height, AZ::u32 pixelSize); + + //! Build a deterministic random set of image pixel data + //! @param width Width of the image + //! @param height Height of the image + //! @param pixelSize Number of bytes per pixel + //! @param seed The random seed for generating the data + //! @return A vector of bytes for the image data + AZStd::vector BuildBasicImageData(AZ::u32 width, AZ::u32 height, AZ::u32 pixelSize, AZ::s32 seed); + + //! Build a mip chain asset that contains the basic image data from BuildBasicImageData + //! @param mipLevels Number of mip levels in the chain + //! @param arraySize Number of sub images within a mip level + //! @param width The width of the image + //! @param height The height of the image + //! @param pixelSize The number of bytes per pixel + //! @param seed The random seed for generating the data + //! @return A mip chain asset with the specified basic image data + AZ::Data::Asset BuildBasicMipChainAsset(AZ::u16 mipLevels, AZ::u16 arraySize, AZ::u32 width, AZ::u32 height, AZ::u32 pixelSize, AZ::s32 seed); + + //! Construct an array of image data where all the pixels are 0 except for one at the given coordinate + //! @param width Width of the image + //! @param height Height of the image + //! @param pixelSize Number of bytes per pixel + //! @param pixelX The X coordinate of the pixel to set to 1 + //! @param pixelY The Y coordinate of the pixel to set to 1 + //! @return A vector of bytes for the image data + AZStd::vector BuildSpecificPixelImageData(AZ::u32 width, AZ::u32 height, AZ::u32 pixelSize, AZ::u32 pixelX, AZ::u32 pixelY); + + //! Build a mip chain asset that contains the specific image data from BuildSpecificPixelImageData + //! @param mipLevels Number of mip levels in the chain + //! @param arraySize Number of sub images within a mip level + //! @param width The width of the image + //! @param height The height of the image + //! @param pixelSize The number of bytes per pixel + //! @param pixelX The X coordinate of the pixel to set to 1 + //! @param pixelY The Y coordinate of the pixel to set to 1 + //! @return A mip chain asset with the specific pixel image data + AZ::Data::Asset BuildSpecificPixelMipChainAsset(AZ::u16 mipLevels, AZ::u16 arraySize, AZ::u32 width, AZ::u32 height, AZ::u32 pixelSize, AZ::u32 pixelX, AZ::u32 pixelY); + + //! Creates a deterministically random set of pixel data as an AZ::RPI::StreamingImageAsset. + //! \param width The width of the AZ::RPI::StreamingImageAsset + //! \param height The height of the AZ::RPI::StreamingImageAsset + //! \param seed The random seed to use for generating the random data + //! \return The AZ::RPI::StreamingImageAsset in a loaded ready state + AZ::Data::Asset CreateImageAsset(AZ::u32 width, AZ::u32 height, AZ::s32 seed); + + //! Creates an AZ::RPI::StreamingImageAsset where all the pixels are 0 except for the one pixel at the given coordinates, which is set to 1. + //! \param width The width of the AZ::RPI::StreamingImageAsset + //! \param height The height of the AZ::RPI::StreamingImageAsset + //! \param pixelX The X coordinate of the pixel to set to 1 + //! \param pixelY The Y coordinate of the pixel to set to 1 + //! \return The AZ::RPI::StreamingImageAsset in a loaded ready state + AZ::Data::Asset CreateSpecificPixelImageAsset(AZ::u32 width, AZ::u32 height, AZ::u32 pixelX, AZ::u32 pixelY); + class GradientSignalTestHelpers { public: diff --git a/Gems/GradientSignal/Code/Tests/GradientSignalTestMocks.cpp b/Gems/GradientSignal/Code/Tests/GradientSignalTestMocks.cpp deleted file mode 100644 index 7ac6ef1ecc..0000000000 --- a/Gems/GradientSignal/Code/Tests/GradientSignalTestMocks.cpp +++ /dev/null @@ -1,74 +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 - -namespace UnitTest -{ - AZ::Data::Asset ImageAssetMockAssetHandler::CreateImageAsset(AZ::u32 width, AZ::u32 height, AZ::s32 seed) - { - auto imageAsset = AZ::Data::AssetManager::Instance().CreateAsset( - AZ::Data::AssetId(AZ::Uuid::CreateRandom()), AZ::Data::AssetLoadBehavior::Default); - - imageAsset->m_imageWidth = width; - imageAsset->m_imageHeight = height; - imageAsset->m_bytesPerPixel = 1; - imageAsset->m_imageFormat = ImageProcessingAtom::EPixelFormat::ePixelFormat_R8; - imageAsset->m_imageData.reserve(width * height); - - size_t value = 0; - AZStd::hash_combine(value, seed); - - for (AZ::u32 x = 0; x < width; ++x) - { - for (AZ::u32 y = 0; y < height; ++y) - { - AZStd::hash_combine(value, x); - AZStd::hash_combine(value, y); - imageAsset->m_imageData.push_back(static_cast(value)); - } - } - - return imageAsset; - } - - AZ::Data::Asset ImageAssetMockAssetHandler::CreateSpecificPixelImageAsset( - AZ::u32 width, AZ::u32 height, AZ::u32 pixelX, AZ::u32 pixelY) - { - auto imageAsset = AZ::Data::AssetManager::Instance().CreateAsset( - AZ::Data::AssetId(AZ::Uuid::CreateRandom()), AZ::Data::AssetLoadBehavior::Default); - - imageAsset->m_imageWidth = width; - imageAsset->m_imageHeight = height; - imageAsset->m_bytesPerPixel = 1; - imageAsset->m_imageFormat = ImageProcessingAtom::EPixelFormat::ePixelFormat_R8; - imageAsset->m_imageData.reserve(width * height); - - const AZ::u8 pixelValue = 255; - - // Image data should be stored inverted on the y axis relative to our engine, so loop backwards through y. - for (int y = static_cast(height) - 1; y >= 0; --y) - { - for (AZ::u32 x = 0; x < width; ++x) - { - if ((x == static_cast(pixelX)) && (y == static_cast(pixelY))) - { - imageAsset->m_imageData.push_back(pixelValue); - } - else - { - imageAsset->m_imageData.push_back(0); - } - } - } - - return imageAsset; - } -} - diff --git a/Gems/GradientSignal/Code/Tests/GradientSignalTestMocks.h b/Gems/GradientSignal/Code/Tests/GradientSignalTestMocks.h index 30f262d9b4..997e88ce17 100644 --- a/Gems/GradientSignal/Code/Tests/GradientSignalTestMocks.h +++ b/Gems/GradientSignal/Code/Tests/GradientSignalTestMocks.h @@ -27,53 +27,6 @@ namespace UnitTest { - // Mock asset handler for GradientSignal::ImageAsset that we can use in unit tests to pretend to load an image asset with. - // Also includes utility functions for creating image assets with specific testable patterns. - struct ImageAssetMockAssetHandler : public AZ::Data::AssetHandler - { - //! Creates a deterministically random set of pixel data as an ImageAsset. - //! \param width The width of the ImageAsset - //! \param height The height of the ImageAsset - //! \param seed The random seed to use for generating the random data - //! \return The ImageAsset in a loaded ready state - static AZ::Data::Asset CreateImageAsset(AZ::u32 width, AZ::u32 height, AZ::s32 seed); - - //! Creates an ImageAsset where all the pixels are 0 except for the one pixel at the given coordinates, which is set to 1. - //! \param width The width of the ImageAsset - //! \param height The height of the ImageAsset - //! \param pixelX The X coordinate of the pixel to set to 1 - //! \param pixelY The Y coordinate of the pixel to set to 1 - //! \return The ImageAsset in a loaded ready state - static AZ::Data::Asset CreateSpecificPixelImageAsset( - AZ::u32 width, AZ::u32 height, AZ::u32 pixelX, AZ::u32 pixelY); - - AZ::Data::AssetPtr CreateAsset(const AZ::Data::AssetId& id, [[maybe_unused]] const AZ::Data::AssetType& type) override - { - // For our mock handler, always mark our assets as immediately ready. - return aznew GradientSignal::ImageAsset(id, AZ::Data::AssetData::AssetStatus::Ready); - } - - void DestroyAsset(AZ::Data::AssetPtr ptr) override - { - if (ptr) - { - delete ptr; - } - } - - void GetHandledAssetTypes([[maybe_unused]] AZStd::vector& assetTypes) override - { - } - - AZ::Data::AssetHandler::LoadResult LoadAssetData( - [[maybe_unused]] const AZ::Data::Asset& asset, - [[maybe_unused]] AZStd::shared_ptr stream, - [[maybe_unused]] const AZ::Data::AssetFilterCB& assetLoadFilterCB) override - { - return AZ::Data::AssetHandler::LoadResult::LoadComplete; - } - }; - struct MockGradientRequestsBus : public GradientSignal::GradientRequestBus::Handler { diff --git a/Gems/GradientSignal/Code/gradientsignal_shared_tests_files.cmake b/Gems/GradientSignal/Code/gradientsignal_shared_tests_files.cmake index 98ab57b7b0..5e11406bae 100644 --- a/Gems/GradientSignal/Code/gradientsignal_shared_tests_files.cmake +++ b/Gems/GradientSignal/Code/gradientsignal_shared_tests_files.cmake @@ -11,6 +11,5 @@ set(FILES Tests/GradientSignalTestHelpers.h Tests/GradientSignalTestFixtures.cpp Tests/GradientSignalTestFixtures.h - Tests/GradientSignalTestMocks.cpp Tests/GradientSignalTestMocks.h )