-
-
Notifications
You must be signed in to change notification settings - Fork 6.8k
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 normal map palette #3114
Fix normal map palette #3114
Conversation
Thank you @cs-altshift! I reviewed your contribution. Taking your commit as base, I'll add a few steps more to the Discrete mode and I'll replace the if-elseif-else condition by a couple of formulas. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll talk with David prior to merge this fix. Surely, we'll change the if
block and some minor details. Thank you again!🎉
if (normalizedDistance < 0.25) { | ||
normalizedDistance = 0; | ||
blueAngleDegrees = 90; | ||
angle = PI / 2; | ||
angle = 0; | ||
} | ||
else if (normalizedDistance < 0.5) { | ||
normalizedDistance = 0.25; | ||
blueAngleDegrees = 45; | ||
} | ||
else if (normalizedDistance < 0.75) { | ||
normalizedDistance = 0.5; | ||
blueAngleDegrees = 30; | ||
} | ||
else { | ||
normalizedDistance = 0.75; | ||
blueAngleDegrees = 15; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will be replaced with:
if (normalizedDistance < 1.0/6.0)
angle = 0;
normalizedDistance = (std::floor((normalizedDistance) * 6.0 + 1.0) - 1) / 5.0;
blueAngleDegrees = 90.0 * (6.0 - std::floor(normalizedDistance * 6.0)) / 5.0;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This calculates the discrete mode along 6 steps (along the radius), and simplifies the if-elseif-else
statement.
Great news everything works as expected for you. AFAIK we do not use the discrete mode so I just matched what existed before. But indeed more steps will be better. |
Sorry @cs-altshift that we weren't able to merge this PR yet, I'm preparing a corporate CLA and probably changing the CLA-signing process soon. It will take some time until we can merge this but we'll keep you informed. |
No problem. We have a working build on our side anyway. |
Thanks @cs-altshift for this and the patience, we've receive the signed corporate CLA from Alt Shift. I've just pushed these changes in an anternative branch (rebasing the changes to the current https://github.com/aseprite/aseprite/tree/fix-normal-map-palette I'll add an extra new commit in these days implementing the new normal map wheel using shaders (because color selectors have changed the way they are painted). |
Finally merge in 20902e3, thanks a lot @cs-altshift 🙏 |
You're welcome. Glad we will finally be able to use the mainstream Aseprite soon! |
Closes #3015
I agree that my contributions are licensed under the Individual Contributor License Agreement V3.0 ("CLA") as stated in https://github.com/aseprite/sourcecode/blob/main/cla.md
I have signed the CLA following the steps given in https://github.com/aseprite/sourcecode/blob/main/sign-cla.md#sign-the-cla