Page MenuHome

Fix T84638: Wrong scale for primitives with radius
ClosedPublic

Authored by Falk David (filedescriptor) on Jan 12 2021, 5:03 PM.

Details

Summary

Creating some primitives allows for a scale value (via python) that will scale
the object accordingly. For objects with a radius parameter (like
cylinders, spheres, etc.) passing a scale different to (1,1,1) would
result in unexpected behavior.

For example:
>>> bpy.ops.mesh.primitive_uv_sphere_add(radius=2, scale=(1,1,2))
We would expect this to create a sphere with a radius of 2
(dimensions 4,4,4) and then be scaled *2 along the z-axis
(dimensions 4,4,8). But this would previously create a scaled sphere
with dimensions (2,2,4).

The scale was simply divided by two. Maybe because the "radius"
parameter for creating the primitives was confusingly named "diameter"
(but used as the radius).

The fix adds a scale parameter to ED_object_new_primitive_matrix
and also renames the wrongly named "diameter" parameters to "radius".

Diff Detail

Repository
rB Blender
Branch
T (branched from master)
Build Status
Buildable 16953
Build 16953: arc lint + arc unit

Event Timeline

Falk David (filedescriptor) requested review of this revision.Jan 12 2021, 5:03 PM
Falk David (filedescriptor) created this revision.

While this is correct, it changes the behavior of any Python scripts that create new primitives.

With this patch applied, the following line creates a mesh which is twice as big as the geometry created in 2.91.

bpy.ops.mesh.primitive_uv_sphere_add(radius=2, scale=(1,1,2))

I think this has come up before, interested in feedback from other devs.


Suggest to wait until Blender 3.0, since it's likely to break scripts.

Until then, the current incorrect behavior can be noted the docs/comments, so anyone who runs into this is aware whats going on.


Accepting, but suggest to postpone the fix, will check with other developers.

This revision is now accepted and ready to land.Jan 13 2021, 4:28 AM

I would also wait for 3.0 for this, better not potentially break a lot of scripts to fix an issue that is not critical imho. +1 for properly documenting the current incorrect behavior too.

I agree on postponing this to the next major release.

  • Add scale factor of 0.5 and hint to docstring

@Campbell Barton (campbellbarton) suggested reverting the incorrect behavior (so that we only have to delete a small block for 3.0) and adding a hint to the docstring to make it clear what's going on. So now this would not break any scripts, but add the hint so users know how to get the behavior they need and we can just delete the following lines for the next major release to get the correct behavior:

if (scale != NULL && !equals_v3v3(scale, (const float[3]){1.0f, 1.0f, 1.0f})) {
  mul_v3_fl(scale, 0.5f);
}
  • Merge branch 'master' into T84638

Remove hint, I'm not sure what it means exactly but from testing the patch the scale and radius are working as I would expect them to.
So this additional information doesn't seem necessary.

@Campbell Barton (campbellbarton) The hint in this diff was just added so that it could be committed before 3.0 (so people know why it's behaving the way it does). Now there is no point, so removing it was the right call.