New header, 0.5.0, now posted


Jeremy Selan <jeremy...@...>
 

On Apr 22, 11:31 am, Larry Gritz <l....@imageworks.com> wrote:
Random questions:

What is the role of the 'direction' parameter to
ApplyColorSpaceTransform? Why would the user not just reverse the
InputColorSpace and OutputColorSpace parameter ordering to get the
reverse transformation?
Yah, the simple version is what we've had for 6 years. I added the
direction parameter for completeness yesterday. On second thought,
I'll drop it. ;)


Any reason why 'long' rather than 'int' in so many places? (I'm not
necessarily objecting, just curious.)
I'm just super used to working in CPython land so often they look
natural to me. (Python's native 'Int' data type is long - so using
them all over makes writing python glue a tiny bit more braindead).
But this is not a great reason to use longs, i'll roll it back.
http://docs.python.org/release/2.5/api/intObjects.html#l2h-384

Any role for OpenCL in addition to GLSL and Cg?
Sure, if anyone ever needs it it would make good sense. Does OpenCL
support accelerated 3d texture lookups? If so, it'll map well.
Otherwise, a bit more complex but still do-able. Probably not for 1.0
though...

Do you think "FilmLook" might seem anachronistic in the future or hurt
adoption in non-film pipelines? DisplayLook? OutputLook?
Yah, I agree FilmLook is not very forward-looking. I like your ideas,
I'll probably steal one of em.


Is it long/redundant to have ApplyASCColorCorrectionTransform,
ApplyFilmlookTransform, etc? If they were all called ApplyTransform,
they could still be distinguished by the argument types
(ASCColorCorrection& vs FilmLook&, etc.). A matter of style, not
critical, just bringing it up in case others prefer to avoid names
that are extra long only because they have redundancy in the naming
and arguments.
Yes, these names are redundant. I'll shorten them up. I think part
of the reason I went for a long name is that I've been looking at the
code for so long (at least a prior version) that I'm a bit dead to
it. Fresh eyes on an API help quite a bit.


stride_t -- you never define. Either change to ptrdiff_t, or typedef
them and use stride_t everywhere.
Fixed.

ImageView will surely need pixel accessor methods.
Not externally, is my hope. Once the user constructs an ImageView,
other than passing it in I dont want them to call anything on it.
Internally, I can decorate the Imp with any private accessors I'd like
to use.

What does ColorSpace::isData() do?
Colorspaces that are 'data' are a bit special. Basically, any
colorspace transforms you try to apply to them are ignored. (Think of
applying a ColorCorrection to an ID pass). Also, in the Filmlook pass
it obeys special 'data min' and 'data max' args. I will document it
as such.

Thanks for all the comments! They are hugely appreciated.


--
Subscription settings: http://groups.google.com/group/ocs-dev/subscribe?hl=en


Jeremy Selan <jeremy...@...>
 

ApplyColorSpaceTransform does not use the roles.

GetHWFilmlook does. For example, if the fStopExposure arg is
specified, the exposure is adjusted internally in the SCENE_LINEAR
space (as specified by the role). If an ASCColorCorrection is
specified, this occurs in ROLE_COLOR_TIMING colorspace.

Additionally, plugins often rely on them. In our current Nuke
plugin, we have a LogConvert node that essentially does
ApplyColorSpaceTransform between ROLE_SCENE_LINEAR and
ROLE_COMPOSITING_LOG. This is convenient as end-user compositors
often know they want a 'Log Convert', but dont really want to concern
themselves with the specifics for which conversions to use.

I will update the header docs to make the functions which rely on
roles more obvious.

Would you agree with this concept? Assigning 'roles' at a high level
really is useful for a facility, but it's a slippery slope for sure in
having too many of them, and/or making them too facility specific.

The alternative would be to expose the roles as additional arguments.
(For example, in the HW filmlook exposure call an arg would be to
explicitly pass in the fstop exposure colorspace).

-- Jeremy

On Apr 22, 11:03 am, Rod Bogart <bog...@gmail.com> wrote:
Is the ColorSpace "role" just a database sorting tag, or does the
"role" impact the internal process of conversion?

RGB


Larry Gritz <l...@...>
 

Random questions:

What is the role of the 'direction' parameter to
ApplyColorSpaceTransform? Why would the user not just reverse the
InputColorSpace and OutputColorSpace parameter ordering to get the
reverse transformation?

Any reason why 'long' rather than 'int' in so many places? (I'm not
necessarily objecting, just curious.)

Any role for OpenCL in addition to GLSL and Cg?

Do you think "FilmLook" might seem anachronistic in the future or hurt
adoption in non-film pipelines? DisplayLook? OutputLook?

Is it long/redundant to have ApplyASCColorCorrectionTransform,
ApplyFilmlookTransform, etc? If they were all called ApplyTransform,
they could still be distinguished by the argument types
(ASCColorCorrection& vs FilmLook&, etc.). A matter of style, not
critical, just bringing it up in case others prefer to avoid names
that are extra long only because they have redundancy in the naming
and arguments.

stride_t -- you never define. Either change to ptrdiff_t, or typedef
them and use stride_t everywhere.

ImageView will surely need pixel accessor methods.

What does ColorSpace::isData() do?



--
Subscription settings: http://groups.google.com/group/ocs-dev/subscribe?hl=en


Rod Bogart <bog...@...>
 

Is the ColorSpace "role" just a database sorting tag, or does the
"role" impact the internal process of conversion?

RGB

On Thu, Apr 22, 2010 at 10:13 AM, Jeremy Selan <jeremy...@gmail.com> wrote:
This addresses almost all major comments (I hope) with the prior
header.

The one (big) missing chunk is that it does not expose function for
dynamically manipulating color configurations. (example being
OCSConfig->addColorspace, OCSConfig->writeToFile).  These will be
coming in a future revision, and will allow for authoring apps /
dynamic color workflows.


--
Subscription settings: http://groups.google.com/group/ocs-dev/subscribe?hl=en


Jeremy Selan <jeremy...@...>
 

This addresses almost all major comments (I hope) with the prior
header.

The one (big) missing chunk is that it does not expose function for
dynamically manipulating color configurations. (example being
OCSConfig->addColorspace, OCSConfig->writeToFile). These will be
coming in a future revision, and will allow for authoring apps /
dynamic color workflows.


--
Subscription settings: http://groups.google.com/group/ocs-dev/subscribe?hl=en