Color indicating animation status missing from many icon buttons (all defined with emboss=False) #89350

New Issue

raylee · 2021-06-22T06:25:48+02:00

raylee commented

2021-06-22 06:25:48 +02:00

System Information
Operating system:window 10
Graphics card:GTX1060

Blender Version
Broken:2.92
Worked: 2.91

Caused by 600fb28b62

Short description of error
You can for example add keyframes for GP layer visibility, but you can’t see the colors (green=animated, yellow=keyframe etc)

**See the video link for more details
https://youtu.be/v0ktPrHQyLc

Exact steps for others to reproduce the error
from the default startup file:

add a shapekey to the default cube
add a keyframe to the shapekey Mute property

2.91

2.92

**System Information** Operating system:window 10 Graphics card:GTX1060 **Blender Version** Broken:2.92 Worked: 2.91 Caused by 600fb28b62 **Short description of error** You can for example add keyframes for GP layer visibility, but you can’t see the colors (green=animated, yellow=keyframe etc) **See the video link for more details https://youtu.be/v0ktPrHQyLc **Exact steps for others to reproduce the error** from the default startup file: - add a shapekey to the default cube - add a keyframe to the shapekey `Mute` property 2.91 ![image.png](https://archive.blender.org/developer/F10187671/image.png) 2.92 ![image.png](https://archive.blender.org/developer/F10187673/image.png)

raylee commented

2021-06-22 06:25:48 +02:00

Added subscriber: @RayLee

blender-admin commented

2021-06-22 06:25:48 +02:00

#86299 was marked as duplicate of this issue

Philipp Oeser commented

2021-06-22 10:54:45 +02:00

Added subscriber: @lichtwerk

Philipp Oeser commented

2021-06-22 10:54:45 +02:00

Changed status from 'Needs Triage' to: 'Confirmed'

Philipp Oeser self-assigned this 2021-06-22 10:54:45 +02:00

Philipp Oeser commented

2021-06-22 10:54:45 +02:00

Please only one issue per report (so sticking with "Problem 1" for now, please report "Problem 2" separately):

I can confirm that (icon) toggle buttons inside UI Lists dont show the background color indicating whether they are animated or not.
This was working at some point, will check.

Please only one issue per report (so sticking with "Problem 1" for now, please report "Problem 2" separately): I can confirm that (icon) toggle buttons inside UI Lists dont show the background color indicating whether they are animated or not. This was working at some point, will check.

Philipp Oeser changed title from ~~Greasepencil layer visibility color display missing, drive exception~~ to Color indicating animation status missing from UIList toggle buttons

2021-06-22 10:56:38 +02:00

Philipp Oeser commented

2021-06-22 12:06:25 +02:00

Caused by 600fb28b62 apparently

Caused by 600fb28b62 apparently

Philipp Oeser changed title from ~~Color indicating animation status missing from UIList toggle buttons~~ to Color indicating animation status missing from many icon buttons (all defined with emboss=False)

2021-06-22 13:39:55 +02:00

Philipp Oeser removed their assignment 2021-06-22 14:04:12 +02:00

Philipp Oeser commented

2021-06-22 14:04:12 +02:00

Added subscriber: @HooglyBoogly

Philipp Oeser commented

2021-06-22 14:04:12 +02:00

So, the culprit commit checks for UI_EMBOSS_NONE in widget_icon_has_anim and there was another check for it in widget_color_blend_from_flags, and with these checks active, we dont get colored icon backgrounds here.

Isolated, this seems to make sense (since it is supposed to do /* Nothing, only icon and/or text */ in the case of UI_EMBOSS_NONE.
However, there are many, many places where the prop gets shown in the UI via layout.prop(... emboss=False)
This gets translated into UI_EMBOSS_NONE eventualy (rna_uiItemR sets UI_ITEM_R_NO_BG, then uiItemFullO_ptr_ex translates this to layout->emboss = UI_EMBOSS_NONE)

Note we have row.emboss = 'UI_EMBOSS_NONE_OR_STATUS' for such cases, so I wonder if it would make sense to remove all emboss=False?

Something like this:
P2199: #89350 emboss=False problem



diff --git a/release/scripts/startup/bl_ui/properties_data_bone.py b/release/scripts/startup/bl_ui/properties_data_bone.py
index daf64642f68..8a44f223a95 100644
--- a/release/scripts/startup/bl_ui/properties_data_bone.py
+++ b/release/scripts/startup/bl_ui/properties_data_bone.py
@@ -85,8 +85,9 @@ class BONE_PT_transform(BoneButtonsPanel, Panel):
                 row.prop(pchan, "rotation_quaternion", text="Rotation")
                 sub = row.column(align=True)
                 sub.use_property_decorate = False
-                sub.prop(pchan, "lock_rotation_w", text="", emboss=False, icon='DECORATE_UNLOCKED')
-                sub.prop(pchan, "lock_rotation", text="", emboss=False, icon='DECORATE_UNLOCKED')
+                sub.emboss = 'UI_EMBOSS_NONE_OR_STATUS'
+                sub.prop(pchan, "lock_rotation_w", text="", icon='DECORATE_UNLOCKED')
+                sub.prop(pchan, "lock_rotation", text="", icon='DECORATE_UNLOCKED')
             elif rotation_mode == 'AXIS_ANGLE':
                 col = layout.column()
                 row = col.row(align=True)
diff --git a/release/scripts/startup/bl_ui/properties_data_mesh.py b/release/scripts/startup/bl_ui/properties_data_mesh.py
index a9d7b8d71f3..94abff09f57 100644
--- a/release/scripts/startup/bl_ui/properties_data_mesh.py
+++ b/release/scripts/startup/bl_ui/properties_data_mesh.py
@@ -126,7 +126,7 @@ class MESH_UL_shape_keys(UIList):
                 row.prop(key_block, "value", text="")
             else:
                 row.label(text="")
-            row.prop(key_block, "mute", text="", emboss=False)
+            row.prop(key_block, "mute", text="")
         elif self.layout_type == 'GRID':
             layout.alignment = 'CENTER'
             layout.label(text="", icon_value=icon)
diff --git a/release/scripts/startup/bl_ui/properties_grease_pencil_common.py b/release/scripts/startup/bl_ui/properties_grease_pencil_common.py
index 0da0716e850..6048cea6f67 100644
--- a/release/scripts/startup/bl_ui/properties_grease_pencil_common.py
+++ b/release/scripts/startup/bl_ui/properties_grease_pencil_common.py
@@ -657,10 +657,11 @@ class GPENCIL_UL_layer(UIList):
             row.prop(gpl, "info", text="", emboss=False)
 
             row = layout.row(align=True)
+            row.emboss = 'UI_EMBOSS_NONE_OR_STATUS'
 
             icon_mask = 'MOD_MASK' if gpl.use_mask_layer else 'LAYER_ACTIVE'
 
-            row.prop(gpl, "use_mask_layer", text="", icon=icon_mask, emboss=False)
+            row.prop(gpl, "use_mask_layer", text="", icon=icon_mask)
 
             subrow = row.row(align=True)
             subrow.prop(
@@ -670,8 +671,8 @@ class GPENCIL_UL_layer(UIList):
                 icon='ONIONSKIN_ON' if gpl.use_onion_skinning else 'ONIONSKIN_OFF',
                 emboss=False,
             )
-            row.prop(gpl, "hide", text="", emboss=False)
-            row.prop(gpl, "lock", text="", emboss=False)
+            row.prop(gpl, "hide", text="")
+            row.prop(gpl, "lock", text="")
         elif self.layout_type == 'GRID':
             layout.alignment = 'CENTER'
             layout.label(
diff --git a/source/blender/editors/interface/interface_widgets.c b/source/blender/editors/interface/interface_widgets.c
index ce1109ad9df..ca9834d8fe5 100644
--- a/source/blender/editors/interface/interface_widgets.c
+++ b/source/blender/editors/interface/interface_widgets.c
@@ -2544,6 +2544,7 @@ static const uchar *widget_color_blend_from_flags(const uiWidgetStateColors *wco
                                                   const eUIEmbossType emboss)
 {
   /* Explicitly require #UI_EMBOSS_NONE_OR_STATUS for color blending with no emboss. */
+  // this check does not survive
   if (emboss == UI_EMBOSS_NONE) {
     return NULL;
   }
@@ -3936,6 +3937,7 @@ static void widget_unitvec(
 static void widget_icon_has_anim(
     uiBut *but, uiWidgetColors *wcol, rcti *rect, int state, int roundboxalign)
 {
+  // this check does not survive
   if (state & (UI_BUT_ANIMATED | UI_BUT_ANIMATED_KEY | UI_BUT_DRIVEN | UI_BUT_REDALERT) &&
       but->emboss != UI_EMBOSS_NONE) {
     uiWidgetBase wtb;

I guess this is something @HooglyBoogly should have a look at though

So, the culprit commit checks for `UI_EMBOSS_NONE` in `widget_icon_has_anim` and there was another check for it in `widget_color_blend_from_flags`, and with these checks active, we dont get colored icon backgrounds here. Isolated, this seems to make sense (since it is supposed to do /* Nothing, only icon and/or text */ in the case of `UI_EMBOSS_NONE`. However, there are many, many places where the prop gets shown in the UI via `layout.prop(... emboss=False)` This gets translated into `UI_EMBOSS_NONE` eventualy (`rna_uiItemR` sets `UI_ITEM_R_NO_BG`, then `uiItemFullO_ptr_ex` translates this to l`ayout->emboss = UI_EMBOSS_NONE`) Note we have `row.emboss = 'UI_EMBOSS_NONE_OR_STATUS'` for such cases, so I wonder if it would make sense to remove all `emboss=False`? Something like this: [P2199: #89350 emboss=False problem](https://archive.blender.org/developer/P2199.txt) ``` diff --git a/release/scripts/startup/bl_ui/properties_data_bone.py b/release/scripts/startup/bl_ui/properties_data_bone.py index daf64642f68..8a44f223a95 100644 --- a/release/scripts/startup/bl_ui/properties_data_bone.py +++ b/release/scripts/startup/bl_ui/properties_data_bone.py @@ -85,8 +85,9 @@ class BONE_PT_transform(BoneButtonsPanel, Panel): row.prop(pchan, "rotation_quaternion", text="Rotation") sub = row.column(align=True) sub.use_property_decorate = False - sub.prop(pchan, "lock_rotation_w", text="", emboss=False, icon='DECORATE_UNLOCKED') - sub.prop(pchan, "lock_rotation", text="", emboss=False, icon='DECORATE_UNLOCKED') + sub.emboss = 'UI_EMBOSS_NONE_OR_STATUS' + sub.prop(pchan, "lock_rotation_w", text="", icon='DECORATE_UNLOCKED') + sub.prop(pchan, "lock_rotation", text="", icon='DECORATE_UNLOCKED') elif rotation_mode == 'AXIS_ANGLE': col = layout.column() row = col.row(align=True) diff --git a/release/scripts/startup/bl_ui/properties_data_mesh.py b/release/scripts/startup/bl_ui/properties_data_mesh.py index a9d7b8d71f3..94abff09f57 100644 --- a/release/scripts/startup/bl_ui/properties_data_mesh.py +++ b/release/scripts/startup/bl_ui/properties_data_mesh.py @@ -126,7 +126,7 @@ class MESH_UL_shape_keys(UIList): row.prop(key_block, "value", text="") else: row.label(text="") - row.prop(key_block, "mute", text="", emboss=False) + row.prop(key_block, "mute", text="") elif self.layout_type == 'GRID': layout.alignment = 'CENTER' layout.label(text="", icon_value=icon) diff --git a/release/scripts/startup/bl_ui/properties_grease_pencil_common.py b/release/scripts/startup/bl_ui/properties_grease_pencil_common.py index 0da0716e850..6048cea6f67 100644 --- a/release/scripts/startup/bl_ui/properties_grease_pencil_common.py +++ b/release/scripts/startup/bl_ui/properties_grease_pencil_common.py @@ -657,10 +657,11 @@ class GPENCIL_UL_layer(UIList): row.prop(gpl, "info", text="", emboss=False) row = layout.row(align=True) + row.emboss = 'UI_EMBOSS_NONE_OR_STATUS' icon_mask = 'MOD_MASK' if gpl.use_mask_layer else 'LAYER_ACTIVE' - row.prop(gpl, "use_mask_layer", text="", icon=icon_mask, emboss=False) + row.prop(gpl, "use_mask_layer", text="", icon=icon_mask) subrow = row.row(align=True) subrow.prop( @@ -670,8 +671,8 @@ class GPENCIL_UL_layer(UIList): icon='ONIONSKIN_ON' if gpl.use_onion_skinning else 'ONIONSKIN_OFF', emboss=False, ) - row.prop(gpl, "hide", text="", emboss=False) - row.prop(gpl, "lock", text="", emboss=False) + row.prop(gpl, "hide", text="") + row.prop(gpl, "lock", text="") elif self.layout_type == 'GRID': layout.alignment = 'CENTER' layout.label( diff --git a/source/blender/editors/interface/interface_widgets.c b/source/blender/editors/interface/interface_widgets.c index ce1109ad9df..ca9834d8fe5 100644 --- a/source/blender/editors/interface/interface_widgets.c +++ b/source/blender/editors/interface/interface_widgets.c @@ -2544,6 +2544,7 @@ static const uchar *widget_color_blend_from_flags(const uiWidgetStateColors *wco const eUIEmbossType emboss) { /* Explicitly require #UI_EMBOSS_NONE_OR_STATUS for color blending with no emboss. */ + // this check does not survive if (emboss == UI_EMBOSS_NONE) { return NULL; } @@ -3936,6 +3937,7 @@ static void widget_unitvec( static void widget_icon_has_anim( uiBut *but, uiWidgetColors *wcol, rcti *rect, int state, int roundboxalign) { + // this check does not survive if (state & (UI_BUT_ANIMATED | UI_BUT_ANIMATED_KEY | UI_BUT_DRIVEN | UI_BUT_REDALERT) && but->emboss != UI_EMBOSS_NONE) { uiWidgetBase wtb; ``` I guess this is something @HooglyBoogly should have a look at though

Hans Goudey commented

2021-06-24 05:52:37 +02:00

Added subscriber: @JulianEisel

Hans Goudey commented

2021-06-24 05:52:37 +02:00

Thanks for looking at this @lichtwerk. Personally I'd be happy with either of these two options:

Remove all uses of emboss=False, set UI_EMBOSS_NONE_OR_STATUS explicitly.
Change emboss=False to set UI_EMBOSS_NONE_OR_STATUS instead of "None".
However, I'm not sure how to decide between these options, as the initial idea for the new flag came from Julian I think.

@JulianEisel, I'd be happy to do the work for either option, which would you prefer?

Thanks for looking at this @lichtwerk. Personally I'd be happy with either of these two options: - Remove all uses of `emboss=False`, set `UI_EMBOSS_NONE_OR_STATUS` explicitly. - Change `emboss=False` to set `UI_EMBOSS_NONE_OR_STATUS` instead of "None". However, I'm not sure how to decide between these options, as the initial idea for the new flag came from Julian I think. @JulianEisel, I'd be happy to do the work for either option, which would you prefer?

Julian Eisel commented

2021-06-24 13:08:33 +02:00

In #89350#1181814, @HooglyBoogly wrote:

Remove all uses of emboss=False, set UI_EMBOSS_NONE_OR_STATUS explicitly.

This would essentially mean we accept a regression as API breaking change. Even if we would accept this as solution, we could only change Blender's bundled scripts, other add-ons and scripts would have to be updated by their authors. And since this is a rather subtle change, they probably won't even realize it needs to be done.

Change emboss=False to set UI_EMBOSS_NONE_OR_STATUS instead of "None".

This seems quite sensible to me. It matches the old behavior if I'm not mistaken. Showing too much status is better than showing too little. New is that script authors can actually entirely disable the status coloring using layout.emboss = 'NONE'. Just mention what emboss flag this translates to in the API documentation.

Also - eeek, why is the Python identifier called UI_EMBOSS_NONE_OR_STATUS? Should be just NONE_OR_STATUS.

> In #89350#1181814, @HooglyBoogly wrote: > - Remove all uses of `emboss=False`, set `UI_EMBOSS_NONE_OR_STATUS` explicitly. This would essentially mean we accept a regression as API breaking change. Even if we would accept this as solution, we could only change Blender's bundled scripts, other add-ons and scripts would have to be updated by their authors. And since this is a rather subtle change, they probably won't even realize it needs to be done. > - Change `emboss=False` to set `UI_EMBOSS_NONE_OR_STATUS` instead of "None". This seems quite sensible to me. It matches the old behavior if I'm not mistaken. Showing too much status is better than showing too little. New is that script authors can actually entirely disable the status coloring using `layout.emboss = 'NONE'`. Just mention what emboss flag this translates to in the API documentation. --- Also - eeek, why is the Python identifier called `UI_EMBOSS_NONE_OR_STATUS`? Should be just `NONE_OR_STATUS`.

Hans Goudey self-assigned this 2021-06-25 15:59:30 +02:00

Hans Goudey commented

2021-06-25 15:59:30 +02:00

D11701 is for that last point, yikes.

I'll submit a patch for the second option when I get it to work.

[D11701](https://archive.blender.org/developer/D11701) is for that last point, yikes. I'll submit a patch for the second option when I get it to work.

blender-admin commented

2021-06-28 17:00:15 +02:00

This issue was referenced by c184516c37

This issue was referenced by c184516c37b3ef81507558daaff4d8268ecc7d2c

blender-admin commented

2021-06-28 17:00:15 +02:00

This issue was referenced by 6c97c7f767

This issue was referenced by 6c97c7f767c9127469e39222b19f45eba15b41e0

Hans Goudey commented

2021-06-28 17:00:35 +02:00

Changed status from 'Confirmed' to: 'Resolved'

Hans Goudey closed this issue