cnr-isti-vclab / meshlab

The open source mesh processing system
http://www.meshlab.net
GNU General Public License v3.0
4.7k stars 821 forks source link

Assertion failed when MeshLab automatically compacts data #500

Closed rpavlik closed 4 years ago

rpavlik commented 4 years ago

To reproduce, you can follow this video's steps: The crash is caught on video. https://www.youtube.com/watch?v=bDHJM6nAKtc

Happens when I'm cleaning up a surface reconstruction created out of fused dense points and the sparse raster alignment from COLMAP (per instruction video) after texturing the model. Since I built from source with latest devel, I have a console log message with valid information about the failure/crash.

LOG: 2 TEXTURE PAINTING: 0.688 sec.
LOG: 0 Applied filter Parameterization + texturing from registered rasters in 2429 msec
LOG: 0 Selected new Mesh 2
LOG: 0 Started Mode Select Faces in a rectagular region
LOG: 2 Deleted 0 faces.
LOG: 0 Applied filter Delete Selected Faces in 10 msec
LOG: 0 Started Mode Select Faces in a rectagular region
LOG: 2 Nothing done: no vertex selected
LOG: 0 Applied filter Delete Selected Vertices in 9 msec
LOG: 2 Deleted 280 faces, 135 vertices.
LOG: 0 Applied filter Delete Selected Faces and Vertices in 11 msec
LOG: 0 Started Mode Select Faces in a rectagular region
LOG: 2 Deleted 116 faces, 57 vertices.
meshlab: ../../../vcglib/vcg/complex/allocate.h:1252: static void vcg::tri::Allocator<MeshType>::CompactFaceVector(MeshType&, vcg::tri::Allocator<MeshType>::PointerUpdater<typename MeshType::FacePointer>&) [with MeshType = CMeshO; typename MeshType::FacePointer = CFaceO*]: Assertion `fbase <= (*vi).cVFp() && oldIndex < pu.remap.size()' failed.
rpavlik commented 4 years ago

Turns out this can also be triggered by Quadric Edge Collapse Decimation:

LOG: 0 Applied filter Simplification: Quadric Edge Collapse Decimation (with texture) in 3618 msec
meshlab: ../../../vcglib/vcg/complex/allocate.h:1288: static void vcg::tri::Allocator<MeshType>::CompactFaceVector(MeshType&, vcg::tri::Allocator<MeshType>::PointerUpdater<typename MeshType::FacePointer>&) [with MeshType = CMeshO; typename MeshType::FacePointer = CFaceO*]: Assertion `fbase <= (*fi).FFp(i) && oldIndex < pu.remap.size()' failed.
jmespadero commented 4 years ago

Hi @rpavlik, Could you provide a minimal reproducible example for this? Please, include a link to an input mesh (or data) and the parameters that you use into the Quadric Edge Collapse Decimation filter to launch the assertion. Also include your OS and meshlab version.

rpavlik commented 4 years ago

I could do this with a different model (also over-detailed, from photogrammetry) with the following steps

  1. Quadric Edge Collapse Decimation with Texture x3, default parameters each time
  2. Select some faces and delete
  3. Select some more faces and delete, crashes

https://imgur.com/a/Vjx3ehJ

I suspect anything that changes the detail substantially would trigger it, since the assert looks like it's in a face allocator.

LOG: 0 Opened mesh /home/ryan/extra/psvr-photos/dslr-flour/MeshroomCache/Texturing/dcc08fd0e38ebd4f9df4fae5e57155cfeafa16e6/texturedMesh.obj in 4837 msec
LOG: 0 All files opened in 4838 msec
LOG: 0 Applied filter Simplification: Quadric Edge Collapse Decimation (with texture) in 3064 msec
LOG: 0 Applied filter Simplification: Quadric Edge Collapse Decimation (with texture) in 1628 msec
LOG: 0 Applied filter Simplification: Quadric Edge Collapse Decimation (with texture) in 751 msec
LOG: 0 Started Mode Select Faces in a rectagular region
libpng warning: iCCP: known incorrect sRGB profile
LOG: 2 Deleted 1852 faces.
LOG: 0 Applied filter Delete Selected Faces in 37 msec
LOG: 0 Started Mode Select Faces in a rectagular region
LOG: 2 Deleted 2518 faces.
meshlab: ../../../vcglib/vcg/complex/allocate.h:1252: static void vcg::tri::Allocator<MeshType>::CompactFaceVector(MeshType&, vcg::tri::Allocator<MeshType>::PointerUpdater<typename MeshType::FacePointer>&) [with MeshType = CMeshO; typename MeshType::FacePointer = CFaceO*]: Assertion `fbase <= (*vi).cVFp() && oldIndex < pu.remap.size()' failed.
fish: “./meshlab” terminated by signal SIGABRT (Abort)

Here's the model I used there - it was too big for GitHub issue uploads. Will expire in 7 days. https://send.firefox.com/download/e73c3bdcf07aecd2/#OCaXEwOaDmPiCCngmZ4tyw

rpavlik commented 4 years ago

OK, so if I use the mesh before it was textured, I can get it to fit in GitHub's limit. It will still reproduce the bug just fine. mesh.zip

LOG: 0 Opened mesh /home/ryan/extra/psvr-photos/dslr-flour/MeshroomCache/MeshFiltering/7f8e4114d5d9ee46a710d572725a00232203be55/mesh.obj in 1110 msec
LOG: 0 All files opened in 1111 msec
libpng warning: iCCP: known incorrect sRGB profile
LOG: 2 PostSimplification Cleaning: Removed 6 unreferenced vertices
LOG: 0 Applied filter Simplification: Quadric Edge Collapse Decimation in 4085 msec
LOG: 2 PostSimplification Cleaning: Removed 7 unreferenced vertices
LOG: 0 Applied filter Simplification: Quadric Edge Collapse Decimation in 1977 msec
LOG: 2 PostSimplification Cleaning: Removed 4 unreferenced vertices
LOG: 0 Applied filter Simplification: Quadric Edge Collapse Decimation in 920 msec
LOG: 0 Started Mode Select Faces in a rectagular region
LOG: 2 Deleted 14073 faces.
LOG: 0 Applied filter Delete Selected Faces in 17 msec
LOG: 0 Started Mode Select Faces in a rectagular region
LOG: 2 Deleted 6137 faces.
meshlab: ../../../vcglib/vcg/complex/allocate.h:1252: static void vcg::tri::Allocator<MeshType>::CompactFaceVector(MeshType&, vcg::tri::Allocator<MeshType>::PointerUpdater<typename MeshType::FacePointer>&) [with MeshType = CMeshO; typename MeshType::FacePointer = CFaceO*]: Assertion `fbase <= (*vi).cVFp() && oldIndex < pu.remap.size()' failed.
fish: “./meshlab” terminated by signal SIGABRT (Abort)

Same steps.

rpavlik commented 4 years ago

I'm using something very near to master for meshlab https://github.com/rpavlik/meshlab/tree/build-tweaks and vcglib devel https://github.com/rpavlik/vcglib/tree/fix-build - mostly just changes required to build on debian buster x64 (my OS)

jmespadero commented 4 years ago

Thanks! I can reproduce your crash following those same steps. Even just one "Quadric with Texture" followed by two "select" and "erase" are enough to trigger the assertion.

I have separate the assert line in allocate.h into two lines so it does not combine two conditions in the assert.

    // Loop on the vertices to correct VF relation
    if(HasVFAdjacency(m))
    {
      for (VertexIterator vi=m.vert.begin(); vi!=m.vert.end(); ++vi)
        if(!(*vi).IsD())
        {
          if ((*vi).IsVFInitialized() && (*vi).VFp()!=0 )
          {
            size_t oldIndex = (*vi).cVFp() - fbase;
            assert(fbase <= (*vi).cVFp() && "fbase <= (*vi).cVFp()");            //asser1
            assert(oldIndex < pu.remap.size() && "oldIndex < pu.remap.size()");   //asser2
            (*vi).VFp() = fbase+pu.remap[oldIndex];
          }
        }
    }

And I got the first condition: assert(fbase <= (*vi).cVFp() is false.

vcglib/vcg/complex/allocate.h:1252: static void vcg::tri::Allocator<MeshType>::CompactFaceVector(MeshType&, vcg::tri::Allocator<MeshType>::PointerUpdater<typename MeshType::FacePointer>&) [with MeshType = CMeshO; typename MeshType::FacePointer = CFaceO*]: Assertion `fbase <= (*vi).cVFp() && "fbase <= (*vi).cVFp()"' failed.
jmespadero commented 4 years ago

Problem solved:

Finally, it was not a problem with Quadric Edge Collapse Decimation but with Delete Selected Faces itself.

To solve the problem you need to edit the file meshlab/src/meshlabplugins/filter_select/meshselect.cpp and add this section of code at line 361 (link to your own file )

    //BugFix: Update Topology of result mesh
    if (tri::HasFFAdjacency(m.cm))
      tri::UpdateTopology<CMeshO>::FaceFace(m.cm);    
    if (tri::HasVFAdjacency(m.cm))
      tri::UpdateTopology<CMeshO>::VertexFace(m.cm);

And I also would add same code to the sections for filters FP_SELECT_DELETE_VERT and FP_SELECT_DELETE_FACEVERT.

I will provide a Pull Request for this.

jmespadero commented 4 years ago

Other way of solving this is just invalidate every vertex-Face pointers adding this line instead of previous code.

        m.clearDataMask(MeshModel::MM_VERTFACETOPO);
rpavlik commented 4 years ago

awesome, thanks for tracking this down! Hopefully it will get merged and a release (or at least a build) is made, since it's been around in the workflow shown in that video since at least 2017.

(As long as it's effective, the approach in your pull request seems like it would be more efficient than regenerating the whole data structure as your second suggestion mentioned.)

jmespadero commented 4 years ago

You are welcome. For me, it is a mystery why some PR are aging and not being merged in the master branch of meshlab, even if they are crystal clear bugfixes (as #387 #386 #421 #433 #445 #490 #491 #503).

This is pushing more and more people to do their own fork and bugfix there, which is a tragedy for a potentially great program like this.

rpavlik commented 4 years ago

Fix merged, thanks!