Curve: CurveMapping Extend Option
ClosedPublic
Actions

Authored by Jeroen Bakker (jbakker) on Nov 1 2019, 2:29 PM.

Details

Reviewers

Campbell Barton (campbellbarton)

Maniphest Tasks

T67981: Extend Horizontal (RGB Curves Node) does not work outside the Clipping boundaries

Commits

rCd51c39546b93: Curve: CurveMapping Extend Option
rB2e6159a4948c: Curve: CurveMapping Extend Option

Summary

Extend options are currently stored per curve. This was not clearly
communicated to the user and they expected this to be a setting per
CurveMapping.

This change will move the option from Curve to CurveMapping. In
order to support this the API had to be changed.

BPY: CurveMap.evaluate is also moved to CurveMapping.evaluate what
breaks Python API. Cycles has been updated but other add-ons have
not. After release of 2.81 we can merge this to master and adapt
the add-ons.

Diff Detail

Repository

rB Blender

Branch

T67981 (branched from master)

Build Status

Buildable 5518
Build 5518: arc lint + arc unit

Event Timeline

Jeroen Bakker (jbakker) created this revision.Nov 1 2019, 2:29 PM

Harbormaster completed remote builds in B5518: Diff 19319.Nov 1 2019, 2:29 PM

Jeroen Bakker (jbakker) added a reviewer: Campbell Barton (campbellbarton).Nov 1 2019, 2:32 PM

Suggest to keep only CurveMapping.evaluate and remove the other, regarding the Python API.

Re:

Extend options are currently stored per curve. This was not clearly communicated to the user and they expected this to be a setting per CurveMapping.

This seems like something that could be solved by the UI instead of changing internal behavior.

The menu could be a popover with labels: "Curve Mapping" and "Curve" each with their own settings underneath them.

The down side of this patch is you can't change this option without impacting other curves in the mapping, it will likely break some small number of files with complex node setups that happen to set the option for some channels and not others.

Not to say this is a bad change, it just doesn't seem like this is an obvious case where this should be "solved" by changing internal behavior.

See decision in T67981 where this change was chosen. The overhaul was bigger than I had in mind as curves are used everywhere.
Will do the changes to the API as requested, is more a style thing.

Campbell Barton (campbellbarton) accepted this revision.Nov 1 2019, 4:10 PM

Removed the Curve.evaluate function

This revision is now accepted and ready to land.Nov 5 2019, 8:57 AM

Harbormaster completed remote builds in B5552: Diff 19395.Nov 5 2019, 8:57 AM

Jeroen Bakker (jbakker) edited the summary of this revision. (Show Details)Nov 5 2019, 8:59 AM

Jeroen Bakker (jbakker) edited the summary of this revision. (Show Details)

Patch currently fails regression tests

Patch vs Master

12 - object_modifier_array (SEGFAULT)
31 - cycles_mesh (Failed)
51 - eevee_mesh_test (Failed)
55 - eevee_shader_test (Failed)
71 - workbench_mesh_test (Failed)

Fixed render tests

This revision is now accepted and ready to land.Nov 5 2019, 10:28 AM

Harbormaster completed remote builds in B5553: Diff 19397.Nov 5 2019, 10:29 AM

Closed by commit rB2e6159a4948c: Curve: CurveMapping Extend Option (authored by Jeroen Bakker (jbakker)). · Explain WhyNov 27 2019, 4:06 PM

This revision was automatically updated to reflect the committed changes.

Jeroen Bakker (jbakker) added a commit: rB2e6159a4948c: Curve: CurveMapping Extend Option.

Nathan Letwory (jesterking) added a commit: rCd51c39546b93: Curve: CurveMapping Extend Option.Jan 13 2020, 11:28 PM