Contact Z distance cannot be changed to values other than the defaults #2752

Closed
cocoknight opened this Issue Mar 22, 2015 · 8 comments

Projects

None yet

4 participants

@cocoknight

It is not possible to use other values for the "contact Z distance" for support materials. If I put in a value different to the defaults it is changed back to 0 as soon as a change enter another option.
(1.2.6 on Windows 7 x64)

@alexrj
Owner
alexrj commented Mar 23, 2015

What value are you entering for example?

@umfan92
umfan92 commented Mar 23, 2015

I experienced the same issue. I was just playing around with the settings. I tried everything from 0.10 to 0.18. I also tried to leave the gap equal to one layer height. So if my layer height was 0.16, I tried to leave the gap as 0.16mm as well. After saving the settings and slicing the object, it seems to revert to 0 or 0.2 mm. I'm also using 1.2.6 on Windows 7 x64.

@cocoknight

I set it up just how umfan92 explained. I tried using different distances to assess the change on the support material adhesion and peelability. I used values between 0.1 to 0.3mm but it always reset after leving the tab.

@alexrj
Owner
alexrj commented Mar 23, 2015

I suspect this might be a Windows-specific issue.

@alexrj alexrj added this to the 1.2.7 milestone Mar 23, 2015
@alexrj alexrj added the OS: Windows label Mar 23, 2015
@markwal
Contributor
markwal commented Mar 24, 2015

Repros on Ubuntu 14.04 for me though I had to back off Wx to version 0.9923 (Build.PL only requires 0.9918, but cpanm tried to install 0.9926 which doesn't compile on my machine). I could imagine that it is something to do with the widgets, perhaps.

OS: Ubuntu 14.04
Version 1.2.7-dev: 13b63d0
config.ini: https://www.dropbox.com/s/pelthf400dw1gpv/testconfig.ini?dl=0

Steps:

  • choose Print Settings.Support material
  • type a value other than 0 or 0.2 in Contact Z distance
  • hit "tab" three times

Result:
Contact Z distance snaps to 0 when the control focus leaves pattern spacing. These aren't the only steps that produce the result, but they're a quick and reliable way to get it to happen.

Expected:
No change to contact Z distance.

I'll take a gander and see if I can figure it out.

@markwal
Contributor
markwal commented Mar 24, 2015

It's in Slic3r::GUI::OptionsGroup::Field::NumericChoice::set_value:

sub set_value {
    my ($self, $value) = @_;

    $self->disable_change_event(1);

    my $field = $self->wxWindow;
    if ($self->option->gui_flags =~ /\bshow_value\b/) {
        $field->SetValue($value);
    } else {
        if ($self->option->values) {
            # check whether we have a value index
            my $value_idx = first { $self->option->values->[$_] eq $value } 0..$#{$self->option->values};
            if (defined $value_idx) {
                $field->SetSelection($value_idx);
                $self->disable_change_event(0);
                return;
            }
        }
        if ($self->option->labels && $value <= $#{$self->option->labels}) {
#------- next line stomps on the users input
            $field->SetValue($self->option->labels->[$value]);
            $self->disable_change_event(0);
            return;
        }
        $field->SetValue($value);
    }

    $self->disable_change_event(0);
}

I think what is happening here is that it's trying to use the labels (since !show_value) but it's using the value as an index, but the value is actually the measurement in mm, not the index. It rounds any fractional measurement to 0 and you git the first label.

For this field, that whole if seems unnecessary since the SetSelection above will already cover the case where the value matches a label in SetSelection, but perhaps there is a different kind of NumericChoice where there isn't a values array and $value is an index? If so, then the fix would be to make that if an else if? I'll poke around a little more.

@markwal
Contributor
markwal commented Mar 24, 2015

@alexrj If I understand PrintConfig.cpp correctly, there are only three options of this type: "extruder", "fill_density" and "support_material_contact_distance". "fill_density" uses show_value, so takes the first if. "extruder" only has labels, not values, so I conclude that the label lookup by index is for that case. So I propose the following:

diff --git a/lib/Slic3r/GUI/OptionsGroup/Field.pm b/lib/Slic3r/GUI/OptionsGroup/Field.pm
index 13b86cd..b5c8981 100644
--- a/lib/Slic3r/GUI/OptionsGroup/Field.pm
+++ b/lib/Slic3r/GUI/OptionsGroup/Field.pm
@@ -296,7 +296,7 @@ sub set_value {
                 return;
             }
         }
-        if ($self->option->labels && $value <= $#{$self->option->labels}) {
+        elsif ($self->option->labels && $value <= $#{$self->option->labels}) {
             $field->SetValue($self->option->labels->[$value]);
             $self->disable_change_event(0);
             return;

This also matches the logic of the constructor.

While testing out the extruder box before and after this change I found a couple of other bugs, one Windows only, the other in both Ubuntu and Windows, but since they both repro without the fix, I think they aren't regressions of my proposal above. I'll report them separately.

@alexrj
Owner
alexrj commented Mar 27, 2015

@markwal, thank you for investigating this. Your patch is indeed correct. I also made a minor fix in the constructor. Maybe this fixes your other issues?

@alexrj alexrj closed this Mar 27, 2015
@alexrj alexrj added Fixed and removed OS: Windows labels Mar 27, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment