Improved Bezier curve extrusion
ClosedPublic
Actions

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

Details

Reviewers

Campbell Barton (campbellbarton)
Nathan Letwory (jesterking)

Commits

rB1d32aa2b7f71: Curves: Improved Bezier Curve Extrusion
rBa768fd17e0be: Curves: Improved Bezier curve extrusion
rB9733d8416fb1: Improved Bezier curve extrusion

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) updated this revision to Diff 6726.May 20 2016, 9:30 PM

João Araújo (genio84) retitled this revision from to Improved Bezier curve extrusion.

João Araújo (genio84) updated this object.

João Araújo (genio84) updated this object.May 20 2016, 9:34 PM

João Araújo (genio84) added reviewers: Nathan Letwory (jesterking), Campbell Barton (campbellbarton).

João Araújo (genio84) added a subscriber: Duarte Farrajota Ramos (duarteframos).

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
1785	Negation is unnecessary complication.
1791	remove
1794	remove
1797	remove
1800	remove
1804	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

Curves: Improved Bezier curve extrusion

Modified the code according to jesterKing's suggestions.

Curves: Improved Bezier curve extrusion

Some whitespace was not fixed. Solved it.

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.

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

In D2017#46329, @João Araújo (genio84) wrote:

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.

In D2017#46414, @Campbell Barton (campbellbarton) wrote:

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.

In D2017#46421, @Nathan Letwory (jesterking) wrote:

In D2017#46329, @João Araújo (genio84) wrote:

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.

In D2017#46414, @Campbell Barton (campbellbarton) wrote:

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.