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

Colors are not exact RGB values #6096

Closed
scarmain opened this issue Dec 26, 2023 · 3 comments · Fixed by #6100
Closed

Colors are not exact RGB values #6096

scarmain opened this issue Dec 26, 2023 · 3 comments · Fixed by #6100

Comments

@scarmain
Copy link
Contributor

Describe the bug
The RGB values that are rounded are not rounding properly. For example, Color.kRed returns red = 1, green = 2.44140625E-4, blue = 2.44140625E-4.

I think this has to do with the Color.roundAndClamp(), this should probably only be run with the equals() function, not all the colors.

To Reproduce
(Using java test)
assertEquals(1, Color.kRed.red, 1e-4);
assertEquals(0, Color.kRed.green, 1e-4); //fails, 2.44140625E-4
assertEquals(0, Color.kRed.blue, 1e-4); //fails, 2.44140625E-4

Expected behavior
The values should match 1,0,0 exact

Desktop (please complete the following information):

  • OS: Win 10
  • Project Information:
    Project Version: 2024.1.1-beta-3
    VS Code Version: 1.84.0
    WPILib Extension Version: 2024.1.1-beta-3
    C++ Extension Version: 1.17.5
    Java Extension Version: 1.23.0
    Java Debug Extension Version: 0.52.0
    Java Dependencies Extension Version 0.23.0
    Java Version: 17
    Java Location: C:\Users\Public\wpilib\2024\jdk
    Vendor Libraries:
    PathplannerLib (2024.0.0-beta-6)
    CTRE-Phoenix (v5) (5.32.0-beta-2)
    CTRE-Phoenix (v6) (24.0.0-beta-4)
    REVLib (2024.0.0)
    WPILib-New-Commands (1.0.0)

Additional context
I was trying to make a Color.getHSV function, and it was throwing off results (red saturation would be 254.9377, calculated max(rgb)-min(rgb)/max * 255)

@calcmogul
Copy link
Member

calcmogul commented Dec 26, 2023

From https://github.wpilib.org/allwpilib/docs/development/java/edu/wpi/first/wpilibj/util/Color.html, the rounding is to limit colors to 12 bits of precision.

2^-12 works out to 0.000244140625. Maybe it's an off-by-one error in the rounding?

@scarmain
Copy link
Contributor Author

scarmain commented Dec 26, 2023

Also related, the fromHSV has a ton of rounding also, which doesn't need it, as Color accepts doubles... (although you would have to divide by 255 for all of them)

Color.fromHSV(115, 23, 15).toHexString() = #0E0F0F
From Website https://www.rapidtables.com/convert/color/hsv-to-rgb.html (to use this, the hue must be doubled from 180* to 360*, and the sat/value should x*100/255, as we use 255 as max)
hsv(230,9.02,5.88) = #0E0E0F

Edit: I know it's knit-picky, but it was really throwing off my inverse HSV formula... Again using that site:
RBG(14, 15, 15) = HSV (180, 6.7, 5.9) (their units, not WPILIB)
RGB(14, 14, 15) = HSV (240, 6.7, 5.9)
Maybe I need to lower my test sensitivity, but it's crazy to have a 30* (in WpiLib units) error.

@calcmogul
Copy link
Member

calcmogul commented Dec 26, 2023

We'd accept a PR that fixes the HSV conversion accuracy. The test tolerances are like 1e-2, which is massive for the (0, 1) scale. I'm not personally familiar with the color conversion code, or color conversion in general, so I'm not the right person to mess with that code.

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 a pull request may close this issue.

2 participants