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

Color blending fixes #208

Merged
merged 3 commits into from
Jul 24, 2021
Merged

Color blending fixes #208

merged 3 commits into from
Jul 24, 2021

Conversation

jac8888
Copy link

@jac8888 jac8888 commented Jul 17, 2021

The gradient code had the same rounding error in the color lerp that the border radius code did. I also fixed an error in the way the blending is done in the animation code. (More details here.)

Copy link
Owner

@mikke89 mikke89 left a comment

Choose a reason for hiding this comment

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

Thanks a lot! These are some great changes.

Could you perhaps move all the color interpolations into a separate function? They are quite verbose, and especially now that they are used several places.

Source/Core/DecoratorGradient.cpp Outdated Show resolved Hide resolved
@mikke89 mikke89 added the enhancement New feature or request label Jul 17, 2021
@jac8888
Copy link
Author

jac8888 commented Jul 17, 2021

Sure thing; would you say that the lerp should be overloaded for Colourb to be like this?
Will add the clamp as well.

@mikke89
Copy link
Owner

mikke89 commented Jul 17, 2021

Great. I think it's better to make a separate function for this one.

@jac8888
Copy link
Author

jac8888 commented Jul 19, 2021

I do think it makes sense to overload the lerp since it doesn't behave in the correct way for Colourbs (truncating rather than rounding the result leading to errors), and someone is likely to use the wrong function unknowingly if there is another one. I'm not really sure where else it would make sense to go (locally, another function in Math)?

@mikke89
Copy link
Owner

mikke89 commented Jul 19, 2021

I figured it would be easier to separate them in case we later provide a way to select different color interpolation strategies. We don't really have a better place right now than math, that should be fine.

We could do a static assert to make sure people don't use lerp with Colourb.

@jac8888
Copy link
Author

jac8888 commented Jul 24, 2021

All right, I've made those changes. :)

@mikke89 mikke89 merged commit 8b6a361 into mikke89:master Jul 24, 2021
@mikke89
Copy link
Owner

mikke89 commented Jul 24, 2021

Awesome work! Thank you! :)

@jac8888 jac8888 deleted the blending branch July 27, 2021 16:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants