3D Panel Pose Library filter doesn't appear to be working correctly. #91461

New Issue

Jason schleifer · 2021-09-16T19:37:25+02:00

Jason schleifer commented

2021-09-16 19:37:25 +02:00

System Information
Operating system: macOS-11.5.2-x86_64-i386-64bit 64 Bits
Graphics card: AMD Radeon Pro 560X OpenGL Engine ATI Technologies Inc. 4.1 ATI-4.6.20

Blender Version
Broken: version: 3.0.0 Alpha, branch: master, commit date: 2021-09-13 12:13, hash: blender/blender@4b06420e65
Worked: (newest version of Blender that worked as expected)

Short description of error
I'm using the new pose library in the 3D viewport, and when I try and filter the list of poses, the poses all disappear.

Exact steps for others to reproduce the error
Load the attached file (poseLib.blend)
In the 3d viewport, choose the Animation tab
there are two poses saved in the file - Bend and Neutral
In the filter field, type Bend to search for the Bend pose.
The list of poses clears out - and bend doesn't show up.

It should just filter the list.
poseLib.blend

**System Information** Operating system: macOS-11.5.2-x86_64-i386-64bit 64 Bits Graphics card: AMD Radeon Pro 560X OpenGL Engine ATI Technologies Inc. 4.1 ATI-4.6.20 **Blender Version** Broken: version: 3.0.0 Alpha, branch: master, commit date: 2021-09-13 12:13, hash: `blender/blender@4b06420e65` Worked: (newest version of Blender that worked as expected) **Short description of error** I'm using the new pose library in the 3D viewport, and when I try and filter the list of poses, the poses all disappear. **Exact steps for others to reproduce the error** Load the attached file (poseLib.blend) In the 3d viewport, choose the Animation tab there are two poses saved in the file - Bend and Neutral In the filter field, type Bend to search for the Bend pose. The list of poses clears out - and bend doesn't show up. It should just filter the list. [poseLib.blend](https://archive.blender.org/developer/F10431200/poseLib.blend)

Jason schleifer commented

2021-09-16 19:37:25 +02:00

Added subscriber: @JasonSchleifer

blender-admin commented

2021-09-16 19:37:25 +02:00

blender/blender#91231 was marked as duplicate of this issue

FOUNTMEDIA commented

2021-09-16 19:47:09 +02:00

(Migrated from localhost:3001)

Added subscriber: @FOUNTMEDIA

FOUNTMEDIA commented

2021-09-16 19:47:09 +02:00

(Migrated from localhost:3001)

Changed status from 'Needs Triage' to: 'Needs User Info'

FOUNTMEDIA commented

2021-09-16 19:47:09 +02:00

(Migrated from localhost:3001)

This comment was removed by @FOUNTMEDIA

*This comment was removed by @FOUNTMEDIA*

Thomas Dinges commented

2021-09-17 08:16:38 +02:00

Added subscriber: @ThomasDinges

Thomas Dinges commented

2021-09-17 08:17:14 +02:00

Changed status from 'Needs User Info' to: 'Needs Triage'

Philipp Oeser commented

2021-09-17 10:32:30 +02:00

Added subscriber: @lichtwerk

Philipp Oeser self-assigned this 2021-09-17 10:32:30 +02:00

Philipp Oeser commented

2021-09-17 10:32:30 +02:00

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

Philipp Oeser commented

2021-09-17 10:32:30 +02:00

Can confirm, checking...

blender-admin commented

2021-09-17 11:30:05 +02:00

This issue was referenced by blender/blender@a229a9dd64

This issue was referenced by blender/blender@a229a9dd64821575b27e6e6e317a1ce97e23f6d7

Philipp Oeser removed their assignment 2021-09-17 11:35:04 +02:00

Philipp Oeser commented

2021-09-17 11:35:04 +02:00

Added subscribers: @JulianEisel, @dr.sybren

Philipp Oeser commented

2021-09-17 11:35:04 +02:00

Well, since AssetHandle does not have a name_property (RNA_def_struct_name_property), and the UIList is just using the default uilist_filter_items_default it simply cannot filter on names (RNA_struct_name_get_alloc wont succeed).

Not sure what the plans are, there are comments about this design using FileSelectEntry (and getting names via ED_asset_handle_get_name) pretty much being a moving target atm.
Super quicky bandaid (but pretty sure there are other plans here):

P2401: T91461_snippet



diff --git a/source/blender/editors/interface/interface_template_list.cc b/source/blender/editors/interface/interface_template_list.cc
index 8246759ad36..08f28396dd9 100644
--- a/source/blender/editors/interface/interface_template_list.cc
+++ b/source/blender/editors/interface/interface_template_list.cc
@@ -31,6 +31,7 @@
 
 #include "BLT_translation.h"
 
+#include "ED_asset.h"
 #include "ED_screen.h"
 
 #include "MEM_guardedalloc.h"
@@ -216,7 +217,18 @@ static void uilist_filter_items_default(struct uiList *ui_list,
     RNA_PROP_BEGIN (dataptr, itemptr, prop) {
       bool do_order = false;
 
-      char *namebuf = RNA_struct_name_get_alloc(&itemptr, nullptr, 0, nullptr);
+      char *namebuf;
+      bool free_namebuf = false;
+      if (RNA_struct_is_a(itemptr.type, &RNA_AssetHandle)) {
+        AssetHandle *asset_handle = (AssetHandle *)itemptr.data;
+        const char *asset_name = ED_asset_handle_get_name(asset_handle);
+        BLI_strncpy(namebuf, asset_name, sizeof(asset_name));
+      }
+      else {
+        namebuf = RNA_struct_name_get_alloc(&itemptr, nullptr, 0, nullptr);
+        free_namebuf = true;
+      }
+
       const char *name = namebuf ? namebuf : "";
 
       if (filter[0]) {
@@ -244,7 +256,7 @@ static void uilist_filter_items_default(struct uiList *ui_list,
       }
 
       /* free name */
-      if (namebuf) {
+      if (namebuf && free_namebuf) {
         MEM_freeN(namebuf);
       }
       i++;
diff --git a/source/blender/makesrna/intern/rna_asset.c b/source/blender/makesrna/intern/rna_asset.c
index 1e583f4ca52..7649cb4f2dc 100644
--- a/source/blender/makesrna/intern/rna_asset.c
+++ b/source/blender/makesrna/intern/rna_asset.c
@@ -303,6 +303,8 @@ static void rna_def_asset_handle(BlenderRNA *brna)
   RNA_def_property_ui_text(
       prop, "File Entry", "TEMPORARY, DO NOT USE - File data used to refer to the asset");
 
+  /* RNA_def_struct_name_property of some sorts please. */
+
   prop = RNA_def_property(srna, "local_id", PROP_POINTER, PROP_NONE);
   RNA_def_property_struct_type(prop, "ID");
   RNA_def_property_pointer_funcs(prop, "rna_AssetHandle_local_id_get", NULL, NULL, NULL);

CC @dr.sybren
CC @JulianEisel

Well, since `AssetHandle` does not have a `name_property` (`RNA_def_struct_name_property`), and the UIList is just using the default `uilist_filter_items_default` it simply cannot filter on names (`RNA_struct_name_get_alloc` wont succeed). Not sure what the plans are, there are comments about this design using `FileSelectEntry` (and getting names via `ED_asset_handle_get_name`) pretty much being a moving target atm. Super quicky bandaid (but pretty sure there are other plans here): [P2401: T91461_snippet](https://archive.blender.org/developer/P2401.txt) ``` diff --git a/source/blender/editors/interface/interface_template_list.cc b/source/blender/editors/interface/interface_template_list.cc index 8246759ad36..08f28396dd9 100644 --- a/source/blender/editors/interface/interface_template_list.cc +++ b/source/blender/editors/interface/interface_template_list.cc @@ -31,6 +31,7 @@ #include "BLT_translation.h" +#include "ED_asset.h" #include "ED_screen.h" #include "MEM_guardedalloc.h" @@ -216,7 +217,18 @@ static void uilist_filter_items_default(struct uiList *ui_list, RNA_PROP_BEGIN (dataptr, itemptr, prop) { bool do_order = false; - char *namebuf = RNA_struct_name_get_alloc(&itemptr, nullptr, 0, nullptr); + char *namebuf; + bool free_namebuf = false; + if (RNA_struct_is_a(itemptr.type, &RNA_AssetHandle)) { + AssetHandle *asset_handle = (AssetHandle *)itemptr.data; + const char *asset_name = ED_asset_handle_get_name(asset_handle); + BLI_strncpy(namebuf, asset_name, sizeof(asset_name)); + } + else { + namebuf = RNA_struct_name_get_alloc(&itemptr, nullptr, 0, nullptr); + free_namebuf = true; + } + const char *name = namebuf ? namebuf : ""; if (filter[0]) { @@ -244,7 +256,7 @@ static void uilist_filter_items_default(struct uiList *ui_list, } /* free name */ - if (namebuf) { + if (namebuf && free_namebuf) { MEM_freeN(namebuf); } i++; diff --git a/source/blender/makesrna/intern/rna_asset.c b/source/blender/makesrna/intern/rna_asset.c index 1e583f4ca52..7649cb4f2dc 100644 --- a/source/blender/makesrna/intern/rna_asset.c +++ b/source/blender/makesrna/intern/rna_asset.c @@ -303,6 +303,8 @@ static void rna_def_asset_handle(BlenderRNA *brna) RNA_def_property_ui_text( prop, "File Entry", "TEMPORARY, DO NOT USE - File data used to refer to the asset"); + /* RNA_def_struct_name_property of some sorts please. */ + prop = RNA_def_property(srna, "local_id", PROP_POINTER, PROP_NONE); RNA_def_property_struct_type(prop, "ID"); RNA_def_property_pointer_funcs(prop, "rna_AssetHandle_local_id_get", NULL, NULL, NULL); ``` CC @dr.sybren CC @JulianEisel

Philipp Oeser commented

2021-09-17 12:55:35 +02:00

Quoting @JulianEisel from chat:

We can just add a name property to the AssetHandle RNA
Which would just have a getter callback, like local_id there

Problem here is that AssetHandle inherits from PropertyGroup which already sets the name property :/
Why cant RNA_def_struct_name_property just overwrite this? (commented out the check there and it seems to work :))

Quoting @JulianEisel from chat: > We can just add a name property to the AssetHandle RNA > Which would just have a getter callback, like local_id there Problem here is that `AssetHandle` inherits from `PropertyGroup` which already sets the name property :/ Why cant `RNA_def_struct_name_property` just overwrite this? (commented out the check there and it seems to work :))

Philipp Oeser commented

2021-09-17 13:01:33 +02:00

In #91461#1221358, @lichtwerk wrote:
Problem here is that AssetHandle inherits from PropertyGroup which already sets the name property :/
Why cant RNA_def_struct_name_property just overwrite this? (commented out the check there and it seems to work :))

So this isnt working:
P2402: T91461_snippet2



diff --git a/source/blender/makesrna/intern/rna_asset.c b/source/blender/makesrna/intern/rna_asset.c
index 7649cb4f2dc..44b19260a92 100644
--- a/source/blender/makesrna/intern/rna_asset.c
+++ b/source/blender/makesrna/intern/rna_asset.c
@@ -160,6 +160,25 @@ static PointerRNA rna_AssetHandle_local_id_get(PointerRNA *ptr)
   return rna_pointer_inherit_refine(ptr, &RNA_ID, id);
 }
 
+static int rna_AssetHandle_name_length(PointerRNA *ptr)
+{
+  const AssetHandle *asset_handle = ptr->data;
+  return strlen(ED_asset_handle_get_name(asset_handle));
+}
+
+static void rna_AssetHandle_name_get(PointerRNA *ptr, char *value)
+{
+  const AssetHandle *asset_handle = ptr->data;
+  const char *asset_name = ED_asset_handle_get_name(asset_handle);
+
+  if (asset_name) {
+    strcpy(value, asset_name);
+  }
+  else {
+    value- [x] = '\0';
+  }
+}
+
 const EnumPropertyItem *rna_asset_library_reference_itemf(bContext *UNUSED(C),
                                                           PointerRNA *UNUSED(ptr),
                                                           PropertyRNA *UNUSED(prop),
@@ -303,7 +322,10 @@ static void rna_def_asset_handle(BlenderRNA *brna)
   RNA_def_property_ui_text(
       prop, "File Entry", "TEMPORARY, DO NOT USE - File data used to refer to the asset");
 
-  /* RNA_def_struct_name_property of some sorts please. */
+  prop = RNA_def_property(srna, "name", PROP_STRING, PROP_NONE);
+  RNA_def_property_clear_flag(prop, PROP_EDITABLE);
+  RNA_def_property_string_funcs(prop, "rna_AssetHandle_name_get", "rna_AssetHandle_name_length", NULL);
+  RNA_def_struct_name_property(srna, prop);
 
   prop = RNA_def_property(srna, "local_id", PROP_POINTER, PROP_NONE);
   RNA_def_property_struct_type(prop, "ID");

gives (for above reasons):

ERROR (rna.define): rna_define.c:1129 RNA_def_struct_name_property: "AssetHandle.name", name property is already set.

> In #91461#1221358, @lichtwerk wrote: > Problem here is that `AssetHandle` inherits from `PropertyGroup` which already sets the name property :/ > Why cant `RNA_def_struct_name_property` just overwrite this? (commented out the check there and it seems to work :)) So this isnt working: [P2402: T91461_snippet2](https://archive.blender.org/developer/P2402.txt) ``` diff --git a/source/blender/makesrna/intern/rna_asset.c b/source/blender/makesrna/intern/rna_asset.c index 7649cb4f2dc..44b19260a92 100644 --- a/source/blender/makesrna/intern/rna_asset.c +++ b/source/blender/makesrna/intern/rna_asset.c @@ -160,6 +160,25 @@ static PointerRNA rna_AssetHandle_local_id_get(PointerRNA *ptr) return rna_pointer_inherit_refine(ptr, &RNA_ID, id); } +static int rna_AssetHandle_name_length(PointerRNA *ptr) +{ + const AssetHandle *asset_handle = ptr->data; + return strlen(ED_asset_handle_get_name(asset_handle)); +} + +static void rna_AssetHandle_name_get(PointerRNA *ptr, char *value) +{ + const AssetHandle *asset_handle = ptr->data; + const char *asset_name = ED_asset_handle_get_name(asset_handle); + + if (asset_name) { + strcpy(value, asset_name); + } + else { + value- [x] = '\0'; + } +} + const EnumPropertyItem *rna_asset_library_reference_itemf(bContext *UNUSED(C), PointerRNA *UNUSED(ptr), PropertyRNA *UNUSED(prop), @@ -303,7 +322,10 @@ static void rna_def_asset_handle(BlenderRNA *brna) RNA_def_property_ui_text( prop, "File Entry", "TEMPORARY, DO NOT USE - File data used to refer to the asset"); - /* RNA_def_struct_name_property of some sorts please. */ + prop = RNA_def_property(srna, "name", PROP_STRING, PROP_NONE); + RNA_def_property_clear_flag(prop, PROP_EDITABLE); + RNA_def_property_string_funcs(prop, "rna_AssetHandle_name_get", "rna_AssetHandle_name_length", NULL); + RNA_def_struct_name_property(srna, prop); prop = RNA_def_property(srna, "local_id", PROP_POINTER, PROP_NONE); RNA_def_property_struct_type(prop, "ID"); ``` gives (for above reasons): ``` ERROR (rna.define): rna_define.c:1129 RNA_def_struct_name_property: "AssetHandle.name", name property is already set. ```

Julian Eisel commented

2021-09-17 15:18:41 +02:00

There are some other ways we could solve this, but they are more of a hassle than worth it. Esp. considering that the whole AssetHandle design is meant to be temporary anyway. So I'm fine with the if (RNA_struct_is_a(itemptr.type, &RNA_AssetHandle)) hack for now. Just please add a XXX comment to note that this is not meant as a permanent solution.

There are some other ways we could solve this, but they are more of a hassle than worth it. Esp. considering that the whole `AssetHandle` design is meant to be temporary anyway. So I'm fine with the `if (RNA_struct_is_a(itemptr.type, &RNA_AssetHandle))` hack for now. Just please add a `XXX` comment to note that this is not meant as a permanent solution.

Philipp Oeser commented

2021-09-18 08:25:45 +02:00

Changed status from 'Confirmed' to: 'Resolved'

Philipp Oeser closed this issue

2021-09-18 08:25:45 +02:00

Philipp Oeser self-assigned this 2021-09-18 08:25:45 +02:00

Sign in to join this conversation.