Skip to content

Commit 93858a2

Browse files
Evgeni Raikhelclaude
andcommitted
Fix crosshair overlay: video-only, backfill defaults, protect from unrelated saves
Addresses Nir-Az's review comments on PR realsenseai#15093: - Restrict the grid/crosshair toggle button and rendering to 2D video streams (profile.as<rs2::video_stream_profile>()), matching the pattern used elsewhere for stream-details fields. Previously it also rendered on motion (IMU) streams. - Backfill viewer_model.grid_overlay.* defaults into pre-existing, non-empty config files. set_nested_default() already only writes a missing key, so the is_empty() gate around it was redundant and wrong - it meant the grid keys were only ever written into a brand-new config file, never into an existing user's realsense-config.json. Also fixes a related bug found while testing the above: hand-editing viewer_model.grid_overlay.* in realsense-config.json while the viewer is running got silently reverted by the next unrelated config save, because config_file caches the whole document in memory and every set()/set_nested() call blindly overwrites the entire file with that stale copy. Adds config_file::set_protected(), used only by the specific call sites known to fire often enough to have caused this - the window position/size callbacks (drag/resize) and processing-block persistence (stream start, filter enable/disable toggle) - which re-adopt the on-disk grid_overlay section before writing. Every other config_file caller (device options, DDS settings, calibration timestamps, ...) is unaffected and pays no extra I/O. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 2d3036a commit 93858a2

4 files changed

Lines changed: 73 additions & 20 deletions

File tree

common/rs-config.cpp

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,7 @@ config_file& config_file::instance()
113113
config_file::config_file( std::string const & filename )
114114
: _filename( filename )
115115
, _dirty( false )
116+
, _grid_overlay_dirty( false )
116117
, _save_stop( false )
117118
{
118119
try
@@ -127,8 +128,39 @@ config_file::config_file( std::string const & filename )
127128
_save_thread = std::thread( &config_file::save_loop, this );
128129
}
129130

131+
bool config_file::is_grid_overlay_path( const std::string & path )
132+
{
133+
static const std::string prefix = "viewer_model.grid_overlay.";
134+
return path.rfind( prefix, 0 ) == 0;
135+
}
136+
130137
void config_file::save()
131138
{
139+
std::lock_guard< std::recursive_mutex > lk( _mutex );
140+
141+
// If nothing in this process has intentionally changed the crosshair/grid overlay
142+
// section since the last flush, adopt whatever is currently on disk for it before
143+
// writing. This flush can be triggered by any unrelated set() (window move/resize,
144+
// device options, ...) up to once per SAVE_INTERVAL; without this, it would blindly
145+
// overwrite a hand-edit made to realsense-config.json while the viewer is running
146+
// with our stale in-memory copy.
147+
if( ! _grid_overlay_dirty.exchange( false ) && ! _filename.empty() )
148+
{
149+
try
150+
{
151+
auto on_disk = rsutils::json_config::load_from_file( _filename );
152+
if( on_disk.exists() && on_disk.contains( "viewer_model" )
153+
&& on_disk["viewer_model"].contains( "grid_overlay" ) )
154+
{
155+
_j["viewer_model"]["grid_overlay"] = on_disk["viewer_model"]["grid_overlay"];
156+
}
157+
}
158+
catch( ... )
159+
{
160+
// Best effort - if the file can't be read, fall back to whatever _j already has.
161+
}
162+
}
163+
132164
if( ! _filename.empty() )
133165
save( _filename.c_str() );
134166
}
@@ -162,6 +194,7 @@ void config_file::save_loop()
162194
config_file::config_file()
163195
: _j( rsutils::json::object() )
164196
, _dirty( false )
197+
, _grid_overlay_dirty( false )
165198
, _save_stop( false )
166199
{
167200
}
@@ -181,7 +214,12 @@ config_file& config_file::operator=(const config_file& other)
181214
std::lock_guard< std::recursive_mutex > lk_this( _mutex );
182215
_j = std::move( j_copy );
183216
_defaults = std::move( defaults_copy );
217+
// Assignment is always an intentional, wholesale replacement (Load Settings,
218+
// Restore Defaults + Apply, etc.) - mark grid_overlay dirty too, or the next
219+
// deferred flush would discard whatever value came with this assignment by
220+
// merging stale on-disk content back over it.
184221
_dirty = true;
222+
_grid_overlay_dirty = true;
185223
}
186224
return *this;
187225
}

common/rs-config.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,8 @@ namespace rs2
158158

159159
( *current )[keys.back()] = val;
160160
_dirty = true;
161+
if( is_grid_overlay_path( path ) )
162+
_grid_overlay_dirty = true;
161163
}
162164

163165
// Sets a default value to the config and default map
@@ -201,6 +203,8 @@ namespace rs2
201203
}
202204
( *current )[keys.back()] = default_val;
203205
_dirty = true;
206+
if( is_grid_overlay_path( path ) )
207+
_grid_overlay_dirty = true;
204208
}
205209
}
206210

@@ -212,6 +216,11 @@ namespace rs2
212216

213217
static constexpr std::chrono::milliseconds SAVE_INTERVAL{ 1000 };
214218

