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

Boundary checking for Color #49

Open
behreajj opened this issue Jul 3, 2021 · 0 comments
Open

Boundary checking for Color #49

behreajj opened this issue Jul 3, 2021 · 0 comments

Comments

@behreajj
Copy link
Contributor

behreajj commented Jul 3, 2021

Hello,

I'd like to suggest some extra validation for inputs given to Color. Since hue is a periodic value, that would mean wrapping negative values. For other inputs, that would be clamping to the lower and upper bounds of the range.

local x = Color(-3, 255, 257)
local y = x.rgbaPixel
local z = Color(y)

print("RGB boundary checking.")
print(string.format("%08X", y))  -- FF01FFFD
print(string.format("%d, %d, %d", x.red, x.green, x.blue))
print(string.format("%d, %d, %d", z.red, z.green, z.blue))

-- The % operator is floor modulo in Lua.
local u = Color{ h = -18.0, s = 1.2, v = 1.0 }
local v = Color{ h = (-18.0 % 360.0), s = 1.0, v = 1.0 }

print("\nHue boundary checking.")
print(string.format("u hue: %d", u.hsvHue))
print(string.format("v hue: %d", v.hsvHue))
print(string.format("u sat: %d", u.hsvSaturation))
print(string.format("%d, %d, %d", u.red, u.green, u.blue))
print(string.format("%d, %d, %d", v.red, v.green, v.blue))
print(string.format("%08X", u.rgbaPixel))  -- FF00B4FF
print(string.format("%08X", v.rgbaPixel))  -- FF4C00FF

As it stands, there is overflow when a color is converted to an integer. It looks like there may be some boundary checking for other arguments given to HSV conversion, such as saturation.

I'm referring more to the public-facing color_class, not to internal color class methods: https://github.com/aseprite/aseprite/blob/6b2c296ef0c00a42d76d3f76ecf6efb50009a88a/src/app/script/color_class.cpp#L38 .

Thank you for considering this idea.
Best,
Jeremy

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

No branches or pull requests

1 participant