Fix #119296 : Image object data - opacity tooltips text make no sense #119415

Yuting-Chen · 2024-03-13T11:09:24+01:00

Yuting-Chen commented

2024-03-13 11:09:24 +01:00

First-time contributor

Senario of the bug:
The tooltips for the image data property opacity make no sense, and seem to have been copied from the viewport display setting tooltips.
With an reference or background image object selected, in the properties editor goto object data properties.
Hover over opacity checkbox: "Use alpha blending instead of alpha test (can produce sorting artefacts)"
Hover over opacity value slider: "Object color and alpha, used when the Object Color mode is enabled"

Summary of changes:

The description of RNA property "empty_image_flag" makes no sense，I change the text to a clearer one.
The "color" property in viewport display ("blender\scripts\startup\bl_ui\properties_object.py") and in image object data
("blender\scripts\startup\bl_ui\properties_data_empty.py") share one RNA property, but the two needs different tooltips, which makes the tooltips text of image object incoherent.
Therefore, I create a new RNA property "empty_image_alpha" in "blender\source\blender\makesrna\intern\rna_object.cc" and write a new description for it.

Senario of the bug: The tooltips for the image data property opacity make no sense, and seem to have been copied from the viewport display setting tooltips. With an reference or background image object selected, in the properties editor goto object data properties. Hover over opacity checkbox: "Use alpha blending instead of alpha test (can produce sorting artefacts)" Hover over opacity value slider: "Object color and alpha, used when the Object Color mode is enabled" Summary of changes: 1. The description of RNA property "empty_image_flag" makes no sense，I change the text to a clearer one. 2. The "color" property in viewport display ("blender\scripts\startup\bl_ui\properties_object.py") and in image object data ("blender\scripts\startup\bl_ui\properties_data_empty.py") share one RNA property, but the two needs different tooltips, which makes the tooltips text of image object incoherent. Therefore, I create a new RNA property "empty_image_alpha" in "blender\source\blender\makesrna\intern\rna_object.cc" and write a new description for it.

Yuting-Chen added 3 commits 2024-03-13 11:09:34 +01:00

d4af0f251b change layout prop to new defined rna

cbae48d8d5 define a new property rna for empty image

buildbot/vexp-code-patch-lint Build done. Details

buildbot/vexp-code-patch-darwin-x86_64 Build done. Details

buildbot/vexp-code-patch-linux-x86_64 Build done. Details

buildbot/vexp-code-patch-windows-amd64 Build done. Details

buildbot/vexp-code-patch-darwin-arm64 Build done. Details

buildbot/vexp-code-patch-coordinator Build done. Details

04ec9a213b change to new rna

Yuting-Chen changed title from ~~Fix #119296 : Image object data - opacity tooltips text make no sense~~ to WIP: Fix #119296 : Image object data - opacity tooltips text make no sense

2024-03-13 11:14:04 +01:00

Yuting-Chen changed title from ~~WIP: Fix #119296 : Image object data - opacity tooltips text make no sense~~ to Fix #119296 : Image object data - opacity tooltips text make no sense

2024-03-13 11:14:24 +01:00

Blender Bot commented

2024-03-13 11:17:42 +01:00

Only blender organization members with write access can start builds. See documentation for details.

