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

PLY point clouds without a mesh crash the program #1917

Open
parkerlreed opened this issue Jan 16, 2025 · 15 comments
Open

PLY point clouds without a mesh crash the program #1917

parkerlreed opened this issue Jan 16, 2025 · 15 comments

Comments

@parkerlreed
Copy link

Describe the bug
When trying to open a PLY that is just a colored point cloud, F3D says there's no faces, but also crashes.

This is generated from RTAB-Map's default export options. RTAB-Map's viewer is also VTK so I believe this should work.

To Reproduce
Steps to reproduce the behavior:

  1. Open the file f3d dog.ply
  2. Observe crash

Expected behavior
A graceful exit at least (possibly supporting this type of PLY)

System Information:

  • OS: Arch Linux
  • GPU and GPU driver: 780M AMDGPU

F3D Information

F3D 3.0.0

F3D - A fast and minimalist 3D viewer
Version: 3.0.0-RC3-9-g8d72f2d0.
Build date: 2025-01-15 15:07:23.
Build system: Linux 64-bits.
Compiler: GNU 14.2.1.
Module ImGui: ON.
Module OpenEXR: ON.
Module Raytracing: ON.
VTK version: 9.4.1.
Copyright (C) 2019-2021 Kitware SAS.
Copyright (C) 2021-2024 Michael Migliore, Mathieu Westphal.
License BSD-3-Clause.

Additional context

[parker@parker-framework RTAB-Map]$ f3d dog.ply 
Warning:  Can't find property 'vertex_indices' in element 'face'
free(): double free detected in tcache 2
Aborted (core dumped)

Image

File

dog.zip

@mwestphal
Copy link
Contributor

Stack trace:

(gdb) bt
#0  0x00007ffff76a53f4 in ?? () from /usr/lib/libc.so.6
#1  0x00007ffff764c120 in raise () from /usr/lib/libc.so.6
#2  0x00007ffff76334c3 in abort () from /usr/lib/libc.so.6
#3  0x00007ffff7634354 in ?? () from /usr/lib/libc.so.6
#4  0x00007ffff76af765 in ?? () from /usr/lib/libc.so.6
#5  0x00007ffff76b1e1f in ?? () from /usr/lib/libc.so.6
#6  0x00007ffff76b45ce in free () from /usr/lib/libc.so.6
#7  0x00007ffff74e2426 in vtkPLY::ply_close (plyfile=0x5555561f2c40) at /home/glow/dev/vtk/vtk1/src/IO/PLY/vtkPLY.cxx:1365
#8  0x00007ffff74ead2a in vtkPLYReader::RequestData (this=0x555555c9bf60, outputVector=0x55555617e310) at /home/glow/dev/vtk/vtk1/src/IO/PLY/vtkPLYReader.cxx:622
#9  0x00007ffff1ebe248 in vtkPolyDataAlgorithm::ProcessRequest (this=0x555555c9bf60, request=0x5555561e9ea0, inputVector=0x0, outputVector=0x55555617e310) at /home/glow/dev/vtk/vtk1/src/Common/ExecutionModel/vtkPolyDataAlgorithm.cxx:76
#10 0x00007ffff1e8e445 in vtkExecutive::CallAlgorithm (this=0x55555617f2c0, request=0x5555561e9ea0, direction=1, inInfo=0x0, outInfo=0x55555617e310) at /home/glow/dev/vtk/vtk1/src/Common/ExecutionModel/vtkExecutive.cxx:723
#11 0x00007ffff1e7fe5e in vtkDemandDrivenPipeline::ExecuteData (this=0x55555617f2c0, request=0x5555561e9ea0, inInfo=0x0, outInfo=0x55555617e310) at /home/glow/dev/vtk/vtk1/src/Common/ExecutionModel/vtkDemandDrivenPipeline.cxx:450

@mwestphal
Copy link
Contributor

I think there is two issues here:

  1. Unable to read this file for some reason
  2. Crash on closing the file because PLY reader does not cleanup correctly

@mwestphal mwestphal added this to F3D Jan 16, 2025
@mwestphal mwestphal moved this to Investigate in F3D Jan 16, 2025
@exbluesbreaker
Copy link

