Better fix for non-global options not being replaced in filename placeholders. Includes refactoring and a new PlaceholderParser class. Also includes regression tests. #1831

This commit is contained in:
Alessandro Ranellucci 2014-03-17 00:39:07 +01:00
parent bc054e613c
commit 34f1511e0c
10 changed files with 146 additions and 128 deletions

View File

@ -52,6 +52,7 @@ use Slic3r::GCode::ArcFitting;
use Slic3r::GCode::CoolingBuffer; use Slic3r::GCode::CoolingBuffer;
use Slic3r::GCode::Layer; use Slic3r::GCode::Layer;
use Slic3r::GCode::MotionPlanner; use Slic3r::GCode::MotionPlanner;
use Slic3r::GCode::PlaceholderParser;
use Slic3r::GCode::Reader; use Slic3r::GCode::Reader;
use Slic3r::GCode::SpiralVase; use Slic3r::GCode::SpiralVase;
use Slic3r::GCode::VibrationLimit; use Slic3r::GCode::VibrationLimit;

View File

@ -359,46 +359,6 @@ sub validate {
return 1; 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) # min object distance is max(duplicate_distance, clearance_radius)
sub min_object_distance { sub min_object_distance {
my $self = shift; my $self = shift;

View File

@ -9,7 +9,7 @@ use Slic3r::Geometry::Clipper qw(union_ex);
use Slic3r::Surface ':types'; use Slic3r::Surface ':types';
has 'print_config' => (is => 'ro', default => sub { Slic3r::Config::Print->new }); 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 'standby_points' => (is => 'rw');
has 'enable_loop_clipping' => (is => 'rw', default => sub {1}); has 'enable_loop_clipping' => (is => 'rw', default => sub {1});
has 'enable_wipe' => (is => 'rw', default => sub {0}); # at least one extruder has wipe enabled 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 # append custom toolchange G-code
if (defined $self->extruder && $self->print_config->toolchange_gcode) { 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, previous_extruder => $self->extruder->id,
next_extruder => $extruder_id, next_extruder => $extruder_id,
}); });
@ -748,9 +748,4 @@ sub set_bed_temperature {
return $gcode; return $gcode;
} }
sub replace_variables {
my ($self, $string, $extra) = @_;
return $self->print_config->replace_options($string, { %{$self->extra_variables}, %{ $extra || {} } });
}
1; 1;

View File

@ -72,7 +72,7 @@ sub process_layer {
# set new layer - this will change Z and force a retraction if retract_layer_change is enabled # 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->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, layer_num => $self->gcodegen->layer->id,
}) . "\n" if $self->print->config->layer_gcode; }) . "\n" if $self->print->config->layer_gcode;

View File

@ -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;

View File

@ -670,16 +670,31 @@ sub export_gcode {
return; return;
} }
# get config before spawning the thread because it needs GetParent and it's not available there # It looks like declaring a local $SIG{__WARN__} prevents the ugly
our $config = $self->skeinpanel->config; # "Attempt to free unreferenced scalar" warning...
our $extra_variables = $self->skeinpanel->extra_variables; 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 # select output file
$self->{output_file} = $main::opt{output}; $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 $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($self->{output_file})), my $dlg = Wx::FileDialog->new($self, 'Save G-code file as:', Slic3r::GUI->output_path(dirname($output_file)),
basename($self->{output_file}), &Slic3r::GUI::SkeinPanel::FILE_WILDCARDS->{gcode}, wxFD_SAVE); basename($output_file), &Slic3r::GUI::SkeinPanel::FILE_WILDCARDS->{gcode}, wxFD_SAVE);
if ($dlg->ShowModal != wxID_OK) { if ($dlg->ShowModal != wxID_OK) {
$dlg->Destroy; $dlg->Destroy;
return; return;
@ -692,10 +707,6 @@ sub export_gcode {
$self->statusbar->StartBusy; $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) { if ($Slic3r::have_threads) {
@_ = (); @_ = ();
@ -704,8 +715,6 @@ sub export_gcode {
$self->{export_thread} = threads->create(sub { $self->{export_thread} = threads->create(sub {
$_thread_self->export_gcode2( $_thread_self->export_gcode2(
$config,
$extra_variables,
$_thread_self->{output_file}, $_thread_self->{output_file},
progressbar => sub { Wx::PostEvent($_thread_self, Wx::PlThreadEvent->new(-1, $PROGRESS_BAR_EVENT, shared_clone([@_]))) }, 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([@_]))) }, message_dialog => sub { Wx::PostEvent($_thread_self, Wx::PlThreadEvent->new(-1, $MESSAGE_DIALOG_EVENT, shared_clone([@_]))) },
@ -727,8 +736,6 @@ sub export_gcode {
}); });
} else { } else {
$self->export_gcode2( $self->export_gcode2(
$config,
$extra_variables,
$self->{output_file}, $self->{output_file},
progressbar => sub { progressbar => sub {
my ($percent, $message) = @_; my ($percent, $message) = @_;
@ -747,7 +754,7 @@ sub export_gcode {
sub export_gcode2 { sub export_gcode2 {
my $self = shift; my $self = shift;
my ($config, $extra_variables, $output_file, %params) = @_; my ($output_file, %params) = @_;
local $SIG{'KILL'} = sub { local $SIG{'KILL'} = sub {
Slic3r::debugf "Exporting cancelled; exiting thread...\n"; Slic3r::debugf "Exporting cancelled; exiting thread...\n";
Slic3r::thread_cleanup(); Slic3r::thread_cleanup();
@ -756,16 +763,7 @@ sub export_gcode2 {
my $print = $self->{print}; my $print = $self->{print};
eval { 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 = (); my @warnings = ();
local $SIG{__WARN__} = sub { push @warnings, $_[0] }; local $SIG{__WARN__} = sub { push @warnings, $_[0] };
@ -840,7 +838,7 @@ sub _get_export_file {
my $output_file = $main::opt{output}; 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; $output_file =~ s/\.gcode$/$suffix/i;
my $dlg = Wx::FileDialog->new($self, "Save $format file as:", dirname($output_file), 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); basename($output_file), &Slic3r::GUI::SkeinPanel::MODEL_WILDCARD, wxFD_SAVE | wxFD_OVERWRITE_PROMPT);

View File

@ -15,7 +15,7 @@ use Slic3r::Print::State ':steps';
has 'config' => (is => 'ro', default => sub { Slic3r::Config::Print->new }); has 'config' => (is => 'ro', default => sub { Slic3r::Config::Print->new });
has 'default_object_config' => (is => 'ro', default => sub { Slic3r::Config::PrintObject->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 '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 'objects' => (is => 'rw', default => sub {[]});
has 'status_cb' => (is => 'rw'); has 'status_cb' => (is => 'rw');
has 'regions' => (is => 'rw', default => sub {[]}); has 'regions' => (is => 'rw', default => sub {[]});
@ -32,6 +32,9 @@ has 'brim' => (is => 'rw', default => sub { Slic3r::ExtrusionPath::Collection->n
sub apply_config { sub apply_config {
my ($self, $config) = @_; my ($self, $config) = @_;
# apply variables to placeholder parser
$self->placeholder_parser->apply_config($config);
# handle changes to print config # handle changes to print config
my $print_diff = $self->config->diff($config); my $print_diff = $self->config->diff($config);
if (@$print_diff) { if (@$print_diff) {
@ -187,12 +190,6 @@ sub add_model_object {
push @{$self->objects}, $o; 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_SKIRT);
$self->_state->invalidate(STEP_BRIM); $self->_state->invalidate(STEP_BRIM);
} }
@ -825,10 +822,13 @@ sub write_gcode {
print $fh "\n"; 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( my $gcodegen = Slic3r::GCode->new(
print_config => $self->config, print_config => $self->config,
extra_variables => $self->extra_variables, placeholder_parser => $self->placeholder_parser,
layer_count => $self->layer_count, layer_count => $self->layer_count,
); );
$gcodegen->set_extruders($self->extruders); $gcodegen->set_extruders($self->extruders);
@ -853,7 +853,7 @@ sub write_gcode {
} }
}; };
$print_first_layer_temperature->(0); $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); $print_first_layer_temperature->(1);
# set other general things # set other general things
@ -997,7 +997,7 @@ sub write_gcode {
# write end commands to file # write end commands to file
print $fh $gcodegen->retract if $gcodegen->extruder; # empty prints don't even set an extruder print $fh $gcodegen->retract if $gcodegen->extruder; # empty prints don't even set an extruder
print $fh $gcodegen->set_fan(0); 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_used_filament(0);
$self->total_extruded_volume(0); $self->total_extruded_volume(0);
@ -1029,15 +1029,18 @@ sub write_gcode {
# format variables with their values # format variables with their values
sub expanded_output_filepath { sub expanded_output_filepath {
my $self = shift; my $self = shift;
my ($path, $input_file) = @_; my ($path) = @_;
my $extra_variables = {}; return undef if !@{$self->objects};
if ($input_file) { my $input_file = first { defined $_ } map $_->model_object->input_file, @{$self->objects};
@$extra_variables{qw(input_filename input_filename_base)} = parse_filename($input_file); return undef if !defined $input_file;
} else {
# if no input file was supplied, take the first one from our objects my $filename = my $filename_base = basename($input_file);
$input_file = $self->objects->[0]->model_object->input_file // return undef; $filename_base =~ s/\.[^.]+$//; # without suffix
} my $extra = {
input_filename => $filename,
input_filename_base => $filename_base,
};
if ($path && -d $path) { if ($path && -d $path) {
# if output path is an existing directory, we take that and append # 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 # 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 return $self->placeholder_parser->process($path, $extra);
# (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;
} }
sub invalidate_step { sub invalidate_step {

View File

@ -101,7 +101,7 @@ sub model {
$mesh->scale($params{scale}) if $params{scale}; $mesh->scale($params{scale}) if $params{scale};
my $model = Slic3r::Model->new; 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_volume(mesh => $mesh);
$object->add_instance( $object->add_instance(
offset => [0,0], offset => [0,0],

View File

@ -1,4 +1,4 @@
use Test::More tests => 2; use Test::More tests => 6;
use strict; use strict;
use warnings; use warnings;
@ -44,10 +44,27 @@ use Slic3r::Test;
#========================================================== #==========================================================
{ {
my $config = Slic3r::Config->new_from_defaults; my $parser = Slic3r::GCode::PlaceholderParser->new;
is $config->replace_options('[temperature_[foo]]', { foo => '0' }), $parser->apply_config(my $config = Slic3r::Config->new_from_defaults);
200, is $parser->process('[temperature_[foo]]', { foo => '0' }),
$config->temperature->[0],
"nested config options"; "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__ __END__

View File

@ -1,4 +1,4 @@
use Test::More tests => 4; use Test::More tests => 2;
use strict; use strict;
use warnings; 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)'; 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__ __END__