Only blender organization members with write access can start builds. See [documentation](https://projects.blender.org/infrastructure/blender-bot/src/branch/main/README.md) for details.

Iliya Katushenock added this to the User Interface project 2024-03-13 11:22:37 +01:00

Yuting-Chen requested review from Jacques Lucke 2024-03-13 13:31:38 +01:00

Jacques Lucke commented

2024-03-14 10:04:08 +01:00

@blender-bot build

Yuting-Chen added 1 commit 2024-03-14 12:53:57 +01:00

94cb257468 make format of changed code

Yuting-Chen requested review from Harley Acheson 2024-03-16 08:14:59 +01:00

Harley Acheson commented

2024-03-18 19:51:28 +01:00

Excuse me if I overexplain this a bit.

The central problem here is that we are using the object "color" in two different ways as far as the user is concerned, which results in confusion here. With that background image selected, you can go to Properties / Object / Viewport Display and see that the "Color" there is what we are changing the alpha channel of in the Data area and showing it as "Opacity". The "index=3" is specifying that we are just altering the one value in the array of values.

Your solution works, in that it gets us a second property for that one color so you can have a separate name and description. But that does add a new color to the API. As in if someone is manipulating these these from Python they would see a item called "empty_image_alpha" that is actually a color, which isn't really what you want.

I think it would be a bit clearer if the new property you make is just a float. This could be done like the following:

diff --git a/scripts/startup/bl_ui/properties_data_empty.py b/scripts/startup/bl_ui/properties_data_empty.py
index a0affa206c0..9fec1aa864b 100644
--- a/scripts/startup/bl_ui/properties_data_empty.py
+++ b/scripts/startup/bl_ui/properties_data_empty.py
@@ -53,8 +53,8 @@ class DATA_PT_empty(DataButtonsPanel, Panel):
             sub.prop(ob, "use_empty_image_alpha", text="")
             sub = sub.row(align=True)
             sub.active = ob.use_empty_image_alpha
-            sub.prop(ob, "color", text="", index=3, slider=True)
-            row.prop_decorator(ob, "color", index=3)
+            sub.prop(ob, "empty_image_alpha", text="", slider=True)
+            row.prop_decorator(ob, "empty_image_alpha")
 
 
 class DATA_PT_empty_image(DataButtonsPanel, Panel):
diff --git a/source/blender/makesrna/intern/rna_object.cc b/source/blender/makesrna/intern/rna_object.cc
index 44ac12562dc..059df2fd93e 100644
--- a/source/blender/makesrna/intern/rna_object.cc
+++ b/source/blender/makesrna/intern/rna_object.cc
@@ -788,6 +788,18 @@ static void rna_Object_empty_display_type_set(PointerRNA *ptr, int value)
   BKE_object_empty_draw_type_set(ob, value);
 }
 
