Technical design for attribute ui list #88460

Closed
opened 2021-05-21 12:26:19 +02:00 by Jacques Lucke · 13 comments
Member

image.png

Similar to how we have lists for vertex groups, uv maps and more, we would like to have a list of attributes. For pointcloud objects (which are an experimental feature), we actually have the attributes list already. However, for those it was easy, because all attributes (including builtin ones like position) are stored the same way. That made it easy to populate a ui list that contains all attributes. Getting the same to work for meshes is more complicated, because mesh attributes are stored in many different ways.

I can think of three main ways to get a list of all mesh attributes (including uv layers etc.) in the ui:

  1. Provide an rna collection property that contains all the attributes and use the existing UIList. Here the main difficulty is to create the collection property in rna for very heterogenuous data. More about that later.
  2. Refactor UIList itself, so that it can display more arbitrary data that is not stored in an rna collection. This refactor could also be useful for other features, but I can't estimate the feasibility of this change yet.
  3. Don't use UIList and draw the list more manually instead. This is surely the simplest and most straight solution. However, it is also ugly and can't match the look and feel of UIList.

The remainder of this post explores different ways to create an rna collection that can contain all attributes, including builtin (e.g. position and material_index) and special purpose (e.g. uv layers) ones.


First, let's have a look at the different ways attributes are stored on meshes. All the attributes mentioned below can be accessed by geometry nodes already.

  • Generic attributes are stored in generic custom data layers.
  • position is stored as part of MVert.
  • material_index is stored as part of MPoly.
  • shade_smooth is stored as part of MPoly.
  • crease is stored as part of MEdge.
  • normal is special and I hope that we can remove it as attribute at some point (because it is derived data).
  • UV maps are stored as special uv data layers.
  • Vertex colors are stored as special vertex color data layers.
  • All vertex groups are stored in a single custom data layer. However, that is the smaller issue. The real problem is that vertex group names are not stored on the mesh, but on the object. No one was able to tell me why yet. Having the names on the object might be a dealbreaker for all the approaches below. Looks like we will have to investigate if we can move the vertex group names to the geometry..

The existing attribute api in rna only provides access to generic custom data layers and does not include builtin and special purpose attributes. Below are a few ideas on how the attribute api can be extended to support the different types.

(a) Extend existing attribute iterator with other custom data layer types.

This mainly involves extending the rna_AttributeGroup_iterator_* functions.
To handle multiple attributes in the same custom data layer (e.g. material_index and shade_smooth in the CD_MPOLY layer), more information might need to be stored in CollectionPropertyIterator.internal.
Furthermore, new rna types like RNA_UVAttribute and RNA_MeshPositionAttribute might be necessary that handle the data access for builtin and special purpose attributes.

(b) Index based iterator.

This is similar to the solution above, but instead of having the iterator reference the attributes directly, it only uses an index. With this we just need to implement a function that counts the number of attributes and one that that gets the correct PointerRNA for an index. The main benefit here is that rna_AttributeGroup_iterator_next becomes trivial.
Unfortunately, this solution might be less efficient under some circumstances, because one has to iterate over all attributes more often.

(c) Attribute handles.

Yet another solution would be to not reference the attribute data directly but to create an indirection.
We could have a struct AttributeHandle that contains the name and information about where the attribute is stored. The rna attribute iterator could just iterate over a list of AttributeHandle that is stored in the ID. Only when the data in an attribute is actually accessed, a PointerRNA for the specific data storage is created. This removes the handling of specific data storages from the rna iterator, simplifying it a lot.

The main problem with this approach is of course that the attribute handle list needs be kept in sync with the actual set of attributes on the data-block.


Looking at this list now, it seems like (a) is the way to go. There is still the problem with vertex group names though, which are stored on the object...

![image.png](https://archive.blender.org/developer/F10133666/image.png) Similar to how we have lists for vertex groups, uv maps and more, we would like to have a list of attributes. For pointcloud objects (which are an experimental feature), we actually have the attributes list already. However, for those it was easy, because all attributes (including builtin ones like `position`) are stored the same way. That made it easy to populate a ui list that contains all attributes. Getting the same to work for meshes is more complicated, because mesh attributes are stored in many different ways. I can think of three main ways to get a list of all mesh attributes (including uv layers etc.) in the ui: 1. Provide an rna collection property that contains all the attributes and use the existing `UIList`. Here the main difficulty is to create the collection property in rna for very heterogenuous data. More about that later. 2. Refactor `UIList` itself, so that it can display more arbitrary data that is not stored in an rna collection. This refactor could also be useful for other features, but I can't estimate the feasibility of this change yet. 3. Don't use `UIList` and draw the list more manually instead. This is surely the simplest and most straight solution. However, it is also ugly and can't match the look and feel of `UIList`. The remainder of this post explores different ways to create an rna collection that can contain all attributes, including builtin (e.g. `position` and `material_index`) and special purpose (e.g. uv layers) ones. -------- First, let's have a look at the different ways attributes are stored on meshes. All the attributes mentioned below can be accessed by geometry nodes already. * Generic attributes are stored in generic custom data layers. * `position` is stored as part of `MVert`. * `material_index` is stored as part of `MPoly`. * `shade_smooth` is stored as part of `MPoly`. * `crease` is stored as part of `MEdge`. * `normal` is special and I hope that we can remove it as attribute at some point (because it is derived data). * UV maps are stored as special uv data layers. * Vertex colors are stored as special vertex color data layers. * All vertex groups are stored in a single custom data layer. However, that is the smaller issue. The real problem is that vertex group names are *not* stored on the mesh, but on the object. No one was able to tell me why yet. Having the names on the object might be a dealbreaker for all the approaches below. Looks like we will have to investigate if we can move the vertex group names to the geometry.. The existing attribute api in rna only provides access to generic custom data layers and does not include builtin and special purpose attributes. Below are a few ideas on how the attribute api can be extended to support the different types. **(a) Extend existing attribute iterator with other custom data layer types.** This mainly involves extending the `rna_AttributeGroup_iterator_*` functions. To handle multiple attributes in the same custom data layer (e.g. `material_index` and `shade_smooth` in the `CD_MPOLY` layer), more information might need to be stored in `CollectionPropertyIterator.internal`. Furthermore, new rna types like `RNA_UVAttribute` and `RNA_MeshPositionAttribute` might be necessary that handle the data access for builtin and special purpose attributes. **(b) Index based iterator.** This is similar to the solution above, but instead of having the iterator reference the attributes directly, it only uses an index. With this we just need to implement a function that counts the number of attributes and one that that gets the correct `PointerRNA` for an index. The main benefit here is that `rna_AttributeGroup_iterator_next` becomes trivial. Unfortunately, this solution might be less efficient under some circumstances, because one has to iterate over all attributes more often. **(c) Attribute handles.** Yet another solution would be to not reference the attribute data directly but to create an indirection. We could have a `struct AttributeHandle` that contains the name and information about where the attribute is stored. The rna attribute iterator could just iterate over a list of `AttributeHandle` that is stored in the `ID`. Only when the data in an attribute is actually accessed, a `PointerRNA` for the specific data storage is created. This removes the handling of specific data storages from the rna iterator, simplifying it a lot. The main problem with this approach is of course that the attribute handle list needs be kept in sync with the actual set of attributes on the data-block. ----- Looking at this list now, it seems like (a) is the way to go. There is still the problem with vertex group names though, which are stored on the object...
Author
Member

Added subscriber: @JacquesLucke

Added subscriber: @JacquesLucke
Author
Member

Added subscriber: @HooglyBoogly

Added subscriber: @HooglyBoogly
Author
Member

@HooglyBoogly Guess we should check the feasibility of moving vertex group names to geometry data-blocks soon, before annoying others with this topic :D

@HooglyBoogly Guess we should check the feasibility of moving vertex group names to geometry data-blocks soon, before annoying others with this topic :D
Member

Added subscriber: @JulianEisel

Added subscriber: @JulianEisel
Member

I have the same issue with UI-Lists requiring an RNA collection for asset views:
Screenshot 2021-04-08 at 18.59.26.png
(Note that I've also done quite some refactoring for the UI list code and want to do another pass, so I recommend not changing that for now.)

The assets somehow need to be passed as RNA collection. This case has some further difficulties since we want script authors to be able to place such an asset view anywhere in the UI, so it can't be bound to a specific editor/context.

Don't use UIList and draw the list more manually instead. This is surely the simplest and most straight solution. However, it is also ugly and can't match the look and feel of UIList.

Doing this would mean scrolling, box size, filter and order settings will not be stored in files. Only few UI types can be stored, UI-List being one of them.


How I did it

What I currently do is require a dummy pointer-property combination for a collection, which can be defined as custom property (reminder, scripts must be able to define own asset views). The items of the collection had to be of type PropertyGroup, although that may be just because it had to be usable with custom properties (don't remember the details). So I added an AssetHandle which could now be put in the collections.
Before drawing the list we just always populate the collection from scratch. We could avoid repopulating a lot. Performance would probably be fine in practice, but I hate the unnecessary work.

{https://developer.blender.org/diffusion/B/browse/asset-browser-poselib/source/blender/editors/interface/interface_template_asset_view.cc$160-194}


How I Plan to do it Properly

For me the UI list design ties the model too close to the view. What you have in other widget systems is some kind of model-view as interface to give efficient views into complex data. From #88184 (Asset System: Data Storage, Reading & UI Access):

Viewing

If the storage part represents the model, this part represents the view. Idea is a general abstraction: uiModelView. It gives a view into a (potentially big) data-set, i.e. a model. This could be useful for the rest of Blender too, but we can keep it very simple for the start.

For assets, there would be the following model-view:

class AssetView : public uiModelView {
 // ...
 // Methods for lazy-caching, filtering, and querying information about the 
 // model (e.g. the name and preview of an item).
}

UIs like the UIList or the File Browser can get their view onto data via the uiModelView interface. Once the data is fetched into such a view, they do not need to know about the exact implementation (e.g. if this is an asset list, a file list, an RNA collection, ...).

I think the UI-List would just use an iterator provided by the uiModelView.

For a timeline, I don't think it will take long to get to this, I got a good chunk of #88184 (Asset System: Data Storage, Reading & UI Access) already implemented.

I have the same issue with UI-Lists requiring an RNA collection for asset views: ![Screenshot 2021-04-08 at 18.59.26.png](https://archive.blender.org/developer/F10133735/Screenshot_2021-04-08_at_18.59.26.png) (Note that I've also done quite some refactoring for the UI list code and want to do another pass, so I recommend not changing that for now.) The assets somehow need to be passed as RNA collection. This case has some further difficulties since we want script authors to be able to place such an asset view anywhere in the UI, so it can't be bound to a specific editor/context. > Don't use UIList and draw the list more manually instead. This is surely the simplest and most straight solution. However, it is also ugly and can't match the look and feel of UIList. Doing this would mean scrolling, box size, filter and order settings will not be stored in files. Only few UI types can be stored, UI-List being one of them. --- **How I did it** What I currently do is require a dummy pointer-property combination for a collection, which can be defined as custom property (reminder, scripts must be able to define own asset views). The items of the collection had to be of type `PropertyGroup`, although that may be just because it had to be usable with custom properties (don't remember the details). So I added an `AssetHandle` which could now be put in the collections. Before drawing the list we just always populate the collection from scratch. We could avoid repopulating a lot. Performance would probably be fine in practice, but I hate the unnecessary work. {https://developer.blender.org/diffusion/B/browse/asset-browser-poselib/source/blender/editors/interface/interface_template_asset_view.cc$160-194} --- **How I Plan to do it Properly** For me the UI list design ties the model too close to the view. What you have in other widget systems is some kind of model-view as interface to give efficient views into complex data. From #88184 (Asset System: Data Storage, Reading & UI Access): >#### Viewing > > If the storage part represents the *model*, this part represents the *view*. Idea is a general abstraction: `uiModelView`. It gives a view into a (potentially big) data-set, i.e. a model. This could be useful for the rest of Blender too, but we can keep it very simple for the start. > > For assets, there would be the following model-view: >``` >class AssetView : public uiModelView { > // ... > // Methods for lazy-caching, filtering, and querying information about the > // model (e.g. the name and preview of an item). > } > ``` > > UIs like the `UIList` or the File Browser can get their view onto data via the `uiModelView` interface. Once the data is fetched into such a view, they do not need to know about the exact implementation (e.g. if this is an asset list, a file list, an RNA collection, ...). I think the UI-List would just use an iterator provided by the `uiModelView`. For a timeline, I don't think it will take long to get to this, I got a good chunk of #88184 (Asset System: Data Storage, Reading & UI Access) already implemented.
Author
Member

Nice, thanks for the info. That is quite helpful already!

From what I understand, your current solution is quite similar to my attribute handles idea. And you solved the "main problem" that I mentioned in the original post by updating the "handles" explicitly before drawing by calling asset_view_template_refresh_asset_collection.
That can definitely work, would just be a bit annoying if addon developers would have to call some update function before they can access attributes.

Your plan on how to do it properly sounds good to me. I just wasn't sure about the feasibility of a solution like that.

All of that still does not solve the vertex group name issue unfortunately (although it can be worked around more easily if all we want is to display the attributes of an object (instead of geometry)).

Nice, thanks for the info. That is quite helpful already! From what I understand, your current solution is quite similar to my `attribute handles` idea. And you solved the "main problem" that I mentioned in the original post by updating the "handles" explicitly before drawing by calling `asset_view_template_refresh_asset_collection`. That can definitely work, would just be a bit annoying if addon developers would have to call some update function before they can access attributes. Your plan on how to do it properly sounds good to me. I just wasn't sure about the feasibility of a solution like that. All of that still does not solve the vertex group name issue unfortunately (although it can be worked around more easily if all we want is to display the attributes of an object (instead of geometry)).

Added subscriber: @GeorgiaPacific

Added subscriber: @GeorgiaPacific
Author
Member

Added subscriber: @brecht

Added subscriber: @brecht
Author
Member

@brecht Did you think about this problem when you worked on the initial attribute api? Do you have a preferred solution?

@brecht Did you think about this problem when you worked on the initial attribute api? Do you have a preferred solution?

I think (a) and (c) are reasonable. It's useful to be able to access the full list of attributes from Python scripts.

The way I imagined it to work is that the API in BKE_attribute.h would be extended to support more types, like UVs and vertex groups. Currently it assumes each attribute to correspond to a CustomDataLayer, but further abstraction as in (c) can be added. I think that kind of abstraction can also be used by spreadsheet editor or geometry nodes.

I think (a) and (c) are reasonable. It's useful to be able to access the full list of attributes from Python scripts. The way I imagined it to work is that the API in `BKE_attribute.h` would be extended to support more types, like UVs and vertex groups. Currently it assumes each attribute to correspond to a `CustomDataLayer`, but further abstraction as in (c) can be added. I think that kind of abstraction can also be used by spreadsheet editor or geometry nodes.
Author
Member

@brecht thanks, that sounds reasonable. Do you remember why vertex group names are on the object instead of the geometry? Are you aware of any major issues we would run into by moving them to the geometry?

@brecht thanks, that sounds reasonable. Do you remember why vertex group names are on the object instead of the geometry? Are you aware of any major issues we would run into by moving them to the geometry?

I don't know, that was implemented before Blender became open source. Perhaps it was just copying the shape keys implementation, which I suspect are on the object so you can animate their weights differently for each objects.

Whatever the historical reason, I think it belongs on the geometry. I don't see any major issues moving this. If the mesh and object are in different .blend files, there will be some data loss since you can't version that. But I think that's extremely rare and not a real problem.

I don't know, that was implemented before Blender became open source. Perhaps it was just copying the shape keys implementation, which I suspect are on the object so you can animate their weights differently for each objects. Whatever the historical reason, I think it belongs on the geometry. I don't see any major issues moving this. If the mesh and object are in different .blend files, there will be some data loss since you can't version that. But I think that's extremely rare and not a real problem.
Author
Member

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

Changed status from 'Needs Triage' to: 'Archived'
Sign in to join this conversation.
No Label
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset Browser
Interest
Asset Browser Project Overview
Interest
Audio
Interest
Automated Testing
Interest
Blender Asset Bundle
Interest
BlendFile
Interest
Collada
Interest
Compatibility
Interest
Compositing
Interest
Core
Interest
Cycles
Interest
Dependency Graph
Interest
Development Management
Interest
EEVEE
Interest
EEVEE & Viewport
Interest
Freestyle
Interest
Geometry Nodes
Interest
Grease Pencil
Interest
ID Management
Interest
Images & Movies
Interest
Import Export
Interest
Line Art
Interest
Masking
Interest
Metal
Interest
Modeling
Interest
Modifiers
Interest
Motion Tracking
Interest
Nodes & Physics
Interest
OpenGL
Interest
Overlay
Interest
Overrides
Interest
Performance
Interest
Physics
Interest
Pipeline, Assets & IO
Interest
Platforms, Builds & Tests
Interest
Python API
Interest
Render & Cycles
Interest
Render Pipeline
Interest
Sculpt, Paint & Texture
Interest
Text Editor
Interest
Translations
Interest
Triaging
Interest
Undo
Interest
USD
Interest
User Interface
Interest
UV Editing
Interest
VFX & Video
Interest
Video Sequencer
Interest
Virtual Reality
Interest
Vulkan
Interest
Wayland
Interest
Workbench
Interest: X11
Legacy
Blender 2.8 Project
Legacy
Milestone 1: Basic, Local Asset Browser
Legacy
OpenGL Error
Meta
Good First Issue
Meta
Papercut
Meta
Retrospective
Meta
Security
Module
Animation & Rigging
Module
Core
Module
Development Management
Module
EEVEE & Viewport
Module
Grease Pencil
Module
Modeling
Module
Nodes & Physics
Module
Pipeline, Assets & IO
Module
Platforms, Builds & Tests
Module
Python API
Module
Render & Cycles
Module
Sculpt, Paint & Texture
Module
Triaging
Module
User Interface
Module
VFX & Video
Platform
FreeBSD
Platform
Linux
Platform
macOS
Platform
Windows
Priority
High
Priority
Low
Priority
Normal
Priority
Unbreak Now!
Status
Archived
Status
Confirmed
Status
Duplicate
Status
Needs Info from Developers
Status
Needs Information from User
Status
Needs Triage
Status
Resolved
Type
Bug
Type
Design
Type
Known Issue
Type
Patch
Type
Report
Type
To Do
No Milestone
No project
No Assignees
4 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: blender/blender#88460
No description provided.