Skip to content
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

Merged
merged 8 commits into from
Sep 25, 2023

Conversation

jhanca-robotecai
Copy link
Contributor

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:
texture.zip
Outcome:
image

@jhanca-robotecai jhanca-robotecai requested review from a team as code owners September 5, 2023 14:08
@jhanca-robotecai jhanca-robotecai marked this pull request as draft September 5, 2023 14:08
@jhanca-robotecai
Copy link
Contributor Author

Please note, that I also fixed some typos and removed unused code...

@jhanca-robotecai
Copy link
Contributor Author

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.

@jhanca-robotecai
Copy link
Contributor Author

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.

Fixed. The code was tested for the following configurations:

texture
├── face.dae
├── face.jpg
└── face.urdf
texture1
├── face.urdf
└── mesh
    ├── face.dae
    └── textures
        └── face.jpg
texture2
├── face.urdf
├── mesh
│   └── face.dae
└── textures
    └── face.jpg

and resulted in the following structure in Assets folder:

├── Importer
│   ├── texture1.prefab
│   ├── texture2.prefab
│   └── texture.prefab
└── UrdfImporter
    ├── 1266655632_face
    │   ├── face.dae
    │   └── face.dae.assetinfo
    ├── 1535885513_face
    │   ├── face.dae
    │   ├── face.dae.assetinfo
    │   └── face.jpg
    ├── 3533555601_face
    │   ├── face.dae
    │   ├── face.dae.assetinfo
    │   └── textures
    │       └── face.jpg
    └── textures
        └── face.jpg

@jhanca-robotecai jhanca-robotecai marked this pull request as ready for review September 8, 2023 09:53
@jhanca-robotecai
Copy link
Contributor Author

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.

@jhanca-robotecai jhanca-robotecai changed the title copy texture files Robot Importer: copy texture files Sep 13, 2023
Signed-off-by: Jan Hanca <[email protected]>
Signed-off-by: Jan Hanca <[email protected]>
Comment on lines 362 to 383
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();
}
}
Copy link
Contributor

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... :(

@michalpelka michalpelka merged commit 7775e09 into o3de:development Sep 25, 2023
michalpelka pushed a commit to RobotecAI/o3de-extras that referenced this pull request Sep 25, 2023
* 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]>
michalpelka pushed a commit to RobotecAI/o3de-extras that referenced this pull request Sep 25, 2023
* 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]>
michalpelka pushed a commit to RobotecAI/o3de-extras that referenced this pull request Sep 25, 2023
* 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]>
michalpelka pushed a commit to RobotecAI/o3de-extras that referenced this pull request Oct 2, 2023
* 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]>
michalpelka pushed a commit that referenced this pull request Oct 3, 2023
* 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]>
@jhanca-robotecai jhanca-robotecai deleted the jh/texture_file branch November 7, 2023 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants