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

Use assimp as mesh importer and exporter #1090

Merged
merged 16 commits into from
Aug 9, 2021
Merged

Use assimp as mesh importer and exporter #1090

merged 16 commits into from
Aug 9, 2021

Conversation

ghost
Copy link

@ghost ghost commented Aug 4, 2021

Use assimp to be able to import and export mesh instead of custom code.

Currently just replacing the obj import and export function, but maybe updated later to be more generic

@ghost ghost requested a review from fabiencastan August 4, 2021 12:52
Copy link
Member

@simogasp simogasp left a comment

Choose a reason for hiding this comment

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

Maybe add the cmake code to build assimp like the other libs

    set(ASSIMP_TARGET assimp)
    set(ASSIMP_BUILD_OPTIONS -DASSIMP_BUILD_ASSIMP_TOOLS:BOOL=OFF -DASSIMP_BUILD_TESTS:BOOL=OFF)
    set(ASSIMP_AV_VERSION 5.0.1)
    set(ASSIMP_FILENAME v${ASSIMP_AV_VERSION}.tar.gz)
    ExternalProject_Add(${ASSIMP_TARGET}
            URL https://github.com/assimp/assimp/archive/${ASSIMP_FILENAME}
            DOWNLOAD_NAME assimp-${ASSIMP_FILENAME}
            PREFIX ${BUILD_DIR}
            BUILD_IN_SOURCE 0
            BUILD_ALWAYS 0
            SOURCE_DIR ${CMAKE_CURRENT_BINARY_DIR}/assimp
            BINARY_DIR ${BUILD_DIR}/assimp_build
            INSTALL_DIR ${CMAKE_INSTALL_PREFIX}
            CONFIGURE_COMMAND ${CMAKE_COMMAND} ${CMAKE_CORE_BUILD_FLAGS} ${ASSIMP_BUILD_OPTIONS} -DCMAKE_INSTALL_PREFIX:PATH=<INSTALL_DIR> <SOURCE_DIR>
            )
    set(ASSIMP_CMAKE_FLAGS -DAssimp_DIR:PATH=${CMAKE_INSTALL_PREFIX}/lib/cmake/assimp-${ASSIMP_AV_VERSION}/)

Verify the last line as I cannot check if it still installs assimp in a folder named assimp-${ASSIMP_VERSION}

src/aliceVision/mesh/Mesh.cpp Outdated Show resolved Hide resolved
src/aliceVision/mesh/Mesh.cpp Outdated Show resolved Hide resolved
src/aliceVision/mesh/Mesh.cpp Outdated Show resolved Hide resolved
src/aliceVision/mesh/Mesh.cpp Outdated Show resolved Hide resolved
map_indices[idPoint] = pts.size();

aiVector3D v = mesh->mVertices[idPoint];
pts.push_back(Point3d(v.x, v.y, v.z));
Copy link
Member

@simogasp simogasp Aug 4, 2021

Choose a reason for hiding this comment

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

Suggested change
pts.push_back(Point3d(v.x, v.y, v.z));
pts.emplace_back(v.x, v.y, v.z);

requires to add the wrapper method to StaticVector (I think it's worth it because if we ever manage to replace StaticVector with a "pure" std:vector (ie not as member of the class) everything will be smooth)

Copy link
Member

Choose a reason for hiding this comment

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

I think adding this to StaticVector should work

    template< class... Args >
    void emplace_back( Args&&... args )
    {
        _data.emplace_back(std::forward<Args>(args)...);
    }

Comment on lines +1040 to +1041
int vertexId = p.first.first;
int uvId = p.first.second;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
int vertexId = p.first.first;
int uvId = p.first.second;
const int vertexId = p.first.first;
const int uvId = p.first.second;

fprintf(fmtl, "# \n\n");
for(int i = 0; i < _atlases[atlasId].size(); i++)
{
int triangleId = _atlases[atlasId][i];
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
int triangleId = _atlases[atlasId][i];
const int triangleId = _atlases[atlasId][i];

fprintf(fmtl, "# Wavefront material file\n");
fprintf(fmtl, "# Created with AliceVision\n");
fprintf(fmtl, "# \n\n");
for(int i = 0; i < _atlases[atlasId].size(); i++)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
for(int i = 0; i < _atlases[atlasId].size(); i++)
for(int i = 0; i < _atlases[atlasId].size(); ++i)

Comment on lines 1068 to 1069
int vertexId = mesh->tris[triangleId].v[k];
int uvId = mesh->trisUvIds[triangleId].m[k];
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
int vertexId = mesh->tris[triangleId].v[k];
int uvId = mesh->trisUvIds[triangleId].m[k];
const int vertexId = mesh->tris[triangleId].v[k];
const int uvId = mesh->trisUvIds[triangleId].m[k];

aimesh->mFaces[i].mNumIndices = 3;
aimesh->mFaces[i].mIndices = new unsigned int[3];

for (int k = 0; k < 3; k++)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
for (int k = 0; k < 3; k++)
for (int k = 0; k < 3; ++k)

fprintf(fobj, "# \n");
fprintf(fobj, "mtllib %s\n\n", mtlName.c_str());
fprintf(fobj, "g TexturedMesh\n");
if (_atlases.size() == 0)
Copy link
Member

@simogasp simogasp Aug 4, 2021

Choose a reason for hiding this comment

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

Suggested change
if (_atlases.size() == 0)
if (_atlases.empty())

@fabiencastan
Copy link
Member

Is there a way to add a message in the OBJ file that it has been generated with AliceVision and specify the version?
Currently it says only assimp and the version of assimp.

@simogasp
Copy link
Member

simogasp commented Aug 9, 2021

Is there a way to add a message in the OBJ file that it has been generated with AliceVision and specify the version?
Currently it says only assimp and the version of assimp.

I think you can add it in the aiMetadata
https://assimp-docs.readthedocs.io/en/master/API/API-Documentation.html#_CPPv410aiMetadata

@fabiencastan
Copy link
Member

I'm merging it for now.
@servantf Can you check if we can add metadata in a new PR?

@fabiencastan fabiencastan merged commit 73fef34 into develop Aug 9, 2021
@fabiencastan fabiencastan deleted the dev/assimp branch August 9, 2021 09:37
@fabiencastan fabiencastan added this to the 2.5.0 milestone Aug 9, 2021
@ghost
Copy link
Author

ghost commented Aug 9, 2021

There is no way to add metadata in obj files.
aiMetadata are almost never exported but used for importers.
A solution may be to fork assimp and modify it ?

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