Page MenuHome
Differential D10353

Fixes T84928 : Lattice vertices at unexpected positions when changing lattice resolution from 1 to 3 or more.
ClosedPublic
Actions

Authored by Pratik Borhade (PratikPB2123) on Feb 7 2021, 10:11 PM.

Details

Reviewers
Philipp Oeser (lichtwerk)
Campbell Barton (campbellbarton)
Commits
rBc13754e6475e: Fixes T84928 : Lattice vertices at unexpected positions when changing lattice…
Summary

Fix for T84928 .
Considering the changes , issue is resolved ( Ignoring readability issues) .

Changes :

  • Change in value assignment of fu/v/w : Observing previous code , I noticed ,value assigned to them is equivalent to -0.5 ( i.e. co-ordinate of left most vertex of lattice size =1 where centre of lattice is origin ) .
  • Change in value assignment of du/v/w : Margin ( distance ) between each division of surface along any axis is equivalent to ( (length of surface along axis ) / (no of division line - 1) ) . that's why is changed it to (default_size/unew -1) .
  • New variable declared "default_size" : As far as I gone through the code , I noticed values 1 < du ,fu < 1 , which indicates these values were calculated with respect to default lattice of size 1 .
  • removed pntsu/v/w != 1 check : Following changes inside the if block worked properly for pntsu/v/w = 1 .

Diff Detail

Repository
rB Blender

Event Timeline

Pratik Borhade (PratikPB2123) requested review of this revision.Feb 7 2021, 10:11 PM
Pratik Borhade (PratikPB2123) created this revision.
Pratik Borhade (PratikPB2123) updated this revision to Diff 33697.Feb 8 2021, 7:07 AM
Pratik Borhade (PratikPB2123) updated this revision to Diff 33698.Feb 8 2021, 7:31 AM
Pratik Borhade (PratikPB2123) edited the summary of this revision. (Show Details)
Pratik Borhade (PratikPB2123) added a reviewer: Campbell Barton (campbellbarton).Feb 8 2021, 10:43 AM
Campbell Barton (campbellbarton) edited reviewers, added: Philipp Oeser (lichtwerk); removed: Campbell Barton (campbellbarton).Feb 8 2021, 11:26 AM
Philipp Oeser (lichtwerk) added a comment.EditedFeb 10 2021, 4:47 PM

First of all: patch seems to work without issues.
From the patch description it was not immediately clear why these changes were made though.

Thats why for review, I tried to wrap my head around the lattice code and here are my findings [which might influence how we proceed here]:

  • I was wondering why on earth we do calc_lat_fudu and immediately after have to correct that once again
  • Since the very first commit rB12315f4d0e0a, the LT_GRID flag seems to be useless/unused? (it is always ON -- I can only assume this was meant to be used for uniform-distance lattice points? Something like the result of Make Regular?)
  • without the LT_GRID flag being set, calc_lat_fudu would actually (almost) do what this patch suggests
  • only other caller of BKE_lattice_resize is Make Regular -- in this case calc_lat_fudu does the right thing -- if (ltOb) is not entered

So in essence, wouldnt something like this be more readable (use only calc_lat_fudu and use LT_GRID to distinguish between "uniform distance vs uniform size"?)

P1955 D10353_alternative
1
2
3diff --git a/source/blender/blenkernel/intern/lattice.c b/source/blender/blenkernel/intern/lattice.c
4index 3d3ade1a529..c3e9464f9d4 100644
5--- a/source/blender/blenkernel/intern/lattice.c
6+++ b/source/blender/blenkernel/intern/lattice.c
7@@ -279,8 +279,8 @@ void calc_lat_fudu(int flag, int res, float *r_fu, float *r_du)
8 *r_du = 1.0f;
9 }
10 else {
11- *r_fu = -1.0f;
12- *r_du = 2.0f / (res - 1);
13+ *r_fu = -0.5f;
14+ *r_du = 1.0f / (res - 1);
15 }
16 }
17
18@@ -311,32 +311,24 @@ void BKE_lattice_resize(Lattice *lt, int uNew, int vNew, int wNew, Object *ltOb)
19
20 vert_coords = MEM_mallocN(sizeof(*vert_coords) * uNew * vNew * wNew, "tmp_vcos");
21
22- calc_lat_fudu(lt->flag, uNew, &fu, &du);
23- calc_lat_fudu(lt->flag, vNew, &fv, &dv);
24- calc_lat_fudu(lt->flag, wNew, &fw, &dw);
25-
26 /* If old size is different than resolution changed in interface,
27 * try to do clever reinit of points. Pretty simply idea, we just
28 * deform new verts by old lattice, but scaling them to match old
29 * size first.
30 */
31 if (ltOb) {
32- if (uNew != 1 && lt->pntsu != 1) {
33- fu = lt->fu;
34- du = (lt->pntsu - 1) * lt->du / (uNew - 1);
35- }
36-
37- if (vNew != 1 && lt->pntsv != 1) {
38- fv = lt->fv;
39- dv = (lt->pntsv - 1) * lt->dv / (vNew - 1);
40- }
41-
42- if (wNew != 1 && lt->pntsw != 1) {
43- fw = lt->fw;
44- dw = (lt->pntsw - 1) * lt->dw / (wNew - 1);
45- }
46+ /* Based on uniform size. */
47+ lt->flag &= ~LT_GRID;
48+ }
49+ else {
50+ /* Set uniform distance (aka "Make Regular"). */
51+ lt->flag |= LT_GRID;
52 }
53
54+ calc_lat_fudu(lt->flag, uNew, &fu, &du);
55+ calc_lat_fudu(lt->flag, vNew, &fv, &dv);
56+ calc_lat_fudu(lt->flag, wNew, &fw, &dw);
57+
58 co = vert_coords[0];
59 for (w = 0, wc = fw; w < wNew; w++, wc += dw) {
60 for (v = 0, vc = fv; v < vNew; v++, vc += dv) {

Philipp Oeser (lichtwerk) added a subscriber: Campbell Barton (campbellbarton).Feb 10 2021, 5:12 PM

@Campbell Barton (campbellbarton) : sorry this backfires to you, got an opinion?

Pratik Borhade (PratikPB2123) edited the summary of this revision. (Show Details)Feb 10 2021, 7:10 PM
Pratik Borhade (PratikPB2123) edited the summary of this revision. (Show Details)
Campbell Barton (campbellbarton) accepted this revision.Feb 16 2021, 12:18 PM

Suggest to apply this patch as-is for 2.92 as it fixes the bug.

After this we could either remove LT_GRID use, or improve support for it.

Use LT_GRID

Store if the lattice is regular or not.

  • Add a "Make-Uniform" operator (to clear it).
  • Changing the resolution should respect the LT_GRID flag (and not set it).
Remove the LT_GRID Flag
  • Assume this is set, remove checks for it.
  • "Make Regular" can simply transform the points without having to depend on a flag (could be a boolean argument to BKE_lattice_resize).

This has the disadvantage that resizing resets to unit length, although this is already the current behavior, with the exception of T84928 report.

This revision is now accepted and ready to land.Feb 16 2021, 12:18 PM
Philipp Oeser (lichtwerk) accepted this revision.Feb 16 2021, 12:26 PM

@Pratik Borhade (PratikPB2123) : will commit on your behalf

Pratik Borhade (PratikPB2123) added a comment.EditedFeb 16 2021, 12:32 PM

@Philipp Oeser (lichtwerk) , if no downside of this patch , I am fine to submit as it is . After this as Campbell suggested , I would like to work further with suggested changes

Closed by commit rBc13754e6475e: Fixes T84928 : Lattice vertices at unexpected positions when changing lattice… (authored by Pratik Borhade (PratikPB2123), committed by Philipp Oeser (lichtwerk)). · Explain WhyFeb 16 2021, 1:55 PM
This revision was automatically updated to reflect the committed changes.
Philipp Oeser (lichtwerk) added a commit: rBc13754e6475e: Fixes T84928 : Lattice vertices at unexpected positions when changing lattice….

Revision Contents
Changeset List

PathSize
source/
blender/
blenkernel/
intern/
20 lines

Diff 33697

View Options

source/blender/blenkernel/intern/lattice.c

Loading...