Keep a Scalar Config Option's Value When Deserializing Fails (#16641)

A failed stream extraction writes 0 into its target, so Float, Int, Percent and FloatOrPercent options were overwritten before deserialize reported the error, and Point/Point3 kept a partially scanned value. Parse into a temporary and assign only on success; FloatOrPercent sets its percent flag only then too.

src/libslic3r/Config.hpp

@@ -841,8 +841,12 @@ public:
     {
         UNUSED(append);
         std::istringstream iss(str);
-        iss >> this->value;
-        return !iss.fail();
+        double value;
+        iss >> value;
+        if (iss.fail())
+            return false;
+        this->value = value;
+        return true;
     }
 
     bool operator==(const ConfigOption &rhs) const override
@@ -1033,8 +1037,12 @@ public:
     {
         UNUSED(append);
         std::istringstream iss(str);
-        iss >> this->value;
-        return !iss.fail();
+        int value;
+        iss >> value;
+        if (iss.fail())
+            return false;
+        this->value = value;
+        return true;
     }
 
     ConfigOptionInt& operator=(const ConfigOption *opt)
@@ -1252,8 +1260,12 @@ public:
         UNUSED(append);
         // don't try to parse the trailing % since it's optional
         std::istringstream iss(str);
-        iss >> this->value;
-        return !iss.fail();
+        double value;
+        iss >> value;
+        if (iss.fail())
+            return false;
+        this->value = value;
+        return true;
     }
 
 private:
@@ -1365,10 +1377,14 @@ public:
     bool deserialize(const std::string &str, bool append = false) override
     {
         UNUSED(append);
-        this->percent = str.find_first_of("%") != std::string::npos;
         std::istringstream iss(str);
-        iss >> this->value;
-        return !iss.fail();
+        double value;
+        iss >> value;
+        if (iss.fail())
+            return false;
+        this->value   = value;
+        this->percent = str.find_first_of("%") != std::string::npos;
+        return true;
     }
 
 private:
@@ -1538,8 +1554,12 @@ public:
     {
         UNUSED(append);
         char dummy;
-        return sscanf(str.data(), " %lf , %lf %c", &this->value(0), &this->value(1), &dummy) == 2 ||
-               sscanf(str.data(), " %lf x %lf %c", &this->value(0), &this->value(1), &dummy) == 2;
+        Vec2d value;
+        if (sscanf(str.data(), " %lf , %lf %c", &value(0), &value(1), &dummy) != 2 &&
+            sscanf(str.data(), " %lf x %lf %c", &value(0), &value(1), &dummy) != 2)
+            return false;
+        this->value = value;
+        return true;
     }
 
 private:
@@ -1658,8 +1678,12 @@ public:
     {
         UNUSED(append);
         char dummy;
-        return sscanf(str.data(), " %lf , %lf , %lf %c", &this->value(0), &this->value(1), &this->value(2), &dummy) == 3 ||
-               sscanf(str.data(), " %lf x %lf x %lf %c", &this->value(0), &this->value(1), &this->value(2), &dummy) == 3;
+        Vec3d value;
+        if (sscanf(str.data(), " %lf , %lf , %lf %c", &value(0), &value(1), &value(2), &dummy) != 3 &&
+            sscanf(str.data(), " %lf x %lf x %lf %c", &value(0), &value(1), &value(2), &dummy) != 3)
+            return false;
+        this->value = value;
+        return true;
     }
 
 private:

src/slic3r/GUI/3DScene.cpp

@@ -700,7 +700,7 @@ void GLVolume::render_with_outline(const GUI::Size& cnv_size)
 }
 
 //BBS add render for simple case
-void GLVolume::simple_render(GLShaderProgram* shader, ModelObjectPtrs& model_objects, std::vector<ColorRGBA>& extruder_colors, bool ban_light, bool ban_color_volume)
+void GLVolume::simple_render(GLShaderProgram* shader, ModelObjectPtrs& model_objects, std::vector<ColorRGBA>& extruder_colors, bool ban_light)
 {
     if (this->is_left_handed())
         glFrontFace(GL_CW);
@@ -734,7 +734,7 @@ void GLVolume::simple_render(GLShaderProgram* shader, ModelObjectPtrs& model_obj
         }
     } while (0);
 
-    if (color_volume && !picking && !ban_color_volume) {
+    if (color_volume && !picking) {
         const bool brighten_selected = selected && !disabled && !force_native_color && !force_neutral_color;
 
         // when force_transparent, we need to keep the alpha

src/slic3r/GUI/3DScene.hpp

@@ -370,7 +370,7 @@ public:
     virtual void render_with_outline(const GUI::Size& cnv_size);
 
     //BBS: add simple render function for thumbnail
-    void simple_render(GLShaderProgram* shader, ModelObjectPtrs& model_objects, std::vector<ColorRGBA>& extruder_colors, bool ban_light =false, bool ban_color_volume = false);
+    void simple_render(GLShaderProgram* shader, ModelObjectPtrs& model_objects, std::vector<ColorRGBA>& extruder_colors, bool ban_light =false);
 
     void                set_bounding_boxes_as_dirty() {
         m_transformed_bounding_box.reset();

src/slic3r/GUI/GLCanvas3D.cpp

@@ -2804,9 +2804,7 @@ void GLCanvas3D::_render_scene(const Camera& camera, const Size& cnv_size)
             _render_bed(camera.get_view_matrix(), camera.get_projection_matrix(), !camera.is_looking_downward(), show_axes);
         }
         m_frame_profiler.mark("bed");
-        // Exception: the painting gizmos hide the bed mesh on purpose and draw the plate instead,
-        // so there the plate list is the only bed reference in the scene.
-        if (show_bed || _is_below_plate_view_active()) //BBS: add outline logic
+        if (show_bed) //BBS: add outline logic
             _render_platelist(camera.get_view_matrix(), camera.get_projection_matrix(), !camera.is_looking_downward(), only_current, only_body, hover_id, true, show_grid);
         if (m_design_canvas && show_bed)
             // Design tab: replace the plate's corner-origin grid with the origin-centred CAD grid.
@@ -2818,7 +2816,6 @@ void GLCanvas3D::_render_scene(const Camera& camera, const Size& cnv_size)
         _render_shadows(camera.get_view_matrix(), camera.get_projection_matrix());
         m_frame_profiler.mark("shadows");
         _render_objects(GLVolumeCollection::ERenderType::Opaque, !m_gizmos.is_running());
-        _render_below_plate_volumes();
         m_frame_profiler.mark("objects");
         _render_sla_slices();
         _render_selection();
@@ -8843,84 +8840,6 @@ void GLCanvas3D::_render_bed(const Transform3d& view_matrix, const Transform3d&
     m_bed.render(*this, view_matrix, projection_matrix, bottom, scale_factor, show_axes);
 }
 
-bool GLCanvas3D::_is_below_plate_view_active() const
-{
-    return dynamic_cast<const GLGizmoPainterBase*>(m_gizmos.get_current()) != nullptr && m_gizmos.is_hiding_instances();
-}
-
-// The painting gizmo clips the selected object at the plate, so the part under the plate is not drawn.
-// This pass draws that part of the selected instance again at 50% opacity, depth writes off.
-void GLCanvas3D::_render_below_plate_volumes()
-{
-    if (!_is_below_plate_view_active() || m_volumes.empty())
-        return;
-
-    GLShaderProgram* shader = wxGetApp().get_shader("gouraud");
-    if (shader == nullptr)
-        return;
-
-    const Camera& camera = wxGetApp().plater()->get_camera();
-    const Transform3d& view_matrix = camera.get_view_matrix();
-    const Transform3d& proj_matrix = camera.get_projection_matrix();
-    const int object_idx = m_selection.get_object_idx();
-    const int instance_idx = m_selection.get_instance_idx();
-
-    // Depth test stays on so whatever is already in the depth buffer hides the ghost; depth writes
-    // are off and back faces are culled, so each pixel blends only the near surface once.
-    const GLboolean cull_face = ::glIsEnabled(GL_CULL_FACE);
-    glsafe(::glEnable(GL_DEPTH_TEST));
-    glsafe(::glEnable(GL_CULL_FACE));
-    glsafe(::glCullFace(GL_BACK));
-    glsafe(::glDepthMask(GL_FALSE));
-    glsafe(::glEnable(GL_BLEND));
-    glsafe(::glBlendFunc(GL_SRC_ALPHA, GL_ONE_MINUS_SRC_ALPHA));
-
-    shader->start_using();
-    // Keep z <= SINKING_Z_THRESHOLD, the complement of the clip the painting gizmo applies.
-    shader->set_uniform("z_range", std::array<float, 2>{ -FLT_MAX, float(SINKING_Z_THRESHOLD) });
-    shader->set_uniform("clipping_plane", m_volumes.get_clipping_plane());
-    shader->set_uniform("use_color_clip_plane", false);
-    shader->set_uniform("is_outline", false);
-    shader->set_uniform("slope.actived", false);
-    shader->set_uniform("print_volume.type", -1);
-    shader->set_uniform("extruder_printable_heights", std::array<float, 3>{ 0.0f, 0.0f, 0.0f });
-    shader->set_uniform("shadow_intensity", 0.0f);
-
-    // simple_render() needs these to reach the plain surface; with ban_color_volume it ignores both.
-    ModelObjectPtrs&       model_objects   = wxGetApp().model().objects;
-    std::vector<ColorRGBA> extruder_colors = wxGetApp().plater()->get_extruders_colors();
-
-    for (GLVolume* volume : m_volumes.volumes) {
-        // is_active cannot gate this pass: the color painter marks the whole instance inactive and
-        // draws it in its own gizmo pass. The instance and print-part checks below are what pick
-        // this ghost out of the scene.
-        if (volume->object_idx() != object_idx || volume->instance_idx() != instance_idx ||
-            volume->is_modifier || volume->is_wipe_tower || volume->volume_idx() < 0)
-            continue;
-
-        volume->set_render_color();
-        ColorRGBA color = volume->render_color;
-        color.a(0.5f);
-        volume->model.set_color(color);
-
-        const Transform3d model_matrix = volume->world_matrix();
-        shader->set_uniform("view_model_matrix", view_matrix * model_matrix);
-        shader->set_uniform("projection_matrix", proj_matrix);
-        shader->set_uniform("volume_world_matrix", model_matrix);
-        const Matrix3d view_normal_matrix = view_matrix.matrix().block(0, 0, 3, 3) * model_matrix.matrix().block(0, 0, 3, 3).inverse().transpose();
-        shader->set_uniform("view_normal_matrix", view_normal_matrix);
-        // Painted facets would send render() through the MMU segmentation sub-models, which carry
-        // their own opaque filament colours; the ghost draws the plain surface so the alpha holds.
-        volume->simple_render(shader, model_objects, extruder_colors, false, true);
-    }
-    shader->stop_using();
-
-    glsafe(::glDisable(GL_BLEND));
-    glsafe(::glDepthMask(GL_TRUE));
-    if (cull_face == GL_FALSE)
-        glsafe(::glDisable(GL_CULL_FACE));
-}
-
 bool GLCanvas3D::_hides_inactive_plates() const
 {
     return m_canvas_type == ECanvasType::CanvasView3D && !m_design_canvas && wxGetApp().focus_mode();
@@ -12049,10 +11968,9 @@ bool GLCanvas3D::is_bed_visible() const
     if (!m_main_toolbar.is_enabled())
         return true;
 
-    // The painting gizmos clip the selected instance at the plate (see InstancesHider), so the bed
-    // mesh would sit in front of the part below it. They draw the plate and its grid instead (see
-    // _render_scene) and the below-plate part as a translucent ghost.
-    return !_is_below_plate_view_active();
+    const auto type = m_gizmos.get_current_type();
+    return type != GLGizmosManager::FdmSupports && type != GLGizmosManager::Seam &&
+        type != GLGizmosManager::MmSegmentation && type != GLGizmosManager::FuzzySkin;
 }
 
 Vec3d GLCanvas3D::get_camera_pan_anchor(Camera& camera, ECameraNavigationType navigation_type,

src/slic3r/GUI/GLCanvas3D.hpp

@@ -1552,8 +1552,6 @@ private:
     void _render_imex_ghost_tooltip();
     void _render_section_view_caps();
     void _render_wireframe_overlay();
-    bool _is_below_plate_view_active() const;
-    void _render_below_plate_volumes();
     bool _is_xray_view_active() const;
     void _render_xray_volumes();
     //BBS: GUI refactor: add canvas size as parameters

src/slic3r/GUI/Gizmos/GLGizmosCommon.cpp

@@ -7,6 +7,7 @@
 
 
 #include <cstddef>
+#include <glad/gl.h>
 #include "slic3r/GUI/Selection.hpp"
 #include <limits>
 #include "libslic3r/Model.hpp"
@@ -173,9 +174,6 @@ void InstancesHider::on_update()
                 m_clippers.emplace_back(new MeshClipper);
                 m_clippers.back()->set_plane(ClippingPlane(-Vec3d::UnitZ(), z_min));
                 m_clippers.back()->set_mesh(mesh->its);
-                // Outline only: a filled cap would read as a surface the object does not have, and it
-                // covers the model below it. 0.4 mm matches the cut gizmo's contour width.
-                m_clippers.back()->set_behaviour(false, 0.4);
             }
             m_old_meshes = meshes;
         }
@@ -216,7 +214,10 @@ void InstancesHider::render_cut() const
         else
             clipper->set_limiting_plane(ClippingPlane::ClipsNothing());
 
-        clipper->render_contour(mv->is_model_part() ? ColorRGBA(0.8f, 0.3f, 0.0f, 1.0f) : color_from_model_volume(*mv));
+        glsafe(::glPushAttrib(GL_DEPTH_TEST));
+        glsafe(::glDisable(GL_DEPTH_TEST));
+        clipper->render_cut(mv->is_model_part() ? ColorRGBA(0.8f, 0.3f, 0.0f, 1.0f) : color_from_model_volume(*mv));
+        glsafe(::glPopAttrib());
 
         ++clipper_id;
     }

