isl-org / Open3D

Open3D: A Modern Library for 3D Data Processing
http://www.open3d.org
Other
10.77k stars 2.23k forks source link

A few lines missing in dense_slam.py #4465

Closed cosama closed 2 years ago

cosama commented 2 years ago

Describe the bug The code in dense_slam.py and dense_slam_gui.py do not totally agree with each other. I think there are a few lines missing around here: https://github.com/isl-org/Open3D/blob/master/examples/python/t_reconstruction_system/dense_slam.py#L49. That section should be something like:

        depth_ref = o3d.t.io.read_image(depth_file_names[0])
        color_ref = o3d.t.io.read_image(color_file_names[0])
        input_frame = o3d.t.pipelines.slam.Frame(depth_ref.rows,
                                                 depth_ref.columns, intrinsic,
                                                 device)
        raycast_frame = o3d.t.pipelines.slam.Frame(depth_ref.rows,
                                                   depth_ref.columns, intrinsic,
                                                   device)

        input_frame.set_data_from_image('depth', depth_ref)
        input_frame.set_data_from_image('color', color_ref)

        raycast_frame.set_data_from_image('depth', depth_ref)
        raycast_frame.set_data_from_image('color', color_ref)

to agree with the dense_slam_gui.py code. If those lines are not there I also get a warning that 'color' is missing in my Frame. Not sure if it affects slam performance.

Additional context Happy to file a pull request if you guys think that is indeed the case.

theNded commented 2 years ago

These lines buffer images in the UI upon initialization, which gives a nice starting screen. They are not necessary for the cli version as they can be naturally overwritten in the iterations.

The warning would not affect the results, but I can add these lines back to address the concerns.

cosama commented 2 years ago

@theNded Thank you so much for the explanation. I didn't really understood why they would be needed or not. If those are not necessary then I feel they might as well be left out. If anybody has similar concern there will be information about it now here.

By the way the exact warning I get is:

[Open3D WARNING] Property not found for color, return an empty tensor!

@theNded Feel free to close this, if you think no changes are needed.

theNded commented 2 years ago

Thanks. The warning is expected, though there could be better ways to suppress it. I will close this issue for now and hopefully I will improve it later.