From e93c5d4f2057c7077e5b1a8225f12b7c350d8e4e Mon Sep 17 00:00:00 2001 From: Lukas Matena Date: Thu, 22 Jul 2021 15:49:00 +0200 Subject: [PATCH] ImGui sliders clamping: ImGuiWrapper::slider_float now clamps the value returned by imgui by default. Bare ImGui::SliderFloat allows entering off-scale values when entered by keyboard, which is not nice. The protection can be turned off by the last optional parameter. --- src/slic3r/GUI/GLCanvas3D.cpp | 2 +- src/slic3r/GUI/Gizmos/GLGizmoFdmSupports.cpp | 6 ++--- src/slic3r/GUI/Gizmos/GLGizmoHollow.cpp | 24 ++++++++++++------- .../GUI/Gizmos/GLGizmoMmuSegmentation.cpp | 6 ++--- src/slic3r/GUI/Gizmos/GLGizmoSeam.cpp | 7 ++---- src/slic3r/GUI/Gizmos/GLGizmoSlaSupports.cpp | 8 +++---- src/slic3r/GUI/ImGuiWrapper.cpp | 15 +++++++----- src/slic3r/GUI/ImGuiWrapper.hpp | 9 ++++--- 8 files changed, 41 insertions(+), 36 deletions(-) diff --git a/src/slic3r/GUI/GLCanvas3D.cpp b/src/slic3r/GUI/GLCanvas3D.cpp index bc4c49fcb..b72156723 100644 --- a/src/slic3r/GUI/GLCanvas3D.cpp +++ b/src/slic3r/GUI/GLCanvas3D.cpp @@ -254,7 +254,7 @@ void GLCanvas3D::LayersEditing::render_overlay(const GLCanvas3D& canvas) const float widget_align = ImGui::GetCursorPosX(); ImGui::PushItemWidth(imgui.get_style_scaling() * 120.0f); m_adaptive_quality = std::clamp(m_adaptive_quality, 0.0f, 1.f); - ImGui::SliderFloat("", &m_adaptive_quality, 0.0f, 1.f, "%.2f"); + imgui.slider_float("", &m_adaptive_quality, 0.0f, 1.f, "%.2f"); ImGui::Separator(); if (imgui.button(_L("Smooth"))) diff --git a/src/slic3r/GUI/Gizmos/GLGizmoFdmSupports.cpp b/src/slic3r/GUI/Gizmos/GLGizmoFdmSupports.cpp index 4c2d00920..f540e30aa 100644 --- a/src/slic3r/GUI/Gizmos/GLGizmoFdmSupports.cpp +++ b/src/slic3r/GUI/Gizmos/GLGizmoFdmSupports.cpp @@ -186,7 +186,7 @@ void GLGizmoFdmSupports::on_render_input_window(float x, float y, float bottom_l m_imgui->text(m_desc.at("cursor_size")); ImGui::SameLine(cursor_slider_left); ImGui::PushItemWidth(window_width - cursor_slider_left); - ImGui::SliderFloat(" ", &m_cursor_radius, CursorRadiusMin, CursorRadiusMax, "%.2f"); + m_imgui->slider_float(" ", &m_cursor_radius, CursorRadiusMin, CursorRadiusMax, "%.2f"); if (ImGui::IsItemHovered()) { ImGui::BeginTooltip(); ImGui::PushTextWrapPos(max_tooltip_width); @@ -194,8 +194,6 @@ void GLGizmoFdmSupports::on_render_input_window(float x, float y, float bottom_l ImGui::PopTextWrapPos(); ImGui::EndTooltip(); } - // Manually inserted values aren't clamped by ImGui. Zero cursor size results in a crash. - m_cursor_radius = std::clamp(m_cursor_radius, CursorRadiusMin, CursorRadiusMax); ImGui::AlignTextToFramePadding(); @@ -252,7 +250,7 @@ void GLGizmoFdmSupports::on_render_input_window(float x, float y, float bottom_l ImGui::SameLine(clipping_slider_left); ImGui::PushItemWidth(window_width - clipping_slider_left); auto clp_dist = float(m_c->object_clipper()->get_position()); - if (ImGui::SliderFloat(" ", &clp_dist, 0.f, 1.f, "%.2f")) + if (m_imgui->slider_float(" ", &clp_dist, 0.f, 1.f, "%.2f")) m_c->object_clipper()->set_position(clp_dist, true); if (ImGui::IsItemHovered()) { diff --git a/src/slic3r/GUI/Gizmos/GLGizmoHollow.cpp b/src/slic3r/GUI/Gizmos/GLGizmoHollow.cpp index 4337290dc..9d2de1d03 100644 --- a/src/slic3r/GUI/Gizmos/GLGizmoHollow.cpp +++ b/src/slic3r/GUI/Gizmos/GLGizmoHollow.cpp @@ -545,7 +545,7 @@ RENDER_AGAIN: m_imgui->text(m_desc.at("offset")); ImGui::SameLine(settings_sliders_left); ImGui::PushItemWidth(window_width - settings_sliders_left); - ImGui::SliderFloat(" ", &offset, offset_min, offset_max, "%.1f mm"); + m_imgui->slider_float(" ", &offset, offset_min, offset_max, "%.1f mm"); if (ImGui::IsItemHovered()) { ImGui::BeginTooltip(); ImGui::PushTextWrapPos(max_tooltip_width); @@ -560,7 +560,7 @@ RENDER_AGAIN: if (current_mode >= quality_mode) { m_imgui->text(m_desc.at("quality")); ImGui::SameLine(settings_sliders_left); - ImGui::SliderFloat(" ", &quality, quality_min, quality_max, "%.1f"); + m_imgui->slider_float(" ", &quality, quality_min, quality_max, "%.1f"); if (ImGui::IsItemHovered()) { ImGui::BeginTooltip(); ImGui::PushTextWrapPos(max_tooltip_width); @@ -576,7 +576,7 @@ RENDER_AGAIN: if (current_mode >= closing_d_mode) { m_imgui->text(m_desc.at("closing_distance")); ImGui::SameLine(settings_sliders_left); - ImGui::SliderFloat(" ", &closing_d, closing_d_min, closing_d_max, "%.1f mm"); + m_imgui->slider_float(" ", &closing_d, closing_d_min, closing_d_max, "%.1f mm"); if (ImGui::IsItemHovered()) { ImGui::BeginTooltip(); ImGui::PushTextWrapPos(max_tooltip_width); @@ -618,15 +618,19 @@ RENDER_AGAIN: ImGui::Separator(); - float diameter_upper_cap = 15.; - if (m_new_hole_radius > diameter_upper_cap) - m_new_hole_radius = diameter_upper_cap; + float diameter_upper_cap = 60.; + if (m_new_hole_radius * 2.f > diameter_upper_cap) + m_new_hole_radius = diameter_upper_cap / 2.f; m_imgui->text(m_desc.at("hole_diameter")); ImGui::SameLine(diameter_slider_left); ImGui::PushItemWidth(window_width - diameter_slider_left); float diam = 2.f * m_new_hole_radius; - ImGui::SliderFloat("", &diam, 1.f, diameter_upper_cap, "%.1f mm"); + m_imgui->slider_float("", &diam, 1.f, 15.f, "%.1f mm", 1.f, false); + // Let's clamp the value (which could have been entered by keyboard) to a larger range + // than the slider. This allows entering off-scale values and still protects against + //complete non-sense. + diam = std::clamp(diam, 0.1f, diameter_upper_cap); m_new_hole_radius = diam / 2.f; bool clicked = ImGui::IsItemClicked(); bool edited = ImGui::IsItemEdited(); @@ -634,7 +638,9 @@ RENDER_AGAIN: m_imgui->text(m_desc["hole_depth"]); ImGui::SameLine(diameter_slider_left); - ImGui::SliderFloat(" ", &m_new_hole_height, 0.f, 10.f, "%.1f mm"); + m_imgui->slider_float(" ", &m_new_hole_height, 0.f, 10.f, "%.1f mm", 1.f, false); + // Same as above: + m_new_hole_height = std::clamp(m_new_hole_height, 0.f, 100.f); clicked |= ImGui::IsItemClicked(); edited |= ImGui::IsItemEdited(); @@ -699,7 +705,7 @@ RENDER_AGAIN: ImGui::SameLine(clipping_slider_left); ImGui::PushItemWidth(window_width - clipping_slider_left); float clp_dist = m_c->object_clipper()->get_position(); - if (ImGui::SliderFloat(" ", &clp_dist, 0.f, 1.f, "%.2f")) + if (m_imgui->slider_float(" ", &clp_dist, 0.f, 1.f, "%.2f")) m_c->object_clipper()->set_position(clp_dist, true); // make sure supports are shown/hidden as appropriate diff --git a/src/slic3r/GUI/Gizmos/GLGizmoMmuSegmentation.cpp b/src/slic3r/GUI/Gizmos/GLGizmoMmuSegmentation.cpp index 90cfb54a6..a35d8c071 100644 --- a/src/slic3r/GUI/Gizmos/GLGizmoMmuSegmentation.cpp +++ b/src/slic3r/GUI/Gizmos/GLGizmoMmuSegmentation.cpp @@ -425,7 +425,7 @@ void GLGizmoMmuSegmentation::on_render_input_window(float x, float y, float bott m_imgui->text(m_desc.at("cursor_size")); ImGui::SameLine(sliders_width); ImGui::PushItemWidth(window_width - sliders_width); - ImGui::SliderFloat(" ", &m_cursor_radius, CursorRadiusMin, CursorRadiusMax, "%.2f"); + m_imgui->slider_float(" ", &m_cursor_radius, CursorRadiusMin, CursorRadiusMax, "%.2f"); if (ImGui::IsItemHovered()) { ImGui::BeginTooltip(); ImGui::PushTextWrapPos(max_tooltip_width); @@ -433,8 +433,6 @@ void GLGizmoMmuSegmentation::on_render_input_window(float x, float y, float bott ImGui::PopTextWrapPos(); ImGui::EndTooltip(); } - // Manually inserted values aren't clamped by ImGui. Zero cursor size results in a crash. - m_cursor_radius = std::clamp(m_cursor_radius, CursorRadiusMin, CursorRadiusMax); m_imgui->checkbox(_L("Split triangles"), m_triangle_splitting_enabled); @@ -480,7 +478,7 @@ void GLGizmoMmuSegmentation::on_render_input_window(float x, float y, float bott ImGui::SameLine(sliders_width); ImGui::PushItemWidth(window_width - sliders_width); auto clp_dist = float(m_c->object_clipper()->get_position()); - if (ImGui::SliderFloat(" ", &clp_dist, 0.f, 1.f, "%.2f")) + if (m_imgui->slider_float(" ", &clp_dist, 0.f, 1.f, "%.2f")) m_c->object_clipper()->set_position(clp_dist, true); if (ImGui::IsItemHovered()) { ImGui::BeginTooltip(); diff --git a/src/slic3r/GUI/Gizmos/GLGizmoSeam.cpp b/src/slic3r/GUI/Gizmos/GLGizmoSeam.cpp index 9ceb220d4..5f0c6b52d 100644 --- a/src/slic3r/GUI/Gizmos/GLGizmoSeam.cpp +++ b/src/slic3r/GUI/Gizmos/GLGizmoSeam.cpp @@ -140,7 +140,7 @@ void GLGizmoSeam::on_render_input_window(float x, float y, float bottom_limit) m_imgui->text(m_desc.at("cursor_size")); ImGui::SameLine(cursor_size_slider_left); ImGui::PushItemWidth(window_width - cursor_size_slider_left); - ImGui::SliderFloat(" ", &m_cursor_radius, CursorRadiusMin, CursorRadiusMax, "%.2f"); + m_imgui->slider_float(" ", &m_cursor_radius, CursorRadiusMin, CursorRadiusMax, "%.2f"); if (ImGui::IsItemHovered()) { ImGui::BeginTooltip(); ImGui::PushTextWrapPos(max_tooltip_width); @@ -148,9 +148,6 @@ void GLGizmoSeam::on_render_input_window(float x, float y, float bottom_limit) ImGui::PopTextWrapPos(); ImGui::EndTooltip(); } - // Manually inserted values aren't clamped by ImGui. Zero cursor size results in a crash. - m_cursor_radius = std::clamp(m_cursor_radius, CursorRadiusMin, CursorRadiusMax); - ImGui::AlignTextToFramePadding(); m_imgui->text(m_desc.at("cursor_type")); @@ -203,7 +200,7 @@ void GLGizmoSeam::on_render_input_window(float x, float y, float bottom_limit) ImGui::SameLine(clipping_slider_left); ImGui::PushItemWidth(window_width - clipping_slider_left); auto clp_dist = float(m_c->object_clipper()->get_position()); - if (ImGui::SliderFloat(" ", &clp_dist, 0.f, 1.f, "%.2f")) + if (m_imgui->slider_float(" ", &clp_dist, 0.f, 1.f, "%.2f")) m_c->object_clipper()->set_position(clp_dist, true); if (ImGui::IsItemHovered()) { diff --git a/src/slic3r/GUI/Gizmos/GLGizmoSlaSupports.cpp b/src/slic3r/GUI/Gizmos/GLGizmoSlaSupports.cpp index 20306f182..f28f060bd 100644 --- a/src/slic3r/GUI/Gizmos/GLGizmoSlaSupports.cpp +++ b/src/slic3r/GUI/Gizmos/GLGizmoSlaSupports.cpp @@ -671,7 +671,7 @@ RENDER_AGAIN: // - keep updating the head radius during sliding so it is continuosly refreshed in 3D scene // - take correct undo/redo snapshot after the user is done with moving the slider float initial_value = m_new_point_head_diameter; - ImGui::SliderFloat("", &m_new_point_head_diameter, 0.1f, diameter_upper_cap, "%.1f"); + m_imgui->slider_float("", &m_new_point_head_diameter, 0.1f, diameter_upper_cap, "%.1f"); if (ImGui::IsItemClicked()) { if (m_old_point_head_diameter == 0.f) m_old_point_head_diameter = initial_value; @@ -731,7 +731,7 @@ RENDER_AGAIN: float density = static_cast(opts[0])->value; float minimal_point_distance = static_cast(opts[1])->value; - ImGui::SliderFloat("", &minimal_point_distance, 0.f, 20.f, "%.f mm"); + m_imgui->slider_float("", &minimal_point_distance, 0.f, 20.f, "%.f mm"); bool slider_clicked = ImGui::IsItemClicked(); // someone clicked the slider bool slider_edited = ImGui::IsItemEdited(); // someone is dragging the slider bool slider_released = ImGui::IsItemDeactivatedAfterEdit(); // someone has just released the slider @@ -740,7 +740,7 @@ RENDER_AGAIN: m_imgui->text(m_desc.at("points_density")); ImGui::SameLine(settings_sliders_left); - ImGui::SliderFloat(" ", &density, 0.f, 200.f, "%.f %%"); + m_imgui->slider_float(" ", &density, 0.f, 200.f, "%.f %%"); slider_clicked |= ImGui::IsItemClicked(); slider_edited |= ImGui::IsItemEdited(); slider_released |= ImGui::IsItemDeactivatedAfterEdit(); @@ -801,7 +801,7 @@ RENDER_AGAIN: ImGui::SameLine(clipping_slider_left); ImGui::PushItemWidth(window_width - clipping_slider_left); float clp_dist = m_c->object_clipper()->get_position(); - if (ImGui::SliderFloat(" ", &clp_dist, 0.f, 1.f, "%.2f")) + if (m_imgui->slider_float(" ", &clp_dist, 0.f, 1.f, "%.2f")) m_c->object_clipper()->set_position(clp_dist, true); diff --git a/src/slic3r/GUI/ImGuiWrapper.cpp b/src/slic3r/GUI/ImGuiWrapper.cpp index 9980ba5a7..c33ac3c0e 100644 --- a/src/slic3r/GUI/ImGuiWrapper.cpp +++ b/src/slic3r/GUI/ImGuiWrapper.cpp @@ -406,20 +406,23 @@ void ImGuiWrapper::text_colored(const ImVec4& color, const wxString& label) this->text_colored(color, label_utf8.c_str()); } -bool ImGuiWrapper::slider_float(const char* label, float* v, float v_min, float v_max, const char* format/* = "%.3f"*/, float power/* = 1.0f*/) +bool ImGuiWrapper::slider_float(const char* label, float* v, float v_min, float v_max, const char* format/* = "%.3f"*/, float power/* = 1.0f*/, bool clamp /*= true*/) { - return ImGui::SliderFloat(label, v, v_min, v_max, format, power); + bool ret = ImGui::SliderFloat(label, v, v_min, v_max, format, power); + if (clamp) + *v = std::clamp(*v, v_min, v_max); + return ret; } -bool ImGuiWrapper::slider_float(const std::string& label, float* v, float v_min, float v_max, const char* format/* = "%.3f"*/, float power/* = 1.0f*/) +bool ImGuiWrapper::slider_float(const std::string& label, float* v, float v_min, float v_max, const char* format/* = "%.3f"*/, float power/* = 1.0f*/, bool clamp /*= true*/) { - return this->slider_float(label.c_str(), v, v_min, v_max, format, power); + return this->slider_float(label.c_str(), v, v_min, v_max, format, power, clamp); } -bool ImGuiWrapper::slider_float(const wxString& label, float* v, float v_min, float v_max, const char* format/* = "%.3f"*/, float power/* = 1.0f*/) +bool ImGuiWrapper::slider_float(const wxString& label, float* v, float v_min, float v_max, const char* format/* = "%.3f"*/, float power/* = 1.0f*/, bool clamp /*= true*/) { auto label_utf8 = into_u8(label); - return this->slider_float(label_utf8.c_str(), v, v_min, v_max, format, power); + return this->slider_float(label_utf8.c_str(), v, v_min, v_max, format, power, clamp); } bool ImGuiWrapper::combo(const wxString& label, const std::vector& options, int& selection) diff --git a/src/slic3r/GUI/ImGuiWrapper.hpp b/src/slic3r/GUI/ImGuiWrapper.hpp index 5484e46c6..441d26ccc 100644 --- a/src/slic3r/GUI/ImGuiWrapper.hpp +++ b/src/slic3r/GUI/ImGuiWrapper.hpp @@ -79,9 +79,12 @@ public: void text_colored(const ImVec4& color, const char* label); void text_colored(const ImVec4& color, const std::string& label); void text_colored(const ImVec4& color, const wxString& label); - bool slider_float(const char* label, float* v, float v_min, float v_max, const char* format = "%.3f", float power = 1.0f); - bool slider_float(const std::string& label, float* v, float v_min, float v_max, const char* format = "%.3f", float power = 1.0f); - bool slider_float(const wxString& label, float* v, float v_min, float v_max, const char* format = "%.3f", float power = 1.0f); + + // Float sliders: Manually inserted values aren't clamped by ImGui.Using this wrapper function does (when clamp==true). + bool slider_float(const char* label, float* v, float v_min, float v_max, const char* format = "%.3f", float power = 1.0f, bool clamp = true); + bool slider_float(const std::string& label, float* v, float v_min, float v_max, const char* format = "%.3f", float power = 1.0f, bool clamp = true); + bool slider_float(const wxString& label, float* v, float v_min, float v_max, const char* format = "%.3f", float power = 1.0f, bool clamp = true); + bool combo(const wxString& label, const std::vector& options, int& selection); // Use -1 to not mark any option as selected bool undo_redo_list(const ImVec2& size, const bool is_undo, bool (*items_getter)(const bool, int, const char**), int& hovered, int& selected, int& mouse_wheel); void search_list(const ImVec2& size, bool (*items_getter)(int, const char** label, const char** tooltip), char* search_str,