-
Notifications
You must be signed in to change notification settings - Fork 65
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Robot Importer: copy texture files #497
Robot Importer: copy texture files #497
Conversation
Please note, that I also fixed some typos and removed unused code... |
TODO: there might be some problems with path resolving if the textures are located in different folder - this will be fixed later if the approach is approved. |
17a9f0c
to
90d2f42
Compare
Gems/ROS2/Code/Source/RobotImporter/Utils/SourceAssetsStorage.cpp
Outdated
Show resolved
Hide resolved
Fixed. The code was tested for the following configurations:
and resulted in the following structure in
|
The code does not solve all possible problems with textures (textures might come from another package), but it solves multiple cases existing in robots (e.g. ANYmal C ) and it is a step forward comparing to the previous codebase. |
1300354
to
e0aa487
Compare
Gems/ROS2/Code/Source/RobotImporter/Utils/SourceAssetsStorage.cpp
Outdated
Show resolved
Hide resolved
Gems/ROS2/Code/Source/RobotImporter/Utils/SourceAssetsStorage.cpp
Outdated
Show resolved
Hide resolved
Gems/ROS2/Code/Source/RobotImporter/Utils/SourceAssetsStorage.cpp
Outdated
Show resolved
Hide resolved
Gems/ROS2/Code/Source/RobotImporter/Utils/SourceAssetsStorage.cpp
Outdated
Show resolved
Hide resolved
Gems/ROS2/Code/Source/RobotImporter/Utils/SourceAssetsStorage.cpp
Outdated
Show resolved
Hide resolved
Gems/ROS2/Code/Source/RobotImporter/Utils/SourceAssetsStorage.cpp
Outdated
Show resolved
Hide resolved
Signed-off-by: Jan Hanca <[email protected]>
Signed-off-by: Jan Hanca <[email protected]>
Signed-off-by: Jan Hanca <[email protected]>
Signed-off-by: Jan Hanca <[email protected]>
Signed-off-by: Jan Hanca <[email protected]>
e0aa487
to
15beb5f
Compare
Signed-off-by: Jan Hanca <[email protected]>
Signed-off-by: Jan Hanca <[email protected]>
for (const auto& unresolvedAssetFilepath : assetPaths) | ||
{ | ||
// Manifest returns local path in Project's directory temp folder | ||
const AZ::IO::Path assetLocalPath( | ||
AZ::IO::Path(AZ::IO::Path(AZ::Utils::GetProjectPath()) / unresolvedAssetFilepath) | ||
.LexicallyRelative(importDirectoryTmp)); | ||
|
||
const AZ::IO::Path assetFullPathSrc(AZ::IO::Path(resolvedPath.ParentPath()) / assetLocalPath); | ||
const AZ::IO::Path assetFullPathDst(importDirectoryDst / assetLocalPath); | ||
|
||
const auto outcomeMkdir = fileIO->CreatePath(AZ::IO::Path(assetFullPathDst.ParentPath()).c_str()); | ||
if (!outcomeMkdir) | ||
{ | ||
break; | ||
} | ||
|
||
const auto outcomeCopy = fileIO->Copy(assetFullPathSrc.c_str(), assetFullPathDst.c_str()); | ||
if (outcomeCopy) | ||
{ | ||
copiedFiles[assetFullPathSrc.String()] = assetFullPathDst.String(); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a feeling this logic is going to need some iteration over time, but I'm OK with it going in and improving it as we hit issues. Some of the things I'm thinking about:
- If the mesh refers to textures that can't be found, this doesn't look like it's doing much in the way of error-detection and reporting yet.
- @lumberyard-employee-dm We might hit an odd problem with some upcoming work, where we add URI prefix resolution for material textures to the asset import. For the URI prefix resolution to work, it needs to be able to find the texture. If we try to do the resolution before copying the textures, the textures won't exist anywhere that we can find them yet. If we try to copy the textures first, we won't have done the prefix resolution yet to know how to copy them. It's possible we'll need to do it in both places... :(
Gems/ROS2/Code/Source/RobotImporter/Utils/SourceAssetsStorage.cpp
Outdated
Show resolved
Hide resolved
Signed-off-by: Jan Hanca <[email protected]>
47174b2
to
5af338a
Compare
* clang-format; typos; obsolete variables * find and copy meshes' materials * code fixed for subdirectories * code review: add static const * code review: split GetMeshAssets method * fix line ends * fix includes * code review: rename method --------- Signed-off-by: Jan Hanca <[email protected]>
* clang-format; typos; obsolete variables * find and copy meshes' materials * code fixed for subdirectories * code review: add static const * code review: split GetMeshAssets method * fix line ends * fix includes * code review: rename method --------- Signed-off-by: Jan Hanca <[email protected]>
* clang-format; typos; obsolete variables * find and copy meshes' materials * code fixed for subdirectories * code review: add static const * code review: split GetMeshAssets method * fix line ends * fix includes * code review: rename method --------- Signed-off-by: Jan Hanca <[email protected]> Signed-off-by: Michał Pełka <[email protected]>
* clang-format; typos; obsolete variables * find and copy meshes' materials * code fixed for subdirectories * code review: add static const * code review: split GetMeshAssets method * fix line ends * fix includes * code review: rename method --------- Signed-off-by: Jan Hanca <[email protected]> Signed-off-by: Michał Pełka <[email protected]>
* clang-format; typos; obsolete variables * find and copy meshes' materials * code fixed for subdirectories * code review: add static const * code review: split GetMeshAssets method * fix line ends * fix includes * code review: rename method --------- Signed-off-by: Jan Hanca <[email protected]> Signed-off-by: Michał Pełka <[email protected]>
This is a PoC code for copying textures (indicated by #477). I am open to discussions.
Please note, that this PR does not solve the problem of missing mesh (e.g.
gltf
file might store mesh in a separate binary file).Test mesh:
![image](https://private-user-images.githubusercontent.com/134940295/265725618-8588bd1b-8790-42b2-8f1a-b9e41965aa89.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MzkyMzcyMjYsIm5iZiI6MTczOTIzNjkyNiwicGF0aCI6Ii8xMzQ5NDAyOTUvMjY1NzI1NjE4LTg1ODhiZDFiLTg3OTAtNDJiMi04ZjFhLWI5ZTQxOTY1YWE4OS5wbmc_WC1BbXotQWxnb3JpdGhtPUFXUzQtSE1BQy1TSEEyNTYmWC1BbXotQ3JlZGVudGlhbD1BS0lBVkNPRFlMU0E1M1BRSzRaQSUyRjIwMjUwMjExJTJGdXMtZWFzdC0xJTJGczMlMkZhd3M0X3JlcXVlc3QmWC1BbXotRGF0ZT0yMDI1MDIxMVQwMTIyMDZaJlgtQW16LUV4cGlyZXM9MzAwJlgtQW16LVNpZ25hdHVyZT1mNGVhMmNmMDM1NTUxMmI5MzliN2JjMjRmMjE4M2VjY2Q5ODVhN2FmZjQwYmZlNTNkNDY1ODU1MWM1NTgxNDBjJlgtQW16LVNpZ25lZEhlYWRlcnM9aG9zdCJ9.NfDz-mymiw-_l9GRVWWbCsWMJkg90pKTtZ8SGeg8dfE)
texture.zip
Outcome: