Fix: Correct range checking for int and float Config Options + QoL changes in tooltips (#11915)

* Fix float number not working properly for option min/max (#11211)

* ConfigOptionDef: min/max values type are changed from INT to FLOAT.

(cherry picked from commit f277bc80c22e0c9a067481a4301922e2c96aed47)

* Fix infinite loop and crash when `fuzzy_skin_point_distance` = 0 (SoftFever/OrcaSlicer#11069)

* Fix Linux build issue

* Fix float comparison due to precision loss

* Fix: Range check added for coInt options; Ranges and defaults added in tooltips

---------

Co-authored-by: Noisyfox <timemanager.rick@gmail.com>
Co-authored-by: SoftFever <softfeverever@gmail.com>
This commit is contained in:
Valerii Bokhan
2026-02-23 06:06:07 +01:00
committed by GitHub
parent 5e6a8e4daf
commit 4343ddf823
7 changed files with 128 additions and 49 deletions

View File

@@ -318,6 +318,24 @@ ConfigOption* ConfigOptionDef::create_default_option() const
return this->create_empty_option(); return this->create_empty_option();
} }
bool ConfigOptionDef::is_value_valid(const double value, const int max_precision /*= 4*/) const
{
// Special handling for the nil values
// The nil value is a valid one only for nullable options
if (std::isnan(value))
return this->nullable;
// Special handling of 0
if (this->min == 0.f && value < 0)
return false;
const double ep = std::pow(0.1, max_precision);
if (is_approx(value, (double) this->min, ep) || is_approx(value, (double) this->max, ep))
return true;
return this->min <= value && value <= this->max;
}
// Assignment of the serialization IDs is not thread safe. The Defs shall be initialized from the main thread! // Assignment of the serialization IDs is not thread safe. The Defs shall be initialized from the main thread!
ConfigOptionDef* ConfigDef::add(const t_config_option_key &opt_key, ConfigOptionType type) ConfigOptionDef* ConfigDef::add(const t_config_option_key &opt_key, ConfigOptionType type)
{ {

View File

@@ -4,6 +4,7 @@
#include <assert.h> #include <assert.h>
#include <map> #include <map>
#include <climits> #include <climits>
#include <cfloat>
#include <cstdio> #include <cstdio>
#include <cstdlib> #include <cstdlib>
#include <functional> #include <functional>
@@ -2455,10 +2456,11 @@ public:
// Optional width of an input field. // Optional width of an input field.
int width = -1; int width = -1;
// <min, max> limit of a numeric input. // <min, max> limit of a numeric input.
// If not set, the <min, max> is set to <INT_MIN, INT_MAX> // If not set, the <min, max> is set to <-FLT_MAX, FLT_MAX>
// By setting min=0, only nonnegative input is allowed. // By setting min=0, only nonnegative input is allowed.
int min = INT_MIN; float min = -FLT_MAX;
int max = INT_MAX; float max = FLT_MAX;
bool is_value_valid(const double value, const int max_precision = 4) const;
// To check if it's not a typo and a % is missing // To check if it's not a typo and a % is missing
double max_literal = 1; double max_literal = 1;
ConfigOptionMode mode = comSimple; ConfigOptionMode mode = comSimple;

View File

@@ -2159,9 +2159,9 @@ void PrintConfigDef::init_fff_params()
"You may be able to tune this value to get a nice flat surface if there is slight overflow or underflow." "You may be able to tune this value to get a nice flat surface if there is slight overflow or underflow."
"\n\nThe final object flow ratio is this value multiplied by the filament flow ratio."); "\n\nThe final object flow ratio is this value multiplied by the filament flow ratio.");
def->mode = comAdvanced; def->mode = comAdvanced;
def->max = 2; def->max = 2.f;
def->min = 0.01; def->min = 0.01f;
def->set_default_value(new ConfigOptionFloat(1)); def->set_default_value(new ConfigOptionFloat(1.f));
def = this->add("enable_pressure_advance", coBools); def = this->add("enable_pressure_advance", coBools);
def->label = L("Enable pressure advance"); def->label = L("Enable pressure advance");
@@ -3051,9 +3051,10 @@ void PrintConfigDef::init_fff_params()
def->category = L("Speed"); def->category = L("Speed");
def->tooltip = L("Marlin Firmware Junction Deviation (replaces the traditional XY Jerk setting)."); def->tooltip = L("Marlin Firmware Junction Deviation (replaces the traditional XY Jerk setting).");
def->sidetext = L("mm"); // milimeters, CIS languages need translation def->sidetext = L("mm"); // milimeters, CIS languages need translation
def->min = 0; def->min = 0.f;
def->max = 0.5f;
def->mode = comAdvanced; def->mode = comAdvanced;
def->set_default_value(new ConfigOptionFloat(0)); def->set_default_value(new ConfigOptionFloat(0.f));
def = this->add("outer_wall_jerk", coFloat); def = this->add("outer_wall_jerk", coFloat);
def->label = L("Outer wall"); def->label = L("Outer wall");
@@ -3312,10 +3313,10 @@ void PrintConfigDef::init_fff_params()
def->category = L("Others"); def->category = L("Others");
def->tooltip = L("The average distance between the random points introduced on each line segment."); def->tooltip = L("The average distance between the random points introduced on each line segment.");
def->sidetext = L("mm"); // milimeters, CIS languages need translation def->sidetext = L("mm"); // milimeters, CIS languages need translation
def->min = 0; def->min = 0.01f; // point distance cannot be 0! Otherwise we get infinite loop + OOM due to infinite line division.
def->max = 5; def->max = 5.f;
def->mode = comSimple; def->mode = comSimple;
def->set_default_value(new ConfigOptionFloat(0.3)); def->set_default_value(new ConfigOptionFloat(0.3f));
def = this->add("fuzzy_skin_first_layer", coBool); def = this->add("fuzzy_skin_first_layer", coBool);
def->label = L("Apply fuzzy skin to first layer"); def->label = L("Apply fuzzy skin to first layer");
@@ -3375,7 +3376,7 @@ void PrintConfigDef::init_fff_params()
def->category = L("Others"); def->category = L("Others");
def->tooltip = L("The base size of the coherent noise features, in mm. Higher values will result in larger features."); def->tooltip = L("The base size of the coherent noise features, in mm. Higher values will result in larger features.");
def->sidetext = L("mm"); // milimeters, CIS languages need translation def->sidetext = L("mm"); // milimeters, CIS languages need translation
def->min = 0.1; def->min = 0.1f;
def->max = 500; def->max = 500;
def->mode = comAdvanced; def->mode = comAdvanced;
def->set_default_value(new ConfigOptionFloat(1.0)); def->set_default_value(new ConfigOptionFloat(1.0));
@@ -3393,7 +3394,7 @@ void PrintConfigDef::init_fff_params()
def->label = L("Fuzzy skin noise persistence"); def->label = L("Fuzzy skin noise persistence");
def->category = L("Others"); def->category = L("Others");
def->tooltip = L("The decay rate for higher octaves of the coherent noise. Lower values will result in smoother noise."); def->tooltip = L("The decay rate for higher octaves of the coherent noise. Lower values will result in smoother noise.");
def->min = 0.01; def->min = 0.01f;
def->max = 1; def->max = 1;
def->mode = comAdvanced; def->mode = comAdvanced;
def->set_default_value(new ConfigOptionFloat(0.5)); def->set_default_value(new ConfigOptionFloat(0.5));
@@ -4205,10 +4206,10 @@ void PrintConfigDef::init_fff_params()
def->category = L("Machine limits"); def->category = L("Machine limits");
def->tooltip = L("Maximum junction deviation (M205 J, only apply if JD > 0 for Marlin Firmware\nIf your Marlin 2 printer uses Classic Jerk set this value to 0.)"); def->tooltip = L("Maximum junction deviation (M205 J, only apply if JD > 0 for Marlin Firmware\nIf your Marlin 2 printer uses Classic Jerk set this value to 0.)");
def->sidetext = L("mm"); // milimeters, CIS languages need translation def->sidetext = L("mm"); // milimeters, CIS languages need translation
def->min = 0; def->min = 0.f;
def->max = 1; def->max = 0.5f;
def->mode = comAdvanced; def->mode = comAdvanced;
def->set_default_value(new ConfigOptionFloats { 0.01}); def->set_default_value(new ConfigOptionFloats{ 0.01f });
// M205 S... [mm/sec] // M205 S... [mm/sec]
def = this->add("machine_min_extruding_rate", coFloats); def = this->add("machine_min_extruding_rate", coFloats);
@@ -9767,13 +9768,13 @@ std::map<std::string, std::string> validate(const FullPrintConfig &cfg, bool und
case coFloatOrPercent: case coFloatOrPercent:
{ {
auto *fopt = static_cast<const ConfigOptionFloat*>(opt); auto *fopt = static_cast<const ConfigOptionFloat*>(opt);
out_of_range = fopt->value < optdef->min || fopt->value > optdef->max; out_of_range = !optdef->is_value_valid(fopt->value);
break; break;
} }
case coFloats: case coFloats:
case coPercents: case coPercents:
for (double v : static_cast<const ConfigOptionVector<double>*>(opt)->values) for (double v : static_cast<const ConfigOptionVector<double>*>(opt)->values)
if (v < optdef->min || v > optdef->max) { if (!optdef->is_value_valid(v)) {
out_of_range = true; out_of_range = true;
break; break;
} }
@@ -9781,12 +9782,12 @@ std::map<std::string, std::string> validate(const FullPrintConfig &cfg, bool und
case coInt: case coInt:
{ {
auto *iopt = static_cast<const ConfigOptionInt*>(opt); auto *iopt = static_cast<const ConfigOptionInt*>(opt);
out_of_range = iopt->value < optdef->min || iopt->value > optdef->max; out_of_range = !optdef->is_value_valid(iopt->value);
break; break;
} }
case coInts: case coInts:
for (int v : static_cast<const ConfigOptionVector<int>*>(opt)->values) for (int v : static_cast<const ConfigOptionVector<int>*>(opt)->values)
if (v < optdef->min || v > optdef->max) { if (!optdef->is_value_valid(v)) {
out_of_range = true; out_of_range = true;
break; break;
} }

View File

@@ -101,6 +101,45 @@ ThumbnailErrors validate_thumbnails_string(wxString& str, const wxString& def_ex
return errors; return errors;
} }
wxString get_formatted_tooltip_text(const ConfigOptionDef& opt, const t_config_option_key& id)
{
wxString tooltip = _(opt.tooltip);
if (tooltip.length() > 0) {
edit_tooltip(tooltip);
std::string opt_id = id;
auto hash_pos = opt_id.find("#");
if (hash_pos != std::string::npos) {
opt_id.replace(hash_pos, 1,"[");
opt_id += "]";
}
tooltip += "\n\n" + _(L("parameter name")) + ": " + opt_id;
if (opt.type == coFloat || opt.type == coInt) {
double default_value = 0.;
if (opt.type == coFloat)
default_value = opt.get_default_value<ConfigOptionFloat>()->value;
else if (opt.type == coInt)
default_value = opt.get_default_value<ConfigOptionInt>()->value;
tooltip += "\n\n" + _(L("Default")) + ": " + _(double_to_string(default_value));
if (opt.min > -FLT_MAX && opt.max < FLT_MAX) {
tooltip += "\n" + _(L("Range")) + ": [" +
_(double_to_string(opt.min)) + ", " +
_(double_to_string(opt.max)) + "]";
}
}
return tooltip;
}
return "";
}
Field::~Field() Field::~Field()
{ {
if (m_on_kill_focus) if (m_on_kill_focus)
@@ -223,23 +262,9 @@ void Field::toggle(bool en) { en && !m_opt.readonly ? enable() : disable(); }
wxString Field::get_tooltip_text(const wxString &default_string) wxString Field::get_tooltip_text(const wxString &default_string)
{ {
wxString tooltip_text(""); wxString tooltip_text = get_formatted_tooltip_text(m_opt, m_opt_id);
#ifdef NDEBUG
wxString tooltip = _(m_opt.tooltip);
::edit_tooltip(tooltip);
std::string opt_id = m_opt_id; return tooltip_text.length() > 0 ? tooltip_text : default_string;
auto hash_pos = opt_id.find("#");
if (hash_pos != std::string::npos) {
opt_id.replace(hash_pos, 1,"[");
opt_id += "]";
}
if (tooltip.length() > 0)
tooltip_text = tooltip + "\n" +
_(L("parameter name")) + "\t: " + opt_id;
#endif
return tooltip_text;
} }
bool Field::is_matched(const std::string& string, const std::string& pattern) bool Field::is_matched(const std::string& string, const std::string& pattern)
@@ -251,9 +276,15 @@ bool Field::is_matched(const std::string& string, const std::string& pattern)
void Field::get_value_by_opt_type(wxString& str, const bool check_value/* = true*/) void Field::get_value_by_opt_type(wxString& str, const bool check_value/* = true*/)
{ {
switch (m_opt.type) { switch (m_opt.type) {
case coInts:
case coInt: { case coInt: {
long val = 0; long val = 0;
if (!str.ToLong(&val)) {
bool is_na_value = m_opt.nullable && str == m_na_value;
if (is_na_value)
val = ConfigOptionIntsNullable::nil_value();
else if (!str.ToLong(&val)) {
if (!check_value) { if (!check_value) {
m_value.clear(); m_value.clear();
break; break;
@@ -261,6 +292,27 @@ void Field::get_value_by_opt_type(wxString& str, const bool check_value/* = true
show_error(m_parent, _(L("Invalid numeric."))); show_error(m_parent, _(L("Invalid numeric.")));
set_value(int(val), true); set_value(int(val), true);
} }
if (!m_opt.is_value_valid(double(val))) {
if (!check_value) {
m_value.clear();
break;
}
if (!is_na_value) {
// Orca: no need to check ranges for the nil value
show_error(m_parent, _L("Value is out of range."));
int min = static_cast<int>(m_opt.min);
int max = static_cast<int>(m_opt.max);
if (min > val) val = min;
if (val > max) val = max;
set_value(int(val), true);
}
}
m_value = int(val); m_value = int(val);
break; break;
} }
@@ -309,7 +361,7 @@ void Field::get_value_by_opt_type(wxString& str, const bool check_value/* = true
show_error(m_parent, _(L("Invalid numeric."))); show_error(m_parent, _(L("Invalid numeric.")));
set_value(double_to_string(val), true); set_value(double_to_string(val), true);
} }
if (m_opt.min > val || val > m_opt.max) if (!m_opt.is_value_valid(val))
{ {
if (!check_value) { if (!check_value) {
m_value.clear(); m_value.clear();
@@ -347,7 +399,8 @@ void Field::get_value_by_opt_type(wxString& str, const bool check_value/* = true
} }
} }
} }
else { else if (!is_na_value) {
// Orca: no need to check ranges for the nil value
show_error(m_parent, _L("Value is out of range.")); show_error(m_parent, _L("Value is out of range."));
if (m_opt.min > val) val = m_opt.min; if (m_opt.min > val) val = m_opt.min;
if (val > m_opt.max) val = m_opt.max; if (val > m_opt.max) val = m_opt.max;
@@ -1088,8 +1141,8 @@ void SpinCtrl::BUILD() {
break; break;
} }
const int min_val = m_opt.min == INT_MIN ? 0 : m_opt.min; const int min_val = m_opt.min == -FLT_MAX ? 0 : (int)m_opt.min;
const int max_val = m_opt.max < 2147483647 ? m_opt.max : 2147483647; const int max_val = m_opt.max < FLT_MAX ? (int)m_opt.max : INT_MAX;
static Builder<SpinInput> builder; static Builder<SpinInput> builder;
auto temp = builder.build(m_parent, "", "", wxDefaultPosition, size, auto temp = builder.build(m_parent, "", "", wxDefaultPosition, size,
@@ -1145,7 +1198,7 @@ void SpinCtrl::BUILD() {
if (!parsed || value < INT_MIN || value > INT_MAX) if (!parsed || value < INT_MIN || value > INT_MAX)
tmp_value = UNDEF_VALUE; tmp_value = UNDEF_VALUE;
else { else {
tmp_value = std::min(std::max((int)value, m_opt.min), m_opt.max); tmp_value = std::min(std::max((int)value, temp->GetMin()), temp->GetMax());
#ifdef __WXOSX__ #ifdef __WXOSX__
#ifdef UNDEFINED__WXOSX__ // BBS #ifdef UNDEFINED__WXOSX__ // BBS
// Forcibly set the input value for SpinControl, since the value // Forcibly set the input value for SpinControl, since the value
@@ -1198,7 +1251,7 @@ void SpinCtrl::set_value(const boost::any& value, bool change_event) {
m_disable_change_event = !change_event; m_disable_change_event = !change_event;
m_value = value; m_value = value;
if (value.empty()) { // BBS: null value if (value.empty()) { // BBS: null value
dynamic_cast<SpinInput*>(window)->SetValue(m_opt.min); dynamic_cast<SpinInput*>(window)->SetValue(dynamic_cast<SpinInput*>(window)->GetMin());
dynamic_cast<SpinInput*>(window)->GetTextCtrl()->SetValue(""); dynamic_cast<SpinInput*>(window)->GetTextCtrl()->SetValue("");
} }
else { else {
@@ -2139,8 +2192,8 @@ boost::any& PointCtrl::get_value()
show_error(m_parent, _L("Invalid numeric.")); show_error(m_parent, _L("Invalid numeric."));
} }
else else
if (m_opt.min > x || x > m_opt.max || if (!m_opt.is_value_valid(x) ||
m_opt.min > y || y > m_opt.max) !m_opt.is_value_valid(y))
{ {
if (m_opt.min > x) x = m_opt.min; if (m_opt.min > x) x = m_opt.min;
if (x > m_opt.max) x = m_opt.max; if (x > m_opt.max) x = m_opt.max;
@@ -2199,8 +2252,8 @@ void SliderCtrl::BUILD()
auto temp = new wxBoxSizer(wxHORIZONTAL); auto temp = new wxBoxSizer(wxHORIZONTAL);
auto def_val = m_opt.get_default_value<ConfigOptionInt>()->value; auto def_val = m_opt.get_default_value<ConfigOptionInt>()->value;
auto min = m_opt.min == INT_MIN ? 0 : m_opt.min; auto min = m_opt.min == -FLT_MAX ? 0 : (int)m_opt.min;
auto max = m_opt.max == INT_MAX ? 100 : m_opt.max; auto max = m_opt.max == FLT_MAX ? 100 : INT_MAX;
m_slider = new wxSlider(m_parent, wxID_ANY, def_val * m_scale, m_slider = new wxSlider(m_parent, wxID_ANY, def_val * m_scale,
min * m_scale, max * m_scale, min * m_scale, max * m_scale,

View File

@@ -43,6 +43,7 @@ using t_back_to_init = std::function<void(const std::string&)>;
wxString double_to_string(double const value, const int max_precision = 4); wxString double_to_string(double const value, const int max_precision = 4);
wxString get_thumbnail_string(const Vec2d& value); wxString get_thumbnail_string(const Vec2d& value);
wxString get_thumbnails_string(const std::vector<Vec2d>& values); wxString get_thumbnails_string(const std::vector<Vec2d>& values);
wxString get_formatted_tooltip_text(const ConfigOptionDef& opt, const t_config_option_key& id);
class UndoValueUIManager class UndoValueUIManager
{ {

View File

@@ -594,11 +594,12 @@ void OptionsGroup::clear(bool destroy_custom_ctrl)
Line OptionsGroup::create_single_option_line(const Option& option, const std::string& path/* = std::string()*/) const Line OptionsGroup::create_single_option_line(const Option& option, const std::string& path/* = std::string()*/) const
{ {
wxString tooltip = _(option.opt.tooltip); wxString tooltip = _(get_formatted_tooltip_text(option.opt, option.opt_id));
edit_tooltip(tooltip);
Line retval{ _(option.opt.label), tooltip }; Line retval{ _(option.opt.label), tooltip };
retval.label_path = path; retval.label_path = path;
retval.append_option(option); retval.append_option(option);
return retval; return retval;
} }

View File

@@ -78,6 +78,9 @@ public:
void SetRange(int min, int max); void SetRange(int min, int max);
int GetMin() const { return this->min; }
int GetMax() const { return this->max; }
protected: protected:
void DoSetToolTipText(wxString const &tip) override; void DoSetToolTipText(wxString const &tip) override;