diff --git a/lib/Slic3r/GUI/OptionsGroup.pm b/lib/Slic3r/GUI/OptionsGroup.pm index efe804b2f..ac2649528 100644 --- a/lib/Slic3r/GUI/OptionsGroup.pm +++ b/lib/Slic3r/GUI/OptionsGroup.pm @@ -3,7 +3,7 @@ use Moo; use List::Util qw(first); use Wx qw(:combobox :font :misc :sizer :systemsettings :textctrl); -use Wx::Event qw(EVT_CHECKBOX EVT_COMBOBOX EVT_SPINCTRL EVT_TEXT); +use Wx::Event qw(EVT_CHECKBOX EVT_COMBOBOX EVT_SPINCTRL EVT_TEXT EVT_KILL_FOCUS); =head1 NAME @@ -52,6 +52,7 @@ has 'label_width' => (is => 'ro', default => sub { 180 }); has 'extra_column' => (is => 'ro'); has 'label_font' => (is => 'ro'); has 'sidetext_font' => (is => 'ro', default => sub { Wx::SystemSettings::GetFont(wxSYS_DEFAULT_GUI_FONT) }); +has 'ignore_on_change_return' => (is => 'ro', default => sub { 1 }); has 'sizer' => (is => 'rw'); has '_triggers' => (is => 'ro', default => sub { {} }); @@ -166,7 +167,7 @@ sub _build_field { my ($opt) = @_; my $opt_key = $opt->{opt_key}; - $self->_triggers->{$opt_key} = $opt->{on_change} || sub {}; + $self->_triggers->{$opt_key} = $opt->{on_change} || sub { return 1 }; my $field; my $tooltip = $opt->{tooltip}; @@ -176,11 +177,16 @@ sub _build_field { # default width on Windows is too large my $size = Wx::Size->new($opt->{width} || 60, $opt->{height} || -1); - my $on_change = sub { $self->_on_change($opt_key, $field->GetValue) }; + my $on_change = sub { + my $value = $field->GetValue; + $value ||= 0 if $opt->{type} =~ /^(i|f|percent)$/; # prevent crash trying to pass empty strings to Config + $self->_on_change($opt_key, $value); + }; if ($opt->{type} eq 'i') { $field = Wx::SpinCtrl->new($self->parent, -1, $opt->{default}, wxDefaultPosition, $size, $style, $opt->{min} || 0, $opt->{max} || 2147483647, $opt->{default}); $self->_setters->{$opt_key} = sub { $field->SetValue($_[0]) }; EVT_SPINCTRL ($self->parent, $field, $on_change); + EVT_KILL_FOCUS($field, sub { $self->on_kill_focus($opt_key) }); } elsif ($opt->{values}) { $field = Wx::ComboBox->new($self->parent, -1, $opt->{default}, wxDefaultPosition, $size, $opt->{labels} || $opt->{values}); $self->_setters->{$opt_key} = sub { @@ -191,10 +197,12 @@ sub _build_field { $self->_on_change($opt_key, $on_change); }); EVT_TEXT($self->parent, $field, $on_change); + EVT_KILL_FOCUS($field, sub { $self->on_kill_focus($opt_key) }); } else { $field = Wx::TextCtrl->new($self->parent, -1, $opt->{default}, wxDefaultPosition, $size, $style); $self->_setters->{$opt_key} = sub { $field->ChangeValue($_[0]) }; - EVT_TEXT ($self->parent, $field, $on_change); + EVT_TEXT($self->parent, $field, $on_change); + EVT_KILL_FOCUS($field, sub { $self->on_kill_focus($opt_key) }); } $field->Disable if $opt->{readonly}; $tooltip .= " (default: " . $opt->{default} . ")" if ($opt->{default}); @@ -220,8 +228,10 @@ sub _build_field { $tooltip . " (default: " . join(",", @{$opt->{default}}) . ")" ) for @items; } - EVT_TEXT($self->parent, $_, sub { $self->_on_change($opt_key, [ $x_field->GetValue, $y_field->GetValue ]) }) - for $x_field, $y_field; + foreach my $field ($x_field, $y_field) { + EVT_TEXT($self->parent, $field, sub { $self->_on_change($opt_key, [ $x_field->GetValue, $y_field->GetValue ]) }); + EVT_KILL_FOCUS($field, sub { $self->on_kill_focus($opt_key) }); + } $self->_setters->{$opt_key} = sub { $x_field->SetValue($_[0][0]); $y_field->SetValue($_[0][1]); @@ -260,7 +270,7 @@ sub _on_change { my ($opt_key, $value) = @_; return if $self->sizer->GetStaticBox->GetParent->{disabled}; - $self->_triggers->{$opt_key}->($value); + $self->_triggers->{$opt_key}->($value) or $self->ignore_on_change_return or return; $self->on_change->($opt_key, $value); } @@ -285,6 +295,8 @@ sub set_value { return 0; } +sub on_kill_focus {} + package Slic3r::GUI::ConfigOptionsGroup; use Moo; @@ -313,6 +325,7 @@ use List::Util qw(first); has 'config' => (is => 'ro', required => 1); has 'full_labels' => (is => 'ro', default => sub {0}); +has '+ignore_on_change_return' => (is => 'ro', default => sub { 0 }); sub _trigger_options { my $self = shift; @@ -323,13 +336,17 @@ sub _trigger_options { my $full_key = $opt; my ($opt_key, $index) = $self->_split_key($full_key); my $config_opt = $Slic3r::Config::Options->{$opt_key}; + + my $default = $config_opt->{default}; + $default = $default->[$index] if defined($index) && $index <= $#$default; + $opt = { opt_key => $full_key, config => 1, label => ($self->full_labels && defined $config_opt->{full_label}) ? $config_opt->{full_label} : $config_opt->{label}, (map { $_ => $config_opt->{$_} } qw(type tooltip sidetext width height full_width min max labels values multiline readonly)), - default => $self->_get_config($opt_key, $index), - on_change => sub { $self->_set_config($opt_key, $index, $_[0]) }, + default => $default, + on_change => sub { return $self->_set_config($opt_key, $index, $_[0]) }, }; } $opt; @@ -367,6 +384,16 @@ sub set_value { return $changed; } +sub on_kill_focus { + my ($self, $full_key) = @_; + + # when a field loses focus, reapply the config value to it + # (thus discarding any invalid input and reverting to the last + # accepted value) + my ($key, $index) = $self->_split_key($full_key); + $self->SUPER::set_value($full_key, $self->_get_config($key, $index)); +} + sub _split_key { my $self = shift; my ($opt_key) = @_; @@ -378,10 +405,10 @@ sub _split_key { sub _get_config { my $self = shift; - my ($opt_key, $index) = @_; + my ($opt_key, $index, $config) = @_; my ($get_m, $serialized) = $self->_config_methods($opt_key, $index); - my $value = $self->config->$get_m($opt_key); + my $value = ($config // $self->config)->$get_m($opt_key); if (defined $index) { $value->[$index] //= $value->[0]; #/ $value = $value->[$index]; @@ -400,9 +427,9 @@ sub _set_config { $self->config->set($opt_key, $values); } else { if ($serialized) { - $self->config->set_deserialize($opt_key, $value); + return $self->config->set_deserialize($opt_key, $value); } else { - $self->config->set($opt_key, $value); + return $self->config->set($opt_key, $value); } } } diff --git a/xs/src/Config.cpp b/xs/src/Config.cpp index de99b96ec..7c3280d5f 100644 --- a/xs/src/Config.cpp +++ b/xs/src/Config.cpp @@ -33,19 +33,20 @@ ConfigBase::serialize(const t_config_option_key opt_key) { return opt->serialize(); } -void +bool ConfigBase::set_deserialize(const t_config_option_key opt_key, std::string str) { if (this->def->count(opt_key) == 0) throw "Calling set_deserialize() on unknown option"; ConfigOptionDef* optdef = &(*this->def)[opt_key]; if (!optdef->shortcut.empty()) { - for (std::vector::iterator it = optdef->shortcut.begin(); it != optdef->shortcut.end(); ++it) - this->set_deserialize(*it, str); - return; + for (std::vector::iterator it = optdef->shortcut.begin(); it != optdef->shortcut.end(); ++it) { + if (!this->set_deserialize(*it, str)) return false; + } + return true; } ConfigOption* opt = this->option(opt_key, true); assert(opt != NULL); - opt->deserialize(str); + return opt->deserialize(str); } double @@ -164,31 +165,37 @@ ConfigBase::get_at(t_config_option_key opt_key, size_t i) { } } -void +bool ConfigBase::set(t_config_option_key opt_key, SV* value) { ConfigOption* opt = this->option(opt_key, true); if (opt == NULL) CONFESS("Trying to set non-existing option"); if (ConfigOptionFloat* optv = dynamic_cast(opt)) { + if (!looks_like_number(value)) return false; optv->value = SvNV(value); } else if (ConfigOptionFloats* optv = dynamic_cast(opt)) { - optv->values.clear(); + std::vector values; AV* av = (AV*)SvRV(value); const size_t len = av_len(av)+1; for (size_t i = 0; i < len; i++) { SV** elem = av_fetch(av, i, 0); - optv->values.push_back(SvNV(*elem)); + if (!looks_like_number(*elem)) return false; + values.push_back(SvNV(*elem)); } + optv->values = values; } else if (ConfigOptionInt* optv = dynamic_cast(opt)) { + if (!looks_like_number(value)) return false; optv->value = SvIV(value); } else if (ConfigOptionInts* optv = dynamic_cast(opt)) { - optv->values.clear(); + std::vector values; AV* av = (AV*)SvRV(value); const size_t len = av_len(av)+1; for (size_t i = 0; i < len; i++) { SV** elem = av_fetch(av, i, 0); - optv->values.push_back(SvIV(*elem)); + if (!looks_like_number(*elem)) return false; + values.push_back(SvIV(*elem)); } + optv->values = values; } else if (ConfigOptionString* optv = dynamic_cast(opt)) { optv->value = std::string(SvPV_nolen(value), SvCUR(value)); } else if (ConfigOptionStrings* optv = dynamic_cast(opt)) { @@ -200,17 +207,18 @@ ConfigBase::set(t_config_option_key opt_key, SV* value) { optv->values.push_back(std::string(SvPV_nolen(*elem), SvCUR(*elem))); } } else if (ConfigOptionPoint* optv = dynamic_cast(opt)) { - optv->point.from_SV(value); + return optv->point.from_SV(value); } else if (ConfigOptionPoints* optv = dynamic_cast(opt)) { - optv->values.clear(); + std::vector values; AV* av = (AV*)SvRV(value); const size_t len = av_len(av)+1; for (size_t i = 0; i < len; i++) { SV** elem = av_fetch(av, i, 0); Pointf point; - point.from_SV(*elem); - optv->values.push_back(point); + if (!point.from_SV(*elem)) return false; + values.push_back(point); } + optv->values = values; } else if (ConfigOptionBool* optv = dynamic_cast(opt)) { optv->value = SvTRUE(value); } else if (ConfigOptionBools* optv = dynamic_cast(opt)) { @@ -222,8 +230,9 @@ ConfigBase::set(t_config_option_key opt_key, SV* value) { optv->values.push_back(SvTRUE(*elem)); } } else { - opt->deserialize( std::string(SvPV_nolen(value)) ); + if (!opt->deserialize( std::string(SvPV_nolen(value)) )) return false; } + return true; } #endif @@ -347,16 +356,17 @@ StaticConfig::option(const t_config_option_key opt_key) const } #ifdef SLIC3RXS -void +bool StaticConfig::set(t_config_option_key opt_key, SV* value) { ConfigOptionDef* optdef = &(*this->def)[opt_key]; if (!optdef->shortcut.empty()) { - for (std::vector::iterator it = optdef->shortcut.begin(); it != optdef->shortcut.end(); ++it) - this->set(*it, value); - return; + for (std::vector::iterator it = optdef->shortcut.begin(); it != optdef->shortcut.end(); ++it) { + if (!this->set(*it, value)) return false; + } + return true; } - static_cast(this)->set(opt_key, value); + return static_cast(this)->set(opt_key, value); } #endif diff --git a/xs/src/Config.hpp b/xs/src/Config.hpp index 226ebe7a9..5b4cb8284 100644 --- a/xs/src/Config.hpp +++ b/xs/src/Config.hpp @@ -21,7 +21,7 @@ class ConfigOption { public: virtual ~ConfigOption() {}; virtual std::string serialize() const = 0; - virtual void deserialize(std::string str) = 0; + virtual bool deserialize(std::string str) = 0; }; template @@ -54,8 +54,9 @@ class ConfigOptionFloat : public ConfigOption return ss.str(); }; - void deserialize(std::string str) { + bool deserialize(std::string str) { this->value = ::atof(str.c_str()); + return true; }; }; @@ -72,13 +73,14 @@ class ConfigOptionFloats : public ConfigOption, public ConfigOptionVectorvalues.clear(); std::istringstream is(str); std::string item_str; while (std::getline(is, item_str, ',')) { this->values.push_back(::atof(item_str.c_str())); } + return true; }; }; @@ -96,8 +98,9 @@ class ConfigOptionInt : public ConfigOption return ss.str(); }; - void deserialize(std::string str) { + bool deserialize(std::string str) { this->value = ::atoi(str.c_str()); + return true; }; }; @@ -114,13 +117,14 @@ class ConfigOptionInts : public ConfigOption, public ConfigOptionVector return ss.str(); }; - void deserialize(std::string str) { + bool deserialize(std::string str) { this->values.clear(); std::istringstream is(str); std::string item_str; while (std::getline(is, item_str, ',')) { this->values.push_back(::atoi(item_str.c_str())); } + return true; }; }; @@ -145,7 +149,7 @@ class ConfigOptionString : public ConfigOption return str; }; - void deserialize(std::string str) { + bool deserialize(std::string str) { // s/\\n/\n/g size_t pos = 0; while ((pos = str.find("\\n", pos)) != std::string::npos) { @@ -154,6 +158,7 @@ class ConfigOptionString : public ConfigOption } this->value = str; + return true; }; }; @@ -171,13 +176,14 @@ class ConfigOptionStrings : public ConfigOption, public ConfigOptionVectorvalues.clear(); std::istringstream is(str); std::string item_str; while (std::getline(is, item_str, ';')) { this->values.push_back(item_str); } + return true; }; }; @@ -199,9 +205,10 @@ class ConfigOptionPercent : public ConfigOption return s; }; - void deserialize(std::string str) { + bool deserialize(std::string str) { // don't try to parse the trailing % since it's optional - sscanf(str.c_str(), "%lf", &this->value); + int res = sscanf(str.c_str(), "%lf", &this->value); + return res == 1; }; }; @@ -228,14 +235,16 @@ class ConfigOptionFloatOrPercent : public ConfigOption return s; }; - void deserialize(std::string str) { + bool deserialize(std::string str) { if (str.find_first_of("%") != std::string::npos) { - sscanf(str.c_str(), "%lf%%", &this->value); + int res = sscanf(str.c_str(), "%lf%%", &this->value); + if (res == 0) return false; this->percent = true; } else { this->value = ::atof(str.c_str()); this->percent = false; } + return true; }; }; @@ -255,8 +264,9 @@ class ConfigOptionPoint : public ConfigOption return ss.str(); }; - void deserialize(std::string str) { - sscanf(str.c_str(), "%lf%*1[,x]%lf", &this->point.x, &this->point.y); + bool deserialize(std::string str) { + int res = sscanf(str.c_str(), "%lf%*1[,x]%lf", &this->point.x, &this->point.y); + return res == 2; }; }; @@ -275,15 +285,17 @@ class ConfigOptionPoints : public ConfigOption, public ConfigOptionVectorvalues.clear(); std::istringstream is(str); std::string point_str; while (std::getline(is, point_str, ',')) { Pointf point; - sscanf(point_str.c_str(), "%lfx%lf", &point.x, &point.y); + int res = sscanf(point_str.c_str(), "%lfx%lf", &point.x, &point.y); + if (res != 2) return false; this->values.push_back(point); } + return true; }; }; @@ -299,8 +311,9 @@ class ConfigOptionBool : public ConfigOption return std::string(this->value ? "1" : "0"); }; - void deserialize(std::string str) { + bool deserialize(std::string str) { this->value = (str.compare("1") == 0); + return true; }; }; @@ -317,13 +330,14 @@ class ConfigOptionBools : public ConfigOption, public ConfigOptionVector return ss.str(); }; - void deserialize(std::string str) { + bool deserialize(std::string str) { this->values.clear(); std::istringstream is(str); std::string item_str; while (std::getline(is, item_str, ',')) { this->values.push_back(item_str.compare("1") == 0); } + return true; }; }; @@ -345,10 +359,11 @@ class ConfigOptionEnum : public ConfigOption return ""; }; - void deserialize(std::string str) { + bool deserialize(std::string str) { t_config_enum_values enum_keys_map = ConfigOptionEnum::get_enum_values(); - assert(enum_keys_map.count(str) > 0); + if (enum_keys_map.count(str) == 0) return false; this->value = static_cast(enum_keys_map[str]); + return true; }; static t_config_enum_values get_enum_values(); @@ -371,9 +386,10 @@ class ConfigOptionEnumGeneric : public ConfigOption return ""; }; - void deserialize(std::string str) { - assert(this->keys_map->count(str) != 0); + bool deserialize(std::string str) { + if (this->keys_map->count(str) == 0) return false; this->value = (*this->keys_map)[str]; + return true; }; }; @@ -435,7 +451,7 @@ class ConfigBase virtual void keys(t_config_option_keys *keys) const = 0; void apply(const ConfigBase &other, bool ignore_nonexistent = false); std::string serialize(const t_config_option_key opt_key); - void set_deserialize(const t_config_option_key opt_key, std::string str); + bool set_deserialize(const t_config_option_key opt_key, std::string str); double get_abs_value(const t_config_option_key opt_key); double get_abs_value(const t_config_option_key opt_key, double ratio_over); @@ -443,7 +459,7 @@ class ConfigBase SV* as_hash(); SV* get(t_config_option_key opt_key); SV* get_at(t_config_option_key opt_key, size_t i); - void set(t_config_option_key opt_key, SV* value); + bool set(t_config_option_key opt_key, SV* value); #endif }; @@ -477,7 +493,7 @@ class StaticConfig : public ConfigBase const ConfigOption* option(const t_config_option_key opt_key) const; #ifdef SLIC3RXS - void set(t_config_option_key opt_key, SV* value); + bool set(t_config_option_key opt_key, SV* value); #endif }; diff --git a/xs/src/Point.cpp b/xs/src/Point.cpp index 57385a9d4..a6c844054 100644 --- a/xs/src/Point.cpp +++ b/xs/src/Point.cpp @@ -201,12 +201,17 @@ Pointf::to_SV_pureperl() const { return newRV_noinc((SV*)av); } -void +bool Pointf::from_SV(SV* point_sv) { AV* point_av = (AV*)SvRV(point_sv); - this->x = SvNV(*av_fetch(point_av, 0, 0)); - this->y = SvNV(*av_fetch(point_av, 1, 0)); + SV* sv_x = *av_fetch(point_av, 0, 0); + SV* sv_y = *av_fetch(point_av, 1, 0); + if (!looks_like_number(sv_x) || !looks_like_number(sv_x)) return false; + + this->x = SvNV(sv_x); + this->y = SvNV(sv_y); + return true; } #endif diff --git a/xs/src/Point.hpp b/xs/src/Point.hpp index 8fb0ab0bf..8f3cec554 100644 --- a/xs/src/Point.hpp +++ b/xs/src/Point.hpp @@ -66,7 +66,7 @@ class Pointf void translate(double x, double y); #ifdef SLIC3RXS - void from_SV(SV* point_sv); + bool from_SV(SV* point_sv); SV* to_SV_pureperl() const; #endif }; diff --git a/xs/src/PrintConfig.hpp b/xs/src/PrintConfig.hpp index 46ed6c790..49cc64067 100644 --- a/xs/src/PrintConfig.hpp +++ b/xs/src/PrintConfig.hpp @@ -228,6 +228,7 @@ class PrintConfigDef Options["fan_below_layer_time"].sidetext = "approximate seconds"; Options["fan_below_layer_time"].cli = "fan-below-layer-time=i"; Options["fan_below_layer_time"].width = 60; + Options["fan_below_layer_time"].min = 0; Options["fan_below_layer_time"].max = 1000; Options["filament_diameter"].type = coFloats; @@ -235,6 +236,7 @@ class PrintConfigDef Options["filament_diameter"].tooltip = "Enter your filament diameter here. Good precision is required, so use a caliper and do multiple measurements along the filament, then compute the average."; Options["filament_diameter"].sidetext = "mm"; Options["filament_diameter"].cli = "filament-diameter=f@"; + Options["filament_diameter"].min = 0; Options["fill_angle"].type = coInt; Options["fill_angle"].label = "Fill angle"; @@ -242,6 +244,7 @@ class PrintConfigDef Options["fill_angle"].tooltip = "Default base angle for infill orientation. Cross-hatching will be applied to this. Bridges will be infilled using the best direction Slic3r can detect, so this setting does not affect them."; Options["fill_angle"].sidetext = "°"; Options["fill_angle"].cli = "fill-angle=i"; + Options["fill_angle"].min = 0; Options["fill_angle"].max = 359; Options["fill_density"].type = coPercent; diff --git a/xs/xsp/Config.xsp b/xs/xsp/Config.xsp index 7b611b39e..61490f967 100644 --- a/xs/xsp/Config.xsp +++ b/xs/xsp/Config.xsp @@ -12,8 +12,8 @@ SV* as_hash(); SV* get(t_config_option_key opt_key); SV* get_at(t_config_option_key opt_key, int i); - void set(t_config_option_key opt_key, SV* value); - void set_deserialize(t_config_option_key opt_key, std::string str); + bool set(t_config_option_key opt_key, SV* value); + bool set_deserialize(t_config_option_key opt_key, std::string str); std::string serialize(t_config_option_key opt_key); double get_abs_value(t_config_option_key opt_key); %name{get_abs_value_over} @@ -34,8 +34,8 @@ SV* as_hash(); SV* get(t_config_option_key opt_key); SV* get_at(t_config_option_key opt_key, int i); - void set(t_config_option_key opt_key, SV* value); - void set_deserialize(t_config_option_key opt_key, std::string str); + bool set(t_config_option_key opt_key, SV* value); + bool set_deserialize(t_config_option_key opt_key, std::string str); std::string serialize(t_config_option_key opt_key); double get_abs_value(t_config_option_key opt_key); %name{get_abs_value_over} @@ -54,8 +54,8 @@ SV* as_hash(); SV* get(t_config_option_key opt_key); SV* get_at(t_config_option_key opt_key, int i); - void set(t_config_option_key opt_key, SV* value); - void set_deserialize(t_config_option_key opt_key, std::string str); + bool set(t_config_option_key opt_key, SV* value); + bool set_deserialize(t_config_option_key opt_key, std::string str); std::string serialize(t_config_option_key opt_key); double get_abs_value(t_config_option_key opt_key); %name{get_abs_value_over} @@ -75,8 +75,8 @@ SV* as_hash(); SV* get(t_config_option_key opt_key); SV* get_at(t_config_option_key opt_key, int i); - void set(t_config_option_key opt_key, SV* value); - void set_deserialize(t_config_option_key opt_key, std::string str); + bool set(t_config_option_key opt_key, SV* value); + bool set_deserialize(t_config_option_key opt_key, std::string str); std::string serialize(t_config_option_key opt_key); double get_abs_value(t_config_option_key opt_key); %name{get_abs_value_over} @@ -96,8 +96,8 @@ SV* as_hash(); SV* get(t_config_option_key opt_key); SV* get_at(t_config_option_key opt_key, int i); - void set(t_config_option_key opt_key, SV* value); - void set_deserialize(t_config_option_key opt_key, std::string str); + bool set(t_config_option_key opt_key, SV* value); + bool set_deserialize(t_config_option_key opt_key, std::string str); std::string serialize(t_config_option_key opt_key); double get_abs_value(t_config_option_key opt_key); %name{get_abs_value_over}