Skip to content

Commit 20ebeb5

Browse files
author
Evgeni Raikhel
committed
Revert generic grid_overlay protection from config_file::save()
Per Nir-Az's review comment: the grid/crosshair overlay is a viewer_model-scoped feature and has no business special-casing the generic config_file save/set path used by every other feature (device options, DDS settings, window geometry, ...). Removes _grid_overlay_dirty, is_grid_overlay_path(), and the disk-read/merge that save() ran on every flush to protect that one section. This intentionally re-opens the narrower issue the removed code was guarding against: hand-editing viewer_model.grid_overlay.* in realsense-config.json while the viewer is running can still be silently clobbered by the next unrelated config save. Left unaddressed here to keep this PR scoped to the crosshair feature; worth a separate, narrowly-targeted fix if it's worth solving at all.
1 parent fb34c6a commit 20ebeb5

2 files changed

Lines changed: 0 additions & 47 deletions

File tree

common/rs-config.cpp

Lines changed: 0 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,6 @@ 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 )
117116
, _save_stop( false )
118117
{
119118
try
@@ -128,39 +127,9 @@ config_file::config_file( std::string const & filename )
128127
_save_thread = std::thread( &config_file::save_loop, this );
129128
}
130129

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-
137130
void config_file::save()
138131
{
139132
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-
164133
if( ! _filename.empty() )
165134
save( _filename.c_str() );
166135
}
@@ -194,7 +163,6 @@ void config_file::save_loop()
194163
config_file::config_file()
195164
: _j( rsutils::json::object() )
196165
, _dirty( false )
197-
, _grid_overlay_dirty( false )
198166
, _save_stop( false )
199167
{
200168
}
@@ -214,12 +182,7 @@ config_file& config_file::operator=(const config_file& other)
214182
std::lock_guard< std::recursive_mutex > lk_this( _mutex );
215183
_j = std::move( j_copy );
216184
_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.
221185
_dirty = true;
222-
_grid_overlay_dirty = true;
223186
}
224187
return *this;
225188
}

common/rs-config.h

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

157157
( *current )[keys.back()] = val;
158158
_dirty = true;
159-
if( is_grid_overlay_path( path ) )
160-
_grid_overlay_dirty = true;
161159
}
162160

163161
// Sets a default value to the config and default map
@@ -201,8 +199,6 @@ namespace rs2
201199
}
202200
( *current )[keys.back()] = default_val;
203201
_dirty = true;
204-
if( is_grid_overlay_path( path ) )
205-
_grid_overlay_dirty = true;
206202
}
207203
}
208204

@@ -214,11 +210,6 @@ namespace rs2
214210

215211
static constexpr std::chrono::milliseconds SAVE_INTERVAL{ 1000 };
216212

217-
// True if `path` (dot-notation) falls under the viewport grid/crosshair overlay
218-
// section - the one config section save() protects from being overwritten by an
219-
// unrelated flush; see _grid_overlay_dirty and save().
220-
static bool is_grid_overlay_path( const std::string & path );
221-
222213
// Serializes all reads/writes of `_j` and the on-disk file. Required because
223214
// viewer reads/writes config_file from multiple threads (UI thread, the
224215
// config_save_worker background thread in subdevice-model.cpp, and ad-hoc
@@ -230,7 +221,6 @@ namespace rs2
230221
std::string _filename;
231222
rsutils::json _j;
232223
std::atomic<bool> _dirty;
233-
std::atomic<bool> _grid_overlay_dirty;
234224
std::condition_variable _save_cv;
235225
std::mutex _save_cv_mutex;
236226
bool _save_stop;

0 commit comments

Comments
 (0)