+static float rna_Object_empty_image_alpha_get(PointerRNA *ptr)
+{
+  Object *ob = static_cast<Object *>(ptr->data);
+  return ob->color[3];
+}
+
+static void rna_Object_empty_image_alpha_set(PointerRNA *ptr, const float value)
+{
+  Object *ob = static_cast<Object *>(ptr->data);
+  ob->color[3] = value;
+}
+
 static void rna_Object_parent_bone_set(PointerRNA *ptr, const char *value)
 {
   Object *ob = static_cast<Object *>(ptr->data);
@@ -3523,12 +3535,22 @@ static void rna_def_object(BlenderRNA *brna)
 
   prop = RNA_def_property(srna, "use_empty_image_alpha", PROP_BOOLEAN, PROP_NONE);
   RNA_def_property_boolean_sdna(prop, nullptr, "empty_image_flag", OB_EMPTY_IMAGE_USE_ALPHA_BLEND);
-  RNA_def_property_ui_text(
-      prop,
-      "Use Alpha",
-      "Use alpha blending instead of alpha test (can produce sorting artifacts)");
+  RNA_def_property_ui_text(prop, "Use Opacity", "Use the opacity property to add transparency");
   RNA_def_property_update(prop, NC_OBJECT | ND_DRAW, nullptr);
 
+  prop = RNA_def_float(srna,
+                       "empty_image_alpha",
+                       1.0f,
+                       0.0f,
+                       1.0f,
+                       "Image Opacity",
+                       "Degree of transparency applied to the image",
+                       0.0f,
+                       1.0f);
+  RNA_def_property_float_funcs(
+      prop, "rna_Object_empty_image_alpha_get", "rna_Object_empty_image_alpha_set", nullptr);
+  RNA_def_property_update(prop, NC_OBJECT | ND_DRAW, "rna_Object_internal_update_draw");
+
   static EnumPropertyItem prop_empty_image_side_items[] = {
       {0, "DOUBLE_SIDED", 0, "Both", ""},
       {OB_EMPTY_IMAGE_HIDE_BACK, "FRONT", 0, "Front", ""},

Instead of making a copy of that color, we make a float with a custom getter and setter that uses that color. Note that I did the above pretty quickly so it would need some thought and testing.

Excuse me if I overexplain this a bit. The central problem here is that we are using the object "color" in two different ways as far as the user is concerned, which results in confusion here. With that background image selected, you can go to Properties / Object / Viewport Display and see that the "Color" there is what we are changing the alpha channel of in the Data area and showing it as "Opacity". The "index=3" is specifying that we are just altering the one value in the array of values. Your solution works, in that it gets us a second property for that one color so you can have a separate name and description. But that does add a new color to the API. As in if someone is manipulating these these from Python they would see a item called "empty_image_alpha" that is actually a color, which isn't really what you want. I think it would be a bit clearer if the new property you make is just a float. This could be done like the following: ```Diff diff --git a/scripts/startup/bl_ui/properties_data_empty.py b/scripts/startup/bl_ui/properties_data_empty.py index a0affa206c0..9fec1aa864b 100644 --- a/scripts/startup/bl_ui/properties_data_empty.py +++ b/scripts/startup/bl_ui/properties_data_empty.py @@ -53,8 +53,8 @@ class DATA_PT_empty(DataButtonsPanel, Panel): sub.prop(ob, "use_empty_image_alpha", text="") sub = sub.row(align=True) sub.active = ob.use_empty_image_alpha - sub.prop(ob, "color", text="", index=3, slider=True) - row.prop_decorator(ob, "color", index=3) + sub.prop(ob, "empty_image_alpha", text="", slider=True) + row.prop_decorator(ob, "empty_image_alpha") class DATA_PT_empty_image(DataButtonsPanel, Panel): diff --git a/source/blender/makesrna/intern/rna_object.cc b/source/blender/makesrna/intern/rna_object.cc index 44ac12562dc..059df2fd93e 100644 --- a/source/blender/makesrna/intern/rna_object.cc +++ b/source/blender/makesrna/intern/rna_object.cc @@ -788,6 +788,18 @@ static void rna_Object_empty_display_type_set(PointerRNA *ptr, int value) BKE_object_empty_draw_type_set(ob, value); } +static float rna_Object_empty_image_alpha_get(PointerRNA *ptr) +{ + Object *ob = static_cast<Object *>(ptr->data); + return ob->color[3]; +} + +static void rna_Object_empty_image_alpha_set(PointerRNA *ptr, const float value) +{ + Object *ob = static_cast<Object *>(ptr->data); + ob->color[3] = value; +} + static void rna_Object_parent_bone_set(PointerRNA *ptr, const char *value) { Object *ob = static_cast<Object *>(ptr->data); @@ -3523,12 +3535,22 @@ static void rna_def_object(BlenderRNA *brna) prop = RNA_def_property(srna, "use_empty_image_alpha", PROP_BOOLEAN, PROP_NONE); RNA_def_property_boolean_sdna(prop, nullptr, "empty_image_flag", OB_EMPTY_IMAGE_USE_ALPHA_BLEND); - RNA_def_property_ui_text( - prop, - "Use Alpha", - "Use alpha blending instead of alpha test (can produce sorting artifacts)"); + RNA_def_property_ui_text(prop, "Use Opacity", "Use the opacity property to add transparency"); RNA_def_property_update(prop, NC_OBJECT | ND_DRAW, nullptr); + prop = RNA_def_float(srna, + "empty_image_alpha", + 1.0f, + 0.0f, + 1.0f, + "Image Opacity", + "Degree of transparency applied to the image", + 0.0f, + 1.0f); + RNA_def_property_float_funcs( + prop, "rna_Object_empty_image_alpha_get", "rna_Object_empty_image_alpha_set", nullptr); + RNA_def_property_update(prop, NC_OBJECT | ND_DRAW, "rna_Object_internal_update_draw"); + static EnumPropertyItem prop_empty_image_side_items[] = { {0, "DOUBLE_SIDED", 0, "Both", ""}, {OB_EMPTY_IMAGE_HIDE_BACK, "FRONT", 0, "Front", ""}, ``` Instead of making a copy of that color, we make a float with a custom getter and setter that uses that color. Note that I did the above pretty quickly so it would need some thought and testing.

Yuting-Chen changed title from ~~Fix #119296 : Image object data - opacity tooltips text make no sense~~ to WIP: Fix #119296 : Image object data - opacity tooltips text make no sense

2024-03-28 07:22:19 +01:00

Yuting-Chen added 2 commits 2024-03-28 09:07:21 +01:00

6a82eef5c2 update solution for Python properties_data_empty.py

0137602c22 update solution to generate a float property

Yuting-Chen changed title from ~~WIP: Fix #119296 : Image object data - opacity tooltips text make no sense~~ to Fix #119296 : Image object data - opacity tooltips text make no sense

2024-03-28 09:08:56 +01:00

Yuting-Chen commented

2024-03-28 09:25:35 +01:00

First-time contributor

Excuse me if I overexplain this a bit.

The central problem here is that we are using the object "color" in two different ways as far as the user is concerned, which results in confusion here. With that background image selected, you can go to Properties / Object / Viewport Display and see that the "Color" there is what we are changing the alpha channel of in the Data area and showing it as "Opacity". The "index=3" is specifying that we are just altering the one value in the array of values.

Your solution works, in that it gets us a second property for that one color so you can have a separate name and description. But that does add a new color to the API. As in if someone is manipulating these these from Python they would see a item called "empty_image_alpha" that is actually a color, which isn't really what you want.

I think it would be a bit clearer if the new property you make is just a float. This could be done like the following:

diff --git a/scripts/startup/bl_ui/properties_data_empty.py b/scripts/startup/bl_ui/properties_data_empty.py
index a0affa206c0..9fec1aa864b 100644
--- a/scripts/startup/bl_ui/properties_data_empty.py
+++ b/scripts/startup/bl_ui/properties_data_empty.py
@@ -53,8 +53,8 @@ class DATA_PT_empty(DataButtonsPanel, Panel):
             sub.prop(ob, "use_empty_image_alpha", text="")
             sub = sub.row(align=True)
             sub.active = ob.use_empty_image_alpha
-            sub.prop(ob, "color", text="", index=3, slider=True)
-            row.prop_decorator(ob, "color", index=3)
+            sub.prop(ob, "empty_image_alpha", text="", slider=True)
+            row.prop_decorator(ob, "empty_image_alpha")
 
 
 class DATA_PT_empty_image(DataButtonsPanel, Panel):
diff --git a/source/blender/makesrna/intern/rna_object.cc b/source/blender/makesrna/intern/rna_object.cc
index 44ac12562dc..059df2fd93e 100644
--- a/source/blender/makesrna/intern/rna_object.cc
+++ b/source/blender/makesrna/intern/rna_object.cc
@@ -788,6 +788,18 @@ static void rna_Object_empty_display_type_set(PointerRNA *ptr, int value)
   BKE_object_empty_draw_type_set(ob, value);
 }
 
+static float rna_Object_empty_image_alpha_get(PointerRNA *ptr)
+{
+  Object *ob = static_cast<Object *>(ptr->data);
+  return ob->color[3];
+}
+
+static void rna_Object_empty_image_alpha_set(PointerRNA *ptr, const float value)
+{
+  Object *ob = static_cast<Object *>(ptr->data);
+  ob->color[3] = value;
+}
+
 static void rna_Object_parent_bone_set(PointerRNA *ptr, const char *value)
 {
   Object *ob = static_cast<Object *>(ptr->data);
@@ -3523,12 +3535,22 @@ static void rna_def_object(BlenderRNA *brna)
 
   prop = RNA_def_property(srna, "use_empty_image_alpha", PROP_BOOLEAN, PROP_NONE);
   RNA_def_property_boolean_sdna(prop, nullptr, "empty_image_flag", OB_EMPTY_IMAGE_USE_ALPHA_BLEND);
-  RNA_def_property_ui_text(
-      prop,
-      "Use Alpha",
-      "Use alpha blending instead of alpha test (can produce sorting artifacts)");
+  RNA_def_property_ui_text(prop, "Use Opacity", "Use the opacity property to add transparency");
   RNA_def_property_update(prop, NC_OBJECT | ND_DRAW, nullptr);
 
+  prop = RNA_def_float(srna,
+                       "empty_image_alpha",
+                       1.0f,
+                       0.0f,
+                       1.0f,
+                       "Image Opacity",
+                       "Degree of transparency applied to the image",
+                       0.0f,
+                       1.0f);
+  RNA_def_property_float_funcs(
+      prop, "rna_Object_empty_image_alpha_get", "rna_Object_empty_image_alpha_set", nullptr);
+  RNA_def_property_update(prop, NC_OBJECT | ND_DRAW, "rna_Object_internal_update_draw");
+
   static EnumPropertyItem prop_empty_image_side_items[] = {
       {0, "DOUBLE_SIDED", 0, "Both", ""},
       {OB_EMPTY_IMAGE_HIDE_BACK, "FRONT", 0, "Front", ""},

Instead of making a copy of that color, we make a float with a custom getter and setter that uses that color. Note that I did the above pretty quickly so it would need some thought and testing.

Hi! Thank you for your advice. It's better to have the new property to be a float than just copy the color property. I have updated the code and added getter and setter. After testing it does work.

> Excuse me if I overexplain this a bit. > > The central problem here is that we are using the object "color" in two different ways as far as the user is concerned, which results in confusion here. With that background image selected, you can go to Properties / Object / Viewport Display and see that the "Color" there is what we are changing the alpha channel of in the Data area and showing it as "Opacity". The "index=3" is specifying that we are just altering the one value in the array of values. > > Your solution works, in that it gets us a second property for that one color so you can have a separate name and description. But that does add a new color to the API. As in if someone is manipulating these these from Python they would see a item called "empty_image_alpha" that is actually a color, which isn't really what you want. > > I think it would be a bit clearer if the new property you make is just a float. This could be done like the following: > > ```Diff > diff --git a/scripts/startup/bl_ui/properties_data_empty.py b/scripts/startup/bl_ui/properties_data_empty.py > index a0affa206c0..9fec1aa864b 100644 > --- a/scripts/startup/bl_ui/properties_data_empty.py > +++ b/scripts/startup/bl_ui/properties_data_empty.py > @@ -53,8 +53,8 @@ class DATA_PT_empty(DataButtonsPanel, Panel): > sub.prop(ob, "use_empty_image_alpha", text="") > sub = sub.row(align=True) > sub.active = ob.use_empty_image_alpha > - sub.prop(ob, "color", text="", index=3, slider=True) > - row.prop_decorator(ob, "color", index=3) > + sub.prop(ob, "empty_image_alpha", text="", slider=True) > + row.prop_decorator(ob, "empty_image_alpha") > > > class DATA_PT_empty_image(DataButtonsPanel, Panel): > diff --git a/source/blender/makesrna/intern/rna_object.cc b/source/blender/makesrna/intern/rna_object.cc > index 44ac12562dc..059df2fd93e 100644 > --- a/source/blender/makesrna/intern/rna_object.cc > +++ b/source/blender/makesrna/intern/rna_object.cc > @@ -788,6 +788,18 @@ static void rna_Object_empty_display_type_set(PointerRNA *ptr, int value) > BKE_object_empty_draw_type_set(ob, value); > } > > +static float rna_Object_empty_image_alpha_get(PointerRNA *ptr) > +{ > + Object *ob = static_cast<Object *>(ptr->data); > + return ob->color[3]; > +} > + > +static void rna_Object_empty_image_alpha_set(PointerRNA *ptr, const float value) > +{ > + Object *ob = static_cast<Object *>(ptr->data); > + ob->color[3] = value; > +} > + > static void rna_Object_parent_bone_set(PointerRNA *ptr, const char *value) > { > Object *ob = static_cast<Object *>(ptr->data); > @@ -3523,12 +3535,22 @@ static void rna_def_object(BlenderRNA *brna) > > prop = RNA_def_property(srna, "use_empty_image_alpha", PROP_BOOLEAN, PROP_NONE); > RNA_def_property_boolean_sdna(prop, nullptr, "empty_image_flag", OB_EMPTY_IMAGE_USE_ALPHA_BLEND); > - RNA_def_property_ui_text( > - prop, > - "Use Alpha", > - "Use alpha blending instead of alpha test (can produce sorting artifacts)"); > + RNA_def_property_ui_text(prop, "Use Opacity", "Use the opacity property to add transparency"); > RNA_def_property_update(prop, NC_OBJECT | ND_DRAW, nullptr); > > + prop = RNA_def_float(srna, > + "empty_image_alpha", > + 1.0f, > + 0.0f, > + 1.0f, > + "Image Opacity", > + "Degree of transparency applied to the image", > + 0.0f, > + 1.0f); > + RNA_def_property_float_funcs( > + prop, "rna_Object_empty_image_alpha_get", "rna_Object_empty_image_alpha_set", nullptr); > + RNA_def_property_update(prop, NC_OBJECT | ND_DRAW, "rna_Object_internal_update_draw"); > + > static EnumPropertyItem prop_empty_image_side_items[] = { > {0, "DOUBLE_SIDED", 0, "Both", ""}, > {OB_EMPTY_IMAGE_HIDE_BACK, "FRONT", 0, "Front", ""}, > ``` > > Instead of making a copy of that color, we make a float with a custom getter and setter that uses that color. Note that I did the above pretty quickly so it would need some thought and testing. > Hi! Thank you for your advice. It's better to have the new property to be a float than just copy the color property. I have updated the code and added getter and setter. After testing it does work.

Merge conflict checking is in progress. Try again in few moments.

View command line instructions.

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u my-feature:Yuting-Chen-my-feature

git checkout Yuting-Chen-my-feature

Sign in to join this conversation.

No reviewers

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Fix #119296 : Image object data - opacity tooltips text make no sense #119415

Checkout