Patch: DatablockProperty and DatablockVectorProperty #37754
Labels
No Label
Interest
Animation & Rigging
Interest
Blender Cloud
Interest
Collada
Interest
Core
Interest
Documentation
Interest
Eevee & Viewport
Interest
Geometry Nodes
Interest
Grease Pencil
Interest
Import and Export
Interest
Modeling
Interest
Modifiers
Interest
Nodes & Physics
Interest
Pipeline, Assets & IO
Interest
Platforms, Builds, Tests & Devices
Interest
Python API
Interest
Rendering & Cycles
Interest
Sculpt, Paint & Texture
Interest
Translations
Interest
User Interface
Interest
UV Editing
Interest
VFX & Video
Meta
Good First Issue
Meta
Papercut
Module
Add-ons (BF-Blender)
Module
Add-ons (Community)
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
11 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: blender/blender-addons#37754
Loading…
Reference in New Issue
No description provided.
Delete Branch "%!s(<nil>)"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
idprop_datablock.patch
This patch allows Python scripts to create ID Properties which reference datablocks. This functionality has been on the cards for some time: I've been expanding stub functions and removing "not implemented" comments all over the place. The absence of datablock properties "will certainly be resolved soon as the need for them is becoming obvious" said the Python Nodes release notes...last May!
I've been writing this patch in conjunction with an add-on that uses it, so the code is quite mature already.
The patch adds
bpy.props.DatablockProperty
andbpy.props.DatablockVectorProperty
which both requires a "type" value derived frombpy.types.ID
. I decided against re-usingPointerProperty
as it doesn't support assignment, is never None and currently only handlesbpy.types.PropertyGroup
objects (which are not compatible with ID).A
DatablockProperty
can hold not only the assigned type but also anything derived from it. This allows the user to create abpy.types.ID
property and use it to store any sort of datablock that their script might need to deal with.In addition to the standard update callback,
DatablockProperty
can have apoll
callback (standard RNA) and acast
callback.cast
exists becauseset
would be self-defeating, as the whole point of ID Properties is to pass an item into C. It's called just before the value is stored and provides a chance for the Python script to interpret or switch the value, which is particularly useful when dealing withUILayout.prop_search
and/orCollectionProperty
objects.One thing that the patch doesn't do is change ID user counts. We can't assume that the relevant script will be running to tell Blender what to do, so it's either on all the time or off all the time.
IMO off is the sensible choice: all ID types already have an established means to gain users, and it would be irritating if an obscure ID Prop somewhere made other IDs hang around forever.
The patch also fixes an oversight in
readfile.c
'stest_pointer_array
. It assumed that the length of pointers in the array was the same as the file in general's, but in an array which has been saved blindly by a version of Blender unaware that it held pointers it might not be. This led to a crash when saving aDatablockProperty
blend in a 64-bit build, re-saving it in a 32-bit build without this patch applied, then loading it in the 64-bit build again. To fix this the function now takes an optional "items" param from which it deduces the array's actual pointer size.Demo blend: idprop_datablock.blend
Changed status to: 'Open'
Added subscriber: @artfunkel
This functionality is tricky to do well, I checked the patch and it looks like this is not handling cases where you unlink data that might be referenced by an id property?
Say for example - you have a scene, reference it from a
DatablockProperty
then remove the scene.What happens?
This can easily explode into having to do many checks which is one reason I didnt want to add this functionality. - Its too much prone to slowing blender down entirely since any data can link to any other data. For test files you wont notice, but for large scenes it will certainly have an effect unless there is some way to better know which relationships are possible. (its possible to be smart of course but checking the patch I didn't see any special handling of these cases).
I'm very wary of adding this functionality into Blender and think it has a high chance of back-firing unless done carefully.
Added subscriber: @dfelinto
It's handled on line 191. I'll have to test performance, but I would be surprised to see much of a difference unless loads of datablocks actually have datablock ID-props set.
If performance is a problem I'd be happy to create something smarter than currently exists.
Added subscriber: @brecht
I didn't look at the code, but for the name, my idea was to have this called just PointerProperty and not have a special DatablockProperty. It seems better if the C and Python properties behave the same. All ID pointers in C are a PointerProperty.
Performance wise it is indeed tricky, if you delete an object then you basically have to loop over all datablocks, bones and nodes to find if any of them points to the object getting deleted. The way around that would be to somehow provide a quick lookup of which datablocks contain such pointers. It would be nice if we could somehow use the dependency graph work, that is planned to include all datablocks in the graph, but it's unclear when it will be ready.
A trick could be to tag IDProperty groups, datablocks, and datablock lists to indicate if they contain a pointer to some datablock type, but I'm not sure how well that fits or if it can be made reliable.
The trouble with PointerProperty is that it doesn't currently support assignment, which I presume is because scripters aren't supposed to be creating their own PropertyGroup objects?
CollectionProperty
is the same.Rolling the functionality of DatablockProperty into PointerProperty wouldn't be much work at all as internally both are
PROP_POINTER
, it just involves making a change I don't currently understand the implications of.In terms of performance, the first thing to come to my mind is a hashed list of IDProps which hold references to a given ID. I still have yet to actually run any tests to see how slow things get though; bear in mind that most objects don't have any ID Properties at all!
I've run some benchmarks with this script , which creates 9999 empties with 3 DatablockVectorProperties containing two references, all to the same object. It then deletes the pointed-to object.
DatablockVectorProperty with this patch: 0.006
FloatVectorProperty without this patch: 0.002
Proportionately it's three times slower, but the difference is only 4ms even in the extreme case of nearly 60k pointers across 9999 objects.
Modifying the script to delete*all// of the objects doesn't change the proportions much, but the difference is now very noticeable:
With: 29.675
Without: 7.629
This is pushing an extreme scenario to a further extreme and personally I wouldn't sweat it too much, but those results also apply to the time required to free the blend file. That's daft - who cares about dependencies when everything is being freed anyway?
Is there any established way for
BKE_libblock_free
to tell whether the whole file is being unloaded?Edit: and for completeness, if no properties are created then performance is identical in both builds.
@artfunkel, it would be good to run this test on real files, for example some of the complex scenes in Sintel or Tears-of-steel.
You won't necessarily just have 1000's of objects - but 1000s of meshes, 100s bones, nodes, images, materials.
Added subscriber: @SatishGoda
Sound good, where can I download one? It looks like they are only distributed on DVD?
Added subscriber: @BartCrouch
Attaching an updated patch which keeps a hash set of ID Prop references.
In the first test case deletion time is up to 0.008s due to the overhead of using GHash. But in the second test the difference has been brought down to a hair's breadth:
5.05s with
4.96s without (I've optimised the test a bit, so this result is also lower than before.)
The patch now skips ID Prop unlinking entirely when the blend file is being unloaded, so there is no longer any penalty at all when loading a new file or undoing/redoing.
I haven't got my hands on a movie scene yet, if that still matters.
Added subscriber: @JoshuaLeung
Attaching a third version which adds support for copying, linking, and appending.
Not heard anything for a while now, I hope this patch hasn't been forgotten...
Maybe upload the patch in differential?
http://developer.blender.org/differential/diff/create/
The ID property hash table seems like a simple solution, not sure what the others think about it. One problem that needs to be solved is that it's not thread-safe.
I think DatablockProperty should be unified with PointerProperty still, I'm not sure what you mean by "it doesn't currently support assignment"? When you create a PointerProperty, it should be possible to inspect the type to see if it's an ID or a PropertyGroup and do the appropriate thing.
What is the purpose of DatablockVectorProperty, do you have a practical use for it? We didn't have a need for these in Blender itself as far as I know, so I would like to understand the use case. I think practically always you want a collection instead, even if it's initially just one pointer it's likely that you want to add a property at some point, and then it's backwards compatible instead of having to convert from a vector to a collection.
I initially went to create a diff, but the diff page suggests that sizeable changes are submitted as patches instead. I've made one now: D113.
If you try to assign a value to a PointerProperty you will receive a Python error telling you that it's read-only. You can only assign to the members of the pointee, which is a provided by Blender and different for each ID. I see three options here:
Have a separate property type for IDs.
If it weren't for this read-only thing I would have no hesitation in extending PointerProperty instead of creating a new prop type. Say what you want and it will be done. :-)
In the node screenshot above I'm using DatablockVectorProperty for [mesh LODs ]] and the https:*developer.valvesoftware.com/wiki/Blend_sequence node, and eventually I'll be using it to reference materials within individual [ https://developer.valvesoftware.com/wiki/$texturegroup | texture groups too. In all of those cases there is a pre-defined number of IDs needed. Using an array is a lot neater than creating a new PropertyGroup every time then making a bunch of
add()
orremove()
calls when/if the number of items needed changes.Ah, probably this is just the fact that pointer properties don't have the PROP_EDITABLE flag by default. That was done for convenience when defining properties in C, but when you define a property that points to an ID property from python, it can set PROP_EDITABLE on by default.
So I prefer option 2, and this can be set at the moment you define the property. The PropertyGroup being a special case is a bit strange, but follows the convention that all data is owned by Blender, there's no loose data, it's either an ID block owned by BlendData or owned by an ID block.
It's less code perhaps, but it still seems more extensible to me to use a collection. What if you need extra properties on a LoD level (for example Reverse from that document)? Do you keep multiple arrays in sync with the same size? What if you want to use a list UI?
The arrays were meant for things like vectors, matrices, pixels, etc. I feel the kind of use cases you propose go against Blender data API conventions, I rather have one way to store this type of data.
Alright then.
Well yes, you would have to go for a PropertyGroup then. I suppose that a new PropertyGroup is only one extra line of code, really...
Can you explain the practical use case for
cast
? The name seems strange, what it does is quite different than C casting, such a thing does not exist in python as far as I know. I don't understand in why you need to cast and ID pointer.The case for
cast
is subtle: you don't need to cast an ID, but you do need to cast other types provided byUILayout.prop_search
. In existing C code the job is done in theset
function (more about that in the patch description above).I realised the need for it when I needed to store a reference to a Datablock wrapped in a PropertyGroup collection. As discussed above you can't have an assignable PropertyGroup pointer (and in my case the collection has
SKIP_SAVE
too). This is the code which implementscast
:I'm pretty sure there is no established solution to this problem.
One alternative is to move
cast
over to prop_search (and then repeat it anywhere else required). Thinking about it, that would be more flexible: in the code above you can't directly assign an ID toexportable
!I'm going to take that last sentence back. There is zero infrastructure for Python callbacks when setting a value from a button; the RNA property is definitely the correct place for it. Anyone writing a cast function will just have to do this:
The
poll
callback is already used to filter the search list, so why iscast
needed as well? Instead of creating ansmd_export_list
collection, you can create a python set or map to do a quick lookup from thepoll
callback.The only possible difference I can see is if you want to display the ID with a different name in the search list, but that seems confusing as it will get the ID name once assigned, and probably is not what you wanted this for.
prop_search
requires abpy_prop_collection
, which unless I'm mistaken scripts can only create via CollectionProperty. Also I'm not sure what you mean by doing a lookup from poll, since a poll function has to return boolean...how would I tell Blender what value I want?To clear up any confusion,
poll
handles the search list and returns bool whilecast
handles the assigned value and returns an object.While writing this I've thought of a simpler alternative: add a "value_path" argument to
prop_search
which selects the appropriate object member. It's less flexible but avoids the need for a callback.Added subscriber: @AndreiIzrantcev
Is this alive? This looks like a very cool thing I (and all third party renderers integrators I believe) need a lot.
I've tried this patch and it work perfectly for me, with only small issue with "Error: Not freed memory blocks".
Check D113 for the latest on this patch. It does seem to have stalled, unfortunately. I can't do much more on it as I'm now employed as a Modo developer. :-/
artfunkel, yes, I was using the patch from D113. Even it's stalled, it works very well for me, good work.
Still works fine for me and on user scenes. I just have to tweak it a bit to support pynodes append/link.
Any chance to see it in 'master'?
Added subscriber: @AditiaA.Pratama
Added subscriber: @AlexanderRomanov
Removed subscriber: @ideasman42
Added subscriber: @Blendify
Changed status from 'Open' to: 'Resolved'
Committed in blender/blender@a7b3047cef