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

Add a new optional command to export textured mesh to a USD format #1455

Merged
merged 2 commits into from
Jul 6, 2023

Conversation

serguei-k
Copy link
Contributor

Description

Add a new optional command to export textured mesh to a USD format.

Features list

  • Add new command exportUSD to export mesh and textures to USD
  • Add a simple Materials class to capture the small subset of material properties and associated textures that is supported by texturing pipeline
  • Update texturing pipeline to make use of new material class
  • Fix bug in normal computation (checking for NaN)
  • Fix bug in loading mesh data not setting the correct number of atlases and materials used
  • Fix bug in loading mesh data using incorrect material ID due to default material
  • Add option to merge coincident vertices when loading meshes with multiple materials

@@ -1333,7 +1334,7 @@ void Mesh::computeNormalsForPts(StaticVector<StaticVector<int>>& ptsNeighTris, S
{
Point3d n1 = computeTriangleNormal(triTmp[j]);
n1 = n1.normalize();
if(!std::isnan(n1.x) && !std::isnan(n1.y) && std::isnan(n1.z)) // check if is not NaN
if(!std::isnan(n1.x) && !std::isnan(n1.y) && !std::isnan(n1.z)) // check if is not NaN
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was getting 0 normals, found this small issue


// set number of materials used
const std::unordered_set<int> materialIds = std::unordered_set<int>(_trisMtlIds.begin(), _trisMtlIds.end());
nmtls = static_cast<int>(materialIds.size());
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nmtls property was never set, so when loading an asset with materials the number of atlases wasn't being set correctly.

@@ -940,6 +944,26 @@ void Texturing::loadWithAtlas(const std::string& filepath, bool flipNormals)
}
}

void Texturing::loadWithMaterial(const std::string& filepath, bool flipNormals)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added as new method for clarity, could edit existing load method instead.

@@ -242,6 +236,12 @@ int aliceVision_main(int argc, char* argv[])
mesh.generateNormalAndHeightMaps(mp, denseMesh, outputFolder, bumpMappingParams);
}

// save final obj file
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moved after texture generation for consistency in logic

@simogasp
Copy link
Member

simogasp commented Jun 7, 2023

Thanks for your contribution!

Would you mind adding the cmake instructions to install the dependency (https://github.com/PixarAnimationStudios/USD I guess) in the main CMakeLists.txt through ExternalProject_Add, as we did for all the others https://github.com/alicevision/AliceVision/blob/develop/CMakeLists.txt#L10?
you can declare a

option(AV_BUILD_USD "Enable building an embedded USD library" ON)
...
message(STATUS "AV_BUILD_USD: ${AV_BUILD_USD}")

and

if(AV_BUILD_USD)
set(USD_TARGET usd)
ExternalProject_Add(${USD_TARGET}
       .... # see others for examples
       DEPENDS ${TBB_TARGET} ${BOOST_TARGET}. # these seem to be the only dependencies?
       )
set(USD_CMAKE_FLAGS -DUSD_DIR:PATH=${CMAKE_INSTALL_PREFIX}/<where to find the USDConfig.cmake>)
endif()

and add USD_CMAKE_FLAGS at the very end around line 1059

set(AV_DEPS
  ${ZLIB_TARGET}
 .....
  ${CLP_TARGET}
  ${USD_TARGET}       # <<<
)

if(AV_BUILD_ALICEVISION)
ExternalProject_Add(aliceVision
       PREFIX ${CMAKE_CURRENT_SOURCE_DIR}
       .
       .
       .
            ${ZLIB_CMAKE_FLAGS}
            ${ASSIMP_CMAKE_FLAGS}
            ${EIGEN_CMAKE_FLAGS}
            .
            .
            ${FLANN_CMAKE_FLAGS}
            ${PCL_CMAKE_FLAGS}
            ${USD_CMAKE_FLAGS}       # <<<
            -DCMAKE_INSTALL_PREFIX:PATH=<INSTALL_DIR> <SOURCE_DIR>
       DEPENDS ${AV_DEPS}
       )
endif()

@fabiencastan fabiencastan added this to the 3.2.0 milestone Jun 7, 2023
@fabiencastan
Copy link
Member

The error in the CI is unrelated to your PR. It's an issue in the CI env since we switched to C++17.

@serguei-k
Copy link
Contributor Author

serguei-k commented Jun 9, 2023

Thanks for your contribution!

Would you mind adding the cmake instructions to install the dependency (https://github.com/PixarAnimationStudios/USD I guess) in the main CMakeLists.txt through ExternalProject_Add, as we did for all the others https://github.com/alicevision/AliceVision/blob/develop/CMakeLists.txt#L10? you can declare a

Thanks for the note, added the optional build step for USD.

@serguei-k
Copy link
Contributor Author

Hi guys, just a heads up that I will be starting a new job next week that will prevent me from working on oss code, so if you would like me to make any changes please let me know :)

@mugulmd
Copy link
Contributor

mugulmd commented Jun 20, 2023

Hi guys, just a heads up that I will be starting a new job next week that will prevent me from working on oss code, so if you would like me to make any changes please let me know :)

I've just started building your branch for testing, I'll let you know if I spot something 😉

@simogasp
Copy link
Member

simogasp commented Jun 20, 2023

Hi guys, just a heads up that I will be starting a new job next week that will prevent me from working on oss code, so if you would like me to make any changes please let me know :)

I was wondering whether it could be better to move the part of the code that actually saves the mesh into a USD file (i.e. the code from main_exportUSD) directly into the library part, so that it could be used everywhere else if one wants to save a mesh in that format. This would also allow the meshroom user to save the mesh directly in USD format in the texturing node, without going through another node.

In practice, we could refactor the code and modify the current Mesh::save() method so that once it detects the format

void Mesh::save(const std::string& filepath)
{
    const std::string fileTypeStr = boost::filesystem::path(filepath).extension().string().substr(1);
    const EFileType fileType = mesh::EFileType_stringToEnum(fileTypeStr);

and then calls another save method accordingly: e.g. saveUSD() method that contains the code you already wrote for the main_exportUSD or e.g. saveAssimp() that saves the mesh using Assimp (i.e. the code of the current Mesh::save() method).

And, obviously, we can modify all the relevant enum such as mesh::EFileType accordingly to add the USD format.

What do you think?

@mugulmd
Copy link
Contributor

mugulmd commented Jul 6, 2023

Okay so building AliceVision with USD is not an easy task since there are conflicts with TBB versions, so the fact that this is optional is definitely a plus 😅

I did some testing with textured meshes generated with the photogrammetry pipeline and visualized them in usdview, everything seems to be working fine.

Great job, thanks for the contribution!

@mugulmd mugulmd merged commit cc327e3 into alicevision:develop Jul 6, 2023
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.

4 participants