Fade height in UBL can cause inconsistent Z position

[qx1147]

I am intending to use UBL with fading on a Cartesian printer, but something doesn't add up. I think that the use of fading can make current_position[Z] inconsistent with the physical stepper position by taking fading into account at the planner level but not when disabling/enabling bed leveling via set_bed_leveling_enabled(). This can screw up Z homing but also Grid Based Leveling (G29 J) and probably other things.

Just one example: I have UBL activated after reset, and auto-activation of UBL after Z homing is enabled. Fade height is set to 3mm, which happens to be very close to the Z height after homing (using BLtouch probe). Now, after homing (G28), Marlin reports the current Z coordinate (2.98, terminal window and LCD), but executing G1 Z2.98 actually causes the head to move a bit down, even though the Z coordinate reported and shown afterwards is still 2.98.

This is what happens: For Z homing, UBL is disabled at the planner-level and re-enabled afterwards. Before re-enabling, Z is reported to be +3.48. My bed is +0.5mm at the homing XY position (according to G29 T). So after UBL is re-enabled via set_bed_leveling_enabled() at the end of the Z homing procedure, Z is reported to be +2.98 (3.48-0.5). Obviously, this is in anticipation of the planner taking care of bed leveling correction from now on, i.e. the planner is assumed to add 0.5 to Z at the current physical head position. But due to fading and Z being basically at fade height already, the planner would actually not do much of Z correction. It still uses the stepper position though which corresponds to Z=+3.48 when planning the move. So when handling the G1 Z2.98 command, the planner only adds some tiny value due to the almost faded leveling correction and makes a move to the stepper position according to Z=+2.98+epsilon, which is actually a move by -0.5mm, even though the current_position[Z] hasn't changed.

The fix might seem simple: just take fading into account in set_bed_leveling_enabled(). But it becomes rather ugly when trying to keep current_position[Z] consistent with the planner's stepper position in case the fade height is changed after homing (or G29 J) instead of before (set_z_fade_height() and planner.set_z_fade_height()).

[Roxy-3D]

Can you post a G29 W T for us? Usually... Problems with Fade Height are result of the mesh points not being close to 0.00 mm. If you have your Z_PROBE_OFFSET_FROM_EXTRUDER value incorrect, your mesh will not be centered about 0.00mm (which it should be!!!!)..

[qx1147]

Weird request, given that I described the root cause already. But apparently the buzz words "fade height" and "Z position" trigger the inevitable request for G29 W T output. Well, if it helps:

echo:Home XYZ first
Unified Bed Leveling System v1.01 active.
Mesh 0 Loaded.
UBL object count: 1
planner.z_fade_height : 3.0000
# of samples: 100
Mean Mesh Height: -0.000000
Standard Deviation: 0.315843
zprobe_zoffset: -0.3500000
MESH_MIN_X  ((((((240) / 2) - (240) / 2) + 1)>(0)?((((240) / 2) - (240) / 2) + 1):(0)))=1
MESH_MIN_Y  ((((((210) / 2) - (210) / 2) + 1)>(0)?((((210) / 2) - (210) / 2) + 1):(0)))=1
MESH_MAX_X  ((((((240) / 2) + (240) / 2) - (1))<(240)?((((240) / 2) + (240) / 2) - (1)):(240)))=239
MESH_MAX_Y  ((((((210) / 2) + (210) / 2) - (1))<(210)?((((210) / 2) + (210) / 2) - (1)):(210)))=209
GRID_MAX_POINTS_X  10
GRID_MAX_POINTS_Y  10
MESH_X_DIST  26.44
MESH_Y_DIST  23.11
X-Axis Mesh Points at: 1.000  27.444  53.889  80.333  106.778  133.222  159.667  186.111  212.556  239.000  
Y-Axis Mesh Points at: 1.000  24.111  47.222  70.333  93.444  116.556  139.667  162.778  185.889  209.000  
Kill pin on :32  state:1

Unified Bed Leveling sanity checks passed.

Bed Topography Report:

    (  1,209)                                                              (239,209)
        0       1       2       3       4       5       6       7       8       9      
 9 | +0.480  +0.346  +0.245  +0.168  +0.082  -0.013  -0.112  -0.216  -0.333  -0.469 
   |
 8 | +0.536  +0.404  +0.298  +0.206  +0.100  -0.015  -0.108  -0.177  -0.281  -0.444 
   |
 7 | +0.499  +0.406  +0.307  +0.200  +0.087  -0.028  -0.120  -0.189  -0.294  -0.457 
   |
 6 | +0.415  +0.375  +0.288  +0.170  +0.059  -0.044  -0.140  -0.229  -0.343  -0.493 
   |
 5 | +0.396  +0.367  +0.283  +0.162  +0.051  -0.049  -0.145  -0.241  -0.361  -0.516 
   |
 4 | +0.433  +0.382  +0.292  +0.175  +0.063  -0.042  -0.139  -0.228  -0.352  -0.526 
   |
 3 | +0.438  +0.403  +0.316  +0.193  +0.073  -0.040  -0.138  -0.224  -0.353  -0.549 
   |
 2 | +0.399  +0.426  +0.353  +0.211  +0.077  -0.044  -0.148  -0.235  -0.373  -0.589 
   |
 1 | +0.395  +0.410  +0.328  +0.181  +0.043  -0.080  -0.191  -0.292  -0.437  -0.647 
   |
 0 |[+0.459] +0.340  +0.212  +0.080  -0.043  -0.158  -0.278  -0.408  -0.554  -0.723 
        0       1       2       3       4       5       6       7       8       9      
    (  1,  1)                                                              (239,  1)

[Roxy-3D]

Just one example: I have UBL activated after reset, and auto-activation of UBL after Z homing is enabled. Fade height is set to 3mm,

I'm not sure a 3mm fade height makes sense with some of your mesh point values being in the .5 mm range. That is a lot of 'correction' to be done in a small amount of range.

The fix might seem simple: just take fading into account in set_bed_leveling_enabled(). But it becomes rather ugly when trying to keep current_position[Z] consistent with the planner's stepper position in case the fade height is changed after homing (or G29 J) instead of before (set_z_fade_height() and planner.set_z_fade_height()).

Yeah... I think you are correct. The root of the problem is UBL never used the planner to do any of the needed corrections. But as the code was cleaned up planner methods were called (for all bed leveling types). UBL always calculates how to move to the destination coordinates. A pre-move should not be necessary. (If the nozzle is in the wrong position, it will be fixed by the end of the first move.)

I'm pretty busy with other items... Can you make a change to fix the item and give it a quick test? If so... I'll get the code updated with your changes.

Update: As a work around... I think if your turn off ARC_SUPPORT and BEZIER_CURVE_SUPPORT, it might make the problem go away...

[qx1147]

I'm not sure a 3mm fade height makes sense with some of your mesh point values being in the .5 mm range. That is a lot of 'correction' to be done in a small amount of range.

Depends totally on the size of the footprint of the part. I use G29 J like a second Z homing, actually with X and Y parameters which define roughly the size of the region being used for the particular print and the according locations of the grid points. That way I not only optimize the flatness/tilt within the region of interest but also minimize the maximal required correction factor and make its mean within this region zero. This helps with making the Z size of the object as accurate as possible and minimizes the Z correction that has to be faded out.

A pre-move should not be necessary. (If the nozzle is in the wrong position, it will be fixed by the end of the first move.)

I don't really follow. The problem is really more fundamental than having a delayed move. In the end, all moves are relative, so keeping the current position in mm-space in sync with current position in steps-space is essential. An out-of-sync situation is not just fixed with the next move.

Can you make a change to fix the item and give it a quick test? If so... I'll get the code updated with your changes.

Yes, I can do that, at least as far as set_bed_leveling_enabled() is concerned. I am not sure yet how to best handle changes to the fade height.

Update: As a work around... I think if your turn off ARC_SUPPORT and BEZIER_CURVE_SUPPORT, it might make the problem go away...

How so?

[Roxy-3D]

A pre-move should not be necessary. (If the nozzle is in the wrong position, it will be fixed by the end of the first move.)

I don't really follow. The problem is really more fundamental than having a delayed move. In the end, all moves are relative, so keeping the current position in mm-space in sync with current position in steps-space is essential. An out-of-sync situation is not just fixed with the next move.

UBL pretty much only looks at the destination coordinates. And any Z-Height correction is factored into that. And then the move is scheduled. Even if the current Z Height is wrong... It should go to the calculated and correct Z-Height at the end of the next move. (But this assumes that current_position[] is not altered to a bad value.)

Can you make a change to fix the item and give it a quick test? If so... I'll get the code updated with your changes.

Yes, I can do that, at least as far as set_bed_leveling_enabled() is concerned. I am not sure yet how to best handle changes to the fade height.

Great! Thank You!

Update: As a work around... I think if your turn off ARC_SUPPORT and BEZIER_CURVE_SUPPORT, it might make the problem go away...

How so?

This is speculation. But if the problem you are seeing is new... Changes have been recently made to Planner:::apply_leveling(). Specifically, to handle the HAS_UBL_AND_CURVES case. Pretty much, for the Cartesian case, ubl_motion.cpp handles all the corrections. That includes the Z Height changes due to position within a mesh cell and the fade of that value. But all of a sudden... Now we have apply_leveling(float &rx, float &ry, float &rz) zapping the values it is called with... And that is not consistent with the philosophy that was used when UBL was written. I think there is a fair chance that if you change the UBL case to add 0.0 to rz in Planner::apply_leveling() (and a similar change to Planner::unapply_leveling() ) the problem may just disappear.

#if PLANNER_LEVELING || HAS_UBL_AND_CURVES
  /**
   * rx, ry, rz - Cartesian positions in mm
   *              Leveled XYZ on completion
   */
  void Planner::apply_leveling(float &rx, float &ry, float &rz) {

    #if ENABLED(SKEW_CORRECTION)
      skew(rx, ry, rz);
    #endif

    if (!leveling_active) return;

    #if ABL_PLANAR

      float dx = rx - (X_TILT_FULCRUM),
            dy = ry - (Y_TILT_FULCRUM);

      apply_rotation_xyz(bed_level_matrix, dx, dy, rz);

      rx = dx + X_TILT_FULCRUM;
      ry = dy + Y_TILT_FULCRUM;

    #elif HAS_MESH

      #if ENABLED(ENABLE_LEVELING_FADE_HEIGHT)
        const float fade_scaling_factor = fade_scaling_factor_for_z(rz);
      #else
        constexpr float fade_scaling_factor = 1.0;
      #endif

      #if ENABLED(AUTO_BED_LEVELING_BILINEAR)
        const float raw[XYZ] = { rx, ry, 0 };
      #endif

      rz += (
        #if ENABLED(MESH_BED_LEVELING)
          mbl.get_z(rx, ry
            #if ENABLED(ENABLE_LEVELING_FADE_HEIGHT)
              , fade_scaling_factor
            #endif
          )
        #elif ENABLED(AUTO_BED_LEVELING_UBL)
//        fade_scaling_factor ? fade_scaling_factor * ubl.get_z_correction(rx, ry) : 0.0
           0.0;      // Try this.   This might fix your problem!  Please make a
                     // similar change to Planner::unapply_leveling()
        #elif ENABLED(AUTO_BED_LEVELING_BILINEAR)
          fade_scaling_factor ? fade_scaling_factor * bilinear_z_offset(raw) : 0.0
        #endif
      );

    #endif
  }
#endif

[gloomyandy]

Interesting! It looks like your mesh has been adjusted to have a zero mean. I wonder if this may be a reason why you are seeing this problem, but others may not? It looks like that as a side effect of zeroing the mean you have a relatively large zoffset (0.5ish) at the home position, I suspect that most people will have a virtually zero offset at this position (because for an unadjusted mesh the home position will be the "base offset"). This in turn will mean that for most folks virtually no adjustment is applied and so this bug may not show up. Just a thought really, the bug may not be new.

Oh and do you really need to zero adjust the mesh? If you are using G29 J then I think this will in effect do the adjustment for you. I also use G29 J in a similar way, and have often wondered about zeroing the mean for the mesh, but came to the conclusion it wasn't really needed. I may be wrong though!

[Roxy-3D]

I also use G29 J in a similar way, and have often wondered about zeroing the mean for the mesh, but came to the conclusion it wasn't really needed. I may be wrong though!

In theory... the mesh should not have to have a zero mean. And my guess is, people very often enable or disable leveling at the location where the Z-Axis is homed. So that probably masks issues.

Either way... I'm all for getting things working regardless of the order or where people do things!

[qx1147]

@gloomyandy A absolutely agree with all you said.

@Roxy-3D

Either way... I'm all for getting things working regardless of the order or where people do things!

Good to hear, so we are on the same page.

[Roxy-3D]

@qx1147 Did you get a chance to try those changes to Planner::apply_leveling() and Planner::unapply_leveling() ?

[qx1147]

@qx1147 Did you get a chance to try those changes to Planner::apply_leveling() and Planner::unapply_leveling() ?

I have made the following changes to set_bed_leveling_enabled(). I include here just the ENABLED(AUTO_BED_LEVELING_UBL) branch which also has the PLANNER_LEVELING case, which is not active in my case as I don't use SKEW_CORRECTION.

      #elif ENABLED(AUTO_BED_LEVELING_UBL)
        #if PLANNER_LEVELING
          if (planner.leveling_active) {                       // leveling from on to off
            // change unleveled current_position to physical current_position without moving steppers.
            planner.apply_leveling(current_position[X_AXIS], current_position[Y_AXIS], current_position[Z_AXIS]);
            planner.leveling_active = false;                   // disable only AFTER calling apply_leveling
          }
          else {                                               // leveling from off to on
            planner.leveling_active = true;                    // enable BEFORE calling unapply_leveling, otherwise ignored
            // change physical current_position to unleveled current_position without moving steppers.
            planner.unapply_leveling(current_position);
          }
        #else
          // UBL equivalents for apply/unapply_leveling.
          if (planner.leveling_active) {
            set_current_from_steppers_for_axis(Z_AXIS);            
            planner.leveling_active = false;
          }
          else {
            #if ENABLED(ENABLE_LEVELING_FADE_HEIGHT)
              float zcorr = ubl.get_z_correction(current_position[X_AXIS], current_position[Y_AXIS]);
              if (planner.z_fade_height) {
                if (current_position[Z_AXIS]<planner.z_fade_height) {      
                  if (planner.z_fade_height - zcorr > 1e-12)
                    current_position[Z_AXIS] = (current_position[Z_AXIS] - zcorr) * planner.z_fade_height / (planner.z_fade_height - zcorr);
                }
              }
              else
                current_position[Z_AXIS] -= zcorr;
            #else
              current_position[Z_AXIS] -= ubl.get_z_correction(current_position[X_AXIS], current_position[Y_AXIS]);
            #endif
            planner.leveling_active = true;
          }
        #endif 

Obviously, I removed the SKEW_CORRECTION as it never is relevant here anyway due to PLANNER_LEVELING being set when SKEW_CORRECTION is set (strange that PLANNER_LEVELING is not used irrespective of SKEW_CORRECTION) . Doing the inverse operation of fading turned out to be more complicated than anticipated (and the implementation in planner.unapply_leveling() seems wrong in this respect). One has to base the calculation of the fading factor on the uncorrected Z (which we want to know), not the corrected Z (which is given), so one has to resolve the fading equation for the uncorrected Z. Although the case zcorr>planner.z_fade_height is of no practical use, I included the according condition anyway in order to prevent division by zero and alike.

The whole thing can only work accurately if the unapply_leveling correctly anticipates what the planner will be doing, which includes the calculation of the mesh value. But since the planner is not using ubl.get_z_correction() and does things slightly differently for xy positions outside the mesh (which is a possibility if MESH_INSET>0), one might get into trouble when calling set_bed_leveling_enabled() while the current xy position is outside the bed (e.g., during homing?). I think one still is safe on the X/Y_MIN sides, but not so much on the X/Y_MAX sides. BTW: unified_bed_leveling::line_to_destination_cartesian() could use a code review, I think: For example, get_cell_index_x() returns clamped values, so the WITHIN()-conditions further below, before FINAL_MOVE, apparently never catch on to prevent e.g. (cell_dest_xi + 1) from being a potentially invalid index into z_values[].

[thinkyhead]

strange that PLANNER_LEVELING is not used irrespective of SKEW_CORRECTION

Well, the thing is that planner.(un)apply_leveling simply isn't used for UBL because UBL does all its leveling adjustment internally (unless SEGMENT_LEVELED_MOVES is enabled).

But, when skew correction is enabled, then it is done for all movement by planner.apply_leveling which is called from the low-level planner methods.

It's all a bit of a muddle at the moment. At some point we may bring UBL more in line with the other leveling systems, but the way it works now is so UBL can run optimally with the least amount of redundant calculation and outside function calls.

[qx1147]

But, when skew correction is enabled, then it is done for all movement by planner.apply_leveling which is called from the low-level planner methods.

Yeah, that's what I don't understand. Why would skew correction make a difference regarding splitting up moves? Skewing is a linear operation (and UBL is), so skewing should not require additional moves within the mesh cells, and UBL optimization could still be used even with SKEW_CORRECTION on.

[thinkyhead]

Why would skew correction make a difference regarding splitting up moves?

I'm not sure I understand the question. Skew is applied to a move's destination point just before it's sent to whatever function will do further processing on the way to the planner.

So if you have some long linear move, only the final endpoint of that line needs to get skew applied. The skewed destination point will then be passed on to the UBL cartesian move handler, and UBL will then just segment the line as it usually does.

So, to your point, skewing does not require or incur any additional moves.

[qx1147]

@thinkyhead

So, to your point, skewing does not require or incur any additional moves.

Ah, yes, you are right. I was just confused by SKEW_CORRECTION triggering PLANNER_LEVELING and by not knowing what PLANNER_LEVELING exactly refers to and where SKEW_CORRECTION would make the difference. But I guess this is just part of the muddle you mentioned earlier.

[boelle]

@qx1147 is this still an issue with latest bugfix 2.0?

[boelle]

@thinkyhead i think we can close this one

[github-actions[bot]]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.