librealsense icon indicating copy to clipboard operation
librealsense copied to clipboard

PLY vertices data should be on a single line per vertex

Open Alexxkrehmen opened this issue 2 years ago • 14 comments

Issue Description

As shown in the PLY file format (http://gamma.cs.unc.edu/POWERPLANT/papers/ply.pdf) all the data for each vertex should be on a single line. Adding a carriage return after each property makes the file unusable in some third party applications (see here : https://developer.blender.org/T88745)

The fix seems easy to do, in librealsense/include/librealsense2/hpp/rs_export.hpp, between line 240 to 260, remove all the carriage returns added after each property, and only put one at the end of the line, like this :

for (size_t i = 0; i <new_verts.size(); ++i)
{
    out << new_verts[i].x << " ";
    out << new_verts[i].y << " ";
    out << new_verts[i].z << " ";

    if (mesh && use_normals)
    {
        out << normals[i].x << " ";
        out << normals[i].y << " ";
        out << normals[i].z << " ";
    }

    if (use_texcoords)
    {
        out << unsigned(new_tex[i][0]) << " ";
        out << unsigned(new_tex[i][1]) << " ";
        out << unsigned(new_tex[i][2]) << " ";
    }
    out << "\n";
}

Alexxkrehmen avatar Sep 29 '21 00:09 Alexxkrehmen

Thanks very much @Alexxkrehmen for sharing your method! If you would like it to be added officially to the RealSense SDK, I would recommend submitting the change yourself as a Pull Request so that it can be considered by the RealSense team for inclusion in the SDK.

https://github.com/IntelRealSense/librealsense/pulls

MartyG-RealSense avatar Sep 29 '21 08:09 MartyG-RealSense

Pull Request posted :)

Alexxkrehmen avatar Sep 29 '21 08:09 Alexxkrehmen

Thanks so much - I have added the Enhancement label to this case as a reminder that it should be kept open whilst the Pull Request is active. Thanks again!

MartyG-RealSense avatar Sep 29 '21 08:09 MartyG-RealSense

Thanks so much - I have added the Enhancement label to this case as a reminder that it should be kept open whilst the Pull Request is active. Thanks again!

I've closed my Pull request since it wasn't right ... I Can't find how to create one properly, either it includes many other requests I don't want or I just simply cannot create it because I don't have the right to do it :(

Alexxkrehmen avatar Oct 03 '21 18:10 Alexxkrehmen

@MartyG-RealSense nevermind... I've understood my mistake, the new Pull request is available here ;) https://github.com/IntelRealSense/librealsense/pull/9819

Alexxkrehmen avatar Oct 03 '21 19:10 Alexxkrehmen

Great, thanks very much @Alexxkrehmen :)

MartyG-RealSense avatar Oct 04 '21 10:10 MartyG-RealSense

Any updated on this? The fix seems trivial and the issue has been reported twice in Blender:

  • https://developer.blender.org/T92130
  • https://developer.blender.org/T88745

ideasman42 avatar Apr 23 '22 03:04 ideasman42

Any updated on this? The fix seems trivial and the issue has been reported twice in Blender:

* https://developer.blender.org/T92130

* https://developer.blender.org/T88745

I tried to send the fix : https://github.com/IntelRealSense/librealsense/pull/9819/commits but my commit is still pending... I don't know why...

Alexxkrehmen avatar Apr 23 '22 03:04 Alexxkrehmen

Thanks very much @Alexxkrehmen for your response to @ideasman42 - it appears that there is a build check failure on your pull at https://github.com/IntelRealSense/librealsense/pull/9819/

Normally a pull provides information about the specific aspects of the automated build check process that have failed but in this particular instance, clicking the Details option on this pull results in a broken link.

image

image

MartyG-RealSense avatar Apr 23 '22 06:04 MartyG-RealSense

Yes... So I strictly have no idea on how to fix this problem...

Alexxkrehmen avatar Apr 23 '22 07:04 Alexxkrehmen

You could perhaps create a new pull with the same fix and close the old one.

MartyG-RealSense avatar Apr 23 '22 07:04 MartyG-RealSense

Force pushing to your branch with an amended commit message should re-run the tests too.

ideasman42 avatar Apr 23 '22 07:04 ideasman42

Sorry, I thought I had made a mistake so I did the request twice >_< This should be ok to review now :)

Alexxkrehmen avatar Apr 23 '22 08:04 Alexxkrehmen

Thanks very much, @Alexxkrehmen

I am not involved in pull request processing, so I cannot unfortunately provide a time estimate for when a reviewer from the RealSense team may be assigned to your pull.

In the meantime, RealSense users should be able to apply your fix themselves by editing rs_export.hpp using your instructions at the top of this case.

MartyG-RealSense avatar Apr 23 '22 08:04 MartyG-RealSense