From 34f1511e0c61c2c9b4354dca8961d1e038c34754 Mon Sep 17 00:00:00 2001 From: Alessandro Ranellucci Date: Mon, 17 Mar 2014 00:39:07 +0100 Subject: [PATCH] Better fix for non-global options not being replaced in filename placeholders. Includes refactoring and a new PlaceholderParser class. Also includes regression tests. #1831 --- lib/Slic3r.pm | 1 + lib/Slic3r/Config.pm | 40 --------------- lib/Slic3r/GCode.pm | 9 +--- lib/Slic3r/GCode/Layer.pm | 2 +- lib/Slic3r/GCode/PlaceholderParser.pm | 73 +++++++++++++++++++++++++++ lib/Slic3r/GUI/Plater.pm | 48 +++++++++--------- lib/Slic3r/Print.pm | 63 +++++++++-------------- lib/Slic3r/Test.pm | 2 +- t/custom_gcode.t | 25 +++++++-- t/print.t | 11 +--- 10 files changed, 146 insertions(+), 128 deletions(-) create mode 100644 lib/Slic3r/GCode/PlaceholderParser.pm diff --git a/lib/Slic3r.pm b/lib/Slic3r.pm index 88f501882..24ad32339 100644 --- a/lib/Slic3r.pm +++ b/lib/Slic3r.pm @@ -52,6 +52,7 @@ use Slic3r::GCode::ArcFitting; use Slic3r::GCode::CoolingBuffer; use Slic3r::GCode::Layer; use Slic3r::GCode::MotionPlanner; +use Slic3r::GCode::PlaceholderParser; use Slic3r::GCode::Reader; use Slic3r::GCode::SpiralVase; use Slic3r::GCode::VibrationLimit; diff --git a/lib/Slic3r/Config.pm b/lib/Slic3r/Config.pm index 82c54b584..e8884d6ad 100644 --- a/lib/Slic3r/Config.pm +++ b/lib/Slic3r/Config.pm @@ -359,46 +359,6 @@ sub validate { return 1; } -sub replace_options { - my $self = shift; - my ($string, $more_variables) = @_; - - $more_variables ||= {}; - $more_variables->{$_} = $ENV{$_} for grep /^SLIC3R_/, keys %ENV; - { - my $variables_regex = join '|', keys %$more_variables; - $string =~ s/\[($variables_regex)\]/$more_variables->{$1}/eg; - } - - my @lt = localtime; $lt[5] += 1900; $lt[4] += 1; - $string =~ s/\[timestamp\]/sprintf '%04d%02d%02d-%02d%02d%02d', @lt[5,4,3,2,1,0]/egx; - $string =~ s/\[year\]/$lt[5]/eg; - $string =~ s/\[month\]/$lt[4]/eg; - $string =~ s/\[day\]/$lt[3]/eg; - $string =~ s/\[hour\]/$lt[2]/eg; - $string =~ s/\[minute\]/$lt[1]/eg; - $string =~ s/\[second\]/$lt[0]/eg; - $string =~ s/\[version\]/$Slic3r::VERSION/eg; - - # build a regexp to match the available options - my @options = grep !$Slic3r::Config::Options->{$_}{multiline}, @{$self->get_keys}; - my $options_regex = join '|', @options; - - # use that regexp to search and replace option names with option values - # it looks like passing $1 as argument to serialize() directly causes a segfault - # (maybe some perl optimization? maybe regex captures are not regular SVs?) - $string =~ s/\[($options_regex)\]/my $opt_key = $1; $self->serialize($opt_key)/eg; - foreach my $opt_key (grep ref $self->$_ eq 'ARRAY', @options) { - my $value = $self->$opt_key; - $string =~ s/\[${opt_key}_${_}\]/$value->[$_]/eg for 0 .. $#$value; - if ($Options->{$opt_key}{type} eq 'point') { - $string =~ s/\[${opt_key}_X\]/$value->[0]/eg; - $string =~ s/\[${opt_key}_Y\]/$value->[1]/eg; - } - } - return $string; -} - # min object distance is max(duplicate_distance, clearance_radius) sub min_object_distance { my $self = shift; diff --git a/lib/Slic3r/GCode.pm b/lib/Slic3r/GCode.pm index 82c576f93..5f75905dc 100644 --- a/lib/Slic3r/GCode.pm +++ b/lib/Slic3r/GCode.pm @@ -9,7 +9,7 @@ use Slic3r::Geometry::Clipper qw(union_ex); use Slic3r::Surface ':types'; has 'print_config' => (is => 'ro', default => sub { Slic3r::Config::Print->new }); -has 'extra_variables' => (is => 'rw', default => sub {{}}); +has 'placeholder_parser' => (is => 'rw', default => sub { Slic3r::GCode::PlaceholderParser->new }); has 'standby_points' => (is => 'rw'); has 'enable_loop_clipping' => (is => 'rw', default => sub {1}); has 'enable_wipe' => (is => 'rw', default => sub {0}); # at least one extruder has wipe enabled @@ -649,7 +649,7 @@ sub set_extruder { # append custom toolchange G-code if (defined $self->extruder && $self->print_config->toolchange_gcode) { - $gcode .= sprintf "%s\n", $self->replace_variables($self->print_config->toolchange_gcode, { + $gcode .= sprintf "%s\n", $self->placeholder_parser->process($self->print_config->toolchange_gcode, { previous_extruder => $self->extruder->id, next_extruder => $extruder_id, }); @@ -748,9 +748,4 @@ sub set_bed_temperature { return $gcode; } -sub replace_variables { - my ($self, $string, $extra) = @_; - return $self->print_config->replace_options($string, { %{$self->extra_variables}, %{ $extra || {} } }); -} - 1; diff --git a/lib/Slic3r/GCode/Layer.pm b/lib/Slic3r/GCode/Layer.pm index 33465eecc..3aeb80de4 100644 --- a/lib/Slic3r/GCode/Layer.pm +++ b/lib/Slic3r/GCode/Layer.pm @@ -72,7 +72,7 @@ sub process_layer { # set new layer - this will change Z and force a retraction if retract_layer_change is enabled $gcode .= $self->gcodegen->change_layer($layer); - $gcode .= $self->gcodegen->replace_variables($self->print->config->layer_gcode, { + $gcode .= $self->gcodegen->placeholder_parser->process($self->print->config->layer_gcode, { layer_num => $self->gcodegen->layer->id, }) . "\n" if $self->print->config->layer_gcode; diff --git a/lib/Slic3r/GCode/PlaceholderParser.pm b/lib/Slic3r/GCode/PlaceholderParser.pm new file mode 100644 index 000000000..fe92a2438 --- /dev/null +++ b/lib/Slic3r/GCode/PlaceholderParser.pm @@ -0,0 +1,73 @@ +package Slic3r::GCode::PlaceholderParser; +use Moo; + +has '_single' => (is => 'ro', default => sub { {} }); +has '_multiple' => (is => 'ro', default => sub { {} }); + +sub BUILD { + my ($self) = @_; + + my $s = $self->_single; + + # environment variables + $s->{$_} = $ENV{$_} for grep /^SLIC3R_/, keys %ENV; + + # timestamp + my @lt = localtime; $lt[5] += 1900; $lt[4] += 1; + $s->{timestamp} = sprintf '%04d%02d%02d-%02d%02d%02d', @lt[5,4,3,2,1,0]; + $s->{year} = $lt[5]; + $s->{month} = $lt[4]; + $s->{day} = $lt[3]; + $s->{hour} = $lt[2]; + $s->{minute} = $lt[1]; + $s->{second} = $lt[0]; + $s->{version} = $Slic3r::VERSION; +} + +sub apply_config { + my ($self, $config) = @_; + + # options with single value + my $s = $self->_single; + my @opt_keys = grep !$Slic3r::Config::Options->{$_}{multiline}, @{$config->get_keys}; + $s->{$_} = $config->serialize($_) for @opt_keys; + + # options with multiple values + my $m = $self->_multiple; + foreach my $opt_key (@opt_keys) { + my $value = $config->$opt_key; + next unless ref($value) eq 'ARRAY'; + $m->{"${opt_key}_${_}"} = $value->[$_] for 0..$#$value; + if ($Slic3r::Config::Options->{$opt_key}{type} eq 'point') { + $m->{"${opt_key}_X"} = $value->[0]; + $m->{"${opt_key}_Y"} = $value->[1]; + } + } +} + +sub set { + my ($self, $key, $val) = @_; + $self->_single->{$key} = $val; +} + +sub process { + my ($self, $string, $extra) = @_; + + # extra variables have priority over the stored ones + if ($extra) { + my $regex = join '|', keys %$extra; + $string =~ s/\[($regex)\]/$extra->{$1}/eg; + } + { + my $regex = join '|', keys %{$self->_single}; + $string =~ s/\[($regex)\]/$self->_single->{$1}/eg; + } + { + my $regex = join '|', keys %{$self->_multiple}; + $string =~ s/\[($regex)\]/$self->_multiple->{$1}/eg; + } + + return $string; +} + +1; diff --git a/lib/Slic3r/GUI/Plater.pm b/lib/Slic3r/GUI/Plater.pm index b15efcc1c..572bf2a56 100644 --- a/lib/Slic3r/GUI/Plater.pm +++ b/lib/Slic3r/GUI/Plater.pm @@ -670,16 +670,31 @@ sub export_gcode { return; } - # get config before spawning the thread because it needs GetParent and it's not available there - our $config = $self->skeinpanel->config; - our $extra_variables = $self->skeinpanel->extra_variables; + # It looks like declaring a local $SIG{__WARN__} prevents the ugly + # "Attempt to free unreferenced scalar" warning... + local $SIG{__WARN__} = Slic3r::GUI::warning_catcher($self); + + # apply config and validate print + my $config = $self->skeinpanel->config; + eval { + # this will throw errors if config is not valid + $config->validate; + $self->{print}->apply_config($config); + $self->{print}->validate; + }; + + # apply extra variables + { + my $extra = $self->skeinpanel->extra_variables; + $self->{print}->placeholder_parser->set($_, $extra->{$_}) for keys %$extra; + } # select output file $self->{output_file} = $main::opt{output}; { - $self->{output_file} = $self->skeinpanel->init_print->expanded_output_filepath($self->{output_file}, $self->{model}->objects->[0]->input_file); - my $dlg = Wx::FileDialog->new($self, 'Save G-code file as:', Slic3r::GUI->output_path(dirname($self->{output_file})), - basename($self->{output_file}), &Slic3r::GUI::SkeinPanel::FILE_WILDCARDS->{gcode}, wxFD_SAVE); + my $output_file = $self->{print}->expanded_output_filepath($self->{output_file}); + my $dlg = Wx::FileDialog->new($self, 'Save G-code file as:', Slic3r::GUI->output_path(dirname($output_file)), + basename($output_file), &Slic3r::GUI::SkeinPanel::FILE_WILDCARDS->{gcode}, wxFD_SAVE); if ($dlg->ShowModal != wxID_OK) { $dlg->Destroy; return; @@ -692,10 +707,6 @@ sub export_gcode { $self->statusbar->StartBusy; - # It looks like declaring a local $SIG{__WARN__} prevents the ugly - # "Attempt to free unreferenced scalar" warning... - local $SIG{__WARN__} = Slic3r::GUI::warning_catcher($self); - if ($Slic3r::have_threads) { @_ = (); @@ -704,8 +715,6 @@ sub export_gcode { $self->{export_thread} = threads->create(sub { $_thread_self->export_gcode2( - $config, - $extra_variables, $_thread_self->{output_file}, progressbar => sub { Wx::PostEvent($_thread_self, Wx::PlThreadEvent->new(-1, $PROGRESS_BAR_EVENT, shared_clone([@_]))) }, message_dialog => sub { Wx::PostEvent($_thread_self, Wx::PlThreadEvent->new(-1, $MESSAGE_DIALOG_EVENT, shared_clone([@_]))) }, @@ -727,8 +736,6 @@ sub export_gcode { }); } else { $self->export_gcode2( - $config, - $extra_variables, $self->{output_file}, progressbar => sub { my ($percent, $message) = @_; @@ -747,7 +754,7 @@ sub export_gcode { sub export_gcode2 { my $self = shift; - my ($config, $extra_variables, $output_file, %params) = @_; + my ($output_file, %params) = @_; local $SIG{'KILL'} = sub { Slic3r::debugf "Exporting cancelled; exiting thread...\n"; Slic3r::thread_cleanup(); @@ -756,16 +763,7 @@ sub export_gcode2 { my $print = $self->{print}; - eval { - # this will throw errors if config is not valid - $config->validate; - - $print->apply_config($config); - $print->apply_extra_variables($extra_variables); - - $print->validate; - { my @warnings = (); local $SIG{__WARN__} = sub { push @warnings, $_[0] }; @@ -840,7 +838,7 @@ sub _get_export_file { my $output_file = $main::opt{output}; { - $output_file = $self->skeinpanel->init_print->expanded_output_filepath($output_file, $self->{model}->objects->[0]->input_file); + $output_file = $self->{print}->expanded_output_filepath($output_file); $output_file =~ s/\.gcode$/$suffix/i; my $dlg = Wx::FileDialog->new($self, "Save $format file as:", dirname($output_file), basename($output_file), &Slic3r::GUI::SkeinPanel::MODEL_WILDCARD, wxFD_SAVE | wxFD_OVERWRITE_PROMPT); diff --git a/lib/Slic3r/Print.pm b/lib/Slic3r/Print.pm index fd5d391cd..347f3458f 100644 --- a/lib/Slic3r/Print.pm +++ b/lib/Slic3r/Print.pm @@ -15,7 +15,7 @@ use Slic3r::Print::State ':steps'; has 'config' => (is => 'ro', default => sub { Slic3r::Config::Print->new }); has 'default_object_config' => (is => 'ro', default => sub { Slic3r::Config::PrintObject->new }); has 'default_region_config' => (is => 'ro', default => sub { Slic3r::Config::PrintRegion->new }); -has 'extra_variables' => (is => 'rw', default => sub {{}}); +has 'placeholder_parser' => (is => 'rw', default => sub { Slic3r::GCode::PlaceholderParser->new }); has 'objects' => (is => 'rw', default => sub {[]}); has 'status_cb' => (is => 'rw'); has 'regions' => (is => 'rw', default => sub {[]}); @@ -32,6 +32,9 @@ has 'brim' => (is => 'rw', default => sub { Slic3r::ExtrusionPath::Collection->n sub apply_config { my ($self, $config) = @_; + # apply variables to placeholder parser + $self->placeholder_parser->apply_config($config); + # handle changes to print config my $print_diff = $self->config->diff($config); if (@$print_diff) { @@ -187,12 +190,6 @@ sub add_model_object { push @{$self->objects}, $o; } - if (!defined $self->extra_variables->{input_filename}) { - if (defined (my $input_file = $object->input_file)) { - @{$self->extra_variables}{qw(input_filename input_filename_base)} = parse_filename($input_file); - } - } - $self->_state->invalidate(STEP_SKIRT); $self->_state->invalidate(STEP_BRIM); } @@ -825,10 +822,13 @@ sub write_gcode { print $fh "\n"; } - # set up our extruder object + # prepare the helper object for replacing placeholders in custom G-code and output filename + + + # set up our helper object my $gcodegen = Slic3r::GCode->new( print_config => $self->config, - extra_variables => $self->extra_variables, + placeholder_parser => $self->placeholder_parser, layer_count => $self->layer_count, ); $gcodegen->set_extruders($self->extruders); @@ -853,7 +853,7 @@ sub write_gcode { } }; $print_first_layer_temperature->(0); - printf $fh "%s\n", $gcodegen->replace_variables($self->config->start_gcode); + printf $fh "%s\n", $gcodegen->placeholder_parser->process($self->config->start_gcode); $print_first_layer_temperature->(1); # set other general things @@ -997,7 +997,7 @@ sub write_gcode { # write end commands to file print $fh $gcodegen->retract if $gcodegen->extruder; # empty prints don't even set an extruder print $fh $gcodegen->set_fan(0); - printf $fh "%s\n", $gcodegen->replace_variables($self->config->end_gcode); + printf $fh "%s\n", $gcodegen->placeholder_parser->process($self->config->end_gcode); $self->total_used_filament(0); $self->total_extruded_volume(0); @@ -1029,15 +1029,18 @@ sub write_gcode { # format variables with their values sub expanded_output_filepath { my $self = shift; - my ($path, $input_file) = @_; + my ($path) = @_; - my $extra_variables = {}; - if ($input_file) { - @$extra_variables{qw(input_filename input_filename_base)} = parse_filename($input_file); - } else { - # if no input file was supplied, take the first one from our objects - $input_file = $self->objects->[0]->model_object->input_file // return undef; - } + return undef if !@{$self->objects}; + my $input_file = first { defined $_ } map $_->model_object->input_file, @{$self->objects}; + return undef if !defined $input_file; + + my $filename = my $filename_base = basename($input_file); + $filename_base =~ s/\.[^.]+$//; # without suffix + my $extra = { + input_filename => $filename, + input_filename_base => $filename_base, + }; if ($path && -d $path) { # if output path is an existing directory, we take that and append @@ -1051,27 +1054,7 @@ sub expanded_output_filepath { # path is a full path to a file so we use it as it is } - # get a full set options for replacing placeholders in output filename format - # (only use the first region's and first object's options) - my $full_config = Slic3r::Config->new; - $full_config->apply_static($self->config); - $full_config->apply_static($self->regions->[0]->config) if @{$self->regions}; - $full_config->apply_static($self->objects->[0]->config) if @{$self->objects}; - return $full_config->replace_options($path, { %{$self->extra_variables}, %$extra_variables }); -} - -# given the path to a file, this function returns its filename with and without extension -sub parse_filename { - my ($path) = @_; - - my $filename = my $filename_base = basename($path); - $filename_base =~ s/\.[^.]+$//; - return ($filename, $filename_base); -} - -sub apply_extra_variables { - my ($self, $extra) = @_; - $self->extra_variables->{$_} = $extra->{$_} for keys %$extra; + return $self->placeholder_parser->process($path, $extra); } sub invalidate_step { diff --git a/lib/Slic3r/Test.pm b/lib/Slic3r/Test.pm index 8d5c93da3..1242cd520 100644 --- a/lib/Slic3r/Test.pm +++ b/lib/Slic3r/Test.pm @@ -101,7 +101,7 @@ sub model { $mesh->scale($params{scale}) if $params{scale}; my $model = Slic3r::Model->new; - my $object = $model->add_object; + my $object = $model->add_object(input_file => "${model_name}.stl"); $object->add_volume(mesh => $mesh); $object->add_instance( offset => [0,0], diff --git a/t/custom_gcode.t b/t/custom_gcode.t index 765946f36..6e678d690 100644 --- a/t/custom_gcode.t +++ b/t/custom_gcode.t @@ -1,4 +1,4 @@ -use Test::More tests => 2; +use Test::More tests => 6; use strict; use warnings; @@ -44,10 +44,27 @@ use Slic3r::Test; #========================================================== { - my $config = Slic3r::Config->new_from_defaults; - is $config->replace_options('[temperature_[foo]]', { foo => '0' }), - 200, + my $parser = Slic3r::GCode::PlaceholderParser->new; + $parser->apply_config(my $config = Slic3r::Config->new_from_defaults); + is $parser->process('[temperature_[foo]]', { foo => '0' }), + $config->temperature->[0], "nested config options"; } +{ + my $config = Slic3r::Config->new_from_defaults; + $config->set('output_filename_format', '[travel_speed]_[layer_height].gcode'); + $config->set('start_gcode', "TRAVEL:[travel_speed] HEIGHT:[layer_height]\n"); + my $print = Slic3r::Test::init_print('20mm_cube', config => $config); + + my $output_file = $print->expanded_output_filepath; + ok $output_file !~ /\[travel_speed\]/, 'print config options are replaced in output filename'; + ok $output_file !~ /\[layer_height\]/, 'region config options are replaced in output filename'; + + my $gcode = Slic3r::Test::gcode($print); + my ($t, $h) = map $config->$_, qw(travel_speed layer_height); + ok $gcode =~ /TRAVEL:$t/, 'print config options are replaced in custom G-code'; + ok $gcode =~ /HEIGHT:$h/, 'region config options are replaced in custom G-code'; +} + __END__ diff --git a/t/print.t b/t/print.t index bd2926717..b4ff20145 100644 --- a/t/print.t +++ b/t/print.t @@ -1,4 +1,4 @@ -use Test::More tests => 4; +use Test::More tests => 2; use strict; use warnings; @@ -30,13 +30,4 @@ use Slic3r::Test; ok abs(unscale($center->[Y]) - $config->print_center->[Y]) < epsilon, 'print is centered around print_center (Y)'; } -{ - my $config = Slic3r::Config->new_from_defaults; - $config->set('output_filename_format', '[travel_speed]_[layer_height].gcode'); - my $print = Slic3r::Test::init_print('20mm_cube', config => $config); - my $output_file = $print->expanded_output_filepath(undef, "foo.stl"); - ok $output_file !~ /\[travel_speed\]/, 'print config options are replaced in output filename'; - ok $output_file !~ /\[layer_height\]/, 'region config options are replaced in output filename'; -} - __END__