search

GafferHQ / gaffer

Gaffer is a node-based application for lookdev, lighting and automation
http://www.gafferhq.org
BSD 3-Clause "New" or "Revised" License
945 stars 205 forks source link

Provide `ImageWriter`'s expected colorspace in python #5937

Open ivanimanishi opened 2 months ago

ivanimanishi commented 2 months ago

Summary

Provide ImageWriter's expected colorspace in python.

User story

At Image Engine we want to track the colorspace use when writing images with the ImageWriter (via our internal subclass of that node). Up until now, we essentially replicated the functionality from ImageWriter.colorspace() (https://github.com/GafferHQ/gaffer/blob/main/include/GafferImage/ImageWriter.h#L121) so we could get that info. We are now in a position that we would need to duplicate that replicated code again (in another node that writes multiple image sequences at once) and it seems more reasonable to just ask for the gaffer implementation to be exposed so we can use it directly.

Additionally, in gaffer 1.3, behaviour for the ImageWriter changed where this method can now return an empty string, which will later be substituted based on the workingspace from the current context.

I guess what we'd really want is a method that returned that final value (so, never an empty string), if provided the context?

What

A way to get the colorspace that the ImageWriter will use to write out an image (given a certain context).

Why

Store the colorspace information in the asset manager, in order to be used later when reading it.

johnhaddon commented 2 months ago

I guess what we'd really want is a method that returned that final value (so, never an empty string), if provided the context?

I might regret saying this because I think it would be much fiddlier to implement (due to ImageWriter not being a ComputeNode), but what would you think to providing the same information via an output plug rather than a method?

ivanimanishi commented 2 months ago

I guess what we'd really want is a method that returned that final value (so, never an empty string), if provided the context?

I might regret saying this because I think it would be much fiddlier to implement (due to ImageWriter not being a ComputeNode), but what would you think to providing the same information via an output plug rather than a method?

Either option (method or plug) would work for us. Whatever you think best.

johnhaddon commented 1 month ago

What's the priority on this one @ivanimanishi, bearing in mind you'll need to be on 1.4 to take advantage of it?

ivanimanishi commented 1 month ago

Yeah. I was assuming this would be 1.4 only. Not a high priority. We'll be duplicating the logic at IE for now, and switch to using the native gaffer functionality once available.

Something else we've been debating internally regarding this is whether or not we want the fully resolved colorspace, or keep role-based colorspaces unresolved.

For example, we could get now something like "scene_linear", which would be resolved to something like "AcesCG" given the current environment's settings. For the asset management, we want to store the fully resolved value ("AcesCG" in this case).

Right now, from the regular gaffer logic, I think we'd be getting "scene_linear", and therefore need to go through the next step to resolve it to "AcesCG" before storing the information in the database.

I'm fine with doing this last step at IE specifically, if it doesn't make sense to do it in gaffer itself. But I thought it was worth mentioning. And we'd definitely want to know what to expect from gaffer.

Thanks!