tests/libslic3r/test_config.cpp

@@ -1263,6 +1263,55 @@ TEST_CASE("read_cli rejects an invalid scalar numeric value", "[Config]") {
     REQUIRE_FALSE(config.read_cli(3, argv, &extra, &keys));
 }
 
+TEST_CASE("A scalar option keeps its value when deserializing fails", "[Config]") {
+    using Catch::Matchers::WithinAbs;
+
+    SECTION("float") {
+        ConfigOptionFloat opt(7.5);
+        REQUIRE_FALSE(opt.deserialize("abc"));
+        CHECK_THAT(opt.value, WithinAbs(7.5, 1e-9));
+    }
+    SECTION("int") {
+        ConfigOptionInt opt(3);
+        REQUIRE_FALSE(opt.deserialize("abc"));
+        CHECK(opt.value == 3);
+    }
+    SECTION("percent") {
+        ConfigOptionPercent opt(40.);
+        REQUIRE_FALSE(opt.deserialize("abc%"));
+        CHECK_THAT(opt.value, WithinAbs(40., 1e-9));
+    }
+    SECTION("float or percent") {
+        ConfigOptionFloatOrPercent opt(0.4, false);
+        REQUIRE_FALSE(opt.deserialize("abc%"));
+        CHECK_THAT(opt.value, WithinAbs(0.4, 1e-9));
+        CHECK_FALSE(opt.percent);
+    }
+    SECTION("point") {
+        ConfigOptionPoint opt(Vec2d(1., 2.));
+        REQUIRE_FALSE(opt.deserialize("5,abc"));
+        CHECK(opt.value == Vec2d(1., 2.));
+    }
+    SECTION("point3") {
+        ConfigOptionPoint3 opt(Vec3d(1., 2., 3.));
+        REQUIRE_FALSE(opt.deserialize("5,6,abc"));
+        CHECK(opt.value == Vec3d(1., 2., 3.));
+    }
+}
+
+TEST_CASE("A scalar option still takes a valid value", "[Config]") {
+    using Catch::Matchers::WithinAbs;
+
+    ConfigOptionFloatOrPercent opt(0.4, false);
+    REQUIRE(opt.deserialize("50%"));
+    CHECK_THAT(opt.value, WithinAbs(50., 1e-9));
+    CHECK(opt.percent);
+
+    ConfigOptionPoint point;
+    REQUIRE(point.deserialize("3x4"));
+    CHECK(point.value == Vec2d(3., 4.));
+}
+
 TEST_CASE("read_cli appends values when a vector option is repeated", "[Config]") {
     Slic3r::DynamicPrintConfig config;
     t_config_option_keys extra, keys;