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>

Author: Evgeni Raikhel

common/processing-block-model.cpp

@@ -114,17 +114,21 @@ namespace rs2
     void save_processing_block_to_config_file(const char* name,
         std::shared_ptr<rs2::processing_block> pb, bool enable)
     {
+        // set_protected() (not plain set()): this fires on every stream Start (via the
+        // debounced _options_invalidated path in subdevice_model::update()) and on every
+        // filter enable/disable toggle - both are frequent enough to have clobbered a
+        // hand-edited crosshair overlay section, the same way window drag/resize did.
         for (auto opt : pb->get_supported_options())
         {
             auto val = pb->get_option(opt);
             std::string key = name;
             key += ".";
             key += pb->get_option_name(opt);
-            config_file::instance().set(key.c_str(), val);
+            config_file::instance().set_protected(key.c_str(), val);
         }
 
         std::string key = name;
         key += ".enabled";
-        config_file::instance().set(key.c_str(), enable);
+        config_file::instance().set_protected(key.c_str(), enable);
     }
 }

common/rs-config.cpp

@@ -22,6 +22,13 @@ void config_file::set(const char* key, const char* value)
     save();
 }
 
+void config_file::set_protected(const char* key, const char* value)
+{
+    std::lock_guard< std::recursive_mutex > lk( _mutex );
+    _j[key] = value;
+    save( skip_all );
+}
+
 void config_file::set_default(const char* key, const char* calculate)
 {
     std::lock_guard< std::recursive_mutex > lk( _mutex );
@@ -116,6 +123,47 @@ config_file::config_file( std::string const & filename )
 
 void config_file::save()
 {
+    // Plain write-through, same as before any protected section existed: set()/remove()/
+    // reset() are used by ~10 unrelated call sites (device options, processing blocks,
+    // DDS settings, calibration timestamps, ...) that have never been shown to clobber a
+    // protected section, so they pay no extra disk read. The few call sites that are
+    // known to (window position/size - see ux-window.cpp) use set_protected() instead.
+    save( skip_none );
+}
+
+void config_file::save( unsigned skip_overwrite_mask )
+{
+    std::lock_guard< std::recursive_mutex > lk( _mutex );
+
+    if( skip_overwrite_mask != skip_none && ! _filename.empty() )
+    {
+        // _j only reflects what was loaded at startup (plus whatever this process has
+        // explicitly set since). If the user hand-edited a protected section directly in
+        // realsense-config.json while the viewer was running, adopt their on-disk values
+        // for every section named in the mask now - otherwise this save (e.g. triggered by
+        // an unrelated window move/resize, or on exit) would blindly overwrite the file
+        // with our stale in-memory copy and silently discard their edit.
+        try
+        {
+            auto on_disk = rsutils::json_config::load_from_file( _filename );
+            if( on_disk.exists() )
+            {
+                if( ( skip_overwrite_mask & skip_grid_overlay )
+                    && on_disk.contains( "viewer_model" )
+                    && on_disk["viewer_model"].contains( "grid_overlay" ) )
+                {
+                    _j["viewer_model"]["grid_overlay"] = on_disk["viewer_model"]["grid_overlay"];
+                }
+                // Add an `if (skip_overwrite_mask & skip_<new_section>) ...` block here for
+                // each additional bit as new protected sections are introduced.
+            }
+        }
+        catch( ... )
+        {
+            // Best effort - if the file can't be read, fall back to whatever _j already has.
+        }
+    }
+
     save( _filename.c_str() );
 }
 
@@ -139,7 +187,10 @@ config_file& config_file::operator=(const config_file& other)
         std::lock_guard< std::recursive_mutex > lk_this( _mutex );
         _j = std::move( j_copy );
         _defaults = std::move( defaults_copy );
-        save();
+        // Assignment is always an intentional, wholesale replacement (Load Settings,
+        // Restore Defaults + Apply, etc.) - protect nothing here, or we'd immediately
+        // discard the very values this assignment means to commit.
+        save( skip_none );
     }
     return *this;
 }

common/rs-config.h

