From 905bdf9627f5c6226624d3343fb2c45f0d707440 Mon Sep 17 00:00:00 2001 From: michabr <82236305+michabr@users.noreply.github.com> Date: Mon, 3 May 2021 16:32:18 -0700 Subject: [PATCH] Fix sprite asset selection in property editor (#384) * Fix sprite asset selection in property editor * Linux compile fix * More fixes for the custom Sprite property handler * PR feedback to use existing constant image extension --- .../Code/Editor/PropertyHandlerSprite.cpp | 26 +++- Gems/LyShine/Code/Source/Sprite.cpp | 125 +++++++++++------- Gems/LyShine/Code/Source/Sprite.h | 8 ++ Gems/LyShine/Code/Source/UiImageComponent.cpp | 3 +- 4 files changed, 106 insertions(+), 56 deletions(-) diff --git a/Gems/LyShine/Code/Editor/PropertyHandlerSprite.cpp b/Gems/LyShine/Code/Editor/PropertyHandlerSprite.cpp index 20721d4624..9263210088 100644 --- a/Gems/LyShine/Code/Editor/PropertyHandlerSprite.cpp +++ b/Gems/LyShine/Code/Editor/PropertyHandlerSprite.cpp @@ -11,6 +11,7 @@ */ #include "UiCanvasEditor_precompiled.h" #include "EditorCommon.h" +#include "Sprite.h" #include "PropertyHandlerSprite.h" @@ -31,6 +32,8 @@ #include +#include + #include #include @@ -44,6 +47,7 @@ PropertySpriteCtrl::PropertySpriteCtrl(QWidget* parent) [ this ]([[maybe_unused]] AZ::Data::AssetId newAssetID) { EBUS_EVENT(AzToolsFramework::PropertyEditorGUIMessages::Bus, RequestWrite, this); + AzToolsFramework::PropertyEditorGUIMessages::Bus::Broadcast(&AzToolsFramework::PropertyEditorGUIMessages::Bus::Handler::OnEditingFinished, m_propertyAssetCtrl); }); setAcceptDrops(true); @@ -150,7 +154,9 @@ void PropertyHandlerSprite::WriteGUIValuesIntoProperty(size_t index, PropertySpr AZStd::string assetPath; EBUS_EVENT_RESULT(assetPath, AZ::Data::AssetCatalogRequestBus, GetAssetPathById, GUI->GetPropertyAssetCtrl()->GetCurrentAssetID()); - instance.SetAssetPath(assetPath.c_str()); + // Convert streaming image's product path to relative source path to assign to the SimpleAssetReference + AZStd::string sourcePath = CSprite::GetImageSourcePathFromProductPath(assetPath); + instance.SetAssetPath(sourcePath.c_str()); } bool PropertyHandlerSprite::ReadValuesIntoGUI(size_t index, PropertySpriteCtrl* GUI, const property_t& instance, AzToolsFramework::InstanceDataNode* node) @@ -162,12 +168,26 @@ bool PropertyHandlerSprite::ReadValuesIntoGUI(size_t index, PropertySpriteCtrl* ctrl->blockSignals(true); { - ctrl->SetCurrentAssetType(instance.GetAssetType()); + // Set the asset type for the PropertyAssetCtrl. + // Use the hardcoded streaming image asset type instead of the passed in instance's asset type + // since the passed in type is the legacy SimpleAssetReference, and the asset picker + // does not associate this type with streaming images + AZ::Data::AssetType assetType = AZ::AzTypeInfo::Uuid(); + ctrl->SetCurrentAssetType(assetType); AZ::Data::AssetId assetId; if (!instance.GetAssetPath().empty()) { - EBUS_EVENT_RESULT(assetId, AZ::Data::AssetCatalogRequestBus, GetAssetIdByPath, instance.GetAssetPath().c_str(), instance.GetAssetType(), false); + // Get the image path from the SimpleAssetReference and fix it up since CSprite still + // allows user specified paths that have the .sprite extension or the deprecated .dds extension + AZStd::string sourcePath = CSprite::GetImageSourcePathFromProductPath(instance.GetAssetPath()); + AZStd::string fixedUpSourcePath; + CSprite::FixUpSourceImagePathFromUserDefinedPath(sourcePath, fixedUpSourcePath); + + AZ::Data::AssetCatalogRequestBus::BroadcastResult( + assetId, &AZ::Data::AssetCatalogRequestBus::Events::GenerateAssetIdTEMP, + fixedUpSourcePath.c_str()); + assetId.m_subId = AZ::RPI::StreamingImageAsset::GetImageAssetSubId(); } ctrl->SetSelectedAssetID(assetId); } diff --git a/Gems/LyShine/Code/Source/Sprite.cpp b/Gems/LyShine/Code/Source/Sprite.cpp index c574a16c0c..26e4056a41 100644 --- a/Gems/LyShine/Code/Source/Sprite.cpp +++ b/Gems/LyShine/Code/Source/Sprite.cpp @@ -26,6 +26,7 @@ namespace { const char* const spriteExtension = "sprite"; + const char* const streamingImageExtension = "streamingimage"; // Increment this when the Sprite Serialize(TSerialize) function // changes to be incompatible with previous data @@ -37,7 +38,7 @@ namespace }; const int numAllowedSpriteTextureExtensions = AZ_ARRAY_SIZE(allowedSpriteTextureExtensions); - bool IsValidSpriteTextureExtension(const AZStd::string& extension) + bool IsValidImageExtension(const AZStd::string& extension) { for (int i = 0; i < numAllowedSpriteTextureExtensions; ++i) { @@ -50,6 +51,13 @@ namespace return false; } + bool IsImageProductPath(const AZStd::string& pathname) + { + AZStd::string extension; + AzFramework::StringFunc::Path::GetExtension(pathname.c_str(), extension, false); + return (extension.compare(streamingImageExtension) == 0); + } + // Check if a file exists. This does not go through the AssetCatalog so that it can identify files that exist but aren't processed yet, // and so that it will work before the AssetCatalog has loaded bool CheckIfFileExists(const AZStd::string& sourceRelativePath, const AZStd::string& cacheRelativePath) @@ -88,61 +96,49 @@ namespace return fileExists; } - bool ReplaceSpriteExtensionWithTextureExtension(const AZStd::string& spritePath, AZStd::string& texturePath) + bool GetSourceAssetPaths(const AZStd::string& pathname, AZStd::string& spritePath, AZStd::string& texturePath) { - for (int i = 0; i < numAllowedSpriteTextureExtensions; ++i) + // Remove product extension from the texture path if it exists + AZStd::string sourcePathname(pathname); + if (IsImageProductPath(pathname)) { - AZStd::string sourceRelativePath(spritePath); - AzFramework::StringFunc::Path::ReplaceExtension(sourceRelativePath, allowedSpriteTextureExtensions[i]); - AZStd::string cacheRelativePath = sourceRelativePath + ".streamingimage"; - - bool textureExists = CheckIfFileExists(sourceRelativePath, cacheRelativePath); - if (textureExists) - { - texturePath = sourceRelativePath; - return true; - } + sourcePathname = CSprite::GetImageSourcePathFromProductPath(pathname); } - return false; - } - - bool GetAssetPaths(const AZStd::string& pathname, AZStd::string& spritePath, AZStd::string& texturePath) - { - // the input string could be in any form. So make it normalized + // the input string could be in any form. So make it normalized (forward slashes and lower case) // NOTE: it should not be a full path at this point. If called from the UI editor it will // have been transformed to a game path. If being called with a hard coded path it should be a // game path already - it is not good for code to be using full paths. - AZStd::string assetPath(pathname); - EBUS_EVENT(AzFramework::ApplicationRequests::Bus, NormalizePath, assetPath); + EBUS_EVENT(AzFramework::ApplicationRequests::Bus, NormalizePath, sourcePathname); // check the extension and work out the pathname of the sprite file and the texture file // currently it works if the input path is either a sprite file or a texture file AZStd::string extension; - AzFramework::StringFunc::Path::GetExtension(assetPath.c_str(), extension, false); + AzFramework::StringFunc::Path::GetExtension(sourcePathname.c_str(), extension, false); if (extension.compare(spriteExtension) == 0) { - spritePath = assetPath; + // The .sprite file has been specified + spritePath = sourcePathname; // look for a texture file with the same name - if (!ReplaceSpriteExtensionWithTextureExtension(spritePath, texturePath)) + if (!CSprite::FixUpSourceImagePathFromUserDefinedPath(spritePath, texturePath)) { gEnv->pSystem->Warning(VALIDATOR_MODULE_SHINE, VALIDATOR_WARNING, VALIDATOR_FLAG_FILE | VALIDATOR_FLAG_TEXTURE, - assetPath.c_str(), "No texture file found for sprite: %s, no sprite will be used", assetPath.c_str()); + spritePath.c_str(), "No texture file found for sprite: %s, no sprite will be used", spritePath.c_str()); return false; } } - else if (IsValidSpriteTextureExtension(extension)) + else if (IsValidImageExtension(extension)) { - texturePath = assetPath; - spritePath = assetPath; + texturePath = sourcePathname; + spritePath = sourcePathname; AzFramework::StringFunc::Path::ReplaceExtension(spritePath, spriteExtension); } else { gEnv->pSystem->Warning(VALIDATOR_MODULE_SHINE, VALIDATOR_WARNING, VALIDATOR_FLAG_FILE | VALIDATOR_FLAG_TEXTURE, - assetPath.c_str(), "Invalid file extension for sprite: %s, no sprite will be used", assetPath.c_str()); + pathname.c_str(), "Invalid file extension for sprite: %s, no sprite will be used", pathname.c_str()); return false; } @@ -665,7 +661,7 @@ CSprite* CSprite::LoadSprite(const string& pathname) { AZStd::string spritePath; AZStd::string texturePath; - bool validAssetPaths = GetAssetPaths(pathname.c_str(), spritePath, texturePath); + bool validAssetPaths = GetSourceAssetPaths(pathname.c_str(), spritePath, texturePath); if (!validAssetPaths) { @@ -760,7 +756,7 @@ bool CSprite::DoesSpriteTextureAssetExist(const AZStd::string& pathname) { AZStd::string spritePath; AZStd::string texturePath; - bool validAssetPaths = GetAssetPaths(pathname, spritePath, texturePath); + bool validAssetPaths = GetSourceAssetPaths(pathname.c_str(), spritePath, texturePath); if (!validAssetPaths) { @@ -785,8 +781,7 @@ bool CSprite::DoesSpriteTextureAssetExist(const AZStd::string& pathname) } // Check if the texture asset exists - AZStd::string cacheRelativePath = texturePath + ".streamingimage"; - bool textureExists = CheckIfFileExists(texturePath, cacheRelativePath); + bool textureExists = CheckIfFileExists(spritePath, texturePath); return textureExists; } @@ -806,6 +801,48 @@ void CSprite::ReplaceSprite(ISprite** baseSprite, ISprite* newSprite) } } +//////////////////////////////////////////////////////////////////////////////////////////////////// +bool CSprite::FixUpSourceImagePathFromUserDefinedPath(const AZStd::string& userDefinedPath, AZStd::string& sourceImagePath) +{ + static const char* textureExtensions[] = { "png", "tif", "tiff", "tga", "jpg", "jpeg", "bmp", "gif" }; + + AZStd::string sourceRelativePath(userDefinedPath); + AZStd::string cacheRelativePath = AZStd::string::format("%s.%s", sourceRelativePath.c_str(), streamingImageExtension); + bool textureExists = CheckIfFileExists(sourceRelativePath, cacheRelativePath); + + if (textureExists) + { + sourceImagePath = userDefinedPath; + return true; + } + + AZStd::string curSourceImagePath(userDefinedPath); + for (const char* extensionReplacement : textureExtensions) + { + AzFramework::StringFunc::Path::ReplaceExtension(curSourceImagePath, extensionReplacement); + cacheRelativePath = AZStd::string::format("%s.%s", curSourceImagePath.c_str(), streamingImageExtension); + textureExists = CheckIfFileExists(curSourceImagePath, cacheRelativePath); + + if (textureExists) + { + sourceImagePath = curSourceImagePath; + return true; + } + } + + return false; +} + +AZStd::string CSprite::GetImageSourcePathFromProductPath(const AZStd::string& productPathname) +{ + AZStd::string sourcePathname(productPathname); + if (IsImageProductPath(sourcePathname)) + { + AzFramework::StringFunc::Path::StripExtension(sourcePathname); + } + return sourcePathname; +} + //////////////////////////////////////////////////////////////////////////////////////////////////// bool CSprite::LoadTexture(const string& texturePathname, const string& pathname, ITexture*& texture) { @@ -850,7 +887,7 @@ void CSprite::ReleaseTexture(ITexture*& texture) bool CSprite::LoadImage(const AZStd::string& nameTex, AZ::Data::Instance& image) { AZStd::string sourceRelativePath(nameTex); - AZStd::string cacheRelativePath = sourceRelativePath + ".streamingimage"; + AZStd::string cacheRelativePath = AZStd::string::format("%s.%s", sourceRelativePath.c_str(), streamingImageExtension); bool textureExists = CheckIfFileExists(sourceRelativePath, cacheRelativePath); if (!textureExists) @@ -859,27 +896,13 @@ bool CSprite::LoadImage(const AZStd::string& nameTex, AZ::Data::InstanceAttribute(AZ::Edit::Attributes::ChangeNotify, AZ_CRC("RefreshEntireTree", 0xefbc823c)); editInfo->DataElement("Sprite", &UiImageComponent::m_spritePathname, "Sprite path", "The sprite path. Can be overridden by another component such as an interactable.") ->Attribute(AZ::Edit::Attributes::Visibility, &UiImageComponent::IsSpriteTypeAsset) - ->Attribute(AZ::Edit::Attributes::ChangeNotify, &UiImageComponent::OnEditorSpritePathnameChange) - ->Attribute(AZ::Edit::Attributes::ChangeNotify, AZ_CRC("RefreshEntireTree", 0xefbc823c)); + ->Attribute(AZ::Edit::Attributes::ChangeNotify, &UiImageComponent::OnEditorSpritePathnameChange); editInfo->DataElement(AZ::Edit::UIHandlers::ComboBox, &UiImageComponent::m_spriteSheetCellIndex, "Index", "Sprite-sheet index. Defines which cell in a sprite-sheet is displayed.") ->Attribute(AZ::Edit::Attributes::Visibility, &UiImageComponent::IsSpriteTypeSpriteSheet) ->Attribute(AZ::Edit::Attributes::ChangeNotify, &UiImageComponent::OnIndexChange)