mirror of
https://github.com/blender/blender
synced 2026-09-29 04:37:17 +03:00
Fix #157653: Incorrect rotation with spin + extrude
Resolve: - Regression in [0] vertices already rotated to step a were being rotated again by the full step a+1 rotation. Reset extruded vertices to their original coordinates each step before applying the rotation. - Regression in [1] the translation (used for screw) was rotated in place each iteration, accumulating rotations across steps. Compute the step translation fresh from the original vector instead. Adds tests covering spin and screw with duplicate enabled/disabled. Ref !156356 [0]:b20e99a626[1]:08afa95a78
This commit is contained in:
Campbell Barton
parent
60c4abc14e
commit
8d139ddc8e
2 changed files with 222 additions and 7 deletions
|
|
@ -560,16 +560,36 @@ void bmo_spin_exec(BMesh *bm, BMOperator *op)
|
|||
const bool do_dupli = BMO_slot_bool_get(op->slots_in, "use_duplicate");
|
||||
const bool use_normal_flip = BMO_slot_bool_get(op->slots_in, "use_normal_flip");
|
||||
/* Caller needs to perform other sanity checks (such as the spin being 360d). */
|
||||
const bool use_merge = BMO_slot_bool_get(op->slots_in, "use_merge") && steps >= 3;
|
||||
const bool use_merge = BMO_slot_bool_get(op->slots_in, "use_merge") &&
|
||||
/* Don't create duplicate geometry. */
|
||||
(steps >= 3) &&
|
||||
/* Only the "extrude" code path supports merging. */
|
||||
(do_dupli == false);
|
||||
|
||||
BMVert **vtable = nullptr;
|
||||
float (*vtable_coords)[3] = nullptr;
|
||||
|
||||
/* When merging, store the original vertices to splice them back together. */
|
||||
if (use_merge) {
|
||||
vtable = MEM_new_array_uninitialized<BMVert *>(bm->totvert, __func__);
|
||||
}
|
||||
|
||||
/* When extruding, always restore the original location before rotating. */
|
||||
if (do_dupli == false) {
|
||||
vtable_coords = MEM_new_array_uninitialized<float[3]>(bm->totvert, __func__);
|
||||
}
|
||||
|
||||
if (vtable || vtable_coords) {
|
||||
int i = 0;
|
||||
BMIter iter;
|
||||
BMVert *v;
|
||||
BM_ITER_MESH_INDEX (v, &iter, bm, BM_VERTS_OF_MESH, i) {
|
||||
vtable[i] = v;
|
||||
if (vtable) {
|
||||
vtable[i] = v;
|
||||
}
|
||||
if (vtable_coords) {
|
||||
copy_v3_v3(vtable_coords[i], v->co);
|
||||
}
|
||||
/* Evil! store original index in normal,
|
||||
* this is duplicated into every other vertex.
|
||||
* So we can read the original from the final.
|
||||
|
|
@ -624,9 +644,23 @@ void bmo_spin_exec(BMesh *bm, BMOperator *op)
|
|||
true);
|
||||
BMO_op_exec(bm, &extop);
|
||||
if ((use_merge && (a == steps - 1)) == false) {
|
||||
/* For extrude mode, rotate the extruded geometry to the current step position.
|
||||
* Use `rmat` which is computed fresh each step from the origin angle to avoid
|
||||
* floating-point error accumulation. */
|
||||
|
||||
/* Reset each new vert's location to its un-rotated origin so the rotate below
|
||||
* runs as a single fresh rotation from the original position
|
||||
* (avoids precision loss from chained rotations, see: #148890). */
|
||||
if (a != 0) {
|
||||
BMOpSlot *slot_geom_out = BMO_slot_get(extop.slots_out, "geom.out");
|
||||
BMElem **elem_array = reinterpret_cast<BMElem **>(slot_geom_out->data.buf);
|
||||
const int elem_array_len = slot_geom_out->len;
|
||||
for (int i = 0; i < elem_array_len; i++) {
|
||||
if (elem_array[i]->head.htype == BM_VERT) {
|
||||
BMVert *v_src = reinterpret_cast<BMVert *>(elem_array[i]);
|
||||
const int index = *(reinterpret_cast<const int *>(&v_src->no[0]));
|
||||
copy_v3_v3(v_src->co, vtable_coords[index]);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
BMO_op_callf(bm,
|
||||
op->flag,
|
||||
"rotate cent=%v matrix=%m3 space=%s verts=%S",
|
||||
|
|
@ -688,11 +722,13 @@ void bmo_spin_exec(BMesh *bm, BMOperator *op)
|
|||
}
|
||||
|
||||
if (use_dvec) {
|
||||
mul_m3_v3(rmat, dvec);
|
||||
float dvec_step[3];
|
||||
mul_v3_m3v3(dvec_step, rmat, dvec);
|
||||
mul_v3_fl(dvec_step, float(a + 1));
|
||||
BMO_op_callf(bm,
|
||||
op->flag,
|
||||
"translate vec=%v space=%s verts=%S",
|
||||
dvec,
|
||||
dvec_step,
|
||||
op,
|
||||
"space",
|
||||
op,
|
||||
|
|
@ -703,6 +739,9 @@ void bmo_spin_exec(BMesh *bm, BMOperator *op)
|
|||
if (vtable) {
|
||||
MEM_delete(vtable);
|
||||
}
|
||||
if (vtable_coords) {
|
||||
MEM_delete(vtable_coords);
|
||||
}
|
||||
}
|
||||
|
||||
} // namespace blender
|
||||
|
|
|
|||
|
|
@ -684,6 +684,182 @@ class TestBMeshUVSelectSimple(unittest.TestCase):
|
|||
# save_to_blend_file_for_testing(bm)
|
||||
|
||||
|
||||
# ------------------------------------------------------------------------------
|
||||
# BMesh Operators
|
||||
|
||||
class TestBMeshOperators(unittest.TestCase):
|
||||
|
||||
def test_spin_primitive(self):
|
||||
import math
|
||||
|
||||
# Regular hexagon with a 1.0 radius.
|
||||
expected_area = (3.0 * math.sqrt(3.0)) / 2.0
|
||||
|
||||
unique_coords_pair = [set(), set()]
|
||||
|
||||
for do_dupli in (False, True):
|
||||
with self.subTest(do_dupli=do_dupli):
|
||||
bm = bmesh.new()
|
||||
v = bm.verts.new((1.0, 0.0, 0.0))
|
||||
|
||||
bmesh.ops.spin(
|
||||
bm,
|
||||
geom=[v],
|
||||
cent=(0.0, 0.0, 0.0),
|
||||
axis=(0.0, 0.0, 1.0),
|
||||
angle=math.radians(360.0),
|
||||
steps=6,
|
||||
use_merge=True,
|
||||
use_duplicate=do_dupli,
|
||||
)
|
||||
|
||||
if do_dupli:
|
||||
# Duplicate mode does not merge first/last.
|
||||
# The trailing vert is rotated one revolution causing it not to be an exact match.
|
||||
# Ensure it's close, then de-duplicate the location so the unique coords test passes.
|
||||
bm.verts.ensure_lookup_table()
|
||||
v0 = bm.verts[0]
|
||||
v_near = bm.verts[-1]
|
||||
self.assertLess((v_near.co - v0.co).length, 1e-4)
|
||||
v_near.co = v0.co
|
||||
|
||||
f = bm.faces.new(bm.verts[:6])
|
||||
self.assertEqual((len(bm.verts), len(bm.edges), len(bm.faces)), (7, 6, 1))
|
||||
else:
|
||||
self.assertEqual((len(bm.verts), len(bm.edges), len(bm.faces)), (6, 6, 0))
|
||||
|
||||
# All edges should have the same length (regular hexagon).
|
||||
edge_lengths = [e.calc_length() for e in bm.edges]
|
||||
for length in edge_lengths[1:]:
|
||||
self.assertAlmostEqual(length, edge_lengths[0], places=5)
|
||||
|
||||
f = bmesh.ops.contextual_create(bm, geom=bm.edges[:])["faces"][0]
|
||||
self.assertEqual((len(bm.verts), len(bm.edges), len(bm.faces)), (6, 6, 1))
|
||||
|
||||
self.assertAlmostEqual(f.calc_area(), expected_area, places=5)
|
||||
|
||||
unique_coords_pair[do_dupli] = {v.co[:] for v in bm.verts}
|
||||
|
||||
bm.free()
|
||||
|
||||
# Both paths should produce the same set of unique vertex positions.
|
||||
self.assertEqual(unique_coords_pair[False], unique_coords_pair[True])
|
||||
|
||||
def test_spin_screw_complex(self):
|
||||
import math
|
||||
from mathutils import Matrix, Vector
|
||||
|
||||
# Non-identity "space" matrix combining translation, rotation, and scale.
|
||||
# So spatial arguments interpreted in this local space.
|
||||
space = (
|
||||
Matrix.Translation((10.0, 0.0, 0.0)) @
|
||||
Matrix.Rotation(math.radians(90.0), 4, Vector((1.0, 1.0, 1.0))) @
|
||||
Matrix.Scale(2.0, 4)
|
||||
)
|
||||
|
||||
steps = 6
|
||||
|
||||
unique_coords_pair = [set(), set()]
|
||||
|
||||
for do_dupli in (False, True):
|
||||
with self.subTest(do_dupli=do_dupli):
|
||||
bm = bmesh.new()
|
||||
|
||||
# Isolated vert (z=0).
|
||||
bm.verts.new((11.0, 0.0, 0.0))
|
||||
# Edge (z=1..2).
|
||||
bm.edges.new([bm.verts.new(co) for co in (
|
||||
(11.0, 0.0, 1.0),
|
||||
(11.0, 0.0, 2.0),
|
||||
)])
|
||||
# Quad (z=3..4 in XZ).
|
||||
bm.faces.new([bm.verts.new(co) for co in (
|
||||
(11.0, 0.0, 3.0),
|
||||
(12.0, 0.0, 3.0),
|
||||
(12.0, 0.0, 4.0),
|
||||
(11.0, 0.0, 4.0),
|
||||
)])
|
||||
|
||||
bmesh.ops.spin(
|
||||
bm,
|
||||
geom=bm.verts[:] + bm.edges[:] + bm.faces[:],
|
||||
cent=(0.0, 0.0, 0.0),
|
||||
axis=(0.0, 0.0, 1.0),
|
||||
space=space,
|
||||
angle=math.radians(360.0),
|
||||
steps=steps,
|
||||
dvec=(0.0, 0.0, 0.5),
|
||||
use_duplicate=do_dupli,
|
||||
)
|
||||
|
||||
total_area = sum(f.calc_area() for f in bm.faces)
|
||||
total_length = sum(e.calc_length() for e in bm.edges)
|
||||
if do_dupli:
|
||||
self.assertEqual((len(bm.verts), len(bm.edges), len(bm.faces)), (49, 35, 7))
|
||||
self.assertAlmostEqual(total_area, 7.0, places=4)
|
||||
self.assertAlmostEqual(total_length, 35.0, places=4)
|
||||
else:
|
||||
self.assertEqual((len(bm.verts), len(bm.edges), len(bm.faces)), (49, 77, 32))
|
||||
self.assertAlmostEqual(total_area, 297.830433, places=4)
|
||||
self.assertAlmostEqual(total_length, 651.346448, places=4)
|
||||
|
||||
unique_coords_pair[do_dupli] = {v.co[:] for v in bm.verts}
|
||||
|
||||
if not do_dupli:
|
||||
save_to_blend_file_for_testing(bm)
|
||||
|
||||
bm.free()
|
||||
|
||||
# Both paths should produce the same set of unique vertex positions.
|
||||
self.assertEqual(unique_coords_pair[False], unique_coords_pair[True])
|
||||
|
||||
def test_spin_screw(self):
|
||||
import math
|
||||
|
||||
steps = 8
|
||||
|
||||
unique_coords_pair = [set(), set()]
|
||||
|
||||
for do_dupli in (False, True):
|
||||
with self.subTest(do_dupli=do_dupli):
|
||||
bm = bmesh.new()
|
||||
|
||||
# Vertical edge at radius 1.0.
|
||||
bm.edges.new([bm.verts.new(co) for co in (
|
||||
(1.0, 0.0, 0.0),
|
||||
(1.0, 0.0, 1.0),
|
||||
)])
|
||||
|
||||
bmesh.ops.spin(
|
||||
bm,
|
||||
geom=bm.verts[:] + bm.edges[:],
|
||||
cent=(-1.0, -1.0, -1.0),
|
||||
axis=(1.0, 1.0, 1.0),
|
||||
angle=math.radians(360.0),
|
||||
steps=steps,
|
||||
dvec=(0.0, 0.0, 1.0 / steps),
|
||||
use_duplicate=do_dupli,
|
||||
)
|
||||
|
||||
total_area = sum(f.calc_area() for f in bm.faces)
|
||||
total_length = sum(e.calc_length() for e in bm.edges)
|
||||
if do_dupli:
|
||||
self.assertEqual((len(bm.verts), len(bm.edges), len(bm.faces)), (18, 9, 0))
|
||||
self.assertAlmostEqual(total_area, 0.0, places=4)
|
||||
self.assertAlmostEqual(total_length, 9.0, places=4)
|
||||
else:
|
||||
self.assertEqual((len(bm.verts), len(bm.edges), len(bm.faces)), (18, 25, 8))
|
||||
self.assertAlmostEqual(total_area, 3.600270, places=4)
|
||||
self.assertAlmostEqual(total_length, 19.121252, places=4)
|
||||
|
||||
unique_coords_pair[do_dupli] = {v.co[:] for v in bm.verts}
|
||||
|
||||
bm.free()
|
||||
|
||||
# Both paths should produce the same set of unique vertex positions.
|
||||
self.assertEqual(unique_coords_pair[False], unique_coords_pair[True])
|
||||
|
||||
|
||||
def main():
|
||||
import sys
|
||||
sys.argv = [__file__] + (sys.argv[sys.argv.index("--") + 1:] if "--" in sys.argv else [])
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue