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

Fixed issues with Shading tool Saturation and Value not always being right in Hue Shading mode #519

Merged
merged 1 commit into from
Aug 19, 2021

Conversation

mrtripie
Copy link
Contributor

This should fix some problems with the Saturation and Value part of the Shading Tool (NOT the Hue problems).

image
Left grey theme is previous build, right side is new one.

Problem 1: If you darkening value is more than the value of the color, it can end up brightening instead (I think Godot's Color class might wrap around values when setting them)
image
(Checked the color with the color picker to make sure it was right hue and at the value limit of 10)

Problem 2: If you darken not quite enough for that, but still too much, it will hit the value limit and ignore the darkening. Now it will just be set to the Value limit:
image

Problem 3: When lightening too much using Saturation, the hue could get messed up, I'm not entirely sure why this happened, but applying the same fix I did to the value part seemed it:
image

Problem 4: If you brightened the Saturation too much, it would also through out the Hue and Value brightening, so I erased that part. Its not really obvious between the old and new version, so for these screenshots I added it back:
Before: (No value change as it increased the saturation all the way)
image

After: Both Saturation goes down and Value go up, making it White.
image

Not sure if it should need more HSV brightening/darkening limits, but it should feel much better to use now.

Copy link
Member

@OverloadedOrama OverloadedOrama left a comment

Choose a reason for hiding this comment

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

Looks good, thank you!

@OverloadedOrama OverloadedOrama merged commit c196a32 into Orama-Interactive:master Aug 19, 2021
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