UBL G29 J Level on Mesh points

[jamespearson04]

Description

G29 J Currently probes the edges of the probeable area, typically not on top of existing mesh points. This means that the mesh Z height value used to compare against is likely to be interpolated, and if the point lies in a mesh area populated by G29 P3 (On the outside of the probed mesh points) it's Z height will almost certainly be innacurate.

This push uses the already existing find_closest_mesh_point_of_type() to find the nearest probeable mesh point to each G29 J probe point, and probes there instead. It should work for all enumerations of G29 Jx, assuming sufficient points exist. In the event insufficient points exist, some will most likely be probed twice, which shouldn't have a significant negative effect, but may bias the LSF results (however this is currently untested).

Testing on Ender 3 Max and Ender 3 Neo has proven successful.

All that remains is to decide how to handle PROBE_PT_1_X etc. Most likely a second define that determines whether their raw position or their nearest mesh point is used.

Requirements

Requires Unified Bed Leveling and some kind of probe.

Benefits

Improves G29 J accuracy.

Configurations

Requires the addition of #define TILT_PROBE_ON_MESH_POINTS to Configuration.h/Configuration_adv.h.

Related Issues

#25485

[thisiskeithb]

Requires the addition of #define TILT_PROBE_ON_MESH_POINTS to Configuration.h/Configuration_adv.h.

That should be added so users can find/use this feature. Somewhere under the AUTO_BED_LEVELING_UBL flag/header would be a good place:

Marlin/Marlin/Configuration.h

Lines 2045 to 2049 in 26f96dc

	#elif ENABLED(AUTO_BED_LEVELING_UBL)
	//===========================================================================
	//========================= Unified Bed Leveling ============================
	//===========================================================================

[jamespearson04]

Added in #defines to enable the feature, and specify whether or not it applies to fixed 3 point leveling points.

//#define UBL_LEVEL_ON_MESH_POINT: Enables the feature.

https://github.com/jamespearson04/Marlin/blob/bugfix-2.1.x/Marlin/Configuration.h#L2045-L2065

//#define UBL_LEVEL_ON_MESH_POINT_FIXED_3POINT: Probe the 3 mesh points nearest the defined points, not the defined points themselves.

https://github.com/jamespearson04/Marlin/blob/17085101789d8f69c596ee69b0cfd4a5baeb50b4/Marlin/Configuration_adv.h#L2145-L2167

[jamespearson04]

That should be working now, though I haven't had chance to test it and won't until Monday.

There may be a better way of selecting points, as opposed to subdividing the probeable area and selecting the nearest ones. However I can't really see how that would play nicely with the way the leveling grid is specified (G29 Jx -> x*x grid), as the available mesh points are virtually never going to correspond to a square grid, and even so it would only work for one size. A different method of selecting points from the mesh would be required in order to work better than the current system.

[thinkyhead]

I'm not sure I understand the reasoning for this option. But perhaps I misunderstand the purpose of the G29 J command.

It is my understanding that the G29 J tilt is based on either nearest-squares from probing a mesh or from the rotation matrix derived from probing a triangle. In both cases the tilt angle is based on real measurements of the physical bed at points that were (most likely) interpolated, and the correction is being made specifically because the interpolated points might not match actual measurements.

What am I missing?

[jamespearson04]

I'm not sure I understand your comment, are you saying the whole feature is unwarranted, or having the defines for it? Because I agree with the latter, I'll gladly change it if others agree it should be the standard.

G29 J is for tilting the loaded mesh to match the current bed tilt, to be used after releveling or before prints etc. It uses least squares fit incremental_LSF() to match the stored points (or their interpolation/extrapolation) to the measured points, and translates the whole mesh accordingly.

Without this PR, the selected points for probing aren't even necessarily confined to the mesh, so depending on configuration, the measured value can be compared against extrapolations (or whatever method is used to work out the Z correction beyond the mesh, none of which will yield good values).

As a result, with such a configuration, you can probe a fresh mesh with G29 P1 etc, and immediately running G29 J will introduce a seemingly random tilt, because the reference points it uses are just incorrect. Running it again shouldn't cause any further changes, but at that point the mesh is basically useless. If working correctly, with a fresh mesh it should introduce virtually no changes, maybe a very minor tilt due to probing variations.

Opting to limit this probing to mesh points means we're comparing pairs of two actual measurements, not a measurement and a inter/extrapolation, eliminating any potential for errors due to them.

[jamespearson04]

Even without UBL_TILT_ON_MESH_POINTS_3POINT I think this should be confined to the bounds of the mesh. (unless otherwise specified I suppose). Currently it is based on the probing bounds, so If MESH_INSET > PROBING_MARGIN it will perform poorly due to extrapolated points.

Marlin/Marlin/src/feature/bedlevel/ubl/ubl_G29.cpp

