Skip to content

AdjustmentCurve: added new property-based modes#293

Merged
deinhofer merged 2 commits intomasterfrom
klaus/adjustment-curve-property-based-modes
Apr 15, 2019
Merged

AdjustmentCurve: added new property-based modes#293
deinhofer merged 2 commits intomasterfrom
klaus/adjustment-curve-property-based-modes

Conversation

@klues
Copy link
Contributor

@klues klues commented Mar 8, 2019

This PR adds a new property operationMode to AdjustmentCurve with the following modes:

  • use file and GUI --> original mode, acts as before, is the default operation mode
  • use property curvePoints (percent values) --> curve points can be defined by an additional property curvePoints and they are interpreted as percentage of the given min/max range of input and output.
  • use property curvePoints (absolute values) --> curve points can be defined by an additional property curvePoints and they are interpreted as absolute values.

The new modes can be handy to rapidly define a simple curve without the GUI and they do not need any external file in bin/ARE/data like the existing mode (which is now called use file and GUI).

updated documentation is still missing and coming.

@klues klues changed the title added new property-based modes to adjustmentcurve plugin AdjustmentCurve: added new property-based modes Mar 8, 2019
@klues
Copy link
Contributor Author

klues commented Mar 11, 2019

updated documentation with e16988f

}
int i = 0;
while ((i < curvePoints.size() - 1) && (x > curvePoints.get(i).x)) {
while ((i < curvePoints.size() - 1) && (x > getX(i))) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some comments would be nice:
You are looking for the first point, where x > curvePoint.x(i)??

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, looks like that this is what the code is doing... ;)
It's just the old logic where the x/y values now aren't directly retrieved form curvePoints, but using helper methods getX() and getY() for making a mode possible where the points are interpreted as percentage values. I'm sure the logic could be somehow improved/beautified but since there are no automatic tests I thought I'll better leave it as it is. However I did some manual tests with the test models existing for AdjustmentCurve and they worked the same way as before after my changes.


if ("operationMode".equalsIgnoreCase(propertyName)) {
final int oldValue = propOperationMode;
propOperationMode = Integer.parseInt(newValue.toString());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in theory newValue could be null.

But this would also be a problem for the other properties

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same question as in #294 (comment)

@deinhofer
Copy link
Contributor

looks good and very good idea to add properties for easy configuration of curvePoints

@deinhofer deinhofer merged commit bdc438b into master Apr 15, 2019
@deinhofer deinhofer deleted the klaus/adjustment-curve-property-based-modes branch April 15, 2019 10:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants