api icon indicating copy to clipboard operation
api copied to clipboard

Boundary checking for Color

Open behreajj opened this issue 4 years ago • 0 comments

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

behreajj avatar Jul 03 '21 11:07 behreajj