Page MenuHome

Apply transforms on parents and children
Open, Confirmed, HighPublic

Description

Blender Version
Broken: 2.8 5073ee0bb27

Short description of error
When a parent and it's children are selected and you apply the transforms, the parenting relation is ignored.
If the parent is scaled to 0.5 in all axes and the child is still at a scale of 1, and all get their scale applied, the parent gets set back to 1 and the child will grow to twice the size with a scale of 1.

Details

Type
Bug

Event Timeline

Looked into this, the issue is apply-transform modifies the original data, then immediately evaluates parent/child relationships - which now use CoW evaluated meshes which don't have the changes from the source objects.

Looking into this further, the evaluated objects are used inconsistently, see: BKE_object_workob_calc_parent.

Note: issue is in apply_objects_internal - problem funcs are around ignore_parent_tx.

As far as I can see the solution is not to mix evaluated & original object matrices - Or, rewrite this operator to work in a completely different way,

This relates to policy for handling copy on write, assigning to @Sergey Sharybin (sergey).

Bastien Montagne (mont29) triaged this task as Confirmed, Medium priority.Jul 20 2018, 4:23 PM
Brecht Van Lommel (brecht) raised the priority of this task from Confirmed, Medium to Confirmed, High.Mon, Apr 1, 2:51 PM

@Sergey Sharybin (sergey): Would it make sense to have a version of BKE_object_workob_calc_parent that works strictly with the originals [parent] (since that would fix this bug afaict)?
(Or is this even more inconsistent?)

Or we could prepare the evaluated object as best as we can like so?

1possible fix for T55494
2
3note: ignore the (commented out) stuff about applying to object data [not needed afaict]
4but object matrix, scale etc. on the evaluated object needs to be up to date...
5
6This would go in line with the fixes for T62601/ T60623 where we prepare the evaluated object as best as we can as well.
7(in order for immediate calls to 'BKE_object_workob_calc_parent()' to work - before depsgraph did its magic...)
8
9
10diff --git a/source/blender/editors/object/object_transform.c b/source/blender/editors/object/object_transform.c
11index f7a49f3fcb7..c7480e49d2b 100644
12--- a/source/blender/editors/object/object_transform.c
13+++ b/source/blender/editors/object/object_transform.c
14@@ -57,6 +57,7 @@
15 #include "BKE_gpencil.h"
16
17 #include "DEG_depsgraph.h"
18+#include "DEG_depsgraph_query.h"
19
20 #include "RNA_define.h"
21 #include "RNA_access.h"
22@@ -532,6 +533,8 @@ static int apply_objects_internal(
23 CTX_DATA_BEGIN (C, Object *, ob, selected_editable_objects)
24 {
25
26+ Object *ob_eval = DEG_get_evaluated_object(depsgraph, ob);
27+
28 /* calculate rotation/scale matrix */
29 if (apply_scale && apply_rot)
30 BKE_object_to_mat3(ob, rsmat);
31@@ -571,35 +574,45 @@ static int apply_objects_internal(
32 /* apply to object data */
33 if (ob->type == OB_MESH) {
34 Mesh *me = ob->data;
35+ //Mesh *me_eval = ob_eval->data;
36
37 if (apply_scale)
38 multiresModifier_scale_disp(depsgraph, scene, ob);
39
40 /* adjust data */
41 BKE_mesh_transform(me, mat, true);
42+ //BKE_mesh_transform(me_eval, mat, true);
43
44 /* update normals */
45 BKE_mesh_calc_normals(me);
46+ //BKE_mesh_calc_normals(me_eval);
47 }
48 else if (ob->type == OB_ARMATURE) {
49 ED_armature_transform_apply(bmain, ob, mat, do_props);
50+ //ED_armature_transform_apply(bmain, ob_eval, mat, do_props);
51 }
52 else if (ob->type == OB_LATTICE) {
53 Lattice *lt = ob->data;
54-
55+ //Lattice *lt_eval = ob_eval->data;
56 BKE_lattice_transform(lt, mat, true);
57+ //BKE_lattice_transform(lt_eval, mat, true);
58 }
59 else if (ob->type == OB_MBALL) {
60 MetaBall *mb = ob->data;
61+ //MetaBall *mb_eval = ob_eval->data;
62 BKE_mball_transform(mb, mat, do_props);
63+ //BKE_mball_transform(mb_eval, mat, do_props);
64 }
65 else if (ELEM(ob->type, OB_CURVE, OB_SURF)) {
66 Curve *cu = ob->data;
67+ //Curve *cu_eval = ob_eval->data;
68 scale = mat3_to_scale(rsmat);
69 BKE_curve_transform_ex(cu, mat, true, do_props, scale);
70+ //BKE_curve_transform_ex(cu_eval, mat, true, do_props, scale);
71 }
72 else if (ob->type == OB_FONT) {
73 Curve *cu = ob->data;
74+ //Curve *cu_eval = ob_eval->data;
75 int i;
76
77 scale = mat3_to_scale(rsmat);
78@@ -610,15 +623,24 @@ static int apply_objects_internal(
79 tb->y *= scale;
80 tb->w *= scale;
81 tb->h *= scale;
82+
83+ //TextBox *tb_eval = &cu_eval->tb[i];
84+ //tb_eval->x *= scale;
85+ //tb_eval->y *= scale;
86+ //tb_eval->w *= scale;
87+ //tb_eval->h *= scale;
88 }
89
90 if (do_props) {
91 cu->fsize *= scale;
92+ //cu_eval->fsize *= scale;
93 }
94 }
95 else if (ob->type == OB_GPENCIL) {
96 bGPdata *gpd = ob->data;
97+ //bGPdata *gpd_eval = ob_eval->data;
98 BKE_gpencil_transform(gpd, mat);
99+ //BKE_gpencil_transform(gpd_eval, mat);
100 }
101 else if (ob->type == OB_CAMERA) {
102 MovieClip *clip = BKE_object_movieclip_get(scene, ob, false);
103@@ -651,10 +673,13 @@ static int apply_objects_internal(
104 {
105 float max_scale = max_fff(fabsf(ob->scale[0]), fabsf(ob->scale[1]), fabsf(ob->scale[2]));
106 ob->empty_drawsize *= max_scale;
107+ //float max_scale_eval = max_fff(fabsf(ob_eval->scale[0]), fabsf(ob_eval->scale[1]), fabsf(ob_eval->scale[2]));
108+ //ob_eval->empty_drawsize *= max_scale_eval;
109 }
110 }
111 else if (ob->type == OB_LAMP) {
112 Light *la = ob->data;
113+ //Light *la_eval = ob_eval->data;
114 if (la->type != LA_AREA) {
115 continue;
116 }
117@@ -663,36 +688,54 @@ static int apply_objects_internal(
118 if ((la->area_shape == LA_AREA_SQUARE) && !keeps_aspect_ratio) {
119 la->area_shape = LA_AREA_RECT;
120 la->area_sizey = la->area_size;
121+ //la_eval->area_shape = LA_AREA_RECT;
122+ //la_eval->area_sizey = la->area_size;
123 }
124 else if ((la->area_shape == LA_AREA_DISK) && !keeps_aspect_ratio) {
125 la->area_shape = LA_AREA_ELLIPSE;
126 la->area_sizey = la->area_size;
127+ //la_eval->area_shape = LA_AREA_ELLIPSE;
128+ //la_eval->area_sizey = la->area_size;
129 }
130
131 la->area_size *= rsmat[0][0];
132 la->area_sizey *= rsmat[1][1];
133 la->area_sizez *= rsmat[2][2];
134+ //la_eval->area_size *= rsmat[0][0];
135+ //la_eval->area_sizey *= rsmat[1][1];
136+ //la_eval->area_sizez *= rsmat[2][2];
137 }
138 else {
139 continue;
140 }
141
142- if (apply_loc)
143+ if (apply_loc) {
144 zero_v3(ob->loc);
145- if (apply_scale)
146+ zero_v3(ob_eval->loc);
147+ }
148+ if (apply_scale) {
149 ob->scale[0] = ob->scale[1] = ob->scale[2] = 1.0f;
150+ ob_eval->scale[0] = ob_eval->scale[1] = ob_eval->scale[2] = 1.0f;
151+ }
152 if (apply_rot) {
153 zero_v3(ob->rot);
154 unit_qt(ob->quat);
155 unit_axis_angle(ob->rotAxis, &ob->rotAngle);
156+ zero_v3(ob_eval->rot);
157+ unit_qt(ob_eval->quat);
158+ unit_axis_angle(ob_eval->rotAxis, &ob_eval->rotAngle);
159 }
160
161 BKE_object_where_is_calc(depsgraph, scene, ob);
162+ BKE_object_where_is_calc(depsgraph, scene, ob_eval);
163+
164 if (ob->type == OB_ARMATURE) {
165 BKE_pose_where_is(depsgraph, scene, ob); /* needed for bone parents */
166+ BKE_pose_where_is(depsgraph, scene, ob_eval); /* needed for bone parents */
167 }
168
169 ignore_parent_tx(C, bmain, scene, ob);
170+ ignore_parent_tx(C, bmain, scene, ob_eval);
171
172 DEG_id_tag_update(&ob->id, ID_RECALC_TRANSFORM | ID_RECALC_GEOMETRY);
173

@Brecht Van Lommel (brecht), i've been looking into the change. Not really sure yet why it is required to do some calculations in original object, and some of them are on both. To me it seems that where_is will return same matrix for both ob and ob_eval?

Also, there is BKE_object_transform_copy(), which is probably easier to use right before where_is, and ignore the rest of the change?

Hm, without having looked at this again (will do on monday), just want to note that the following are all very tightly related:

(This commit is where it "started", in BKE_object_workob_calc_parent calculation of matrix for workobject is now based on the evaluated parent)
Above commit left a couple of places "broken" where code prior to BKE_object_workob_calc_parent was changing original (flags/matrix, etc), but BKE_object_workob_calc_parent was using evaluated version [which didnt have these changes available yet]

  • rB2894e75121d7 / T60623 [this compensated by setting BONE_RELATIVE_PARENTING flag on evaluated version as well, as well as using partype, par1, par2, par3 from the original again]
  • rB9ad9d38786c5 / T62601 [this compensated by setting CU_FOLLOW flag on evaluated version as well]

So right now this is really a bit mixed up... [as @Campbell Barton (campbellbarton) also mentioned above]

So on Monday will also look at why rBbc5482337669 was necessary to do that way...
If it needs to be like that, then it kind of makes sense to make sure the evaluated version is getting any updates as well [because evaluated will be used later on]

Hrmf, after a bit more digging, I still lack a "proper" solution [at least it feels like that...]

Here is the "cleaned up" ex- P946 solution from above (with the unneeded stuff removed and suggestions from @Sergey Sharybin (sergey)):

1
2
3diff --git a/source/blender/editors/object/object_transform.c b/source/blender/editors/object/object_transform.c
4index f7a49f3fcb7..1912f3d0f53 100644
5--- a/source/blender/editors/object/object_transform.c
6+++ b/source/blender/editors/object/object_transform.c
7@@ -57,6 +57,7 @@
8 #include "BKE_gpencil.h"
9
10 #include "DEG_depsgraph.h"
11+#include "DEG_depsgraph_query.h"
12
13 #include "RNA_define.h"
14 #include "RNA_access.h"
15@@ -531,6 +532,7 @@ static int apply_objects_internal(
16 /* now execute */
17 CTX_DATA_BEGIN (C, Object *, ob, selected_editable_objects)
18 {
19+ Object *ob_eval = DEG_get_evaluated_object(depsgraph, ob);
20
21 /* calculate rotation/scale matrix */
22 if (apply_scale && apply_rot)
23@@ -687,9 +689,14 @@ static int apply_objects_internal(
24 unit_axis_angle(ob->rotAxis, &ob->rotAngle);
25 }
26
27+ BKE_object_transform_copy(ob_eval, ob);
28+
29 BKE_object_where_is_calc(depsgraph, scene, ob);
30+ BKE_object_where_is_calc(depsgraph, scene, ob_eval);
31 if (ob->type == OB_ARMATURE) {
32- BKE_pose_where_is(depsgraph, scene, ob); /* needed for bone parents */
33+ /* needed for bone parents */
34+ BKE_pose_where_is(depsgraph, scene, ob);
35+ BKE_pose_where_is(depsgraph, scene, ob_eval);
36 }
37
38 ignore_parent_tx(C, bmain, scene, ob);

other stuff I tried to look into:

  • rBbc5482337669: in the case of this report, working with the original in BKE_object_workob_calc_parent will actually work fine, but will run into problems elsewhere... [wrong evaluation of curve parents etc...]
  • instead of "manually" copying transform etc to the evaluated version: tag with ID_RECALC_COPY_ON_WRITE and update depsgraph [but that will update dependencies as well, causing problems with nested parents etc...]
  • I will also decouple/reopen T63387 again [because solution from here wont actually solve that one...]

Long story short: Some other eyes will probably need to look at this in depth...

Maybe @Dalai Felinto (dfelinto) also wants to join?