Skip to content

Changed ui.disable() to modify opacity. #6765

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

Closed
wants to merge 13 commits into from

Conversation

tye-exe
Copy link
Contributor

@tye-exe tye-exe commented Apr 16, 2025

It previously tinted elements, which could have lackluster effects.

New looks

(I say new but I tried to keep them as similar as possible to previous)

Light mode

light_mode.webm

Dark mode

dark_mode.webm

Additional remarks

Since the opacity is being change, elements that are on-top of other elements now show some of the underneath element (this can be seen with the slider in the sent videos).

It previously tinted elements, which could have lackluster effects.
@juh9870
Copy link

juh9870 commented Apr 16, 2025

This has far-reaching consequences for the visual of existing egui apps other than just fixing the linked issue.

I think this change should be configurable; otherwise, significant considerations should be put into messing up existing visuals.

@tye-exe
Copy link
Contributor Author

tye-exe commented Apr 17, 2025

That's a fair viewpoint.

What other possible solutions to this issue do you recommend?

I tried modifying the fade_out_to_color method (as also suggested in the issue) to modify the alpha channel. However, this was not able to prevent the issue on light mode, even at the extremes of the alpha channel.

Relevant method:

egui/crates/egui/src/style.rs

Lines 1055 to 1060 in 5d6aaa2

/// When fading out things, we fade the colors towards this.
// TODO(emilk): replace with an alpha
#[inline(always)]
pub fn fade_out_to_color(&self) -> Color32 {
self.widgets.noninteractive.weak_bg_fill
}

Out of interest in which situations will this be an issue for (so I could potentially work on solving them)?

@juh9870
Copy link

juh9870 commented Apr 17, 2025

The problem is exactly what you mentioned in the Additional remarks sections.

I believe that a method of fading out (color or transparency) should be an option in the style. Maybe as an enum variant akin to

enum FadeOutMethod {
    NonInteractiveWeakBgFill,
    CustomColor(Color32),
    DimTransparency(f32)
}

the exact shape of variants and naming should be discussed ofc

@tye-exe
Copy link
Contributor Author

tye-exe commented Apr 17, 2025

I agree with being able to override the fade out colour, but that does lean towards a more major change, as Visuals is a commonly used data structure.

Also adding more default colours increases the maintenance burden in the future. i'm not sure if that would be needed if the user can customise the fade out colour.

@tye-exe
Copy link
Contributor Author

tye-exe commented Apr 17, 2025

A possible option for setting a custom fade out colour would be to add a feild like override_fade: Option<Color32>, which can default to None.

@emilk emilk added bug Something is broken visuals Renderings / graphics releated labels Apr 22, 2025
Copy link

Preview available at https://egui-pr-preview.github.io/pr/6765-defaultaffectsopacity
Note that it might take a couple seconds for the update to show up after the preview_build workflow has completed.

"fade_out_to_color" was replaced by "fade_out_opacity" for this purpose.
@tye-exe
Copy link
Contributor Author

tye-exe commented Apr 25, 2025

Will it be a good idea to add an attribute to Visuals that is returned from fade_out_opacity?

Because currently the value is hard-coded, so it cannot be changed.

The fade amount for UI elements can be customised.
@tye-exe
Copy link
Contributor Author

tye-exe commented Apr 30, 2025

I have added the option for user-customization that i suggested & created a pull request against the branch in my fork for anyone to look at.

I can easily merge these changes into the branch in this egui pull request.

@emilk emilk added this to the 0.32.0 milestone May 18, 2025
@emilk emilk added the egui label May 18, 2025
@tye-exe
Copy link
Contributor Author

tye-exe commented May 18, 2025

emilk, do you want me to merge the user-customisation code?

@emilk
Copy link
Owner

emilk commented May 19, 2025

Yes, adding a Visuals::disabled_alpha: f32 that default to 0.5 sounds like a great idea

@tye-exe
Copy link
Contributor Author

tye-exe commented May 19, 2025

Yes, adding a Visuals::disabled_alpha: f32 that default to 0.5 sounds like a great idea

👍 I have done so.

@emilk
Copy link
Owner

emilk commented May 21, 2025

There is a problem with this approach: Visuals::weak_text_color uses Visuals::gray_out, but gray_out no longer makes the text gray; it makes it transparent. This may be fine, but we should probably rename gray_out, and we also need to update snapshot tests.

@tye-exe
Copy link
Contributor Author

tye-exe commented May 22, 2025

I will be able to work on this at the weekend

Split these two behaviours into different functions, as either behaviour is sometimes desirable.
@tye-exe
Copy link
Contributor Author

tye-exe commented May 25, 2025

I've restored gray_out to its previous state, and created the new function disable that returns the colour with the alpha modified.

This allows for either to be used if modifying the alpha if undesirable.

@tye-exe
Copy link
Contributor Author

tye-exe commented May 25, 2025

An issue with the modify opacity approach is that the "colour picker" widget does not look very good when disabled on light mode.

Screencast_20250525_095916.webm

@tye-exe
Copy link
Contributor Author

tye-exe commented May 25, 2025

Should i re-add the fade_out_to_color function, but annotated it as deprecated?
This will give time for users to stop using this function instead of having an instant compile error.

@tye-exe tye-exe requested a review from lucasmerlin as a code owner June 3, 2025 15:11
@emilk
Copy link
Owner

emilk commented Jun 16, 2025

Should i re-add the fade_out_to_color function, but annotated it as deprecated? This will give time for users to stop using this function instead of having an instant compile error.

Yeah, why not.

You also need to update the snapshot images (again), because of a merge race (sorry!)

@tye-exe
Copy link
Contributor Author

tye-exe commented Jun 17, 2025

I think that you have to run the CI to generate the images again for me to pull them from the CI, apologies.

@tye-exe
Copy link
Contributor Author

tye-exe commented Jun 18, 2025

This should do it!

@emilk emilk closed this in #7282 Jul 1, 2025
emilk added a commit that referenced this pull request Jul 1, 2025
* Closes #6765


Branched off of #6765 by @tye-exe.

I needed to branch off to update snapshot images

---------

Co-authored-by: tye-exe <[email protected]>
Co-authored-by: Tye <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is broken egui visuals Renderings / graphics releated
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Disabled image button doesn't fade out in light mode
3 participants