Page MenuHome

Fix T80356: Icon alpha theme property affects previews
Needs RevisionPublic

Authored by Hans Goudey (HooglyBoogly) on Sep 2 2020, 8:48 PM.

Details

Summary

Check the icon's type before using the icon_alpha theme property.

Diff Detail

Repository
rB Blender
Branch
icon-alpha-no-previews (branched from master)
Build Status
Buildable 9943
Build 9943: arc lint + arc unit

Event Timeline

Hans Goudey (HooglyBoogly) requested review of this revision.Sep 2 2020, 8:48 PM
Hans Goudey (HooglyBoogly) created this revision.
This revision is now accepted and ready to land.Sep 3 2020, 10:43 AM

What if it is the custom UI icon? Like here T78630, T64628 for example.

Julian Eisel (Severin) requested changes to this revision.Sep 3 2020, 3:21 PM

Custom icons are indeed an issue. Matcaps and such use ICON_TYPE_BUFFER, other previews like material previews use ICON_TYPE_PREVIEW. However ICON_TYPE_PREVIEW is also used for custom icons (should maybe be mentioned in a comment at its definition). So we can't just rely on that currently.
I think there could just be a bool use_theme_icon_alpha passed to icon_create() and stored in Icon. That gives better control and seems generally a bit nicer, since the option is controlled at a higher level.

Another issue is that custom icons are used for both, regular icons, as well as custom previews. So we'd need the information on how the icon is used to know if we should apply the alpha. For now I'd say just always apply the alpha for custom icons. If needed we can let the Python API have more control.

source/blender/editors/interface/interface_icons.c
1783

Event icons may be mixed with other icons, so we should apply alpha for them too, otherwise:


Note that this is already an issue in master, but that's separate. So I'd just add ICON_TYPE_EVENT here.

This revision now requires changes to proceed.Sep 3 2020, 3:21 PM

Do we even need this option? This was for color icons ("to reduce contrast"), now the color of icons respects the theme settings.

Removing it would be fine with me. I also don't see a need and initially thought it was added during 2.8. But seems it's an old option and probably redundant by now.

What about just doing that? The setting seems pretty useless to me too. IMO there should be better ways to theme the icons, but I don't see why this needs to be one of them.

What about just doing that? The setting seems pretty useless to me too. IMO there should be better ways to theme the icons, but I don't see why this needs to be one of them.

That wouldn't be the end of the world... but I actually like that feature...
I got a quite dark custom theme and (depending on the mood hehehe) sometimes I feel like the icons are in the way.... so be able to adjust the opacity of all the icons at same time with just one slider was kinda neat....

Do we even need this option? This was for color icons ("to reduce contrast"), now the color of icons respects the theme settings.

Removing it would be fine with me. I also don't see a need and initially thought it was added during 2.8. But seems it's an old option and probably redundant by now.

What about just doing that? The setting seems pretty useless to me too. IMO there should be better ways to theme the icons, but I don't see why this needs to be one of them.

I will leave the final word on this up to @Brecht Van Lommel (brecht), he added this option years ago (rB7217518179d9ed).

I'm not particularly attached to keeping the feature, it's not important. It's still convenient somewhat, so we might as well keep it and not break API/theme compatibility?

However ICON_TYPE_PREVIEW is also used for custom icons (should maybe be mentioned in a comment at its definition). So we can't just rely on that currently.

I'm not sure how deep this goes, but using a different ICON_TYPE for custom icons would be good then, since using ICON_TYPE_PREVIEW makes little sense.

Yo, any news on this? πŸ€”

Okay, I looked at this again. The issue with this patch is that it affect's custom icons too.

The relationship between custom icons a does indeed go pretty deep-- as far as I can tell, they ARE previews.
So there is no way to differentiate between the two without exposing some new parameter to python, where both can be created.

I'd like a test case before I work on this though. Do you know of an addon included in Blender that uses custom icons?

Do you know of an addon included in Blender that uses custom icons?

Unfortunately no. But if I find something I'll let you know...

Well, this icon_alpha does not applied to the big Alert icon.
In the File Browser, it is not applied to the large folder icon (thumbnails mode) but it does to small ones (list mode).
It not applied to the EVENT and the VECTOR (key type, handle type) icons.

But it is applied where it is not expected...

For me, icon_alpha option only makes sense for GEOM toolbar icons, together with the Icon Saturation.

Also, for example, if the addon uses custom icons, they do not respect the text color from the theme
as internal ones do, so they are likely to be different anyway.