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

Add DragValues for RGB(A) in the color picker #2734

Merged
merged 29 commits into from Jan 7, 2024

Conversation

IVAN-MK7
Copy link
Contributor

Added some DragValue widgets in the color_picker widget as input fields for managing the RGBA values.
In case the provided values result in a not valid premultiplied alpha RGBA color, a button will appear next to the input fields, to be used to multiply the values with the alpha channel.

image

Closes #2716.

@IVAN-MK7 IVAN-MK7 marked this pull request as ready for review February 13, 2023 20:11
Copy link
Owner

@emilk emilk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is quite confusing how this works with pre-multiplied alpha.

I think we should keep premultiplied alpha out of the UI.

Instead, unmultiply the alpha before showing the buttons, and then special-case alpha=0 (additive).

Otherwise it is very easy to end up with things like this:

image

crates/egui/src/widgets/color_picker.rs Outdated Show resolved Hide resolved
crates/egui/src/widgets/color_picker.rs Outdated Show resolved Hide resolved
crates/egui/src/widgets/color_picker.rs Outdated Show resolved Hide resolved
IVAN-MK7 and others added 2 commits March 2, 2023 15:27
Co-authored-by: Emil Ernerfeldt <emil.ernerfeldt@gmail.com>
@IVAN-MK7
Copy link
Contributor Author

IVAN-MK7 commented Mar 2, 2023

I've applied the suggestions, had to work around the case when Additive blending is signaled with a negative alpha on Alpha::BlendOrAdditive.
Now the DragValues take unmultiplied RGBA values and the Alpha DragValue is hidden in case the provided Alpha is Alpha::Opaque (Alpha::Opaque is automatically sent to the fn in case of a negative alpha on Alpha::BlendOrAdditive).

color_picker_srgba_unmultiplied_alpha_input_values

Given the negative alpha thing, users still can't switch to "Additive" blending when alpha is 0, even after this PR change.

@IVAN-MK7 IVAN-MK7 requested a review from emilk March 2, 2023 15:20
@IVAN-MK7 IVAN-MK7 requested a review from emilk March 29, 2023 16:18
Copy link
Contributor

@tosti007 tosti007 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, I'd really like this addition! To maybe help this further I gave it a review.

crates/egui/src/widgets/color_picker.rs Outdated Show resolved Hide resolved
crates/egui/src/widgets/color_picker.rs Outdated Show resolved Hide resolved
IVAN-MK7 and others added 3 commits June 7, 2023 12:30
Remove unnecessary check on Alpha::Opaque for alpha_control

Co-authored-by: Brian Janssen <tosti007@users.noreply.github.com>
Copy link
Contributor

@tosti007 tosti007 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added another small nit i just realised. Also: Maybe it'd be good to support f32 drag values, since that would allow for HDR color pickers (which is my use case)?

crates/egui/src/widgets/color_picker.rs Outdated Show resolved Hide resolved
@IVAN-MK7
Copy link
Contributor Author

IVAN-MK7 commented Jun 8, 2023

Added another small nit i just realised. Also: Maybe it'd be good to support f32 drag values, since that would allow for HDR color pickers (which is my use case)?

image

Setting 3 decimal values seems the best option for the f32, but the layout would need a rearrange, also adding a button to switch between u8 and f32 appears necessary, which would require to add a new field needed to store the preferred choise (maybe to ui.style.visuals.).

@IVAN-MK7 IVAN-MK7 closed this Jun 8, 2023
@IVAN-MK7 IVAN-MK7 deleted the color-picker-rgba-input branch June 8, 2023 14:18
@IVAN-MK7 IVAN-MK7 restored the color-picker-rgba-input branch June 8, 2023 14:18
@IVAN-MK7 IVAN-MK7 deleted the color-picker-rgba-input branch June 8, 2023 14:18
@IVAN-MK7 IVAN-MK7 restored the color-picker-rgba-input branch June 8, 2023 14:18
@IVAN-MK7 IVAN-MK7 reopened this Jun 8, 2023
@imsi32
Copy link

imsi32 commented Oct 7, 2023

Hello, while I was looking demos, I faced hard time to get the correct colors, so I find this pull request. Because I don't know egui yet, I couldn't help with my suggestion.

I think buttons on the top doesn't look good. Instead of putting them there replacing with rgba(1,1,1,1) text will be better idea like this image:
egui-color-picker-1

Secondly, even though I don't understand the additive page. When I open the additive pages, alpha area disables and all page change place. So putting alpha area in end of the page will make page less shaking. Unfortunately, color changing area needs to put upper of the alpha area to eliminate color mixing between color choosing area and alpha area.
egui-color-picker-2

@IVAN-MK7
Copy link
Contributor Author

IVAN-MK7 commented Oct 8, 2023

I've added the

pub color_picker_input_values_type: ColorPickerInputType,

field to style's Visuals.
Also rearranged color_picker's color sliders, and made color_text_ui display unmultiplied rgba values, this is the final result:

colorpicker_u8_f32_rearrange_2

The resulting appearance definitely needs some polish, it would be better to put buttons and drag values into a StripBuilder, but it must be imported from the egui_extras crate. Help and advices are welcome.

@emilk emilk added feature New feature or request egui labels Nov 10, 2023
crates/egui/src/widgets/color_picker.rs Outdated Show resolved Hide resolved
crates/egui/src/widgets/color_picker.rs Outdated Show resolved Hide resolved
crates/egui/src/widgets/color_picker.rs Outdated Show resolved Hide resolved
crates/egui/src/widgets/color_picker.rs Outdated Show resolved Hide resolved
crates/egui/src/widgets/color_picker.rs Outdated Show resolved Hide resolved
@IVAN-MK7
Copy link
Contributor Author

This is the resulting UI following the recent changes:

u8f32rearranged

Needs a finishing touch for the layout, I tried to make so that the change of RGBA values and input type (U8/F32) made the least amount of popping, but could't go very far without a StripBuilder.

@Ved-s
Copy link

Ved-s commented Jan 7, 2024

Would be nice seeing this implemented in. Also I'd suggest adding hex color field so it's easier to copy/paste, it could accept data in few formats #rrggbbaa/#rrggbb/#rgba/#rgb/#w/#ww (last two are grayscales), but show only #rrggbbaa

crates/egui/src/style.rs Outdated Show resolved Hide resolved
crates/egui/src/style.rs Outdated Show resolved Hide resolved
@emilk emilk changed the title color_picker RGBA input fields Add DragValues for RGB(A) in the color picker Jan 7, 2024
@emilk emilk merged commit bfed2b4 into emilk:master Jan 7, 2024
19 checks passed
@IVAN-MK7 IVAN-MK7 deleted the color-picker-rgba-input branch January 7, 2024 17:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
egui feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RGBA values input fields for color_picker
5 participants