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

Fix colors in gamma space being treated as if they were in linear space #20708

Merged
merged 1 commit into from
Feb 28, 2023

Conversation

PunkPun
Copy link
Member

@PunkPun PunkPun commented Feb 27, 2023

Fixes #20415 (comment)

PlayerColorRemap expected colors in linear space yet we provided them in gamma. We fix this by instead expecting gamma space colors and then converting them into linear space ourselves.

Fix proposed by @pchote

@PunkPun PunkPun added this to the Next Release milestone Feb 27, 2023
@pchote
Copy link
Member

pchote commented Feb 27, 2023

On second thought, it would be better to pass the original color into PlayerColorRemap and do the linearisation internally.

@PunkPun
Copy link
Member Author

PunkPun commented Feb 27, 2023

Now passing color directly

pchote
pchote previously approved these changes Feb 27, 2023
@PunkPun PunkPun changed the title Fix colors being operating in the wrong space Fix colors in gamma space being treated as if they were in lineer space Feb 27, 2023
@penev92
Copy link
Member

penev92 commented Feb 28, 2023

OK, from what I read this does make sense.
The input Hue and Saturation in PlayerColorRemap, now input color, is assumingly in gamma color space. GetRemappedColor() already clearly states that the remapping is done in linear color space and converts the original color to linear before it starts, so makes sense that the Hue and Saturation (ignoring Value) of the remapping (input) color would need to be in the same space.
The conversion from RGB to HSV of the input color remains the same, but now the RGB is in linear space and so are the resulting Hue and Saturation (ignoring the Value).

That explanation was for me to make sure I understood correctly, for someone to confirm or deny if I did understand correctly and possibly to help others that like me did not have previous knowledge of the matter understand what's going on.

PlayerColorRemap expected colors in linear space yet we provided them in gamma. We fix this by instead expecting gamma space colors and then converting them into linear space ourselves.
@PunkPun
Copy link
Member Author

PunkPun commented Feb 28, 2023

Renamed the commit / added a description

Copy link
Member

@penev92 penev92 left a comment

Choose a reason for hiding this comment

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

👍

@penev92 penev92 merged commit 422a228 into OpenRA:bleed Feb 28, 2023
@penev92
Copy link
Member

penev92 commented Feb 28, 2023

Changelog

And commit on prep.

@Mailaender Mailaender changed the title Fix colors in gamma space being treated as if they were in lineer space Fix colors in gamma space being treated as if they were in linear space Feb 28, 2023
@PunkPun PunkPun deleted the fix-color branch March 6, 2023 22:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Issues with the new colorpicker
4 participants