Date   

Re: Review: DisplayTransform interface update

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

Here's an update that addresses all previous comments.
http://github.com/jeremyselan/OpenColorIO/commit/2b926355196bcd1690f9dd5a924c3d04ff24cd53

This commit updates the APIs to use our newer terminology + functionality:
- Display - a virtual or physical display device
- View - a meaningful view of the reference space on a Display

It also changes the .ocio config format:

displays:
DCIP3:
- !<View> {name: Raw, colorspace: nc10}
- !<View> {name: Log, colorspace: lg10}
- !<View> {name: Film, colorspace: p3dci8}
sRGB:
- !<View> {name: Raw, colorspace: nc10}
- !<View> {name: Log, colorspace: lg10}
- !<View> {name: Film, colorspace: srgb8}
Cheese:
- !<View> {name: Raw, colorspace: nc10}
- !<View> {name: Log, colorspace: lg10}
active_displays: [sRGB, DCIP3, Cheese]
active_views: [Film, Log, Raw]

(The old format is still allowed for backwards compatibility purposes.)

New environment variables are exposed:
$OCIO_ACTIVE_DISPLAYS
$OCIO_ACTIVE_VIEWS
which allow these lists to be pruned / prioritized at runtime.

Upon this commit being approved, I will put out updated configurations
that match this spec.

-- Jeremy


Re: Review: added ociobuildicc app

Malcolm Humphreys <malcolmh...@...>
 

Hi Joesph,

When building ICC profiles I've found it very helpful to use the InputTables of the AtoB0 to hold an intermediate conversion, for instance the a 1024 entry 1-d conversion from Cineon to video . The CLUT can then just hold the 3d portion of the conversion I found the match was much improved going that way without having to create a large 3d lut. It's really helps out if you are working with linear space source files in Photoshop.
Effectively this is what is being done with our matte painting allocation, and it does help out a lot.

It does bring up something, I'm looking at replacing the lut builder (non icc luts) I wrote before the switch to OCIO, with OCIO. I was hoping to implement a 1D Sampler (prelut/shaper) and a 3D Sampler for the cube. It would be cool to be able to tag a colorspace as being able to used for the 1D Samplers.

I was thinking about some group of allocation spaces?

.malcolm

-Joseph

-----Original Message-----
From: ocio...@... [mailto:ocio...@...] On Behalf Of Malcolm Humphreys
Sent: Thursday, January 27, 2011 11:00 PM
To: ocio...@...
Subject: Re: [ocio-dev] Review: added ociobuildicc app

Also added Little CMS to LICENSE file

Commit:
https://github.com/malcolmhumphreys/OpenColorIO/commit/8ff296f8ab234ea7b06704036037bf2a132ebd6a

.malcolm

On 28/01/2011, at 5:49 PM, Malcolm Humphreys wrote:

Attaching some slightly larger examples :)


On 28/01/2011, at 5:46 PM, Malcolm Humphreys wrote:

Added ociobuildicc app which will build a soft-proofing icc profile for a given working space.
(added LCMS2 into ext)

Commits:
https://github.com/malcolmhumphreys/OpenColorIO/commit/1b0d39fb10db10
f4edbf1fb0fed124b548b836d9

Attached is an example of two profiles in action.

From the left
* sRGB reference image (ocio cpu)
* matte paint allocation space
* log space image.

The visual difference in the log image is most likely caused by 16bit
quantization errors in the CLUT of the AToB0Tag. These errors are
less with the matte paint allocation as the distance to travel is less.

I did play around with supporting the new MPE D2Bx and B2Dx tags
which should make this problem go away
(http://www.color.org/ICCSpecRevision_02_11_06_Float.pdf) but couldn't get it too work reliably so I pulled it out till I can.

.malcolm



<ocioicc_softp_off.jpeg><ocioicc_softp_on.jpeg>
<ocioicc_softp_off.jpeg><ocioicc_softp_on.jpeg>


Re: Review: added ociobuildicc app

Malcolm Humphreys <malcolmh...@...>
 

Hi Joesph,

It has been approved but not added to a new spec yet.

http://www.color.org/icc_specs2.xalter
--snip--
Approved revisions since ICC.1:2004-10
25 Feb 2010 Dictionary Type and Metadata TAG Definition
7 Nov 2007 Deletion of mediaBlackPointTag
11 Apr 2007 Colorimetric Intent Image State tag
11 Apr 2007 Profile Sequence Identifier tag
2 Nov 2006 Floating Point Device Encoding Range
13 Jun 2006 Motion Picture technology tags
22 Feb 2005 Perceptual Intent Reference Medium Color Gamut
--snip--

It's also referenced in the book 'Color Management: Understanding and Using ICC Profiles'. Both http://sourceforge.net/projects/sampleicc/ and littleCMS support MPE tags, and the book also suggests that their is a version of Adobe CMM that also supports MPE.

I have only recently been aware of the new MPE tags and haven't gotten it to work in photoshop, would be nice if it did work. I'm sure their is some secrete sauce lying around somewhere that will make it work.

.malcolm

On 01/02/2011, at 12:49 PM, Joseph Slomka wrote:

Malcolm,

This is slightly off topic.

Is there a new ICC standard support for the MPE tags? The newest document I found on this was http://ntust-ciic.no-ip.info/ciic/Phil_Green-Taiwan-Presentations/ITRI-ICC-6-10-09.pdf That document is not even a proposal.

It still looks like the D2Bx tags are not officially supported. In your expecience do most CMM's and applications implement D2Bx support?

-Joseph

-----Original Message-----
From: ocio...@... [mailto:ocio...@...] On Behalf Of Malcolm Humphreys
Sent: Thursday, January 27, 2011 10:47 PM
To: OpenColorIO Developers
Subject: [ocio-dev] Review: added ociobuildicc app

Added ociobuildicc app which will build a soft-proofing icc profile for a given working space.
(added LCMS2 into ext)

Commits:
https://github.com/malcolmhumphreys/OpenColorIO/commit/1b0d39fb10db10f4edbf1fb0fed124b548b836d9

Attached is an example of two profiles in action.

From the left
* sRGB reference image (ocio cpu)
* matte paint allocation space
* log space image.

The visual difference in the log image is most likely caused by 16bit quantization errors in the CLUT of the AToB0Tag. These errors are less with the matte paint allocation as the distance to travel is less.

I did play around with supporting the new MPE D2Bx and B2Dx tags which should make this problem go away (http://www.color.org/ICCSpecRevision_02_11_06_Float.pdf) but couldn't get it too work reliably so I pulled it out till I can.

.malcolm


Re: Review: added ociobuildicc app

Joseph Slomka <jsl...@...>
 

Malcolm,

This is slightly off topic.

Is there a new ICC standard support for the MPE tags? The newest document I found on this was http://ntust-ciic.no-ip.info/ciic/Phil_Green-Taiwan-Presentations/ITRI-ICC-6-10-09.pdf That document is not even a proposal.

It still looks like the D2Bx tags are not officially supported. In your expecience do most CMM's and applications implement D2Bx support?

-Joseph

-----Original Message-----
From: ocio...@... [mailto:ocio...@...] On Behalf Of Malcolm Humphreys
Sent: Thursday, January 27, 2011 10:47 PM
To: OpenColorIO Developers
Subject: [ocio-dev] Review: added ociobuildicc app

Added ociobuildicc app which will build a soft-proofing icc profile for a given working space.
(added LCMS2 into ext)

Commits:
https://github.com/malcolmhumphreys/OpenColorIO/commit/1b0d39fb10db10f4edbf1fb0fed124b548b836d9

Attached is an example of two profiles in action.

From the left
* sRGB reference image (ocio cpu)
* matte paint allocation space
* log space image.

The visual difference in the log image is most likely caused by 16bit quantization errors in the CLUT of the AToB0Tag. These errors are less with the matte paint allocation as the distance to travel is less.

I did play around with supporting the new MPE D2Bx and B2Dx tags which should make this problem go away (http://www.color.org/ICCSpecRevision_02_11_06_Float.pdf) but couldn't get it too work reliably so I pulled it out till I can.

.malcolm


Re: Review: added ociobuildicc app

Joseph Slomka <jsl...@...>
 

Malcolm,

When building ICC profiles I've found it very helpful to use the InputTables of the AtoB0 to hold an intermediate conversion, for instance the a 1024 entry 1-d conversion from Cineon to video . The CLUT can then just hold the 3d portion of the conversion I found the match was much improved going that way without having to create a large 3d lut. It's really helps out if you are working with linear space source files in Photoshop.

It may be handy to specify that intermediate space to be placed in Add3GammaCurves()

So for example,
Source=Cineon
DST=Srgb_Film
Intermediate=video10

could be broken into 2 OCIO transformations
one transformation from Cineon to video10. (This transformation would be 1024 entries and placed into the 52nd byte of the Lut16 structure)
The next transformation would be the video10 to Srgb_Film, placed in the CLUT , at the end of the InputTables list.

It looks like that you are creating a gamma 1.0 table in that spot right now.

Let me know if that makes sense.

-Joseph

-----Original Message-----
From: ocio...@... [mailto:ocio...@...] On Behalf Of Malcolm Humphreys
Sent: Thursday, January 27, 2011 11:00 PM
To: ocio...@...
Subject: Re: [ocio-dev] Review: added ociobuildicc app

Also added Little CMS to LICENSE file

Commit:
https://github.com/malcolmhumphreys/OpenColorIO/commit/8ff296f8ab234ea7b06704036037bf2a132ebd6a

.malcolm

On 28/01/2011, at 5:49 PM, Malcolm Humphreys wrote:

Attaching some slightly larger examples :)


On 28/01/2011, at 5:46 PM, Malcolm Humphreys wrote:

Added ociobuildicc app which will build a soft-proofing icc profile for a given working space.
(added LCMS2 into ext)

Commits:
https://github.com/malcolmhumphreys/OpenColorIO/commit/1b0d39fb10db10
f4edbf1fb0fed124b548b836d9

Attached is an example of two profiles in action.

From the left
* sRGB reference image (ocio cpu)
* matte paint allocation space
* log space image.

The visual difference in the log image is most likely caused by 16bit
quantization errors in the CLUT of the AToB0Tag. These errors are
less with the matte paint allocation as the distance to travel is less.

I did play around with supporting the new MPE D2Bx and B2Dx tags
which should make this problem go away
(http://www.color.org/ICCSpecRevision_02_11_06_Float.pdf) but couldn't get it too work reliably so I pulled it out till I can.

.malcolm



<ocioicc_softp_off.jpeg><ocioicc_softp_on.jpeg>
<ocioicc_softp_off.jpeg><ocioicc_softp_on.jpeg>


Re: Pedantic tidy up of CMakeLists

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

Looks good to me. Committed.
-- Jeremy

On Sun, Jan 30, 2011 at 3:53 AM, dbr/Ben <b...@...> wrote:
Boring changes \o/

https://github.com/dbr/OpenColorIO/commit/f103709ec085bdaa03a3ea446d5b928d8327ffbc
Add Boost 1.44 and 1.45 to additional versions list (1.45 works fine, can't
imagine 1.44 is incompatible)
Move the find OpenGL/OIIO stuff into OCIOMacros. Probably belongs in a
Find___.cmake file
Try and find OIIO/OpenGL/etc, build applications if they are found
Components more consistently check if their requirements are met in the root
CMakeLists.txt, rather than wrapping the components CMakeList in an if()
statement. This makes it easy to see what components have requirements by
looking at one file
Tidier and hopefully more helpful messages regarding applications and their
requirements


Pedantic tidy up of CMakeLists

"dbr/Ben" <b...@...>
 

Boring changes \o/



Add Boost 1.44 and 1.45 to additional versions list (1.45 works fine, can't imagine 1.44 is incompatible)

Move the find OpenGL/OIIO stuff into OCIOMacros. Probably belongs in a Find___.cmake file

Try and find OIIO/OpenGL/etc, build applications if they are found

Components more consistently check if their requirements are met in the root CMakeLists.txt, rather than wrapping the components CMakeList in an if() statement. This makes it easy to see what components have requirements by looking at one file

Tidier and hopefully more helpful messages regarding applications and their requirements


Re: Review: added ociobuildicc app

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

Committed.

(Additional argument tweaks can come as needed later).

-- Jeremy


Re: Review: added ociobuildicc app

"dbr/Ben" <b...@...>
 

This is great :D

I generated a LUT without issue, and it was as close as I would expect from Photoshop...

Only nitpick'y problem I can see is the --outputfile argument seems to be redundant given the positional "outputprofile.icc" arg

On 28/01/2011, at 5:16 PM, Malcolm Humphreys wrote:

Added ociobuildicc app which will build a soft-proofing icc profile for a given working space.
(added LCMS2 into ext)

Commits:
https://github.com/malcolmhumphreys/OpenColorIO/commit/1b0d39fb10db10f4edbf1fb0fed124b548b836d9

Attached is an example of two profiles in action.

From the left
* sRGB reference image (ocio cpu)
* matte paint allocation space
* log space image.

The visual difference in the log image is most likely caused by 16bit quantization errors in the
CLUT of the AToB0Tag. These errors are less with the matte paint allocation as the distance
to travel is less.

I did play around with supporting the new MPE D2Bx and B2Dx tags which should make this
problem go away (http://www.color.org/ICCSpecRevision_02_11_06_Float.pdf) but couldn't
get it too work reliably so I pulled it out till I can.

.malcolm



<ocioicc_softp_off.jpeg><ocioicc_softp_on.jpeg>


Re: Review: added ociobuildicc app

Malcolm Humphreys <malcolmh...@...>
 

On 29/01/2011, at 5:17 AM, Jeremy Selan wrote:

Malcolm,

This is awesome! I've wanted to have icc export for so long, this is
hugely appreciated. (We'll use it at SPI immediately)

* What are the command-line arguments used in your example? I'd like
to give it a shot.
ociobuildicc --description="spi-vfx log test" --workingspace compositing_log --viewspace sRGB /Users/malcolm/Library/ColorSync/Profiles/spi-vfx_compositing_log_test.icc

* Would you consider renaming this app? ocio2icc, ocioicc, etc.
sure, it depends on how comfortable you are with the output as the profiles are modelling the transform:
scene_linear -> sRGB (Film) D65 -> Lab D50 (chromatic adaptation tag, takes it back to D65)

ocio2icc would be my vote

.malcolm


-- Jeremy


On Thu, Jan 27, 2011 at 10:59 PM, Malcolm Humphreys
<malcolmh...@...> wrote:
Also added Little CMS to LICENSE file

Commit:
https://github.com/malcolmhumphreys/OpenColorIO/commit/8ff296f8ab234ea7b06704036037bf2a132ebd6a

.malcolm

On 28/01/2011, at 5:49 PM, Malcolm Humphreys wrote:

Attaching some slightly larger examples :)


On 28/01/2011, at 5:46 PM, Malcolm Humphreys wrote:

Added ociobuildicc app which will build a soft-proofing icc profile for a given working space.
(added LCMS2 into ext)

Commits:
https://github.com/malcolmhumphreys/OpenColorIO/commit/1b0d39fb10db10f4edbf1fb0fed124b548b836d9

Attached is an example of two profiles in action.

From the left
* sRGB reference image (ocio cpu)
* matte paint allocation space
* log space image.

The visual difference in the log image is most likely caused by 16bit quantization errors in the
CLUT of the AToB0Tag. These errors are less with the matte paint allocation as the distance
to travel is less.

I did play around with supporting the new MPE D2Bx and B2Dx tags which should make this
problem go away (http://www.color.org/ICCSpecRevision_02_11_06_Float.pdf) but couldn't
get it too work reliably so I pulled it out till I can.

.malcolm



<ocioicc_softp_off.jpeg><ocioicc_softp_on.jpeg>
<ocioicc_softp_off.jpeg><ocioicc_softp_on.jpeg>


Re: Review: added ociobuildicc app

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

Malcolm,

This is awesome! I've wanted to have icc export for so long, this is
hugely appreciated. (We'll use it at SPI immediately)

* What are the command-line arguments used in your example? I'd like
to give it a shot.

* Would you consider renaming this app? ocio2icc, ocioicc, etc.

-- Jeremy


On Thu, Jan 27, 2011 at 10:59 PM, Malcolm Humphreys
<malcolmh...@...> wrote:
Also added Little CMS to LICENSE file

Commit:
https://github.com/malcolmhumphreys/OpenColorIO/commit/8ff296f8ab234ea7b06704036037bf2a132ebd6a

.malcolm

On 28/01/2011, at 5:49 PM, Malcolm Humphreys wrote:

Attaching some slightly larger examples :)


On 28/01/2011, at 5:46 PM, Malcolm Humphreys wrote:

Added ociobuildicc app which will build a soft-proofing icc profile for a given working space.
(added LCMS2 into ext)

Commits:
https://github.com/malcolmhumphreys/OpenColorIO/commit/1b0d39fb10db10f4edbf1fb0fed124b548b836d9

Attached is an example of two profiles in action.

From the left
* sRGB reference image (ocio cpu)
* matte paint allocation space
* log space image.

The visual difference in the log image is most likely caused by 16bit quantization errors in the
CLUT of the AToB0Tag. These errors are less with the matte paint allocation as the distance
to travel is less.

I did play around with supporting the new MPE D2Bx and B2Dx tags which should make this
problem go away (http://www.color.org/ICCSpecRevision_02_11_06_Float.pdf) but couldn't
get it too work reliably so I pulled it out till I can.

.malcolm



<ocioicc_softp_off.jpeg><ocioicc_softp_on.jpeg>
<ocioicc_softp_off.jpeg><ocioicc_softp_on.jpeg>


Re: Review: added ociobuildicc app

Malcolm Humphreys <malcolmh...@...>
 

On 28/01/2011, at 5:49 PM, Malcolm Humphreys wrote:

Attaching some slightly larger examples :)


On 28/01/2011, at 5:46 PM, Malcolm Humphreys wrote:

Added ociobuildicc app which will build a soft-proofing icc profile for a given working space.
(added LCMS2 into ext)

Commits:
https://github.com/malcolmhumphreys/OpenColorIO/commit/1b0d39fb10db10f4edbf1fb0fed124b548b836d9

Attached is an example of two profiles in action.

From the left
* sRGB reference image (ocio cpu)
* matte paint allocation space
* log space image.

The visual difference in the log image is most likely caused by 16bit quantization errors in the
CLUT of the AToB0Tag. These errors are less with the matte paint allocation as the distance
to travel is less.

I did play around with supporting the new MPE D2Bx and B2Dx tags which should make this
problem go away (http://www.color.org/ICCSpecRevision_02_11_06_Float.pdf) but couldn't
get it too work reliably so I pulled it out till I can.

.malcolm



<ocioicc_softp_off.jpeg><ocioicc_softp_on.jpeg>
<ocioicc_softp_off.jpeg><ocioicc_softp_on.jpeg>


Re: Review: added ociobuildicc app

Malcolm Humphreys <malcolmh...@...>
 

Attaching some slightly larger examples :)

On 28/01/2011, at 5:46 PM, Malcolm Humphreys wrote:

Added ociobuildicc app which will build a soft-proofing icc profile for a given working space.
(added LCMS2 into ext)

Commits:
https://github.com/malcolmhumphreys/OpenColorIO/commit/1b0d39fb10db10f4edbf1fb0fed124b548b836d9

Attached is an example of two profiles in action.

From the left
* sRGB reference image (ocio cpu)
* matte paint allocation space
* log space image.

The visual difference in the log image is most likely caused by 16bit quantization errors in the
CLUT of the AToB0Tag. These errors are less with the matte paint allocation as the distance
to travel is less.

I did play around with supporting the new MPE D2Bx and B2Dx tags which should make this
problem go away (http://www.color.org/ICCSpecRevision_02_11_06_Float.pdf) but couldn't
get it too work reliably so I pulled it out till I can.

.malcolm



<ocioicc_softp_off.jpeg><ocioicc_softp_on.jpeg>


Review: added ociobuildicc app

Malcolm Humphreys <malcolmh...@...>
 

Added ociobuildicc app which will build a soft-proofing icc profile for a given working space.
(added LCMS2 into ext)

Commits:
https://github.com/malcolmhumphreys/OpenColorIO/commit/1b0d39fb10db10f4edbf1fb0fed124b548b836d9

Attached is an example of two profiles in action.

From the left
* sRGB reference image (ocio cpu)
* matte paint allocation space
* log space image.

The visual difference in the log image is most likely caused by 16bit quantization errors in the
CLUT of the AToB0Tag. These errors are less with the matte paint allocation as the distance
to travel is less.

I did play around with supporting the new MPE D2Bx and B2Dx tags which should make this
problem go away (http://www.color.org/ICCSpecRevision_02_11_06_Float.pdf) but couldn't
get it too work reliably so I pulled it out till I can.

.malcolm


Re: Review: FileTransform supports .cc and .ccc files

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

LGTM, committed.
Thanks!

-- Jeremy

On Thu, Jan 27, 2011 at 2:52 AM, dbr/Ben <b...@...> wrote:
Oh,
https://github.com/dbr/OpenColorIO/commit/0247f16ae202eeb5316f695c1310dac523272a0e
On 23/01/2011, at 4:41 AM, Jeremy Selan wrote:

Nice job on the docs.  By all means, check them in!  (Even partial
docs are better than nothing). :)

I wonder, could the cccid's be presented in a drop-down, similar to the

colour-space listings?

I like this, but I'm not sure that there's a nuke UI widget rich
enough for this behavior.   What we want is a popdown that lets you
choose from a bunch of pre-defined options, but also lets you type in
new values (such as envvars).  Im not sure that nuke popdowns can
allow text entry capability.  The other difficulty is that in nuke if
you use the popdowns, the underlying value is an int index rather than
a string.  This sucks because if you were to update the array, change
the underlying value, etc, the only precious data for nuke is what
order it was in the list. (Whereas in our case the value is precious).
So I think it's safer to keep it a string for now.   But... what if
we added a button that when pressed provided a popup for the user?
(Calling out to OCIO to enumerate the options).  That would make it
convenient + keep data value 'native'.

I like the use of the invisible flag. I'll investigate setting it up.

-- Jeremy


Re: Review: FileTransform supports .cc and .ccc files

"dbr/Ben" <b...@...>
 

On 23/01/2011, at 4:41 AM, Jeremy Selan wrote:

Nice job on the docs.  By all means, check them in!  (Even partial
docs are better than nothing). :)

I wonder, could the cccid's be presented in a drop-down, similar to the
colour-space listings?

I like this, but I'm not sure that there's a nuke UI widget rich
enough for this behavior.   What we want is a popdown that lets you
choose from a bunch of pre-defined options, but also lets you type in
new values (such as envvars).  Im not sure that nuke popdowns can
allow text entry capability.  The other difficulty is that in nuke if
you use the popdowns, the underlying value is an int index rather than
a string.  This sucks because if you were to update the array, change
the underlying value, etc, the only precious data for nuke is what
order it was in the list. (Whereas in our case the value is precious).
So I think it's safer to keep it a string for now.   But... what if
we added a button that when pressed provided a popup for the user?
(Calling out to OCIO to enumerate the options).  That would make it
convenient + keep data value 'native'.

I like the use of the invisible flag. I'll investigate setting it up.

-- Jeremy


Re: Review: Replaced config->getRoleNameByIndex

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

LGTM - committed.

-- Jeremy

On Wed, Jan 26, 2011 at 10:34 PM, Malcolm Humphreys
<malcolmh...@...> wrote:
Replaced config->getRoleNameByIndex() with config->getRoleName()
 - getRoleNameByIndex returned the colorspace name not the role name, also ..ByIndex seemed redundant in the signature.

Added hasRole(role) convenience function

Commit:
https://github.com/malcolmhumphreys/OpenColorIO/commit/4825ef1c7d105378ef5ebd41609f5d3b49c2a6ac

.malcolm

On 19/01/2011, at 11:12 AM, Malcolm Humphreys wrote:


Thanks!

I like the intent, but have two minor issues:

First, the trivial:  I'd prefer to never allow NULL values as return
types from const char * functions. Please use an empty string ""
instead.

The reason being, OCIO expects the client to copy the returned string
value, if the client intends to hang onto it. So it's often convenient
in your code to do a directly assign the function result to a
std::string,

std::string rolename = config->getRoleNameByIndex(i);

If the API promises to never return a NULL value, this will work
great. However, if OCIO can return a null pointer than the above code
will crash.  Thus for all char * return functions the user would be
required to explicitly add a null check.   This is probably counter to
user expectations, and I expect that a few would slip through and
result in occasionally crashy client code.  (This would be likely
because most of the time it *would* work fine.  Only on invalid
configurations, or other corner cases, would this sharp corner be run
into.  I'd like to keep OCIO as friendly as reasonable.)

(See http://github.com/imageworks/OpenColorIO/issues#issue/20 for more
thoughts on this topic).
Sure that sounds great, i'll fix that.

The second issue is that you've exposed two helper functions
(getRoleColorSpaceName), that are not quite as helpful as they appear.
Is the client supposed to use these at runtime to determine which
colorspace to use for a given role?

Allow me to answer...  "No!"

:)

ConstColorSpaceRcPtr getColorSpace(const char * name) const;
already does this in the 'correct' way, and this should be the *only*
allowed mechanism for getting a ColorSpace from a name.   It
encapsulates a bit of extra mojo your new function does not, such as
looking at the 'strictParsing', and if allowed returning the DEFAULT
colorspace role in fall-through situations. It also resolves the
ambiguity where a role name and colorspace name overlap, and having
this resolution only defined in a single place seems preferable.

What if we renamed your functions to make it super clear that they are
not intended for color-space resolution, but instead as configuration
setup/ui purposes?

I.e.
setRole(const char * role, const char * colorSpaceName);
const char * getRoleName(int index);
const char * getRoleValue(int index);
I don't remind removing them altogether.

I was really just adding them to match the old functionality of getRoleNameByIndex() which returned the colorspace name not the role name, I wasn't sure if you were needing the colorspace name.

Calling getColorSpace(getRoleName(0)) seems nice an clean to me.

.malcolm

Even though the role 'value' is a colorspace name, I think the
function would be far less tempting for the naive API user to
accidentally call.

If we'd like a convenience function to do the resolution, we could add
      const char * getColorSpaceName(const char * name) const;

which would internally rely on config::getColorSpace.

-- Jeremy

On Sun, Jan 16, 2011 at 8:22 PM, Malcolm Humphreys
<malcolmh...@...> wrote:
When building some UI menus, I wanted to get a list of defined roles. Config::getRoleNameByIndex() only returned the resolved colorspace name.

--snip--
Replaced config->getRoleNameByIndex (which returned the colorspace name, not the role name) with the following:

- getRoleName(int): get the role name at index, this will return values like 'scene_linear', 'compositing_log'. Returns NULL if index is out of range

- getRoleColorSpaceName(int): get the actual colorspace name at role index. Returns NULL when index is out of range

- getRoleColorSpaceName(char*): get the actual colorspace name from a role name. Returns NULL if roles is not defined
--snip--

https://github.com/malcolmhumphreys/OpenColorIO/commit/d7c8094f6aad57f980060e87d0f547ff36d844b7

.malcolm


Re: Review: DisplayTransform interface update

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

I think that's a perfect description of all the terminology. I'm
thinking a glossary section (front page of webpage?, inside the FAQ?)
would be appropriate.

I'm thinking of naming the class that handles looks,
LookModificationTransform (LMT). This is the terminology the Academy
/ IIF is using, and it would be a nice fit to use the same terminology
inside OpenColorIO.

-- Jeremy

On Wed, Jan 26, 2011 at 11:01 PM, Malcolm Humphreys
<malcolmh...@...> wrote:
I would think looks would be something separate from views.
Terminology we currently have:
- Transform - functions that transform RGBA data
- ReferenceSpace - a space that connects ColorSpaces
- ColorSpace - a meaningful space that can be transferred to and from the
reference space
- Display - a virtual or physical display device
- View - a meaningful view of the reference space on a Display
- Role - abstract colorspace naming
I would say a Look is a meaningful group of Transforms that can be applied
between the reference space and other transformations to create consistent
creative styles.
.malcolm
On 26/01/2011, at 12:00 PM, Joseph Slomka wrote:

Would the 'view' terminology  be able to encompass your concept of looks? Id
it does then I'm onboard
-Joseph

-----Original Message-----
From: ocio...@... [mailto:ocio...@...] On Behalf
Of Jeremy Selan
Sent: Tuesday, January 25, 2011 4:54 PM
To: ocio...@...
Subject: Re: [ocio-dev] Review: DisplayTransform interface update

Oh, and have we agreed to use 'View' rather than 'Alias'?  I'm cool with
that...

-- Jeremy

On Tue, Jan 25, 2011 at 4:51 PM, Jeremy Selan <jeremy...@...>
wrote:

Would this syntax preserve Display order?  I think that's important.

-- Jeremy

displays:

  sRGB:

     -!<View> {name: Raw, colorspace: nc10}

     -!<View> {name: Log, colorspace: lg10}

     -!<View> {name: Film, colorspace: srgb8}

  DCIP3:

     -!<View> {name: Raw, colorspace: nc10}

     -!<View> {name: Log, colorspace: lg10}

     -!<View> {name: Film, colorspace: p3dci8}





Re: Review: DisplayTransform interface update

Malcolm Humphreys <malcolmh...@...>
 

I would think looks would be something separate from views.

Terminology we currently have:
- Transform - functions that transform RGBA data
- ReferenceSpace - a space that connects ColorSpaces
- ColorSpace - a meaningful space that can be transferred to and from the reference space
- Display - a virtual or physical display device
- View - a meaningful view of the reference space on a Display
- Role - abstract colorspace naming

I would say a Look is a meaningful group of Transforms that can be applied between the reference space and other transformations to create consistent creative styles.

.malcolm

On 26/01/2011, at 12:00 PM, Joseph Slomka wrote:

Would the 'view' terminology  be able to encompass your concept of looks? Id it does then I'm onboard
-Joseph

-----Original Message-----
From: ocio...@... [mailto:ocio...@...] On Behalf Of Jeremy Selan
Sent: Tuesday, January 25, 2011 4:54 PM
To: ocio...@...
Subject: Re: [ocio-dev] Review: DisplayTransform interface update

Oh, and have we agreed to use 'View' rather than 'Alias'?  I'm cool with that...

-- Jeremy

On Tue, Jan 25, 2011 at 4:51 PM, Jeremy Selan <jeremy...@...> wrote:
Would this syntax preserve Display order?  I think that's important.

-- Jeremy

displays:
  sRGB:
     -!<View> {name: Raw, colorspace: nc10}
     -!<View> {name: Log, colorspace: lg10}
     -!<View> {name: Film, colorspace: srgb8}
  DCIP3:
     -!<View> {name: Raw, colorspace: nc10}
     -!<View> {name: Log, colorspace: lg10}
     -!<View> {name: Film, colorspace: p3dci8}





Re: Review: Replaced config->getRoleNameByIndex

Malcolm Humphreys <malcolmh...@...>
 

Replaced config->getRoleNameByIndex() with config->getRoleName()
- getRoleNameByIndex returned the colorspace name not the role name, also ..ByIndex seemed redundant in the signature.

Added hasRole(role) convenience function

Commit:
https://github.com/malcolmhumphreys/OpenColorIO/commit/4825ef1c7d105378ef5ebd41609f5d3b49c2a6ac

.malcolm

On 19/01/2011, at 11:12 AM, Malcolm Humphreys wrote:


Thanks!

I like the intent, but have two minor issues:

First, the trivial: I'd prefer to never allow NULL values as return
types from const char * functions. Please use an empty string ""
instead.

The reason being, OCIO expects the client to copy the returned string
value, if the client intends to hang onto it. So it's often convenient
in your code to do a directly assign the function result to a
std::string,

std::string rolename = config->getRoleNameByIndex(i);

If the API promises to never return a NULL value, this will work
great. However, if OCIO can return a null pointer than the above code
will crash. Thus for all char * return functions the user would be
required to explicitly add a null check. This is probably counter to
user expectations, and I expect that a few would slip through and
result in occasionally crashy client code. (This would be likely
because most of the time it *would* work fine. Only on invalid
configurations, or other corner cases, would this sharp corner be run
into. I'd like to keep OCIO as friendly as reasonable.)

(See http://github.com/imageworks/OpenColorIO/issues#issue/20 for more
thoughts on this topic).
Sure that sounds great, i'll fix that.

The second issue is that you've exposed two helper functions
(getRoleColorSpaceName), that are not quite as helpful as they appear.
Is the client supposed to use these at runtime to determine which
colorspace to use for a given role?

Allow me to answer... "No!"

:)

ConstColorSpaceRcPtr getColorSpace(const char * name) const;
already does this in the 'correct' way, and this should be the *only*
allowed mechanism for getting a ColorSpace from a name. It
encapsulates a bit of extra mojo your new function does not, such as
looking at the 'strictParsing', and if allowed returning the DEFAULT
colorspace role in fall-through situations. It also resolves the
ambiguity where a role name and colorspace name overlap, and having
this resolution only defined in a single place seems preferable.

What if we renamed your functions to make it super clear that they are
not intended for color-space resolution, but instead as configuration
setup/ui purposes?

I.e.
setRole(const char * role, const char * colorSpaceName);
const char * getRoleName(int index);
const char * getRoleValue(int index);
I don't remind removing them altogether.

I was really just adding them to match the old functionality of getRoleNameByIndex() which returned the colorspace name not the role name, I wasn't sure if you were needing the colorspace name.

Calling getColorSpace(getRoleName(0)) seems nice an clean to me.

.malcolm

Even though the role 'value' is a colorspace name, I think the
function would be far less tempting for the naive API user to
accidentally call.

If we'd like a convenience function to do the resolution, we could add
const char * getColorSpaceName(const char * name) const;

which would internally rely on config::getColorSpace.

-- Jeremy

On Sun, Jan 16, 2011 at 8:22 PM, Malcolm Humphreys
<malcolmh...@...> wrote:
When building some UI menus, I wanted to get a list of defined roles. Config::getRoleNameByIndex() only returned the resolved colorspace name.

--snip--
Replaced config->getRoleNameByIndex (which returned the colorspace name, not the role name) with the following:

- getRoleName(int): get the role name at index, this will return values like 'scene_linear', 'compositing_log'. Returns NULL if index is out of range

- getRoleColorSpaceName(int): get the actual colorspace name at role index. Returns NULL when index is out of range

- getRoleColorSpaceName(char*): get the actual colorspace name from a role name. Returns NULL if roles is not defined
--snip--

https://github.com/malcolmhumphreys/OpenColorIO/commit/d7c8094f6aad57f980060e87d0f547ff36d844b7

.malcolm

1801 - 1820 of 2226