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

White Escapes New Normal Map Equator #3595

Closed
behreajj opened this issue Nov 17, 2022 · 1 comment
Closed

White Escapes New Normal Map Equator #3595

behreajj opened this issue Nov 17, 2022 · 1 comment
Assignees
Labels
Milestone

Comments

@behreajj
Copy link
Contributor

behreajj commented Nov 17, 2022

This is an issue related to the new normal map from 20902e3 . I'm expecting the color white to fall around #dada80 which is (45 degrees theta, 0 degrees phi) in spherical coordinates or (0.7071, 0.7071, 0.0) in Cartesian coordinates.

Not sure what to suggest for the current approach because I 1. try to use fewer trigonometric functions and 2. allow the normal sphere to be flipped on the z axis. Below is a snippet of some simplified Lua code.

local xi = center
local yi = center

local r255 = hexSrgb & 0xff
local g255 = (hexSrgb >> 0x08) & 0xff

local x = (r255 + r255 - 255) / 255.0
local y = (255 - (g255 + g255)) / 255.0

local sqMag2d = x * x + y * y
if sqMag2d > 0.000031 then
    local invMag2d = 1.0 / math.sqrt(sqMag2d)
    local xn2d = x * invMag2d
    local yn2d = y * invMag2d
    local xu = 0.5
    local yu = 0.5
    if sqMag2d >= 1.0 then
        xu = xn2d * 0.5 + 0.5
        yu = yn2d * 0.5 + 0.5
    else
        local b255 = (hexSrgb >> 0x10) & 0xff
        local z = (b255 + b255 - 255) / 255.0
        local zn3d = z / math.sqrt(sqMag2d + z * z)

        -- cos(asin(z)) == sqrt(1 - z * z)
        local dist = math.sqrt(1.0 - zn3d * zn3d)
        local xn3d = xn2d * dist
        local yn3d = yn2d * dist

        xu = xn3d * 0.5 + 0.5
        yu = yn3d * 0.5 + 0.5
    end

    xi = floor(0.5 + xu * size)
    yi = floor(0.5 + yu * size)
end

Not sure, but some users may expect a color like white to be normalized in advance, making white into #c9c9c9, (45 degrees theta, 35 degrees phi) or (0.577, 0.577, 0.577). For a color picker, though, I prefer clamping so that the reticule doesn't bounce around on the wheel.

@dacap
Copy link
Member

dacap commented Nov 17, 2022

I think you refer to this right?

image

I guess in ColorWheel::onPaintMainArea(), before we call paintColorIndicator() when can just clamp the distance when we use red/green values only:

diff --git a/src/app/ui/color_wheel.cpp b/src/app/ui/color_wheel.cpp
index 6f0b0ea2b..187b9e2c9 100644
--- a/src/app/ui/color_wheel.cpp
+++ b/src/app/ui/color_wheel.cpp
@@ -344,9 +344,14 @@ void ColorWheel::onPaintMainArea(ui::Graphics* g, const gfx::Rect& rc)
       double x, y;
 
       double approximationThreshold = (246.0 / 255.0) * 2.0 - 1.0;
-      if (normalizedBlue > approximationThreshold) { // If blue is too high, we use red and green only as approximation
-        x = normalizedRed * m_wheelRadius;
-        y = -normalizedGreen * m_wheelRadius;
+      if (normalizedBlue > approximationThreshold) {
+        // If blue is too high, we use red and green only as approximation
+        double angle = std::atan2(normalizedGreen, normalizedRed);
+        double dist = std::sqrt(normalizedRed*normalizedRed + normalizedGreen*normalizedGreen);
+        dist = std::clamp(dist, 0.0, 1.0);
+
+        x = std::cos(angle) * m_wheelRadius * dist;
+        y = -std::sin(angle) * m_wheelRadius * dist;
       }
       else {
         double normalizedDistance = std::cos(std::asin(normalizedBlue));

@dacap dacap self-assigned this Nov 17, 2022
@dacap dacap added this to the v1.3.0 milestone Nov 17, 2022
@dacap dacap added the wip label Nov 17, 2022
@dacap dacap closed this as completed in 01967be Nov 17, 2022
@dacap dacap removed the wip label Nov 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants