Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ofColor subtraction and negative values #1144

Closed
jembezmamy opened this issue Apr 6, 2012 · 21 comments
Closed

ofColor subtraction and negative values #1144

jembezmamy opened this issue Apr 6, 2012 · 21 comments

Comments

@jembezmamy
Copy link

When using the ofColor subtraction operator, negative results are clamped to positive values instead of 0. Values are circulating between 0 and 255, which could be useful for hue in HSV but in most cases (R, G, B..) is not what I would expect.

ofColor a(10, 40, 80);
ofColor b(20, 5, 30);
ofColor c = (a-b); // I get: (246, 45, 50), I would expect: (0, 45, 50)
@damian0815
Copy link
Contributor

A case could be argued for each implementations, actually. Perhaps we need an explicit clamping subtraction operator as well.

@kylemcdonald
Copy link
Contributor

i would say this is a bug, since the behavior is counter-intuitive. opencv deals with this via the saturate_cast function.

this is probably a bug for most math that happens inside ofColor, maybe the best solution is to do something similar to saturate_cast to make sure that ofColor operations are always clamped?

also, @DamianNZ can you give an example of a situation where using non-clamping operators is important? the one i can think of is when you want an unbounded floating point color for averaging (summing then dividing) things.

@bilderbuchi
Copy link
Member

well, the Hue component in HSV/HSB comes to mind, for example, since it loops around, i.e. hue=1 is the same as hue=0.

@damian0815
Copy link
Contributor

also, sometimes you want it to be the case that that subtracting BLUE from BLACK gives you YELLOW, because then you can wrap and loop and get all psychadelic on it.

@damian0815
Copy link
Contributor

also RED minus YELLOW is some kind of GREEN, it's not BLACK. (@liasomething says 'of course it's not black! RED minus RED is black.')

@bilderbuchi
Copy link
Member

yellow (FFFF00) minus red (FF0000) is green (00FF00), not the other way round, no?

yay, colour theory...

@damian0815
Copy link
Contributor

if they're numbers, yes. but they're not numbers, they're colours; minus ought to really act as a difference operator here. although it mathematically makes sense that RED minus YELLOW is BLACK, we have to ask what makes sense in terms of colours, and i say the difference between FF0000 and FFFF00 is always 00FF00.

@elliotwoods
Copy link
Contributor

red (255,0,0) - yellow (255,255,0) = green (0,255,0) ?
i see the colour logic, but how could that be expressed mathematically?

it seems you're intending to 'lock saturation' and simply transform the hue

i.e. something like (pseudo)

color = red;
color.hue -= yellow.hue;
assert(color == green);

p.s. agree with bilderbuchi, only hue should be 'looped' (i.e. integer overflow makes sense)
and +1 for avoiding integer overflow in other situations.

perhaps @DamianNZ usage could be facilitated, e.g.:

assert(green == ofColorDifference(red, yellow));

@ofTheo
Copy link
Member

ofTheo commented May 1, 2012

this should definitely be clamped to 0 - ie no wrapping.
like wise 250 + 10 should be 255.
I see this causing too many issues with people using ofColor otherwise.

@thiagohersan
Copy link
Contributor

this is marked for 0.8.0.
#728 and #1960 seem to be related.

is there consensus for clamping? except for hue ?
if so, I can take a look...

@ofTheo
Copy link
Member

ofTheo commented Jun 19, 2013

I think clamping should be on by default with the option to disable.
@kylemcdonald , @arturoc @ofZach whats your thought on this?

@ofTheo
Copy link
Member

ofTheo commented Jul 7, 2013

@kylemcdonald - can I assign this to you? I feel like this is your (color)wheel-house

@ghost ghost assigned kylemcdonald Jul 7, 2013
@kylemcdonald
Copy link
Contributor

yeah i can get this before 0.8.0, thanks @ofTheo

@kylemcdonald
Copy link
Contributor

@DamianNZ it sounds like you want "difference" to mean "absolute difference"? is that the right interpretation of (red - yellow = green)?

that's a separate issue from the clamping, though -- clamping can't be a mode you enable/disable, but it could be something you pass as an argument to a function. but for now i will just change ofColor so clamping is always enabled.

@bilderbuchi
Copy link
Member

could you leave a note and/or document that HSV hue will not loop (contrary to what could be expected)?
Also, ofColor::clamp() will always be a no-op, then, I guess - deprecate it?

@kylemcdonald
Copy link
Contributor

@bilderbuchi don't you think it makes sense for ofColor_<float> to allow unnormalized colors? in that case clamp() is still useful -- and actually, clamp() was never useful in any other case :)

is it more reasonable to have everything be clamped, or only to keep things that can overflow/underflow from over/underflowing?

@bilderbuchi
Copy link
Member

I have to admit I don't understand what you're saying. :-)
I purely took this

i will just change ofColor so clamping is always enabled.

to mean that ofColors will automatically, always, be clamped, in which case a separate method would be superfluous.
Also, I don't get the distinction between "clamping" and "keeping things that can over/underflow from doing so" - I thought that's the definition of clamping? ^^
as Theo said, your house, I'll leave you to it. :-)

@kylemcdonald
Copy link
Contributor

sorry, i didn't explain clearly enough. there is a difference between clamping and over/underflow. integral colors (unsigned short, unsigned char) can't be clamped, and real colors (float) can't over/underflow.

instead of saying "clamping is always enabled" i should have said "over/underflow is not allowed".

edit: also, it looks like all the operations are already clamped, it just doesn't mean anything most of the time.

@kylemcdonald
Copy link
Contributor

so here's a first pass at what i'm thinking kylemcdonald@61da27e

this just does the add and subtract operations. if anyone has ideas/insight that would be appreciated :)

@kylemcdonald
Copy link
Contributor

regarding (red - yellow) = green, that should be handled as a feature request if it is still desired, rather than a bug, since the current behavior of the subtraction operator is now mathematically correct, useful, and can not be implemented with an "absolute difference" operator (i think).

@ofTheo
Copy link
Member

ofTheo commented Jul 15, 2013

works for me! + 1 to merge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants