Fix T94051: Add parameters to rna_Image_save_render()
Needs ReviewPublic
Actions

Authored by Paul Golter (paulgolter) on Feb 3 2022, 11:27 AM.

Details

Reviewers

Campbell Barton (campbellbarton)
Brecht Van Lommel (brecht)

Summary

This patch fixes: https://developer.blender.org/T94051

By adding frame, layer_index, pass_index parameter, API Users have more control over what exactly they want to export when calling image.save_render(). Because of the way how the ImageUser was created in rna_Image_save_render previously it would throw a 'Could not Acquire Buffer from Image' error when trying to save an Image of a Sequence.

Currently there is an open Design task https://developer.blender.org/T94869 on how to handle Temporary DNA struct creation for argument passing. Depending on what the result of this will be, this patch might need to be adjusted. The Design task wasn't updated for a while tough and it would be great to have a solution for image.save_render() now.

Diff Detail

Event Timeline

Paul Golter (paulgolter) requested review of this revision.Feb 3 2022, 11:27 AM

Paul Golter (paulgolter) created this revision.

Paul Golter (paulgolter) edited the summary of this revision. (Show Details)Feb 3 2022, 11:31 AM

Paul Golter (paulgolter) added inline comments.Feb 3 2022, 11:42 AM

source/blender/makesrna/intern/rna_image_api.c
83	Would be great to hear your opinions here, not sure if we need it

Paul Golter (paulgolter) edited the summary of this revision. (Show Details)Feb 3 2022, 2:15 PM

For the layer and pass, I think it should be a name

source/blender/makesrna/intern/rna_image_api.c
83	I don't think we need it here. If you've tested that passing the frame without it works there is no need. The purpose for this is looping or restricting to a certain frame range, but if you're explicitly accessing a frame number from the API that does not have to be taken into account.
310–320	Passing in indices here is a bit strange, I'm not sure if there is even a way to figure out which index corresponds to what from the API? It would be better to pass a name here, but that may require deeper internal changes to make it work. Not sure if you need that functionality, or are just adding it for completeness along with the frame and could just leave it out.

Paul Golter (paulgolter) added inline comments.Feb 3 2022, 4:42 PM

source/blender/makesrna/intern/rna_image_api.c
83	I was just remembering that sometimes you need to set this when using images sequence in the Compositor otherwise it would not load frames. But you are right we are explicitly accessing a frame, so I will remove it. Tested it and it works without it.
310–320	You are right it would propably be nicer to do it by name. I am not sure if there is an actual way to know which index corresponds to what. It is possible tough to get the active layer and pass index with the current API via the image_user. layer_index = image_editor_area.spaces.active.image_user.multilayer_layer pass_index = image_editor_area.spaces.active.image_user.multilayer_pass I am checking this in an add-on and now I would like to save the active pass and layer to disk with the image.save_render() function.

Only thing needed then is to update the comment about frames I think.

source/blender/makesrna/intern/rna_image_api.c
310–320	Ok, keeping it for that seems reasonable then.

Thanks for the review @Brecht Van Lommel (brecht),

Removed the comment and updated the diff. As I have no commit rights if you have time to commit it, here a sample commit message:

Fix T94051: Add parameters to rna_Image_save_render()

By adding frame, layer_index, pass_index parameter, API Users 
have more control over what exactly they want to export when
calling image.save_render(). Because of the way how the 
ImageUser was created previously in rna_Image_save_render it 
would throw a 'Could not Acquire Buffer from Image' error when 
trying to save an Image of a Sequence.


Differential Revision: https://developer.blender.org/D14003
Reviewed by: Brecht

I can also ask @Sebastian Parborg (zeddb) when he is in later.

Currently I'm looking into supporting passing ImageUser's directly, I'd like to check if this can be supported which might remove the need for this patch.

Campbell Barton (campbellbarton) mentioned this in T94869: Temporary DNA struct creation for argument passing.Feb 16 2022, 6:28 AM

Jeroen Bakker (jbakker) mentioned this in D7789: Fix T73730: UDIM "Edit externally" is capable of opening ONLY the first tile.Apr 20 2022, 5:47 PM