Review: Config.roles data storage is map
Jeremy Selan <jeremy...@...>
The list of defined roles within a config previously maintained order.
Conceptually, this is actually a mapping type where order does not matter (and the yaml serialization dropped ordering). This switches the underlying storage to use a map instead. Commits: 3942e6d8bd72b721b23d3e9991e434f2537a2759
|
|
Review: EnvExpand optimization
Jeremy Selan <jeremy...@...>
Minor optimization: the potentially expensive EnvExpand call (which
expands envvars in file paths), does an early exit for paths which do not contain special characters. ($ + %) Commits: 38d53d46239ec3f7f090509f2273edbedd703d7e -- Jeremy
|
|
Re: Review: CSP bugfix
Jeremy Selan <jeremy...@...>
In the absence of any objections, I will be merging this (and the
previous commits) into OCIO. -- Jeremy
|
|
Re: Review: Inital pass at searchpath impl
Jeremy Selan <jeremy...@...>
Malcolm, I like your suggestions.
My thoughts: * The name 'globals' is good. * I like pre-declaring the list of allowed variables beforehand for the config. * I like the use of env-vars as defaults. This seems very user-friendly. * I like supporting only strings for now. In the future, I can also see numbers being supported (such as allowing for CDL blocks that rely on envvars), but this can be added as needed. * The FileTransform will probably have to get a bit smarter. Say you're using a pershot lut, and for some shots you dont want any such table. This could be solved by either not installing a lut in the expected location, or by installing an identity lut. Which is preferable? In the current implementation, if a lut is not found this is always an error condition. Maybe we should introduce on the FilmTransform a 'silent error mode', where this would just skip the application of that specific LUT? * The "globals" pre-declaration could also define an optional default (used if the envvar is not set). If the envvar is not set, and no default is specified, any references to the global variable would probably be an error. * I have some apprehensions about the config->getGlobal, config->setGlobal implementation. My primary concern is that the GlobalConfig() is const (by design), so clients couldnt actually call setGlobals on the config in use. Instead, they would have to do something like: c = GetGlobalConfig().createEditableCopy() c.setGlobals(...) SetGlobalConfig( c ) This is a lot of copying, particularly as the globals often are tied to the clip. If you're A/B-ing two clips in an image viewer, conceptually there really is only one config, and two sets of globals. Why force the client to copy and have duplicate configs? I will mockup some ideas which solve this and see what the possible solutions look like. -- Jeremy
|
|
Review: CSP bugfix
Jeremy Selan <jeremy...@...>
Upon further testing, it appears the channel swizzling was backwards
during CSP lut importing. I used Nuke as the baseline in terms of lut channel order, so it's possible that I've actually broken things if Nuke's CSP Vector node is broken. But if Nuke's works, OCIO's now agrees with the results. Can anyone confirm Nuke's Vectorfield implementation of .csp files works? I am including the file I used for testing. This gains down the red channel by 0.5 - green and blue are unchanged. Commits: d483d6a3660cec5cafa6868294f3cb62c05f4472 I've confirmed that our 3dl handing is correct. -- Jeremy
|
|
Review: Added two additional Nuke OCIO nodes
Jeremy Selan <jeremy...@...>
* LogConvert, which is a handy node that does the colorspace transform
between the SCENE_LINEAR role, and the COMPOSITING_LOG role. While this isnt strictly needed (it could be accomplished using the ColorSpace node) it's convenient for the artists to have so they dont have to worry about all the options. They can just think, "give me the right log to lin". * FileTransform, which allows for the application of individual files (i.e., luts). While not needed in general use, this will let users apply any lut that OCIO knows about, independent of the normal OCIO workflow. Commits: 27371ec44b878ae787326c89405400ee22dfa1c8 745a7c495b4b3e5ab3faab17e5a40dbb3325d4f0 -- Jeremy
|
|
Review: Switched auto_ptr -> ptr
Jeremy Selan <jeremy...@...>
In the public API we were storing references to m_impl using autoptrs,
which really was unnecessary. This only saved one line of code per use, and made the memory model less obvious. This doesnt affect user code at all, as it's impl related anyways. Commits: 089aaa9297dfedffa59c2998bf4111cd65d53c5d -- Jeremy
|
|
Review: Disable envvar unit tests
Jeremy Selan <jeremy...@...>
Unit tests for envvars were failing, referencing files that weren't
checked into the source tree. Disabling these specific tests for now. Commits: 0a052d9090c9d693f63cdb409260627406790d75 -- Jeremy
|
|
Re: Idea: Remove boost:shared_ptr
Jeremy Selan <jeremy...@...>
Definitely,
I went to add tr1 on mac, and realized we're already doing it. From export/OpenColorIO/OpenColorTypes.h, #ifdef __APPLE__ #include <tr1/memory> #define OCIO_SHARED_PTR std::tr1::shared_ptr #define OCIO_DYNAMIC_POINTER_CAST std::tr1::dynamic_pointer_cast #else #include <boost/shared_ptr.hpp> #define OCIO_SHARED_PTR boost::shared_ptr #define OCIO_DYNAMIC_POINTER_CAST boost::dynamic_pointer_cast #endif So I'll have to see why this didnt work on OSX without boost. Looks like it all should have worked... -- Jeremy
|
|
Re: Idea: Remove boost:shared_ptr
Colin Doncaster <colin.d...@...>
Would
toggle quoted messageShow quoted text
#include <tr1/memory> be an option for now?
On 03-11-2010, at 6:00 AM, Malcolm Humphreys wrote:
+1 it should be pretty easy to add boost back as a dependency if we need to. I have seen a lot of projects copy the shared_ptr so it seems fairly common practice.
|
|
Re: Review: Inital pass at searchpath impl
Malcolm Humphreys <malcolmh...@...>
Hi,
It might have to wait a few weeks, I'm in Hong Kong right now and I'm going to be MIA in asia for a bit. But yes a chat would be good when we are both free.
They are a common mechanism for setting up initial state for a lot of apps. This being similar is a nice entry point for integration with existing systems. But I agree the env vars should only be used to setup the initial state. Even if it is possible to change this state with setenv()'s I would discourage this use with a better interface for changing state during execution, similar to what you have suggested. Most places have some form of setshot system which bootstraps the env for a shot, not just for color but things like the required toolchain for that artist session doing task A (I'm guessing you have something similar at SPI). I can guarantee that I would need to write something that grabbed env's and setup an initial state through which ever interface we settle on. I would prefer that this ocio env wrapper type code was internal rather than each shop doing it slightly differently.
+1 Being able to setup/enfoce the initial state like this is important for multiple dept / group workflows. Who are most likely using diffrent toolchains but require to start with the same consistent viewing env between them. In this style of setup each person gets pushed the currently approved viewing env for that session and are not concerned with modification of it.
Neither do I, that would be a bit clumsy for that specific problem.
I think a mixture of the two ideas could work well (see below).
I was thinking the same kind of thing, but with the possibility of these grabbing their initial value from the env (if it exists). I'm also thinking it might be better to be explicit with globals eg. config->addglobal(name) these would end up as a list in the yaml profile. Only the $globals in this list would be expanded and also the only ones initialized from the env. (This does resolve my niggles with the current expand code which has the potential to be a bit slow, if used in many) These globals could be set/get dynamically at run-time ie config->getglobal(), config->setglobal(). I would prefer to call these globals over variables as it suggests their scope. Right now I only see a direct use for globals with string parameters and would only want to add support for other types if we had a strong need for them. As an example this would be nearly a complete list of globals I could see a use for ($JOB, $SCENE, $SEQ, $SHOT, $TASK, $USERGROUP, $BALLANCE_GRADE, $CREATIVE_GRADE, $EDIT_GRADE). I'm sure these have slightly different names per shop but represent the same concepts.
I definitely would like some form of dynamic parameters/variables, but these should be something different to globals.
With the prospect of multiple image sequences / clips with transforms, configs and or grades. For these case are you proposing that we setup a single config and then store some kind of ocio processor relationship per clip?
It would be great to flesh this out as it would be good for client apps not to need to manage the details of this per clip state storage if this is the way we are thinking of going. .malcolm
|
|
Re: Idea: Remove boost:shared_ptr
Malcolm Humphreys <malcolmh...@...>
+1 it should be pretty easy to add boost back as a dependency if we need
to. I have seen a lot of projects copy the shared_ptr so it seems fairly common practice. .malcolm
On 03 Nov, 2010,at 09:29 AM, Jeremy Selan <jere...@...> wrote:
|
|
Re: Review: Added missing role enum to python
Malcolm Humphreys <malcolmh...@...>
LGTM
On 03 Nov, 2010,at 09:26 AM, Jeremy Selan <jere...@...> wrote:
|
|
Re: Idea: Remove boost:shared_ptr
Malcolm Humphreys <malcolmh...@...>
+1 it should be pretty easy to add boost back as a dependency if we need to. I have seen a lot of projects copy the shared_ptr so it seems fairly common practice. .malcolm
On 03 Nov, 2010,at 09:29 AM, Jeremy Selan <jere...@...> wrote:
|
|
Re: Review: Added missing role enum to python
Malcolm Humphreys <malcolmh...@...>
LGTM
On 03 Nov, 2010,at 09:26 AM, Jeremy Selan <jere...@...> wrote:
|
|
Review: Nuke to OCIO exporter
Jeremy Selan <jeremy...@...>
I've added a nuke python script to src/testbed that lets you export
specified nuke color spaces into an OCIO profile. It also is a good example of using the python API to create an OCIO profile. commit: b9d48f946b4c39d5334055e089d024d322561812 There is one major 'gotcha' I discovered in writing this script, and it relates to the python version used at build time. As this script requires the OCIO module to be imported into the Nuke python namespace, the PyOpenColorIO module has to be compiled with the same version that Nuke uses internally (which for current Nukes appears to be python 2.5). However, the native compilation uses whichever python happens to be the system install (which in my case is 2.6). Unfortunately, running a 2.6 python .so in 2.5 does not work in subtle ways. This implies that a system level install, for use in a 3rd party app (such as nuke) may not be appropriate. Should building OCIO for a 3rd party plugin rebuild all .sos with app-specific settings? Seems like it may need to. -- Jeremy
|
|
New 0.7 Profile posted
Jeremy Selan <jeremy...@...>
A new profile, 'nuke-default' has been added to the 0.7.1 config bundle.
http://sites.google.com/site/opencolorio/profiles This profile emulates the 'out of the box' color processing luts that come with nuke 6.1 (should you choose to use them). I've also checked in the nuke script which is used to generate this profile, which will allow for the generalized OCIO export of working Nuke color configurations. This is exciting because for shops which have configured custom Nuke color processing, you can export an OCIO profile for use in other apps. -- Jeremy
|
|
Re: ocio_core_tests linking issue on Ubuntu 10.10
Andrew Hunter <and...@...>
Hey Jeremy,
On Tue, Nov 2, 2010 at 6:23 PM, Jeremy Selan <jeremy...@...> wrote: I haven't seen that issue before, but it looks like it's related toThe build completes successfully commenting out that line. If things are as Est mentioned, then I think this is a good idea. It would be preferable to use the distribution's supplied boost rather than having to build it locally. Cheers! Andrew
|
|
Re: ocio_core_tests linking issue on Ubuntu 10.10
Est <rame...@...>
I had the same problem. It happens when you link to the
toggle quoted messageShow quoted text
dynamic version of boost's test libraries. You need to build / install boost test as static libraries and point CMake to them. Est.
On Nov 2, 11:23 pm, Jeremy Selan <jeremy...@...> wrote:
I haven't seen that issue before, but it looks like it's related to
|
|
Re: Idea: Remove boost:shared_ptr
Larry Gritz <l...@...>
shared_ptr is part of C++0x (in TR1, I think).
So another approach is to rig it so that sufficiently new compilers don't need the boost dependency, and only the old compilers will need to pull it in to get the shared_ptr. Just a thought. On Nov 2, 2010, at 3:29 PM, Jeremy Selan wrote: Would anyone be against us copying the shared_ptr implementation-- Larry Gritz l...@...
|
|