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

Added subtraction and division operators for sf::Color. #145

Closed
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@ghost

ghost commented Jan 1, 2012

This is a fix for #114.

@TheNexus

This comment has been minimized.

Show comment
Hide comment
@TheNexus

TheNexus Jan 7, 2012

Since the sf::Color members are of type sf::Uint8, I think that the subtraction result (unsigned int by means of integral promotion) is wrapped around, so std::max() will never choose 0 to be larger. You can avoid this by casting the operands to sf::Int8.

By the way, what's the use case for color subtraction and division?

TheNexus commented Jan 7, 2012

Since the sf::Color members are of type sf::Uint8, I think that the subtraction result (unsigned int by means of integral promotion) is wrapped around, so std::max() will never choose 0 to be larger. You can avoid this by casting the operands to sf::Int8.

By the way, what's the use case for color subtraction and division?

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Jan 7, 2012

There are blending modes that use subtraction and division, just like those that use addition and multiplication. It makes sense to have all four.

When two unsigned values are subtracted, they are automatically casted to a signed type (and can be casted back to unsigned to receive an unsigned result if needed). So, my method will work. Specifically, in this case, all of the values are automatically casted to int when operations are applied.

ghost commented Jan 7, 2012

There are blending modes that use subtraction and division, just like those that use addition and multiplication. It makes sense to have all four.

When two unsigned values are subtracted, they are automatically casted to a signed type (and can be casted back to unsigned to receive an unsigned result if needed). So, my method will work. Specifically, in this case, all of the values are automatically casted to int when operations are applied.

@TheNexus

This comment has been minimized.

Show comment
Hide comment
@TheNexus

TheNexus Jan 8, 2012

When two unsigned values are subtracted, they are automatically casted to a signed type

No, this is only the case for types smaller than (unsigned) int. But you are right in this case, I didn't have the exact integral promotion rules in mind. There would only be a problem when sf::Uint8 referred to unsigned int, which is unprobable to happen.

TheNexus commented Jan 8, 2012

When two unsigned values are subtracted, they are automatically casted to a signed type

No, this is only the case for types smaller than (unsigned) int. But you are right in this case, I didn't have the exact integral promotion rules in mind. There would only be a problem when sf::Uint8 referred to unsigned int, which is unprobable to happen.

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Jan 8, 2012

sf::Uint8 can't refer to unsigned int because unsigned int must be at least 16 bits. But yes, I did get my promotion rules wrong.

ghost commented Jan 8, 2012

sf::Uint8 can't refer to unsigned int because unsigned int must be at least 16 bits. But yes, I did get my promotion rules wrong.

@LaurentGomila

This comment has been minimized.

Show comment
Hide comment
@LaurentGomila

LaurentGomila Jan 20, 2012

Member

The division operator doesn't make sense:

  • it crashes with any color that has a zero component (so basically all predefined colors except white)
  • if left is greater than right, the result will be greater than 255 and will become a random number after wrapping is applied
  • what the hell is it supposed to represent? to "invert" a color, one must use the subtraction; division definitely doesn't belong to the color world
Member

LaurentGomila commented Jan 20, 2012

The division operator doesn't make sense:

  • it crashes with any color that has a zero component (so basically all predefined colors except white)
  • if left is greater than right, the result will be greater than 255 and will become a random number after wrapping is applied
  • what the hell is it supposed to represent? to "invert" a color, one must use the subtraction; division definitely doesn't belong to the color world
@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Jan 20, 2012

They're intended to be inverse functions; multiplying by zero yields zero, while dividing by zero is undefined.

However, now that I look at GIMP's page on colour blending, the blending mode that they use for division is (256 * A) / (B + 1), which will prevent division by zero errors.

ghost commented Jan 20, 2012

They're intended to be inverse functions; multiplying by zero yields zero, while dividing by zero is undefined.

However, now that I look at GIMP's page on colour blending, the blending mode that they use for division is (256 * A) / (B + 1), which will prevent division by zero errors.

@LaurentGomila

This comment has been minimized.

Show comment
Hide comment
@LaurentGomila

LaurentGomila Jan 21, 2012

Member

What about my second and third points? Don't you think that values higher than 255 should be clamped instead of producing random numbers?

Member

LaurentGomila commented Jan 21, 2012

What about my second and third points? Don't you think that values higher than 255 should be clamped instead of producing random numbers?

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Jan 21, 2012

I... actually didn't notice that point when reading the comment. -_-

I don't know why the actual manual page doesn't say that, but it does appear that GIMP clamps the colours as well.

As far as the third point goes, division is an actual colour blending mode that acts as an inverse to multiplication. It would make sense, either way, to have a member Invert() function for sf::Color that inverts the colour (which essentially just returns White - Color).

ghost commented Jan 21, 2012

I... actually didn't notice that point when reading the comment. -_-

I don't know why the actual manual page doesn't say that, but it does appear that GIMP clamps the colours as well.

As far as the third point goes, division is an actual colour blending mode that acts as an inverse to multiplication. It would make sense, either way, to have a member Invert() function for sf::Color that inverts the colour (which essentially just returns White - Color).

@ghost ghost assigned LaurentGomila Jul 30, 2013

@LaurentGomila

This comment has been minimized.

Show comment
Hide comment
@LaurentGomila

LaurentGomila Aug 9, 2013

Member

Added operator - manually, and dropped operator / (definitely too weird and I still have no idea how it could be useful).

Member

LaurentGomila commented Aug 9, 2013

Added operator - manually, and dropped operator / (definitely too weird and I still have no idea how it could be useful).

@ghost ghost assigned LaurentGomila Aug 9, 2013

@eXpl0it3r eXpl0it3r added feature and removed s:superseded labels Jul 4, 2014

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