Page MenuHome

Improved Bezier curve extrusion
ClosedPublic

Authored by João Araújo (genio84) on May 20 2016, 9:30 PM.

Details

Summary

This patch attempts to solve the issues with D1841.

This patch implements the "Improved Extrusion" feature proposed in https://docs.google.com/document/d/1NSpMnKHn5LPL9_LUe2sRXiOtU52WFIfug26p3RaJAas/edit#.

The changes performed consist of adding two new interface options: "Extrude Symmetric" and "Reverse Extrude Direction". To ensure backwards compatibility, "Extrude symmetric" is turned on by default.

This patch improves upon D1841 by continuing to work when non-zero bevel values are specified.

Diff Detail

Repository
rB Blender
Branch
gsoc2016-improved_extrusion

Event Timeline

João Araújo (genio84) retitled this revision from to Improved Bezier curve extrusion.May 20 2016, 9:30 PM
João Araújo (genio84) updated this object.
João Araújo (genio84) updated this revision to Diff 6726.

Looks ok, just some improvements needed, especially regarding the unnecessary use of negation. See comments inline.

Next to that please fix whitespace. Use tabs instead of spaces. Also eliminate blank lines after if() { lines (marked with remove)

source/blender/blenkernel/intern/curve.c
1789

Negation is unnecessary complication.

1795

remove

1798

remove

1801

remove

1804

remove

1808

remove

1852

Negation is unnecessary complication.

1853

remove

1857

remove

1859

remove

1862

remove

1864

remove

1883

Negation is unnecessary complication.

1884

remove

1889

remove

1892

remove

1895

remove

1898

remove

1907

Negation is unnecessary complication.

1908

remove

1913

remove

1916

remove

1919

remove

1922

remove

1946

Negation is unnecessary complication.

1947

remove

1951

remove

1953

remove

1956

remove

source/blender/makesdna/DNA_curve_types.h
305

whitespace fix

source/blender/makesrna/intern/rna_curve.c
1514

check whitespace for block.

1515

Use the RNA_def_property_boolean_sdna rather than this negated boolean. This negated boolean makes writing code for this property unnecessary complicated: using the normal boolean takes out all negations as pointed above.

1516

Use this

João Araújo (genio84) updated this revision to Diff 6747.
  • Curves: Improved Bezier curve extrusion

Modified the code according to jesterKing's suggestions.

João Araújo (genio84) marked 4 inline comments as done.May 23 2016, 9:12 PM
João Araújo (genio84) updated this revision to Diff 6749.
  • Curves: Improved Bezier curve extrusion

Some whitespace was not fixed. Solved it.

João Araújo (genio84) marked 29 inline comments as done.May 24 2016, 11:43 PM

I just realized that the way I fixed the unnecessary negations completely breaks backwards compatibility.

At least the idea is here. I'll move away from this for a moment, to clear my ideas, and then make another patch.

Nathan Letwory (jesterking) edited edge metadata.EditedMay 25 2016, 8:42 AM
Nathan Letwory (jesterking) requested changes to this revision.

Much better. Check for the cases where you still have used spaces instead of tabs for indentation.

This revision now requires changes to proceed.May 25 2016, 8:42 AM

@João Araújo (genio84), its typically best to update existing revisions (D1841), not sure this needs to be separate?

@Campbell Barton (campbellbarton) I submitted the first revision manually, while this time I did it using arc. arc automatically created this new revision. Is there a way to merge the two (or maybe I should just close the first one)?

From your new branch, using arc you can do: arc diff --update D1841

I just realized that the way I fixed the unnecessary negations completely breaks backwards compatibility.
At least the idea is here. I'll move away from this for a moment, to clear my ideas, and then make another patch.

I don't see how this breaks backwards compatibility other than test files you had based on the original patch. Since this is still a patch IMO that is fine - no need to worry about backwards compatibility for that case.

From your new branch, using arc you can do: arc diff --update D1841

Since we got into this situation with two differentials and D2017 the one with most recent work done in it I suggest we continue with D2017 and D1841 be closed.

I just realized that the way I fixed the unnecessary negations completely breaks backwards compatibility.
At least the idea is here. I'll move away from this for a moment, to clear my ideas, and then make another patch.

I don't see how this breaks backwards compatibility other than test files you had based on the original patch. Since this is still a patch IMO that is fine - no need to worry about backwards compatibility for that case.

It is very simple actually. As the code currently is, symmetric extrude is turned on by default. After the modifications I did, the way to ensure that was to add the flag "CU_SYM_EXTRUDE" when the curve is created. However, the curves that are in Blender's users files don't have this flag, and therefore when they open their old files with this code all their curves will be only unilaterally extruded.

One way to solve this would be to replace all "symmetric" occurrences" with "unilateral", and change the code to behave accordingly.

From your new branch, using arc you can do: arc diff --update D1841

Since we got into this situation with two differentials and D2017 the one with most recent work done in it I suggest we continue with D2017 and D1841 be closed.

I have been looking everywhere to find how to close a review in differential, but I can't find any information on how to do it.

João Araújo (genio84) updated this revision to Diff 6854.
  • Curves: Improved Bezier Curve Extrusion
Nathan Letwory (jesterking) accepted this revision.

LGTM,

make sure you fix the white space, otherwise go ahead and land this in your own gsoc branch.

This revision is now accepted and ready to land.Jun 11 2016, 8:16 PM