Lines 1488 to 1498 in 94b5d54

	if (do_3_pt_leveling) {
	xy_float_t points[3];
	probe.get_three_points(points);
	#if ENABLED(UBL_TILT_ON_MESH_POINTS_3POINT)
	mesh_index_pair cpos[3];
	LOOP_L_N(ix, 3) { // Convert points to coordinates of mesh points
	cpos[ix] = find_closest_mesh_point_of_type(REAL, points[ix], true);
	points[ix] = cpos[ix].meshpos();
	}
	#endif

[jamespearson04]

I've updated the standard version (non mesh point) to confine the points to the mesh area, as probing outside of it just doesn't make sense, I can't think of a use case where it would be desireable behaviour. A warning has been added for G29J_MESH_TILT_MARGIN as the margin now starts at the mesh edge, not the probing area edge.

[thinkyhead]

the measured value can be compared against extrapolations…

I see what you mean. You could use G29 J to compare new probe readings to old mesh values that were probed maybe a week ago, and fix the mesh for a bed that somehow got tilted in the interim without needing to re-probe the whole bed. One issue I have with the whole theory of G29 J is that tilting the mesh is not quite the same as tilting a matrix, as in the case of planar leveling. However, I grok why you want to do it this way.

In case you wondered, I made it possible to use closest mesh points even in the case where the mesh points are not explicitly set by the user, because it seems reasonable that you might want to do that sometimes. I expect that picking your own mesh points would be the less common case. And, I made it so the two new options are independent of one another, but I expect most users will enable both if they enable one.

[thinkyhead]

I can't think of a use case where it would be desireable behaviour.

I don't see that as being too unusual if you were using ABL with a known-flat bed, but since UBL uses a constant for points outside the mesh it is very likely to be invalid most of the time. So, the added warning is a good idea.

[jamespearson04]

One issue I have with the whole theory of G29 J is that tilting the mesh is not quite the same as tilting a matrix, as in the case of planar leveling

I don't really know how the translation is actually applied, so I can't really say how mathematically correct that portion is, but it does appear to work well when it's using good points, noticeably better than with extrapolated points. Best I can do is assure that this makes it better not worse lol.

I don't see that as being too unusual if you were using ABL with a known-flat bed

Absolutely, but if that's the case you have no need for UBL, you'd be using one of the other leveling variants, so I think it ought to be the norm for UBL.

The warning is just for people who were using the G29J margin, as its behaviour will have changed slightly with the associated commit. Not ideal, but necessary to keep G29 J's behaviour consistent.

[thinkyhead]

The G29J_MESH_TILT_MARGIN is defined as an element of the "avoid obstacles" feature, which is referenced to the size of the bed and the clips that intrude into the bed area. So it does need to be a modifier to the bed area, but it should not apply as a modifier to the probing mesh area (which is not always well-known to the user). Therefore I think this should be used:

const float x_min = _MAX((X_MIN_POS) + (G29J_MESH_TILT_MARGIN), MESH_MIN_X, probe.min_x()),
            x_max = _MIN((X_MAX_POS) - (G29J_MESH_TILT_MARGIN), MESH_MAX_X, probe.max_x()),
            y_min = _MAX((Y_MIN_POS) + (G29J_MESH_TILT_MARGIN), MESH_MIN_Y, probe.min_y()),
            y_max = _MIN((Y_MAX_POS) - (G29J_MESH_TILT_MARGIN), MESH_MAX_Y, probe.max_y());

[jamespearson04]

I agree, that's the right way to handle it, I'll add it in.

edit: nvm, commit hadn't loaded up for me

[jamespearson04]

The only other thing I can think of that might be worth improving on is how it handles the mesh being cut off by unprobeable areas (without UBL_TILT_ON_MESH_POINTS).

The black box is the bed, the green box the probeable area. The vertical lines represent mesh columns. Red are manually probed / estimated, blue are measured. Orange dots the generated probe points for 3 point leveling.

The leftmost point will be less accurate than the others, as it will be the result of an interpolation between measured and manually probed/estimated (extrapolated) values. This is actually the situation that led to me creating the UBL_TILT_ON_MESH_POINTS option, and looking into the way G29 J worked. Whilst that option does prevent this issue, I think it may be worth further altering the default implementation so that in the above example, the leftmost point ends up on the blue line.

(This will have the effect of shifting it on to a mesh point as UBL_TILT_ON_MESH_POINTS does, but it is still distinct as other points won't necessarily reside on mesh points)

[jamespearson04]

Also thank you @thinkyhead for your help on this PR!

[jamespearson04]

something akin to

if (probe.min_x() > MESH_MIN_X){
  x_min = MESH_MIN_X;
  while (x_min < probe.min_x()){
    x_min += MESH_X_DIST;
  }
}