219+
// True if `path` (dot-notation) falls under the viewport grid/crosshair overlay
220+
// section - the one config section save() protects from being overwritten by an
221+
// unrelated flush; see _grid_overlay_dirty and save().
222+
static bool is_grid_overlay_path( const std::string & path );
223+
215224
// Serializes all reads/writes of `_j` and the on-disk file. Required because
216225
// viewer reads/writes config_file from multiple threads (UI thread, the
217226
// config_save_worker background thread in subdevice-model.cpp, and ad-hoc
@@ -223,6 +232,7 @@ namespace rs2
223232
std::string _filename;
224233
rsutils::json _j;
225234
std::atomic<bool> _dirty;
235+
std::atomic<bool> _grid_overlay_dirty;
226236
std::condition_variable _save_cv;
227237
std::mutex _save_cv_mutex;
228238
bool _save_stop;

common/stream-model.cpp

Lines changed: 23 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -453,10 +453,11 @@ namespace rs2
453453
void stream_model::show_stream_header(ImFont* font, const rect &stream_rect, viewer_model& viewer)
454454
{
455455
const auto top_bar_height = 32.f;
456-
auto num_of_buttons = 6; // Crosshair button is the latest addition
456+
auto num_of_buttons = 5;
457457

458458
if (!viewer.allow_stream_close) --num_of_buttons;
459459
if (viewer.streams.size() > 1) ++num_of_buttons;
460+
if (profile.as<rs2::video_stream_profile>()) ++num_of_buttons; // Grid/crosshair button - video streams only
460461
if (RS2_STREAM_DEPTH == profile.stream_type()) ++num_of_buttons; // Color map ruler button
461462
if (RS2_FORMAT_MOTION_XYZ32F == profile.format()) ++num_of_buttons; // Motion graph button
462463
if (RS2_STREAM_OCCUPANCY == profile.stream_type() && _normalized_zoom.w == 1) ++num_of_buttons; // Safety zones button
@@ -599,29 +600,32 @@ namespace rs2
599600
}
600601
ImGui::SameLine();
601602

602-
label = rsutils::string::from() << textual_icons::grid << "##Grid " << profile.unique_id();
603-
if (show_crosshair)
603+
if (profile.as<rs2::video_stream_profile>()) // Grid/crosshair overlay is only meaningful on 2D video streams
604604
{
605-
ImGui::PushStyleColor(ImGuiCol_Text, light_blue);
606-
ImGui::PushStyleColor(ImGuiCol_TextSelectedBg, light_blue);
607-
if (ImGui::Button(label.c_str(), { 24, top_bar_height }))
605+
label = rsutils::string::from() << textual_icons::grid << "##Grid " << profile.unique_id();
606+
if (show_crosshair)
608607
{
609-
show_crosshair = false;
608+
ImGui::PushStyleColor(ImGuiCol_Text, light_blue);
609+
ImGui::PushStyleColor(ImGuiCol_TextSelectedBg, light_blue);
610+
if (ImGui::Button(label.c_str(), { 24, top_bar_height }))
611+
{
612+
show_crosshair = false;
613+
}
614+
if (ImGui::IsItemHovered())
615+
RsImGui::CustomTooltip("Hide crosshair overlay");
616+
ImGui::PopStyleColor(2);
610617
}
611-
if (ImGui::IsItemHovered())
612-
RsImGui::CustomTooltip("Hide crosshair overlay");
613-
ImGui::PopStyleColor(2);
614-
}
615-
else
616-
{
617-
if (ImGui::Button(label.c_str(), { 24, top_bar_height }))
618+
else
618619
{
619-
show_crosshair = true;
620+
if (ImGui::Button(label.c_str(), { 24, top_bar_height }))
621+
{
622+
show_crosshair = true;
623+
}
624+
if (ImGui::IsItemHovered())
625+
RsImGui::CustomTooltip("Show crosshair/grid overlay");
620626
}
621-
if (ImGui::IsItemHovered())
622-
RsImGui::CustomTooltip("Show crosshair/grid overlay");
627+
ImGui::SameLine();
623628
}
624-
ImGui::SameLine();
625629

626630

627631
if (RS2_STREAM_DEPTH == profile.stream_type())
@@ -2113,7 +2117,7 @@ namespace rs2
21132117

21142118
update_ae_roi_rect(stream_rect, g, error_message);
21152119

2116-
if (show_crosshair)
2120+
if (show_crosshair && profile.as<rs2::video_stream_profile>())
21172121
draw_crosshair(stream_rect, grid_h_lines, grid_v_lines, grid_line_width,
21182122
grid_color_r, grid_color_g, grid_color_b);
21192123

common/ux-window.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,8 +87,9 @@ namespace rs2
8787
config_file::instance().set_default(configurations::viewer::commands_xml, "./Commands.xml");
8888
config_file::instance().set_default(configurations::viewer::hwlogger_xml, "./HWLoggerEvents.xml");
8989

90-
if( config_file::instance().is_empty() )
9190
{
91+
// set_nested_default() only writes a key if it's missing, so this is safe to run
92+
// unconditionally - it backfills the grid keys into pre-existing config files too.
9293
namespace cfg = configurations::viewer::viewport_grid_overlay;
9394
auto& cf = config_file::instance();
9495
cf.set_nested_default( cfg::horizontal_lines, 1 );

0 commit comments

Comments
 (0)