Hello!

I'm working on this problem. There are two ply element creations and deletions, but the second one involves uninitialized heap memory that later gets freed. I'll continue tomorrow—I’ve spent some time on it. It seems there should be better initialization since the freeing code assumes all pointers are valid and doesn’t check for NULL(and pointer is not null just not initialized).

Overall it is not good to see C-style memory management in C++ code, but I guess there are reasons for it :)

@mwestphal
Copy link
Contributor

Overall it is not good to see C-style memory management in C++ code, but I guess there are reasons for it :)

That code is OLD.

Thanks for investigating!

(btw we have a discord, feel free to join if thats your thing @exbluesbreaker : https://discord.f3d.app)

@exbluesbreaker
Copy link

I joined, just didn't yet talk there)

@exbluesbreaker
Copy link

Hello! So it seems I figure out the issue.
ply file had those elements:

element vertex 139777- point cloud with a lot of points
element face 0- no polygons (a bit uncommon for ply but could happen with like raw point cloud input)
element camera 1

The issue is the absence of polygons, which means also no properties are allocated, leaving props uninitialized. As a result, free is called on an invalid pointer, causing a crash. In vtkPLY::add_property, properties are allocated, so vtkPLY::ply_close should check elem->nprops > 0 before freeing props. Adding this check prevents the crash.

// vtkPLY::add_property
...
  if (elem->nprops == 0)
    elem->props = (PlyProperty**)myalloc(sizeof(PlyProperty*));
  else
    elem->props = (PlyProperty**)realloc(elem->props, sizeof(PlyProperty*) * (elem->nprops + 1));
...

Place of crash without check:

// vtkPLY::ply_close
...
  PlyElement* elem;
  for (i = 0; i < plyfile->nelems; i++)
  {
    elem = plyfile->elems[i];
    free(elem->name);
    for (j = 0; j < elem->nprops; j++)
    {
      free(const_cast<char*>(elem->props[j]->name));
      free(elem->props[j]);
    }
    free(elem->props);//crash happens here 
    free(elem->store_prop);
    free(elem);
  }
...

I will prepare MR for VTK with fix for it.
The only thing which still bothers me, I may have found another issue—after successfully displaying a PLY file, pressing Ctrl+C sometimes leaves the app unresponsive instead of closing it, requiring a manual kill. I couldn't reproduce this with a different file.

Also here is result of display of ply file. I doesn't look exactly like dog to me, but who knows :)

Image

@mwestphal
Copy link
Contributor

pressing Ctrl+C sometimes leaves the app unresponsive instead of closing it,

Ive seen that sometimes, with other files, i think its another bug.

@exbluesbreaker
Copy link

I created this pull request
https://gitlab.kitware.com/vtk/vtk/-/merge_requests/11961

@parkerlreed
Copy link
Author

I could have sworn this was viewable in something but yeah everything I'm trying now is not opening it due to not having any faces...

Oh found it

Image

https://point.love/

So it has valid data, maybe nothing is coded to display it? (This particular example was only IR so the only color is white but they can have colors as well)

Image

@parkerlreed
Copy link
Author

I guess the distinction here being it's a point cloud and not a 3D model so maybe it's not within scope for F3D to display?

RTAB-Map renders these with VTK so the underlying support should be there.

@exbluesbreaker
Copy link

Yeah maybe there is something of with visualization, according to file there should be 139777 points, but in f3d visualization there are definitely less, I guess small rectangles correspond to single point.

@exbluesbreaker
Copy link

I can try to open it tomorrow in Matlab at work (it probably should support it and I also should have probably necessary toolbox).

@mwestphal
Copy link
Contributor

so maybe it's not within scope for F3D to display?

It definitely is in the scope of F3D :)

@mwestphal
Copy link
Contributor

Lets fix the crash first, we will figure out if a rendering fix is needed after :)

@exbluesbreaker
Copy link

Opens in MATLAB ok.

Actually I thought maybe I need to zoom out in f3d, maybe default viewport onlyl catches small portion of overall point cloud somehow. In MATLAB by default it was zoom out very far away.
Image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Investigate
Development

No branches or pull requests

3 participants