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 options #4906

Closed
Closed

Conversation

AnnaRomanov
Copy link
Contributor

Added PLY configuration options:

  • OPTION_PLY_MESH: if true, export mesh (calculate faces); otherwise export only points.
    Default: true.
  • OPTION_PLY_BINARY: if true, export binary file; otherwise export textual (ascii format) PLY.
    Default: true.
  • OPTION_PLY_NORMALS: if true, add vertex normals (available only when OPTION_PLY_MESH is true).
    Default: false.

The options are available in the rs2::save_to_ply processing block.
Also, realsense viewer now uses the above mentioned processing block when exporting to PLY. Clicking on the "floppy" button allows choosing the PLY configuration.

solves: #4813

class save_to_ply : public filter
{
public:
save_to_ply(std::string filename = "RealSense Pointcloud ", pointcloud pc = pointcloud())
: filter([this](frame f, frame_source& s) { func(f, s); }),
static const auto OPTION_IGNORE_COLOR = rs2_option(RS2_OPTION_COUNT + 1);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Pls sync with @aangerma to avoid conflicts.
This needs to be managed cross-SDK

filter([this](frame f, frame_source& s)
{
std::vector<rs2::frame> frame_vec;
auto tex = owner->get_last_texture()->get_last_frame(true);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Pls check whether the capture is safe if device is disconnected while processing the export

@@ -83,7 +100,7 @@ namespace rs2
//ImGui::End();
}

void viewer_model::show_3dviewer_header(ImFont* font, rs2::rect stream_rect, bool& paused, std::string& error_message)
void viewer_model::show_3dviewer_header(ImFont* large_font, ImFont* font, rs2::rect stream_rect, bool& paused, std::string& error_message)
Copy link
Collaborator

Choose a reason for hiding this comment

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

PLs add a comment what is the 2nd font for

}

auto curr_exporter = exporters.find(tab);
assert(curr_exporter != exporters.end()); // every tab should have a corresponding exporter
Copy link
Collaborator

Choose a reason for hiding this comment

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

The assert will be discarded in release. A run-time error should probably be generated here

{
auto curr_exporter = exporters.find(tab);
assert(curr_exporter != exporters.end()); // every tab should have a corresponding exporter
if (auto ret = file_dialog_open(save_file, curr_exporter->second.filters.c_str(), NULL, NULL))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as in line 392

@@ -264,6 +281,193 @@ namespace rs2
}
}

ImGui::PopStyleColor(3);
Copy link
Collaborator

Choose a reason for hiding this comment

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

It is strange that the section starts with pop and not push - seem to indicate and issue elsewhere.

Please encapsulate this fragment into a function call for maintainability.

}

export_frame(fname, std::move(exporter), not_model, data);
exporter.reset();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do you need to reset it explicitly just before scoping out ?

@@ -2493,6 +2681,11 @@ namespace rs2
return false;
}

std::shared_ptr<texture_buffer> viewer_model::get_last_texture()
Copy link
Collaborator

Choose a reason for hiding this comment

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

This seem to be a public member - what is the getter for ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

last_texture is private

@dorodnic
Copy link
Contributor

dorodnic commented Nov 2, 2019

Superseded by #5110

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