Refactoring: keep height in Flow object and calculate spacing on demand

lib/Slic3r/Fill.pm

@@ -222,7 +222,7 @@ sub make_fill {
         );
         next unless @polylines;
         
-        my $mm3_per_mm = $flow->mm3_per_mm($h);
+        my $mm3_per_mm = $flow->mm3_per_mm;
         
         # save into layer
         push @fills, my $collection = Slic3r::ExtrusionPath::Collection->new;

lib/Slic3r/Layer/Region.pm

@@ -43,19 +43,19 @@ sub make_perimeters {
     
     # external perimeters
     my $ext_perimeter_flow  = $self->flow(FLOW_ROLE_EXTERNAL_PERIMETER);
-    my $ext_mm3_per_mm      = $ext_perimeter_flow->mm3_per_mm($self->height);
+    my $ext_mm3_per_mm      = $ext_perimeter_flow->mm3_per_mm;
     my $ext_pwidth          = $ext_perimeter_flow->scaled_width;
     my $ext_pspacing        = $ext_perimeter_flow->scaled_spacing;
     
     # other perimeters
     my $perimeter_flow      = $self->flow(FLOW_ROLE_PERIMETER);
-    my $mm3_per_mm          = $perimeter_flow->mm3_per_mm($self->height);
+    my $mm3_per_mm          = $perimeter_flow->mm3_per_mm;
     my $pwidth              = $perimeter_flow->scaled_width;
     my $pspacing            = $perimeter_flow->scaled_spacing;
     
     # overhang perimeters
     my $overhang_flow       = $self->region->flow(FLOW_ROLE_PERIMETER, -1, 1, 0, undef, $self->layer->object);
-    my $mm3_per_mm_overhang = $overhang_flow->mm3_per_mm(-1);
+    my $mm3_per_mm_overhang = $overhang_flow->mm3_per_mm;
     
     # solid infill
     my $solid_infill_flow   = $self->flow(FLOW_ROLE_SOLID_INFILL);
@@ -418,7 +418,7 @@ sub _fill_gaps {
     my $flow = $self->flow(FLOW_ROLE_SOLID_INFILL, 0, $w);
     my %path_args = (
         role        => EXTR_ROLE_GAPFILL,
-        mm3_per_mm  => $flow->mm3_per_mm($self->height),
+        mm3_per_mm  => $flow->mm3_per_mm,
         width       => $flow->width,
         height      => $self->height,
     );

lib/Slic3r/Print.pm

@@ -704,7 +704,7 @@ sub make_skirt {
         bridge_flow_ratio   => 0,
     );
     my $spacing = $flow->spacing;
-    my $mm3_per_mm = $flow->mm3_per_mm($first_layer_height);
+    my $mm3_per_mm = $flow->mm3_per_mm;
     
     my @extruders_e_per_mm = ();
     my $extruder_idx = 0;
@@ -763,7 +763,7 @@ sub make_brim {
         layer_height        => $first_layer_height,
         bridge_flow_ratio   => 0,
     );
-    my $mm3_per_mm = $flow->mm3_per_mm($first_layer_height);
+    my $mm3_per_mm = $flow->mm3_per_mm;
     
     my $grow_distance = $flow->scaled_width / 2;
     my @islands = (); # array of polygons

lib/Slic3r/Print/SupportMaterial.pm

@@ -602,7 +602,7 @@ sub generate_toolpaths {
             );
             
             # transform loops into ExtrusionPath objects
-            my $mm3_per_mm = $interface_flow->mm3_per_mm($layer->height);
+            my $mm3_per_mm = $interface_flow->mm3_per_mm;
             @loops = map Slic3r::ExtrusionPath->new(
                 polyline    => $_,
                 role        => EXTR_ROLE_SUPPORTMATERIAL_INTERFACE,
@@ -649,7 +649,7 @@ sub generate_toolpaths {
                     layer_height => $layer->height,
                     complete    => 1,
                 );
-                my $mm3_per_mm = $params->{flow}->mm3_per_mm($layer->height);
+                my $mm3_per_mm = $params->{flow}->mm3_per_mm;
                 
                 push @paths, map Slic3r::ExtrusionPath->new(
                     polyline    => Slic3r::Polyline->new(@$_),
@@ -684,7 +684,7 @@ sub generate_toolpaths {
             } else {
                 # draw a perimeter all around support infill
                 # TODO: use brim ordering algorithm
-                my $mm3_per_mm = $flow->mm3_per_mm($layer->height);
+                my $mm3_per_mm = $flow->mm3_per_mm;
                 push @paths, map Slic3r::ExtrusionPath->new(
                     polyline    => $_->split_at_first_point,
                     role        => EXTR_ROLE_SUPPORTMATERIAL,
@@ -705,7 +705,7 @@ sub generate_toolpaths {
                     layer_height => $layer->height,
                     complete    => 1,
                 );
-                my $mm3_per_mm = $params->{flow}->mm3_per_mm($layer->height);
+                my $mm3_per_mm = $params->{flow}->mm3_per_mm;
                 
                 push @paths, map Slic3r::ExtrusionPath->new(
                     polyline    => Slic3r::Polyline->new(@$_),

xs/src/Flow.cpp

@@ -3,54 +3,81 @@
 
 namespace Slic3r {
 
+/* This constructor builds a Flow object from an extrusion width config setting
+   and other context properties. */
 Flow
 Flow::new_from_config_width(FlowRole role, const ConfigOptionFloatOrPercent &width, float nozzle_diameter, float height, float bridge_flow_ratio) {
     // we need layer height unless it's a bridge
     if (height <= 0 && bridge_flow_ratio == 0) CONFESS("Invalid flow height supplied to new_from_config_width()");
     
     float w;
-    // use automatic extrusion width if user left 0 or we need a bridge flow
+    if (bridge_flow_ratio > 0) {
-    if ((!width.percent && width.value == 0) || bridge_flow_ratio > 0) {
+        // if bridge flow was requested, calculate bridge width
-        w = Flow::_width(role, nozzle_diameter, height, bridge_flow_ratio);
+        w = Flow::_bridge_width(nozzle_diameter, bridge_flow_ratio);
+    } else if (!width.percent && width.value == 0) {
+        // if user left option to 0, calculate a sane default width
+        w = Flow::_auto_width(role, nozzle_diameter, height);
     } else {
+        // if user set a manual value, use it
         w = width.get_abs_value(height);
     }
     
-    Flow flow(w, Flow::_spacing(w, nozzle_diameter, height, bridge_flow_ratio), nozzle_diameter);
+    return Flow(w, height, nozzle_diameter, bridge_flow_ratio > 0);
-    if (bridge_flow_ratio > 0) flow.bridge = true;
-    return flow;
 }
 
+/* This constructor builds a Flow object from a given centerline spacing. */
 Flow
 Flow::new_from_spacing(float spacing, float nozzle_diameter, float height, bool bridge) {
     // we need layer height unless it's a bridge
     if (height <= 0 && !bridge) CONFESS("Invalid flow height supplied to new_from_spacing()");
 
     float w = Flow::_width_from_spacing(spacing, nozzle_diameter, height, bridge);
-    Flow flow(w, spacing, nozzle_diameter);
+    return Flow(w, height, nozzle_diameter, bridge);
-    flow.bridge = bridge;
-    return flow;
 }
 
-double
+/* This method returns the centerline spacing between two adjacent extrusions 
-Flow::mm3_per_mm(float h) {
+   having the same extrusion width (and other properties). */
+float
+Flow::spacing() const {
     if (this->bridge) {
-        return (this->width * this->width) * PI/4.0;
+        return width + BRIDGE_EXTRA_SPACING;
-    } else if (this->width >= (this->nozzle_diameter + h)) {
+    }
+    
+    float min_flow_spacing;
+    if (this->width >= (this->nozzle_diameter + this->height)) {
         // rectangle with semicircles at the ends
-        return this->width * h + (h*h) / 4.0 * (PI-4.0);
+        min_flow_spacing = this->width - this->height * (1 - PI/4.0);
     } else {
         // rectangle with shrunk semicircles at the ends
-        return this->nozzle_diameter * h * (1 - PI/4.0) + h * this->width * PI/4.0;
+        min_flow_spacing = this->nozzle_diameter * (1 - PI/4.0) + this->width * PI/4.0;
+    }
+    return this->width - OVERLAP_FACTOR * (this->width - min_flow_spacing);
+}
+
+/* This method returns extrusion volume per head move unit. */
+double
+Flow::mm3_per_mm() const {
+    if (this->bridge) {
+        return (this->width * this->width) * PI/4.0;
+    } else if (this->width >= (this->nozzle_diameter + this->height)) {
+        // rectangle with semicircles at the ends
+        return this->width * this->height + (this->height*this->height) / 4.0 * (PI-4.0);
+    } else {
+        // rectangle with shrunk semicircles at the ends
+        return this->nozzle_diameter * this->height * (1 - PI/4.0) + this->height * this->width * PI/4.0;
     }
 }
 
+/* This static method returns bridge width for a given nozzle diameter. */
 float
-Flow::_width(FlowRole role, float nozzle_diameter, float height, float bridge_flow_ratio) {
+Flow::_bridge_width(float nozzle_diameter, float bridge_flow_ratio) {
-    if (bridge_flow_ratio > 0) {
+    if (bridge_flow_ratio == 1) return nozzle_diameter;  // optimization to avoid sqrt()
     return sqrt(bridge_flow_ratio * (nozzle_diameter*nozzle_diameter));
-    }
+}
 
+/* This static method returns a sane extrusion width default. */
+float
+Flow::_auto_width(FlowRole role, float nozzle_diameter, float height) {
     // here we calculate a sane default by matching the flow speed (at the nozzle) and the feed rate
     float volume = (nozzle_diameter*nozzle_diameter) * PI/4.0;
     float shape_threshold = nozzle_diameter * height + (height*height) * PI/4.0;
@@ -77,7 +104,7 @@ Flow::_width(FlowRole role, float nozzle_diameter, float height, float bridge_fl
     return width;
 }
 
-
+/* This static method returns the extrusion width value corresponding to the supplied centerline spacing. */
 float
 Flow::_width_from_spacing(float spacing, float nozzle_diameter, float height, bool bridge) {
     if (bridge) {
@@ -96,23 +123,6 @@ Flow::_width_from_spacing(float spacing, float nozzle_diameter, float height, bo
     }
 }
 
-float
-Flow::_spacing(float width, float nozzle_diameter, float height, float bridge_flow_ratio) {
-    if (bridge_flow_ratio > 0) {
-        return width + BRIDGE_EXTRA_SPACING;
-    }
-    
-    float min_flow_spacing;
-    if (width >= (nozzle_diameter + height)) {
-        // rectangle with semicircles at the ends
-        min_flow_spacing = width - height * (1 - PI/4.0);
-    } else {
-        // rectangle with shrunk semicircles at the ends
-        min_flow_spacing = nozzle_diameter * (1 - PI/4.0) + width * PI/4.0;
-    }
-    return width - OVERLAP_FACTOR * (width - min_flow_spacing);
-}
-
 #ifdef SLIC3RXS
 REGISTER_CLASS(Flow, "Flow");
 #endif

xs/src/Flow.hpp

@@ -23,23 +23,26 @@ enum FlowRole {
 class Flow
 {
     public:
-    float width;
+    float width, height, nozzle_diameter;
-    float spacing;
-    float nozzle_diameter;
     bool bridge;
-    coord_t scaled_width;
-    coord_t scaled_spacing;
     
-    Flow(float _w, float _s, float _nd): width(_w), spacing(_s), nozzle_diameter(_nd), bridge(false) {
+    Flow(float _w, float _h, float _nd, bool _bridge = false)
-        this->scaled_width   = scale_(this->width);
+        : width(_w), height(_h), nozzle_diameter(_nd), bridge(_bridge) {};
-        this->scaled_spacing = scale_(this->spacing);
+    float spacing() const;
+    double mm3_per_mm() const;
+    coord_t scaled_width() const {
+        return scale_(this->width);
     };
-    double mm3_per_mm(float h);
+    coord_t scaled_spacing() const {
+        return scale_(this->spacing());
+    };
+    
     static Flow new_from_config_width(FlowRole role, const ConfigOptionFloatOrPercent &width, float nozzle_diameter, float height, float bridge_flow_ratio);
     static Flow new_from_spacing(float spacing, float nozzle_diameter, float height, bool bridge);
     
     private:
-    static float _width(FlowRole role, float nozzle_diameter, float height, float bridge_flow_ratio);
+    static float _bridge_width(float nozzle_diameter, float bridge_flow_ratio);
+    static float _auto_width(FlowRole role, float nozzle_diameter, float height);
     static float _width_from_spacing(float spacing, float nozzle_diameter, float height, bool bridge);
     static float _spacing(float width, float nozzle_diameter, float height, float bridge_flow_ratio);
 };

xs/xsp/Flow.xsp

@@ -7,7 +7,9 @@
 
 %name{Slic3r::Flow} class Flow {
     ~Flow();
-    %name{_new} Flow(float width, float spacing, float nozzle_diameter);
+    %name{_new} Flow(float width, float height, float nozzle_diameter);
+    void set_height(float height)
+        %code{% THIS->height = height; %};
     void set_bridge(bool bridge)
         %code{% THIS->bridge = bridge; %};
     Clone<Flow> clone()
@@ -15,18 +17,16 @@
     
     float width()
         %code{% RETVAL = THIS->width; %};
-    float spacing()
+    float height()
-        %code{% RETVAL = THIS->spacing; %};
+        %code{% RETVAL = THIS->height; %};
     float nozzle_diameter()
         %code{% RETVAL = THIS->nozzle_diameter; %};
     bool bridge()
         %code{% RETVAL = THIS->bridge; %};
-    long scaled_width()
+    float spacing();
-        %code{% RETVAL = THIS->scaled_width; %};
+    long scaled_width();
-    long scaled_spacing()
+    long scaled_spacing();
-        %code{% RETVAL = THIS->scaled_spacing; %};
+    double mm3_per_mm();
-    
-    double mm3_per_mm(float height);
 %{
 
 Flow*