[3.x] Fix scale sensitivity for 3D objects.

[lentsius-bark]

Scaling 3D objects has been broken for a long time, it works well only when scale values stay around their original ones. Meaning that if you try scaling an objects that already at scale 30, the scaling function becomes unusable. Simple as that.

Current godot implementation vs. this PR:

GodotScaleFix2.mp4

This PR also divides the scale factor by the distance of the mouse click from the object. This is neat as it gives users the ability to choose scaling sensitivity in a very simple way: "The further from the object's center you mouse is, the slower it scales and vice versa"

A video showing the variable scaling sensitivity:

VariableScale.mp4

I'm new to contributing and a novice at C++, I welcome all feedback. I heard the codebase for specifically this has changed loads for 4.0, chances are it's already fixed if not I could try my hand at porting it?

[JFonS]

Makes sense usability-wise. The way the scaling factor is computed is a bit convoluted, I left a change suggestion in a comment.

This also needs to be changed in master. Simply applying the same scaling factor to motion right before this piece of code should do it:

godot/editor/plugins/node_3d_editor_plugin.cpp

Lines 1861 to 1874 in c8c1199

	if (se->gizmo.is_valid()) {
	for (Map<int, Transform3D>::Element *GE = se->subgizmos.front(); GE; GE = GE->next()) {
	Transform3D xform = GE->get();
	Transform3D new_xform = _compute_transform(TRANSFORM_SCALE, se->original * xform, xform, motion, snap, local_coords);
	if (!local_coords) {
	new_xform = se->original.affine_inverse() * new_xform;
	}
	se->gizmo->set_subgizmo_transform(GE->key(), new_xform);
	}
	} else {
	Transform3D new_xform = _compute_transform(TRANSFORM_SCALE, se->original, se->original_local, motion, snap, local_coords);
	_transform_gizmo_apply(se->sp, new_xform, local_coords);
	}

[JFonS]

The divisions by original.basis.get_scale() are redundant since they happen on both sides of the larger division. The code can be simplified down to this:

Suggested change

	float click_distance = click.distance_to(_edit.center);
	motion = ((motion / original.basis.get_scale()) / (Vector3(click_distance, click_distance, click_distance) / original.basis.get_scale()));
	motion /= click.distance_to(_edit.center);

[lentsius-bark]

I'll be online later this evening and work the changes in then, thanks for the suggestions! Loving the simplicity.

[akien-mga]

Aside from the code simplification, three more things worth considering (I didn't assess them myself):

• Is this the "best" location to perform this operation? motion is calculated further up.
• Related, we're in a for loop for all the selection - was this tested with multiple scaled objects selected, with different scales? It looks like it will mutate the same motion each time so I suspect you'll see problems.
• Related, further up some calculations are done for when snapping is enabled. Was this tested with snapping on, and if so does it work as expected?

[lentsius-bark]

@JFonS I implemented your suggestions. Thanks for pointing this out, really good stuff.
@akien-mga I moved the code higher, right after where motion is first defined so that it'd work robustly.
I tried scaling multiple objects as well as scale snap and it worked as expected.

I'll rebase and squash commits when this is all resolved :)

[lentsius-bark]

a video with the scaling of a group as well as snap scale

groupScale.mp4

[JFonS]

Looks good. I approve the PR, but it would need a master version before being merged :)

[lentsius-bark]

@JFonS #52685

Should I rebase and squash commits on this one?

[JFonS]

@lentsius-bark yup, this one needs the commits squashed.

[lentsius-bark]

@JFonS will do, thanks!

[lentsius-bark]

@JFonS done.

[akien-mga]

Thanks!