Temporary DNA struct creation for argument passing #94869

New Issue

Campbell Barton · 2022-01-13T10:34:01+01:00

Campbell Barton commented

2022-01-13 10:34:01 +01:00

Description

Currently there are situations where it's not possible to create arguments to pass into functions without first creating data-blocks elsewhere (typically in G.main).

While this is not an urgent problem, there are situations where this is inconvenient.

Motivation

In some cases it's useful to be able to pass in a DNA-struct
two examples of this are:

To construct an ImageUser to pass to bpy.types.Image.gl_load()

Currently we would either need to expand all image user arguments to construct an ImageUser in the `gl_load` function.

To construct key-map properties to pass to wm.keyconfigs.find_item_from_operator().

Currently `release/scripts/modules/bl_keymap_utils/keymap_from_toolbar.py` creates a key-map item so it's possible to create properties.

EDIT: would likely be a special case (even in the context of this topic) as it's type depends on another argument that is passed to the function. It may be that this requires custom parsing logic in the C/Python API.

Solutions

Here are some solutions we could consider to address this limitation:

Support Coerce Dictionaries

This would allow a dictionary to be passed as arguments to RNA functions.

An example of how this could work:

image.gl_load(image_user=dict(
    frame_current=5,
    multilayer_view=2,
    multilayer_pass=1,
))

Details:

Values that are unset would use DNA defaults.
An argument flag would be used to specify coercing is supported (to prevent this being used in situations where a reference to the memory passed in would be kept.)
Any unknown arguments or invalid types would raise an exception.
ID-properties could be supported as well as dictionaries

(if we want).

Pros:

Straightforward to implement and understand.
Doesn't complicate memory management, data creation and free can be done in the scope of a single function.

Cons:

Not very Pythonic (see other possible solution).
Slower in situations where the same argument is passed in many times.

Support Creating Instanced of RNA Types

We could support creating data from Python which is not part of Blender's data-base (G.main).

Since this opens ourselves up for complications regarding data ownership I'd propose only to support this for specific types.
So it should not be possible to create an ID and assign it to object-data.

Types could be created using their types, e.g.

image_user = bpy.type.ImageUser()
image_user.frame_current = 5
image_user.multilayer_view = 2
image_user.multilayer_pass = 1

Details:

The lifetime of these types would be bound to the Python-object.
The Python API would need to check this kind of data is never assigned or passed to functions that hold references to the memory.

Pros:

Convenient, Pythonic.

Cons:

Complicates the Python API & memory management for a corner case.
Types that start simple may be extended later in ways that complicate the API (for example if the image-user points to other ID data-blocks), meaning we would have to handle user-counts differently for data that Python-owned memory references.

A context manager could be used to limit the scope of Python owned data, however this is seems awkward, especially for multiple objects. It would simplify memory management though.

NOTE: this task was created after discussing the topic with @paulgolter

### Description Currently there are situations where it's not possible to create arguments to pass into functions without first creating data-blocks elsewhere (typically in G.main). While this is not an urgent problem, there are situations where this is inconvenient. ### Motivation In some cases it's useful to be able to pass in a DNA-struct two examples of this are: - To construct an `ImageUser` to pass to `bpy.types.Image.gl_load()` ``` Currently we would either need to expand all image user arguments to construct an ImageUser in the `gl_load` function. ``` - To construct key-map properties to pass to `wm.keyconfigs.find_item_from_operator()`. ``` Currently `release/scripts/modules/bl_keymap_utils/keymap_from_toolbar.py` creates a key-map item so it's possible to create properties. ``` ``` EDIT: would likely be a special case (even in the context of this topic) as it's type depends on another argument that is passed to the function. It may be that this requires custom parsing logic in the C/Python API. ``` ### Solutions Here are some solutions we could consider to address this limitation: **Support Coerce Dictionaries** This would allow a dictionary to be passed as arguments to RNA functions. An example of how this could work: ``` image.gl_load(image_user=dict( frame_current=5, multilayer_view=2, multilayer_pass=1, )) ``` **Details:** - Values that are unset would use DNA defaults. - An argument flag would be used to specify coercing is supported (to prevent this being used in situations where a reference to the memory passed in would be kept.) - Any unknown arguments or invalid types would raise an exception. - ID-properties could be supported as well as dictionaries ``` (if we want). ``` **Pros:** - Straightforward to implement and understand. - Doesn't complicate memory management, data creation and free can be done in the scope of a single function. **Cons:** - Not very Pythonic (see other possible solution). - Slower in situations where the same argument is passed in many times. **Support Creating Instanced of RNA Types** We could support creating data from Python which is not part of Blender's data-base (G.main). Since this opens ourselves up for complications regarding data ownership I'd propose only to support this for specific types. So it should not be possible to create an ID and assign it to object-data. Types could be created using their types, e.g. ``` image_user = bpy.type.ImageUser() image_user.frame_current = 5 image_user.multilayer_view = 2 image_user.multilayer_pass = 1 ``` **Details:** - The lifetime of these types would be bound to the Python-object. - The Python API would need to check this kind of data is never assigned or passed to functions that hold references to the memory. **Pros:** - Convenient, Pythonic. **Cons:** - Complicates the Python API & memory management for a corner case. - Types that start simple may be extended later in ways that complicate the API (for example if the image-user points to other ID data-blocks), meaning we would have to handle user-counts differently for data that Python-owned memory references. *A context manager could be used to limit the scope of Python owned data, however this is seems awkward, especially for multiple objects. It would simplify memory management though.* ---- NOTE: this task was created after discussing the topic with @paulgolter