@@ -79,6 +79,21 @@ namespace rs2
             set(key, ss.str().c_str());
         }
 
+        // Same as set(), but the resulting save() also preserves any protected config
+        // section (currently just the crosshair/grid overlay) from an out-of-process
+        // hand-edit. Reserved for the few call sites - window position/size - proven to
+        // fire often enough to have caused exactly that problem; everything else keeps
+        // using plain set() and pays no extra disk read.
+        void set_protected(const char* key, const char* value);
+
+        template<class T>
+        void set_protected(const char* key, T val)
+        {
+            std::stringstream ss;
+            ss << val;
+            set_protected(key, ss.str().c_str());
+        }
+
         bool contains(const char* key) const;
 
         void save(const char* filename);
@@ -142,6 +157,9 @@ namespace rs2
             }
 
             ( *current )[keys.back()] = val;
+            // Plain write-through - set_nested() is also used for e.g. DDS settings, which
+            // have never been shown to clobber a protected section, so it pays no extra
+            // disk read. See set_protected() for the call sites that do need it.
             save();
         }
 
@@ -190,9 +208,27 @@ namespace rs2
         }
 
     private:
+        // Bitmask of config sections that save() must NOT overwrite with our in-memory
+        // copy - their on-disk value is kept instead. skip_none (0x0, the default for
+        // every plain set()/set_nested()/remove()/reset() call) protects nothing, i.e.
+        // save() writes exactly what's in memory and overwrites the whole file. Only
+        // set_protected() opts in to skip_all, and only for the call sites proven to
+        // need it (window position/size - see ux-window.cpp). Add a bit here (and a
+        // matching branch in save(unsigned)) for each additional config section that
+        // should survive an out-of-process hand-edit across an unrelated save.
+        enum skip_overwrite : unsigned
+        {
+            skip_none         = 0x0,
+            skip_grid_overlay = 0x1,  // viewer_model.grid_overlay.* (crosshair overlay config)
+
+            // Bitwise-OR of every bit above. Extend this when a new bit is added.
+            skip_all          = skip_grid_overlay,
+        };
+
         std::string get_default(const char* key, const char* def) const;
 
         void save();
+        void save( unsigned skip_overwrite_mask );
 
         // Serializes all reads/writes of `_j` and the on-disk file. Required because
         // viewer reads/writes config_file from multiple threads (UI thread, the

common/stream-model.cpp

@@ -448,10 +448,11 @@ namespace rs2
     void stream_model::show_stream_header(ImFont* font, const rect &stream_rect, viewer_model& viewer)
     {
         const auto top_bar_height = 32.f;
-        auto num_of_buttons = 6; // Crosshair button is the latest addition
+        auto num_of_buttons = 5;
 
         if (!viewer.allow_stream_close) --num_of_buttons;
         if (viewer.streams.size() > 1) ++num_of_buttons;
+        if (profile.as<rs2::video_stream_profile>()) ++num_of_buttons; // Grid/crosshair button - video streams only
         if (RS2_STREAM_DEPTH == profile.stream_type()) ++num_of_buttons; // Color map ruler button
         if (RS2_FORMAT_MOTION_XYZ32F == profile.format()) ++num_of_buttons; // Motion graph button
         if (RS2_STREAM_OCCUPANCY == profile.stream_type() && _normalized_zoom.w == 1) ++num_of_buttons; // Safety zones button
@@ -590,29 +591,32 @@ namespace rs2
         }
         ImGui::SameLine();
 
