From 934bd43e3560859e85eda83e77f661a419fdd2c4 Mon Sep 17 00:00:00 2001 From: Alessandro Ranellucci Date: Wed, 16 Dec 2015 12:58:06 +0100 Subject: [PATCH] More refactoring on Config XS bindings --- xs/src/libslic3r/PrintConfig.cpp | 8 ++-- xs/src/libslic3r/PrintConfig.hpp | 2 +- xs/src/perlglue.cpp | 70 +++++++++++++++++++------------- xs/src/xsinit.h | 1 + xs/t/15_config.t | 6 +-- xs/t/20_print.t | 6 +-- xs/xsp/Config.xsp | 4 +- 7 files changed, 56 insertions(+), 41 deletions(-) diff --git a/xs/src/libslic3r/PrintConfig.cpp b/xs/src/libslic3r/PrintConfig.cpp index 460b036fb..8296f5dbb 100644 --- a/xs/src/libslic3r/PrintConfig.cpp +++ b/xs/src/libslic3r/PrintConfig.cpp @@ -304,7 +304,7 @@ PrintConfigDef::PrintConfigDef() Options["filament_settings_id"].type = coString; Options["filament_settings_id"].default_value = new ConfigOptionString(""); - Options["fill_angle"].type = coInt; + Options["fill_angle"].type = coFloat; Options["fill_angle"].label = "Fill angle"; Options["fill_angle"].category = "Infill"; 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."; @@ -1220,19 +1220,19 @@ PrintConfigDef::PrintConfigDef() Options["use_firmware_retraction"].label = "Use firmware retraction"; Options["use_firmware_retraction"].tooltip = "This experimental setting uses G10 and G11 commands to have the firmware handle the retraction. This is only supported in recent Marlin."; Options["use_firmware_retraction"].cli = "use-firmware-retraction!"; - Options["use_firmware_retraction"].default_value = new ConfigOptionFloat(false); + Options["use_firmware_retraction"].default_value = new ConfigOptionBool(false); Options["use_relative_e_distances"].type = coBool; Options["use_relative_e_distances"].label = "Use relative E distances"; Options["use_relative_e_distances"].tooltip = "If your firmware requires relative E values, check this, otherwise leave it unchecked. Most firmwares use absolute values."; Options["use_relative_e_distances"].cli = "use-relative-e-distances!"; - Options["use_relative_e_distances"].default_value = new ConfigOptionFloat(false); + Options["use_relative_e_distances"].default_value = new ConfigOptionBool(false); Options["use_volumetric_e"].type = coBool; Options["use_volumetric_e"].label = "Use volumetric E"; Options["use_volumetric_e"].tooltip = "This experimental setting uses outputs the E values in cubic millimeters instead of linear millimeters. If your firmware doesn't already know filament diameter(s), you can put commands like 'M200 D[filament_diameter_0] T0' in your start G-code in order to turn volumetric mode on and use the filament diameter associated to the filament selected in Slic3r. This is only supported in recent Marlin."; Options["use_volumetric_e"].cli = "use-volumetric-e!"; - Options["use_volumetric_e"].default_value = new ConfigOptionFloat(false); + Options["use_volumetric_e"].default_value = new ConfigOptionBool(false); Options["vibration_limit"].type = coFloat; Options["vibration_limit"].label = "Vibration limit (deprecated)"; diff --git a/xs/src/libslic3r/PrintConfig.hpp b/xs/src/libslic3r/PrintConfig.hpp index cfcb75d60..41add7f2c 100644 --- a/xs/src/libslic3r/PrintConfig.hpp +++ b/xs/src/libslic3r/PrintConfig.hpp @@ -170,7 +170,7 @@ class PrintRegionConfig : public virtual StaticPrintConfig ConfigOptionFloatOrPercent external_perimeter_speed; ConfigOptionBool external_perimeters_first; ConfigOptionBool extra_perimeters; - ConfigOptionInt fill_angle; + ConfigOptionFloat fill_angle; ConfigOptionPercent fill_density; ConfigOptionEnum fill_pattern; ConfigOptionFloat gap_fill_speed; diff --git a/xs/src/perlglue.cpp b/xs/src/perlglue.cpp index 0eda2ebbf..8098fd980 100644 --- a/xs/src/perlglue.cpp +++ b/xs/src/perlglue.cpp @@ -73,62 +73,76 @@ ConfigBase__get(ConfigBase* THIS, const t_config_option_key &opt_key) { if (opt == NULL) return &PL_sv_undef; const ConfigOptionDef* def = THIS->def->get(opt_key); - if (def->type == coFloat) { - ConfigOptionFloat* optv = dynamic_cast(opt); + return ConfigOption_to_SV(*opt, *def); +} + +SV* +ConfigOption_to_SV(const ConfigOption &opt, const ConfigOptionDef &def) { + if (def.type == coFloat) { + const ConfigOptionFloat* optv = dynamic_cast(&opt); + if (optv == NULL) { + printf("opt_key = %s\n", def.label.c_str()); + } return newSVnv(optv->value); - } else if (def->type == coFloats) { - ConfigOptionFloats* optv = dynamic_cast(opt); + } else if (def.type == coFloats) { + const ConfigOptionFloats* optv = dynamic_cast(&opt); AV* av = newAV(); av_fill(av, optv->values.size()-1); - for (std::vector::iterator it = optv->values.begin(); it != optv->values.end(); ++it) + for (std::vector::const_iterator it = optv->values.begin(); it != optv->values.end(); ++it) av_store(av, it - optv->values.begin(), newSVnv(*it)); return newRV_noinc((SV*)av); - } else if (def->type == coPercent) { - ConfigOptionPercent* optv = dynamic_cast(opt); + } else if (def.type == coPercent) { + const ConfigOptionPercent* optv = dynamic_cast(&opt); return newSVnv(optv->value); - } else if (def->type == coInt) { - ConfigOptionInt* optv = dynamic_cast(opt); + } else if (def.type == coInt) { + const ConfigOptionInt* optv = dynamic_cast(&opt); + if (optv == NULL) { + printf("opt_key = %s\n", def.label.c_str()); + } return newSViv(optv->value); - } else if (def->type == coInts) { - ConfigOptionInts* optv = dynamic_cast(opt); + } else if (def.type == coInts) { + const ConfigOptionInts* optv = dynamic_cast(&opt); AV* av = newAV(); av_fill(av, optv->values.size()-1); - for (std::vector::iterator it = optv->values.begin(); it != optv->values.end(); ++it) + for (std::vector::const_iterator it = optv->values.begin(); it != optv->values.end(); ++it) av_store(av, it - optv->values.begin(), newSViv(*it)); return newRV_noinc((SV*)av); - } else if (def->type == coString) { - ConfigOptionString* optv = dynamic_cast(opt); + } else if (def.type == coString) { + const ConfigOptionString* optv = dynamic_cast(&opt); // we don't serialize() because that would escape newlines return newSVpvn_utf8(optv->value.c_str(), optv->value.length(), true); - } else if (def->type == coStrings) { - ConfigOptionStrings* optv = dynamic_cast(opt); + } else if (def.type == coStrings) { + const ConfigOptionStrings* optv = dynamic_cast(&opt); AV* av = newAV(); av_fill(av, optv->values.size()-1); - for (std::vector::iterator it = optv->values.begin(); it != optv->values.end(); ++it) + for (std::vector::const_iterator it = optv->values.begin(); it != optv->values.end(); ++it) av_store(av, it - optv->values.begin(), newSVpvn_utf8(it->c_str(), it->length(), true)); return newRV_noinc((SV*)av); - } else if (def->type == coPoint) { - ConfigOptionPoint* optv = dynamic_cast(opt); + } else if (def.type == coPoint) { + const ConfigOptionPoint* optv = dynamic_cast(&opt); return perl_to_SV_clone_ref(optv->value); - } else if (def->type == coPoints) { - ConfigOptionPoints* optv = dynamic_cast(opt); + } else if (def.type == coPoints) { + const ConfigOptionPoints* optv = dynamic_cast(&opt); AV* av = newAV(); av_fill(av, optv->values.size()-1); - for (Pointfs::iterator it = optv->values.begin(); it != optv->values.end(); ++it) + for (Pointfs::const_iterator it = optv->values.begin(); it != optv->values.end(); ++it) av_store(av, it - optv->values.begin(), perl_to_SV_clone_ref(*it)); return newRV_noinc((SV*)av); - } else if (def->type == coBool) { - ConfigOptionBool* optv = dynamic_cast(opt); + } else if (def.type == coBool) { + const ConfigOptionBool* optv = dynamic_cast(&opt); + if (optv == NULL) { + printf("opt_key = %s\n", def.label.c_str()); + } return newSViv(optv->value ? 1 : 0); - } else if (def->type == coBools) { - ConfigOptionBools* optv = dynamic_cast(opt); + } else if (def.type == coBools) { + const ConfigOptionBools* optv = dynamic_cast(&opt); AV* av = newAV(); av_fill(av, optv->values.size()-1); - for (std::vector::iterator it = optv->values.begin(); it != optv->values.end(); ++it) + for (std::vector::const_iterator it = optv->values.begin(); it != optv->values.end(); ++it) av_store(av, it - optv->values.begin(), newSViv(*it ? 1 : 0)); return newRV_noinc((SV*)av); } else { - std::string serialized = opt->serialize(); + std::string serialized = opt.serialize(); return newSVpvn_utf8(serialized.c_str(), serialized.length(), true); } } diff --git a/xs/src/xsinit.h b/xs/src/xsinit.h index b5b1d83f4..69ac337f7 100644 --- a/xs/src/xsinit.h +++ b/xs/src/xsinit.h @@ -113,6 +113,7 @@ public: }; SV* ConfigBase__as_hash(ConfigBase* THIS); +SV* ConfigOption_to_SV(const ConfigOption &opt, const ConfigOptionDef &def); SV* ConfigBase__get(ConfigBase* THIS, const t_config_option_key &opt_key); SV* ConfigBase__get_at(ConfigBase* THIS, const t_config_option_key &opt_key, size_t i); bool ConfigBase__set(ConfigBase* THIS, const t_config_option_key &opt_key, SV* value); diff --git a/xs/t/15_config.t b/xs/t/15_config.t index a4d7751ea..838ce18b0 100644 --- a/xs/t/15_config.t +++ b/xs/t/15_config.t @@ -6,7 +6,7 @@ use warnings; use Slic3r::XS; use Test::More tests => 110; -foreach my $config (Slic3r::Config->new, Slic3r::Config::Full->new) { +foreach my $config (Slic3r::Config->new, Slic3r::Config::Static::new_FullPrintConfig) { $config->set('layer_height', 0.3); ok abs($config->get('layer_height') - 0.3) < 1e-4, 'set/get float'; is $config->serialize('layer_height'), '0.3', 'serialize float'; @@ -140,13 +140,13 @@ foreach my $config (Slic3r::Config->new, Slic3r::Config::Full->new) { # test that no crash happens when using set_deserialize() with a key that hasn't been set() yet $config->set_deserialize('filament_diameter', '3'); - my $config2 = Slic3r::Config::Full->new; + my $config2 = Slic3r::Config::Static::new_FullPrintConfig; $config2->apply_dynamic($config); is $config2->get('perimeters'), 2, 'apply_dynamic'; } { - my $config = Slic3r::Config::Full->new; + my $config = Slic3r::Config::Static::new_FullPrintConfig; my $config2 = Slic3r::Config->new; $config2->apply_static($config); is $config2->get('perimeters'), Slic3r::Config::print_config_def()->{perimeters}{default}, 'apply_static and print_config_def'; diff --git a/xs/t/20_print.t b/xs/t/20_print.t index bef4d050c..e535cdd8c 100644 --- a/xs/t/20_print.t +++ b/xs/t/20_print.t @@ -9,9 +9,9 @@ use Test::More tests => 5; { my $print = Slic3r::Print->new; isa_ok $print, 'Slic3r::Print'; - isa_ok $print->config, 'Slic3r::Config::Print::Ref'; - isa_ok $print->default_object_config, 'Slic3r::Config::PrintObject::Ref'; - isa_ok $print->default_region_config, 'Slic3r::Config::PrintRegion::Ref'; + isa_ok $print->config, 'Slic3r::Config::Static::Ref'; + isa_ok $print->default_object_config, 'Slic3r::Config::Static::Ref'; + isa_ok $print->default_region_config, 'Slic3r::Config::Static::Ref'; isa_ok $print->placeholder_parser, 'Slic3r::GCode::PlaceholderParser::Ref'; } diff --git a/xs/xsp/Config.xsp b/xs/xsp/Config.xsp index b37a49954..a2db75999 100644 --- a/xs/xsp/Config.xsp +++ b/xs/xsp/Config.xsp @@ -95,7 +95,6 @@ SV* print_config_def() CODE: t_optiondef_map &def = Slic3r::print_config_def.options; - FullPrintConfig config; HV* options_hv = newHV(); for (t_optiondef_map::iterator oit = def.begin(); oit != def.end(); ++oit) { @@ -179,7 +178,8 @@ print_config_def() (void)hv_stores( hv, "labels", newRV_noinc((SV*)av) ); } - (void)hv_stores( hv, "default", ConfigBase__get(&config, opt_key) ); + if (optdef->default_value != NULL) + (void)hv_stores( hv, "default", ConfigOption_to_SV(*optdef->default_value, *optdef) ); (void)hv_store( options_hv, opt_key.c_str(), opt_key.length(), newRV_noinc((SV*)hv), 0 ); }