Campbell Barton self-assigned this 2022-01-13 10:34:01 +01:00

Campbell Barton commented

2022-01-13 10:34:01 +01:00

Added subscriber: @ideasman42

Campbell Barton commented

2022-01-13 10:46:40 +01:00

Added subscriber: @paulgolter

Campbell Barton commented

2022-01-13 10:46:57 +01:00

Added subscriber: @mont29

Garek commented

2022-01-13 11:13:56 +01:00

Added subscriber: @Garek

Paul Golter commented

2022-01-13 11:15:33 +01:00

Hey Campbell, nicely written down!

Just to throw in another example: #94051 (Saving single image of image sequence: image.save_render(): Could not acquire buffer from image. ). There the image.save_render() function would also require more input arguments.

If I can speak for myself (perspective of the API user) I really only care about having the parameters to control. While it might be sometimes more convenient to work with an actual object instance of a blender type, passing arguments as dictionary would be totally fine for me! I am not 100% sure but I think it is already used in some functions.

Most of the time I have to look up the exact arguments of things in the documentation anyway so raising an exception for unknown arguments or invalid types sounds good!

It seems like the Instances of RNA Types solution is way more work to implement while I, the API user, can achieve the same thing with both solutions. I guess its more about trying to predict what is more robust and easier to maintain if in the future we have 100s of these cases. I also don't know what it means for the daily developer who has to work with Instances of RNA Types that are owned by Python, it this makes actual developing harder.

Hey Campbell, nicely written down! Just to throw in another example: #94051 (Saving single image of image sequence: image.save_render(): Could not acquire buffer from image. ). There the `image.save_render()` function would also require more input arguments. If I can speak for myself (perspective of the API user) I really only care about having the parameters to control. While it might be sometimes more convenient to work with an actual object instance of a blender type, passing arguments as dictionary would be totally fine for me! I am not 100% sure but I think it is already used in some functions. Most of the time I have to look up the exact arguments of things in the documentation anyway so raising an exception for unknown arguments or invalid types sounds good! It seems like the Instances of RNA Types solution is way more work to implement while I, the API user, can achieve the same thing with both solutions. I guess its more about trying to predict what is more robust and easier to maintain if in the future we have 100s of these cases. I also don't know what it means for the daily developer who has to work with Instances of RNA Types that are owned by Python, it this makes actual developing harder.

Julian Eisel commented

2022-01-13 11:19:10 +01:00

Added subscriber: @JulianEisel

Julian Eisel commented

2022-01-13 11:19:10 +01:00

Another use-case:
For the asset view template (template_asset_view()) we wanted to support passing filter settings. Because we couldn't easily construct a filter-settings object in Python and pass it to the template, we had to limit filtering options to an ID filter flag for now. Other workarounds would've been possible (e.g. using ID-properties), but we kept things simple, the template is too complex already.

Another use-case: For the asset view template (`template_asset_view()`) we wanted to support passing filter settings. Because we couldn't easily construct a filter-settings object in Python and pass it to the template, we had to limit filtering options to an ID filter flag for now. Other workarounds would've been possible (e.g. using ID-properties), but we kept things simple, the template is too complex already.

Jeroen Bakker commented

2022-01-13 15:16:05 +01:00

Added subscriber: @Jeroen-Bakker

Jeroen Bakker commented

2022-01-13 15:17:22 +01:00

Other case is when we make the GPUShaderCreateInfo available via python. D13360: GPUShaderCreateInfo for interface abstraction

Other case is when we make the GPUShaderCreateInfo available via python. [D13360: GPUShaderCreateInfo for interface abstraction](https://archive.blender.org/developer/D13360)

Paul Larson commented

2022-01-13 22:08:43 +01:00

Added subscriber: @GeorgiaPacific

Paul Golter commented

2022-01-18 14:34:43 +01:00

Hey @ideasman42,

Any updates on that topic? :)

Hey @ideasman42, Any updates on that topic? :)

Campbell Barton commented

2022-02-08 12:59:30 +01:00

Submitted initial patch to support this by passing dictionaries that are used to initialize the struct (on top of DNA defaults), although it needs further testing: D14047

Submitted initial patch to support this by passing dictionaries that are used to initialize the struct (on top of DNA defaults), although it needs further testing: [D14047](https://archive.blender.org/developer/D14047)

Brecht Van Lommel commented

2022-02-14 17:54:03 +01:00

Added subscriber: @brecht

Brecht Van Lommel commented

2022-02-14 17:54:03 +01:00

I didn't see this design proposal before the patch, but will leave my feedback here now.

Personally I don't understand how this is better than passing individual arguments to API functions, in the cases of image users, asset filters or GPUShaderCreateInfo. The advantage seems to be that you can then use the same dictionary multiple times, but none of those use cases would do that?

For image users it seems more obscure. You are expected to fill in an ImageUser but only 3 out of 10 fields are actually relevant to the API function. How does a user know which ones?

The ability to create instances of some RNA types would be useful in general. We have clunky APIs like to_mesh and to_mesh_clear for this, but they are not thread safe and may lead to memory leaks. But creating a full datablock like that is a more complex use case of course.

I didn't see this design proposal before the patch, but will leave my feedback here now. Personally I don't understand how this is better than passing individual arguments to API functions, in the cases of image users, asset filters or GPUShaderCreateInfo. The advantage seems to be that you can then use the same dictionary multiple times, but none of those use cases would do that? For image users it seems more obscure. You are expected to fill in an `ImageUser` but only 3 out of 10 fields are actually relevant to the API function. How does a user know which ones? The ability to create instances of some RNA types would be useful in general. We have clunky APIs like `to_mesh` and `to_mesh_clear` for this, but they are not thread safe and may lead to memory leaks. But creating a full datablock like that is a more complex use case of course.

Campbell Barton commented

2022-02-15 04:09:00 +01:00

The use case for this feature is admittedly quite rare.

In the case of an ImageUser.

When the script author has an ImageUser, passing it into a function that internally uses an ImageUser is convenient (without having to worry about details), it should always work as expected and removes the need to expand individual arguments. It's also future proof as new properties won't need to be manually passed into the function.
By passing in individual arguments, all callers that use an image_user internally would need to take the same set of arguments. This could be handled using a shared template (which would at least avoid copy-pasted arguments that could easily get out of sync).
Optionally the function could accept either expanded arguments or an image_user which at least allows both use cases & removes the need to create RNA-structs. Although it is a little confusing to take multiple arguments that do the same thing.
Agree that a weak point of this functionality is that some fields aren't used (the current frame may be used so that adds one extra property).

It could be argued you run into practically the same issue for any kind of data that's constructed to be passed in as an argument as there may always be some members that aren't used, however when these are explicitly included as a kind of sub-keyword-argument, the fact some of them have no effect isn't ideal.

If we really want to avoid accepting dummy arguments that do nothing, we could flag some properties as being excluded from coercing, causing an error if they are included - mentioning that it's possible although it's not foolproof as functions may use a struct differently.

So as far as I can see there are pros/cons with both, the main down-side with splitting into individual arguments is that passing in an existing image user isn't possible, whereas the main downside with coercing is there may be some properties that aren't used.

More generally:

It may be that the ImageUser case isn't compelling enough but other use-cases are, since @JulianEisel & @Jeroen-Bakker replied noting this kind of functionality would have come in handy for them.
We might want to look into creating the data instead of temporarily coercing from a dictionary. While I'm open to this - I'm not convinced it's worth the additional complexity it adds - especially if it's mainly used for passing in arguments.

The use case for this feature is admittedly quite rare. In the case of an `ImageUser`. - When the script author has an `ImageUser`, passing it into a function that internally uses an `ImageUser` is convenient (without having to worry about details), it should always work as expected and removes the need to expand individual arguments. It's also future proof as new properties won't need to be manually passed into the function. - By passing in individual arguments, all callers that use an `image_user` internally would need to take the same set of arguments. This could be handled using a shared template (which would at least avoid copy-pasted arguments that could easily get out of sync). - Optionally the function could accept either expanded arguments or an image_user which at least allows both use cases & removes the need to create RNA-structs. Although it is a little confusing to take multiple arguments that do the same thing. - Agree that a weak point of this functionality is that some fields aren't used (the current frame may be used so that adds one extra property). ``` It could be argued you run into practically the same issue for any kind of data that's constructed to be passed in as an argument as there may always be some members that aren't used, however when these are explicitly included as a kind of sub-keyword-argument, the fact some of them have no effect isn't ideal. ``` - If we really want to avoid accepting dummy arguments that do nothing, we *could* flag some properties as being excluded from coercing, causing an error if they are included - mentioning that it's possible although it's not foolproof as functions may use a struct differently. So as far as I can see there are pros/cons with both, the main down-side with splitting into individual arguments is that passing in an existing image user isn't possible, whereas the main downside with coercing is there may be some properties that aren't used. More generally: - It may be that the `ImageUser` case isn't compelling enough but other use-cases are, since @JulianEisel & @Jeroen-Bakker replied noting this kind of functionality would have come in handy for them. - We might want to look into creating the data instead of temporarily coercing from a dictionary. While I'm open to this - I'm not convinced it's worth the additional complexity it adds - especially if it's mainly used for passing in arguments.

Brecht Van Lommel commented

2022-02-15 16:56:39 +01:00

The API foc dealing with image buffers will need significant additions/changes at some point. What we really need is data types for and iterators over image layers/passes/views. And image users should also uses names instead of indices. So I don't think it's future proof for this use case. And I don't understand how this actually helps the other uses cases.

I'm not suggesting we tackle creating data now, but personally I would not bother doing this dictionary coercing at all.

If others like this design, it's not a big deal either way, I can go along with the majority.

The API foc dealing with image buffers will need significant additions/changes at some point. What we really need is data types for and iterators over image layers/passes/views. And image users should also uses names instead of indices. So I don't think it's future proof for this use case. And I don't understand how this actually helps the other uses cases. I'm not suggesting we tackle creating data now, but personally I would not bother doing this dictionary coercing at all. If others like this design, it's not a big deal either way, I can go along with the majority.

Campbell Barton commented

2022-02-16 06:28:46 +01:00

The motivation for coercing dictionaries into DNA/RNA was a request from @paulgolter to allow passing in ImageUsers (which seemed reasonable to me at the time), and support for some situations where you don't have an image user.

I'm not particularly invested in D14047 as a solution for passing arguments to save_render(..) (although it could still be useful elsewhere).

Taking named arguments (as suggested in D14003) seems fine - if these use a shared template we could avoid duplicating the same 3-4 arguments around.

Is this worth supporting now? Or does it depend on those names being more easily accessible for passing in names to be useful?

The motivation for coercing dictionaries into DNA/RNA was a request from @paulgolter to allow passing in ImageUsers (which seemed reasonable to me at the time), and support for some situations where you don't have an image user. I'm not particularly invested in [D14047](https://archive.blender.org/developer/D14047) as a solution for passing arguments to `save_render(..)` (although it could still be useful elsewhere). Taking named arguments (as suggested in [D14003](https://archive.blender.org/developer/D14003)) seems fine - if these use a shared template we could avoid duplicating the same 3-4 arguments around. Is this worth supporting now? Or does it depend on those names being more easily accessible for passing in names to be useful?

Brecht Van Lommel commented

2022-02-16 13:33:56 +01:00

For names, I don't think there's a way of getting a list of layers/passes/views and their names yet. So using names in save_render right now is not helpful.

Once we have a proper API, probably we'd have something like image.layers- [x].passes- [x].views- [x].buffer.save(...) instead of passing names.

For names, I don't think there's a way of getting a list of layers/passes/views and their names yet. So using names in `save_render` right now is not helpful. Once we have a proper API, probably we'd have something like `image.layers- [x].passes- [x].views- [x].buffer.save(...)` instead of passing names.

Campbell Barton commented

2022-02-21 10:22:43 +01:00

There would need to be a way to first access the frame of the image so we might need an intermediate accessor type to expose layers (or this could be supported by ImageUser).

It might be worth checking on how this could work and how much work this is to support (before spending more time on D14047).

At this point I'd rather have @brecht & @paulgolter discuss if a short term workaround is reasonable or not.

There would need to be a way to first access the frame of the image so we might need an intermediate accessor type to expose layers (or this could be supported by ImageUser). It might be worth checking on how this could work and how much work this is to support (before spending more time on [D14047](https://archive.blender.org/developer/D14047)). ---- At this point I'd rather have @brecht & @paulgolter discuss if a short term workaround is reasonable or not.

Philipp Oeser removed the

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Temporary DNA struct creation for argument passing #94869

Description

Motivation

Solutions