-        label = rsutils::string::from() << textual_icons::grid << "##Grid " << profile.unique_id();
-        if (show_crosshair)
+        if (profile.as<rs2::video_stream_profile>()) // Grid/crosshair overlay is only meaningful on 2D video streams
         {
-            ImGui::PushStyleColor(ImGuiCol_Text, light_blue);
-            ImGui::PushStyleColor(ImGuiCol_TextSelectedBg, light_blue);
-            if (ImGui::Button(label.c_str(), { 24, top_bar_height }))
+            label = rsutils::string::from() << textual_icons::grid << "##Grid " << profile.unique_id();
+            if (show_crosshair)
             {
-                show_crosshair = false;
+                ImGui::PushStyleColor(ImGuiCol_Text, light_blue);
+                ImGui::PushStyleColor(ImGuiCol_TextSelectedBg, light_blue);
+                if (ImGui::Button(label.c_str(), { 24, top_bar_height }))
+                {
+                    show_crosshair = false;
+                }
+                if (ImGui::IsItemHovered())
+                    RsImGui::CustomTooltip("Hide crosshair overlay");
+                ImGui::PopStyleColor(2);
             }
-            if (ImGui::IsItemHovered())
-                RsImGui::CustomTooltip("Hide crosshair overlay");
-            ImGui::PopStyleColor(2);
-        }
-        else
-        {
-            if (ImGui::Button(label.c_str(), { 24, top_bar_height }))
+            else
             {
-                show_crosshair = true;
+                if (ImGui::Button(label.c_str(), { 24, top_bar_height }))
+                {
+                    show_crosshair = true;
+                }
+                if (ImGui::IsItemHovered())
+                    RsImGui::CustomTooltip("Show crosshair/grid overlay");
             }
-            if (ImGui::IsItemHovered())
-                RsImGui::CustomTooltip("Show crosshair/grid overlay");
+            ImGui::SameLine();
         }
-        ImGui::SameLine();
 
 
         if (RS2_STREAM_DEPTH == profile.stream_type())
@@ -2104,7 +2108,7 @@ namespace rs2
 
             update_ae_roi_rect(stream_rect, g, error_message);
 
-            if (show_crosshair)
+            if (show_crosshair && profile.as<rs2::video_stream_profile>())
                 draw_crosshair(stream_rect, grid_h_lines, grid_v_lines, grid_line_width,
                                grid_color_r, grid_color_g, grid_color_b);
 

common/ux-window.cpp

@@ -87,8 +87,9 @@ namespace rs2
         config_file::instance().set_default(configurations::viewer::commands_xml, "./Commands.xml");
         config_file::instance().set_default(configurations::viewer::hwlogger_xml, "./HWLoggerEvents.xml");
 
-        if( config_file::instance().is_empty() )
         {
+            // set_nested_default() only writes a key if it's missing, so this is safe to run
+            // unconditionally - it backfills the grid keys into pre-existing config files too.
             namespace cfg = configurations::viewer::viewport_grid_overlay;
             auto& cf = config_file::instance();
             cf.set_nested_default( cfg::horizontal_lines, 1   );
@@ -372,20 +373,23 @@ namespace rs2
             glDebugMessageCallback(MessageCallback, 0);
         }
 
+        // set_protected() (not plain set()): these fire on every native WM_MOVE/WM_SIZE -
+        // dozens of times per drag gesture - and are the call sites that motivated
+        // protecting the crosshair overlay section from an unrelated save (see rs-config.h).
         glfwSetWindowPosCallback(_win, [](GLFWwindow* w, int x, int y)
         {
-            config_file::instance().set(configurations::window::saved_pos, true);
-            config_file::instance().set(configurations::window::position_x, x);
-            config_file::instance().set(configurations::window::position_y, y);
+            config_file::instance().set_protected(configurations::window::saved_pos, true);
+            config_file::instance().set_protected(configurations::window::position_x, x);
+            config_file::instance().set_protected(configurations::window::position_y, y);
         });
 
         glfwSetWindowSizeCallback( _win, []( GLFWwindow * window, int width, int height ) {
             if( width > 0 && height > 0 )
             {
-                config_file::instance().set( configurations::window::saved_size, true );
-                config_file::instance().set( configurations::window::width, width );
-                config_file::instance().set( configurations::window::height, height );
-                config_file::instance().set( configurations::window::maximized,
+                config_file::instance().set_protected( configurations::window::saved_size, true );
+                config_file::instance().set_protected( configurations::window::width, width );
+                config_file::instance().set_protected( configurations::window::height, height );
+                config_file::instance().set_protected( configurations::window::maximized,
                                              glfwGetWindowAttrib( window, GLFW_MAXIMIZED ) );
             }
         } );