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.

Sign up for GitHub

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

One Viewer Context #5623

Merged
merged 2 commits into from
Feb 3, 2020
Merged

One Viewer Context #5623

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c5a8d7d
realsense-viewer: avoid creating unused contexts
radfordi Jan 10, 2020
f8371f9
Let ux_window's pops use a shared context
radfordi Jan 15, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions common/ux-window.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -354,11 +354,11 @@ namespace rs2
stbi_image_free(r);
}

ux_window::ux_window(const char* title) :
ux_window::ux_window(const char* title, context &ctx) :
Copy link
Collaborator

Choose a reason for hiding this comment

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

@radfordi , can you please provide the rationale for this PR ?
I don't think that it could a performance issue. And if not - then the PR may suggests that multiple context instances are somehow incompatible with a certain HW setup which in turn implies that there is an underlying issue that need to be resolved.
To this points there were no specific limitations on the number of ctx objects except for T265, so adding the constrain is at least a potential compatibility break.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I ran into the multiple contexts when trying to test @bfulkers-i' hotplug PR (#5492). It's annoying/challenging to debug hotplug in the context in realsense-viewer because we keep creating/deleting contexts every 200ms! The (longer term) goal of that PR is to move all polling, checking and looping (with sleeps), to the OS level and to make the realsense-viewer (and tm_boot) to rely on the device changed notification instead.

It's not a large performance issue currently because the viewer is such a resource hog as it is, but this is small step toward making it less so and making the device detection use hotplug only.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, T265's can't currently be detected in multiple contexts at the same time. This should be fixed, but in the mean time it's trivial to have a single context and avoid the issue.

_win(nullptr), _width(0), _height(0), _output_height(0),
_font_14(nullptr), _font_18(nullptr), _app_ready(false),
_first_frame(true), _query_devices(true), _missing_device(false),
_hourglass_index(0), _dev_stat_message{}, _keep_alive(true), _title(title)
_hourglass_index(0), _dev_stat_message{}, _keep_alive(true), _title(title), _ctx(ctx)
{
open_window();

Expand Down Expand Up @@ -419,7 +419,7 @@ namespace rs2
bool do_200ms = every_200ms;
if (_query_devices && do_200ms)
{
_missing_device = rs2::context().query_devices(RS2_PRODUCT_LINE_ANY).size() == 0;
_missing_device = _ctx.query_devices(RS2_PRODUCT_LINE_ANY).size() == 0;
_hourglass_index = (_hourglass_index + 1) % 5;

if (!_missing_device)
Expand Down
4 changes: 3 additions & 1 deletion common/ux-window.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
namespace rs2
{
class visualizer_2d;
class context;

class viewer_ui_traits
{
Expand All @@ -34,7 +35,7 @@ namespace rs2
std::function<void(std::string)> on_file_drop = [](std::string) {};
std::function<bool()> on_load = []() { return false; };

ux_window(const char* title);
ux_window(const char* title, context &ctx);

float width() const { return float(_width); }
float height() const { return float(_height); }
Expand Down Expand Up @@ -121,6 +122,7 @@ namespace rs2

std::string _title;
std::shared_ptr<visualizer_2d> _2d_vis;
context &_ctx;

bool _is_ui_aligned = false;
};
Expand Down
5 changes: 3 additions & 2 deletions common/viewer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -766,8 +766,9 @@ namespace rs2
}
}

viewer_model::viewer_model()
: ppf(*this),
viewer_model::viewer_model(context &ctx_)
: ppf(*this),
ctx(ctx_),
synchronization_enable(true),
zo_sensors(0),
frameset_alloc(this)
Expand Down
4 changes: 2 additions & 2 deletions common/viewer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ namespace rs2

rs2::frame handle_ready_frames(const rect& viewer_rect, ux_window& window, int devices, std::string& error_message);

viewer_model();
viewer_model(context &ctx_);

~viewer_model()
{
Expand Down Expand Up @@ -128,7 +128,7 @@ namespace rs2
std::shared_ptr<syncer_model> syncer;
post_processing_filters ppf;

context ctx;
context &ctx;
notifications_model not_model;
bool is_output_collapsed = false;
bool is_3d_view = false;
Expand Down
7 changes: 5 additions & 2 deletions tools/depth-quality/depth-quality-model.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,11 @@ namespace rs2
{
namespace depth_quality
{
tool_model::tool_model()
: _update_readonly_options_timer(std::chrono::seconds(6)), _roi_percent(0.4f),
tool_model::tool_model(rs2::context &ctx)
: _ctx(ctx),
_pipe(ctx),
_viewer_model(ctx),
_update_readonly_options_timer(std::chrono::seconds(6)), _roi_percent(0.4f),
_roi_located(std::chrono::seconds(4)),
_too_close(std::chrono::seconds(4)),
_too_far(std::chrono::seconds(4)),
Expand Down
4 changes: 2 additions & 2 deletions tools/depth-quality/depth-quality-model.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,7 @@ namespace rs2
class tool_model
{
public:
tool_model();
tool_model(rs2::context& ctx);

bool start(ux_window& win);

Expand Down Expand Up @@ -381,6 +381,7 @@ namespace rs2

std::string capture_description();

rs2::context& _ctx;
pipeline _pipe;
std::shared_ptr<device_model> _device_model;
viewer_model _viewer_model;
Expand Down Expand Up @@ -408,7 +409,6 @@ namespace rs2

float _min_dist, _max_dist, _max_angle;
std::mutex _mutex;
rs2::context _ctx;

bool _use_ground_truth = false;
int _ground_truth = 0;
Expand Down
5 changes: 3 additions & 2 deletions tools/depth-quality/rs-depth-quality.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@

int main(int argc, const char * argv[]) try
{
rs2::ux_window window("Depth Quality Tool");
rs2::depth_quality::tool_model model;
rs2::context ctx;
rs2::ux_window window("Depth Quality Tool", ctx);
rs2::depth_quality::tool_model model(ctx);

using namespace rs2::depth_quality;

Expand Down
7 changes: 3 additions & 4 deletions tools/realsense-viewer/realsense-viewer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,10 +246,10 @@ int main(int argc, const char** argv) try
{
rs2::log_to_console(RS2_LOG_SEVERITY_WARN);

ux_window window("Intel RealSense Viewer");
context ctx;
ux_window window("Intel RealSense Viewer", ctx);

// Create RealSense Context
context ctx;
device_changes devices_connection_changes(ctx);
std::vector<std::pair<std::string, std::string>> device_names;

Expand All @@ -259,8 +259,7 @@ int main(int argc, const char** argv) try
std::shared_ptr<device_models_list> device_models = std::make_shared<device_models_list>();
device_model* device_to_remove = nullptr;

viewer_model viewer_model;
viewer_model.ctx = ctx;
viewer_model viewer_model(ctx);

std::vector<device> connected_devs;
std::mutex m;
Expand Down