slint-ui / slint

Slint is a declarative GUI toolkit to build native user interfaces for Rust, C++, or JavaScript apps.
https://slint.dev
Other
17.53k stars 600 forks source link

Add properties to the color type to get HSV component #911

Open ogoffart opened 2 years ago

ogoffart commented 2 years ago

One should add the following properties to a colour to access individual components

What should these return? Float from 0 to 1 or int from 0 to 255? (Should hue be in deg) How to distinguish hue and saturation from the hsv and HSL colorspace?

hunger commented 2 years ago

0..255 is a bit short nowadays with people using 10+ bits per color in some places.

Float is more future proof there. But we could also extend the color values to a u16. That is still taking less space than a float, is easy to convert to a u8 if needed and should have enough bits for future screens.

ogoffart commented 2 years ago

0..65535 is out of question. This is about the API in .60, it should be what people expect. And i think some people might expect a value between 0 and 255.
Personally i think a value between 0 and 1 is better.

(If you want more precision, we could still have it a float and have the value between 0.0 and 255.0)

Then how it is going to be in the hardware, we only have the int and float type in .60 which maps to i32 and f32. I don't think it's worth trying to optimize this anyway. But if it is, we could still change it to be some wrapper around u16 in the generated code, for this case only

hunger commented 2 years ago

Then let's go for a float between 0.0 and 1.0. That should give enough bits of precision for the foreseeable future:-)

But a f32 (no need for f64, is there?) is harder and more expensive to turn into a u8 than a u16 is though. And you will need u8s regularly -- at least till graphics cards more widely support 10bit per color.

ogoffart commented 2 years ago

remember that this is only when users call some_color.red in a .60 binding. In practice, it basically will just be kept in a register. And in .60 we only have float and int (and a bunch of other unit that's all equivalent to float)

hunger commented 2 years ago

That's a good point. So I'd go for float in the range between 0.0 and 1.0.

All you need to do is multiply that with some constant to get useful values. And there are enough bits in a float to cover even very wide color channels.

flukejones commented 8 months ago

I'm finding I need this functionality now for a color picker and for doing some other color operations that aren't already available as a slint function

flukejones commented 8 months ago

There are 5 things I would love to see accessible in .slint:

My main usecase for these is in RGB keyboards when creating a colour picker. At this time I have a mostly workable example but the code has many callbacks to manage conversions and calculations. Access to a more comprehensive range of parameters for colour would open up quite a few possibilities I imagine.

Blend I've done like:

        let c1 = RgbaColor {
            alpha: 1.0,
            red: c1.red + (c2.red - c1.red) * f,
            green: c1.green + (c2.green - c1.green) * f,
            blue: c1.blue + (c2.blue - c1.blue) * f,
        };

where f is just scaling factor based on the position of the hue slider I have, I don't know if the name is really that appropriate for this.

flukejones commented 8 months ago

I'd like to have a go at this but I'm a little lost as to how to add extra functions to things like this. Is there an overview or guide available?

flukejones commented 8 months ago

@tronical it might be worth discussing exactly how far we want to go with this. There are many colour spaces available.. should we expose just a few? RGBA is a given, but maybe also HSI? What about CMYK given it is in use heavily for image industry.

If we go this far it starts to feel like we either need additional colour types, or we need to store more data in Color which then expands its memory footprint..

Additional types would mean more impl From. But less overhead as it would be on demand. I don't really know how this would all interact with the slint lang. There is a crate available to ease it https://docs.rs/color-art/latest/color_art/

Calculating any part of HSI or HSL is pretty much calculating the entire group of values so it almost justifies a new type simply to store them and prevent a lot of math overhead if using many colours..

In any case these colour spaces need to be done in the slint compiler as it is very unwieldy to attempt in slint functions.

ref: https://github.com/slint-ui/slint/pull/4749

tronical commented 8 months ago

My feeling is that we should keep Color as it is, with its low-precision, memory efficient rgba8 representation and honestly reflect this also in the Slint facing API. We already expose red/green/blue/alpha as u8 in Rust/C++, and it's the lowest common denominator that works for MCU and MPU. It's insufficient for advanced use-cases, but it works.

I think offering designers the ability to work with other color spaces is something we should do with help from designers and using dedicated types that offer higher precision and corresponding functionality.

My preference, API wise, at this point is if we could expose red, green, blue, and alpha as properties of color values (just like we expose width and height of image, when you can write i := Image {} property <int> foo: i.source.width;.

As data type I'd go with values ranging from 0 to 255 (same as in Rust, C++, and Node.js) and int as underlying data type (to not give the impression of precision we can't provide).

When (and that's not an if :), we decide to offer a higher precision color type as input to the rendering, I think we might want to offer a different data type (say colorf), that one could still opt out of for MCU builds - for example.

What do the others think?

flukejones commented 8 months ago

I pretty much agree with your analysis. I'm going to keep what I've done here in a separate branch as I really don't want a nested chain of callbacks littering my code.

There is already the HSV colour type available which is used for very little except some mixing and such. I was wanting to expose that as an additional colour type in .slint code. From researching a bit around it seems like this would be an acceptable space to use with conversion between Color/Hsva.

So I'll trim the scope of this back to the initial property exposure, then have a crack at an additional type.

ogoffart commented 8 months ago

Let's say we have the following properties (not function) on the color slint type (not brush)

the_color.red
the_color.green
the_color.blue
the_color.alpha

Everything in the range 0 255

Regarding HSV, we could either have:

1. the_color.hue , the_color.saturation, the_color.value
OR
2. the_color.hsv.hue , the_color.hsv.saturation, the_color.hsv.value
OR
3. the_color.to-hsv().hue , the_color.to-hsv().saturation, the_color.to-hsv().value

I think Option 1 is probably the easier to use. Otherwise we probably want it to be a method such as to-hsv() similar to string.to-float(), and that communicate a conversion. (and the return value is something like {hue: degree, saturation: float, value: float})

From an API point of view at least. the way this is implemented, the Slint compiler can always do sub expression optimization so that it doesn't compute the hsv several time.

Regarding blend, there is already mix method on Color: how is it different?

to-hex() that return a string, I personally don't see the usecase (see also https://github.com/slint-ui/slint/issues/4745)

flukejones commented 8 months ago

Let's say we have the following properties (not function) on the color slint type (not brush)

the_color.red
the_color.green
the_color.blue
the_color.alpha

Everything in the range 0 255

Agreed

Regarding HSV, we could either have:

1. the_color.hue , the_color.saturation, the_color.value
OR
2. the_color.hsv.hue , the_color.hsv.saturation, the_color.hsv.value
OR
3. the_color.to-hsv().hue , the_color.to-hsv().saturation, the_color.to-hsv().value

I think Option 1 is probably the easier to use. Otherwise we probably want it to be a method such as to-hsv() similar to string.to-float(), and that communicate a conversion. (and the return value is something like {hue: degree, saturation: float, value: float})

From an API point of view at least. the way this is implemented, the Slint compiler can always do sub expression optimization so that it doesn't compute the hsv several time.

Can you explain how this works? I know very little about the compiler beyond what I've managed to fumble out here. I would prefer a conversion with return of {hue: degree, saturation: float, value: float}.

Regarding blend, there is already mix method on Color: how is it different?

It's very different. The linear_blend I implemented is only on RGB, but hsv. And the effect in use is smoother (when using for example on a colour picker slider) as opposed to the mix() which causes stepping.

to-hex() that return a string, I personally don't see the usecase (see also #4745)

That's certainly personal. Right now I have a callback chain to do the conversion so I can show what hex value a colour is in the colour picker I'm doing for RGB keyboard. In fact most of what I'm proposing is just because it's what I need in a colour picker and I don't want a complex series of callbacks for every conversion, hsv or such.

flukejones commented 8 months ago

My feeling is that we should keep Color as it is, with its low-precision, memory efficient rgba8 representation and honestly reflect this also in the Slint facing API. We already expose red/green/blue/alpha as u8 in Rust/C++, and it's the lowest common denominator that works for MCU and MPU. It's insufficient for advanced use-cases, but it works.

I tried to add an Option to it to avoid that, but Option is not FFI safe.

I think offering designers the ability to work with other color spaces is something we should do with help from designers and using dedicated types that offer higher precision and corresponding functionality.

The already existing HSV should be exposed I think. And then add some other common types (CMYK?)

My preference, API wise, at this point is if we could expose red, green, blue, and alpha as properties of color values (just like we expose width and height of image, when you can write i := Image {} property <int> foo: i.source.width;.

This is what I wanted to do in my PR but it was very difficult for me to discover how to do this.

As data type I'd go with values ranging from 0 to 255 (same as in Rust, C++, and Node.js) and int as underlying data type (to not give the impression of precision we can't provide).

For RGB yes. For other colour spaces it will need to be different.

ogoffart commented 8 months ago

Can you explain how this works?

How what works? For example, there is already image.width that in lookup.rs is lowered to something like BuiltinFunction::ImageSize(image).width.

The subexpression optimization is in deduplicate_property_read. Although currently it makes sure that an expression such as: some-property.foo - some-property.bar and we only generate code that reads some-property once. It could be extended to detect more sub-expressions such as pure function calls.

Anyway, what I mean is that in the .slint language, we first try to get easy to understand API for designer and then we can always try to find ways to make it work.

It's very different. The linear_blend I implemented is only on RGB, but hsv. And the effect in use is smoother (when using for example on a colour picker slider) as opposed to the mix() which causes stepping.

Maybe the problem here is a bug in the mix implementation? Also for animation on color, we just interpolate every rgb component, which is just the easier but not the most correct. We may want to fix that anyway.

The already existing HSV should be exposed I think. And then add some other common types (CMYK?)

Agreed, we could just polish a bit the API.

flukejones commented 8 months ago

How what works? For example, there is already image.width that in lookup.rs is lowered to something like BuiltinFunction::ImageSize(image).width.

Thanks! I managed to find that info on my own just before. So i have the red/green/blue fields exposed as properties now.

The subexpression optimization is in deduplicate_property_read. Although currently it makes sure that an expression such as: some-property.foo - some-property.bar and we only generate code that reads some-property once. It could be extended to detect more sub-expressions such as pure function calls.

A bit beyond my knowledge of slint internals.. :)

Anyway, what I mean is that in the .slint language, we first try to get easy to understand API for designer and then we can always try to find ways to make it work.

Most of what I'm doing with a current PR is pretty much this. I'm the active designer of a component and need certain things.

Maybe the problem here is a bug in the mix implementation? Also for animation on color, we just interpolate every rgb component, which is just the easier but not the most correct. We may want to fix that anyway.

No, the mix function is working with HSV, while the very simple linear_mix I've added is directly on rgb(0-255).

The already existing HSV should be exposed I think. And then add some other common types (CMYK?)

Agreed, we could just polish a bit the API.

I'll work on this? I would prefer color<->hsv conversions. It would mean RGB color is kept clean and simple plus compact for embedded cases, while having more complex colour system within reach on demand.

ogoffart commented 8 months ago

Red/green/blue accessor added in https://github.com/slint-ui/slint/pull/4796 Thanks @flukejones

Hsv colour space is not yet implemented.

flukejones commented 8 months ago

I've done the HSV part, just need to rebase on newly merged work.