ocio-dev@lists.aswf.io

OCIO 0.7.0 posted

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

#191

The first release based on YAML is now ready for download. Thanks to
Malcolm for his help on this.

An updated set of 0.7-format color configs (with many more color
spaces!) will be posted tomorrow.

Version 0.7.0 (Oct 21 2010):
    * Switched file format from XML to Yaml

-- Jeremy

Re: Review: Additional Yaml bugfixes

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

#194

All LGTM, sorry most of these were introduced by me in the switch

Show quoted text

On 21/10/2010, at 2:39 PM, Jeremy Selan wrote:

These commits fix a bunch of bugs we discovered when testing our
in-house apps on real production color configurations.

* Yaml serialization fix, resource_path is updated properly
* Relative path loading works again (for FileTransform)
* Yaml output doesn't include default transform direction. (need to
generalize this not to emit default values in general)
* Fixed processing bug related to handling of null transforms
* Yaml bugfix, previously color spaces which defined the transform
"from_reference" were hosed.

js/master

8e6e90b822f21e02b055e9ef9a95e78ac88dc15f
8046c604171470c3a3b8dd31fc51a69d127371a4
e24312c91420a92158658f418e05032ad934d97f
f381144da667b388da86a8492ce28ca7e19a1a2a
c4a68d76662fb74676ac6e94b3f0abcd24c68de7
4a0063ce32c35cffb13bab77eef024971a615266

-- Jeremy

Re: Review: renamed config.writeToStream to config.serialize

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

#193

Yep, I was thinking about doing that but it's not straight forward if we don't want yaml-cpp in the public interfaces for the Transforms()s.

It could also be possible if we have some form of Transform()s parameter types / storage. So something in the core lib can just serialize these params into this ostream. This could pave the way for having Transform plugins which can be serialize themselves with out knowing about yaml-cpp.

.malcolm

Show quoted text

On 21/10/2010, at 2:30 PM, Jeremy Selan wrote:

Thinking about this topic further, I think the right thing to do it to
have __str__ call through to the C++ ostream implementation for each
class.  (If that happens to use yaml, great! But it's not a problem if
it doesn't either).

-- Jeremy


Did you see my comment in one of the threads about also supporting the .__str__() method?

Review: Additional Yaml bugfixes

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

#188

These commits fix a bunch of bugs we discovered when testing our
in-house apps on real production color configurations.

* Yaml serialization fix, resource_path is updated properly
* Relative path loading works again (for FileTransform)
* Yaml output doesn't include default transform direction. (need to
generalize this not to emit default values in general)
* Fixed processing bug related to handling of null transforms
* Yaml bugfix, previously color spaces which defined the transform
"from_reference" were hosed.

js/master

8e6e90b822f21e02b055e9ef9a95e78ac88dc15f
8046c604171470c3a3b8dd31fc51a69d127371a4
e24312c91420a92158658f418e05032ad934d97f
f381144da667b388da86a8492ce28ca7e19a1a2a
c4a68d76662fb74676ac6e94b3f0abcd24c68de7
4a0063ce32c35cffb13bab77eef024971a615266

-- Jeremy

Re: Review: renamed config.writeToStream to config.serialize

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

#187

Thinking about this topic further, I think the right thing to do it to
have __str__ call through to the C++ ostream implementation for each
class.  (If that happens to use yaml, great! But it's not a problem if
it doesn't either).

-- Jeremy

Show quoted text

Did you see my comment in one of the threads about also supporting the .__str__() method?

Re: Review: Misc Yaml Updates

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

#190

Looking at this quickly it would be a pretty small patch to add out << YAML::NewLines(3);

Something along the lines of:

--emittermanip.h--
	struct _NewLines {
		_NewLines(int value_): value(value_) {}
		int value;
	};
	
	inline _NewLines NewLines(int value) {
		return _NewLines(value);
	}
--emittermanip.h--

-- emitter.h --
...
Emitter& Write(const _NewLines& lines);
...
inline Emitter& operator << (Emitter& emitter, const _NewLines& v) { return emitter.Write(v); }
...
-- emitter.h --

-- emmiter.cpp --
	Emitter& Emitter::Write(const _NewLines& lines)
	{
		if(!good())
			return *this;
		for(unsigned int i=0; i< lines.value; ++i)
			m_stream << "\n";
		return *this;
	}
-- emmiter.cpp --

.malcolm

Show quoted text

On 21/10/2010, at 11:24 AM, Malcolm Humphreys wrote:

This looks all pretty good to me

A bunch of changes related to yaml serialization:

* ColorSpace transform a base Transform, instead of GroupTransform.
* GroupTransform Yaml serialization supports nesting
* Direction is now serialized for all yaml elements- Can we call AddBaseTransformPropertiesToYAMLMap = EmitBaseTransformKeyValues
- Can we call ReadBaseTransformPropertiesFromYAMLMap = ReadBaseTransformKeyValues, and can the argument order be swaped to match the other >> operators ie. (TransformRcPtr & t, const YAML::Node & node) -> (const YAML::Node & node, TransformRcPtr &)

* Roles survive a yaml round trip+1 I like this being a map, I'm not sure if order is important.

* Most transforms serialize in Flow form for better readability

Commits: js/master

5f53929774b53ebe6dd9f626e320f70a19251592
a417123d6ec39a5df8add746c3047685be7b8553
dc66f63326b7766c70d8bafd392c826a3a8b37d3
8ec63860a00ebf6301d249270ff43f7dadf8f568
a788950e36d107ec07adb1e2daaf8b399fe90936

I am not totally happy with the look of the Yaml output.  Ideally, I'd
like to add blank lines after each colorspace definition (this is
valid yaml), but havent been able to figure out how do to so with
yaml-cpp.Yeah most of our yamls are hand written with big breaks between sections, so this hasn't really been an issue.

One possible way would be using YAML::Comment with some new line chars, but it would be much nicer to have something like out << YAML::NewLines(3) that you could use.

void MultiLineComment(YAML::Emitter& out, std::string& desiredOutput)
{
	out << YAML::BeginSeq;
	out << "item 1" << YAML::Comment("really really long\ncomment that couldn't possibly\nfit on one line");
	out << "item 2";
	out << YAML::EndSeq;
	desiredOutput = "---\n- item 1  # really really long\n          # comment that couldn't possibly\n          # fit on one line\n- item 2";
}

.malcolm

Re: Review: Misc Yaml Updates

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

#189

This looks all pretty good to me

A bunch of changes related to yaml serialization:

* ColorSpace transform a base Transform, instead of GroupTransform.
* GroupTransform Yaml serialization supports nesting
* Direction is now serialized for all yaml elements- Can we call AddBaseTransformPropertiesToYAMLMap = EmitBaseTransformKeyValues
- Can we call ReadBaseTransformPropertiesFromYAMLMap = ReadBaseTransformKeyValues, and can the argument order be swaped to match the other >> operators ie. (TransformRcPtr & t, const YAML::Node & node) -> (const YAML::Node & node, TransformRcPtr &)

* Roles survive a yaml round trip+1 I like this being a map, I'm not sure if order is important.

* Most transforms serialize in Flow form for better readability

Commits: js/master

5f53929774b53ebe6dd9f626e320f70a19251592
a417123d6ec39a5df8add746c3047685be7b8553
dc66f63326b7766c70d8bafd392c826a3a8b37d3
8ec63860a00ebf6301d249270ff43f7dadf8f568
a788950e36d107ec07adb1e2daaf8b399fe90936

I am not totally happy with the look of the Yaml output.  Ideally, I'd
like to add blank lines after each colorspace definition (this is
valid yaml), but havent been able to figure out how do to so with
yaml-cpp.Yeah most of our yamls are hand written with big breaks between sections, so this hasn't really been an issue.

One possible way would be using YAML::Comment with some new line chars, but it would be much nicer to have something like out << YAML::NewLines(3) that you could use.

void MultiLineComment(YAML::Emitter& out, std::string& desiredOutput)
{
	out << YAML::BeginSeq;
	out << "item 1" << YAML::Comment("really really long\ncomment that couldn't possibly\nfit on one line");
	out << "item 2";
	out << YAML::EndSeq;
	desiredOutput = "---\n- item 1  # really really long\n          # comment that couldn't possibly\n          # fit on one line\n- item 2";
}

.malcolm

Review: Misc Yaml Updates

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

#186

A bunch of changes related to yaml serialization:

* ColorSpace transform a base Transform, instead of GroupTransform.
* GroupTransform Yaml serialization supports nesting
* Direction is now serialized for all yaml elements
* Roles survive a yaml round trip
* Most transforms serialize in Flow form for better readability

Commits: js/master

5f53929774b53ebe6dd9f626e320f70a19251592
a417123d6ec39a5df8add746c3047685be7b8553
dc66f63326b7766c70d8bafd392c826a3a8b37d3
8ec63860a00ebf6301d249270ff43f7dadf8f568
a788950e36d107ec07adb1e2daaf8b399fe90936

I am not totally happy with the look of the Yaml output.  Ideally, I'd
like to add blank lines after each colorspace definition (this is
valid yaml), but havent been able to figure out how do to so with
yaml-cpp.

http://stackoverflow.com/questions/3982901/how-to-emit-a-blank-line-using-yaml-cpp

-- Jeremy

Re: Review: renamed config.writeToStream to config.serialize

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

#184

I did see that comment, forgot to address it in the last post.

I'd prefer to not expose the exact yaml serialization using __str__().

I'm pretty biased against "non-obvious" operator overloading (both in
C and python), so if people want to get the serialized representation
for output purposes I'd prefer an explicit call.

Granted, it may be super handy to have str(config) print out a fully
informative description of the object.  This could be as simple as
<class Ocio.Config FULL YAML EMBEDDED HERE>).   I just want to make
sure the str representation is not valid parsable yaml, so people dont
accidentally rely on str() when they should be using .serialize().

-- Jeremy



On Tue, Oct 19, 2010 at 3:31 PM, Malcolm Humphreys
<malcolmh...@mac.com> wrote:
LGTM,

Did you see my comment in one of the threads about also supporting the .__str__() method?

On 20/10/2010, at 9:05 AM, Jeremy Selan wrote:

This makes the naming unified across C++ / python, and also gets rid
of "getXML" (which is no longer xml).

js/master
commits:
bd559608bdfbbc930634ef78eebd42cda83647da

Re: Review: added missing cmake NAME arg to add_test()

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

#180

looks good to me.

On Tue, Oct 19, 2010 at 5:35 PM, Malcolm Humphreys
<malcolmh...@mac.com> wrote:
- Added missing NAME arg to add_test() as this was failing to work on linux (worked on OSX, small cmake bug)

http://github.com/malcolmhumphreys/OpenColorIO/commit/8e80331866f003b99bbc2bfae627b730980cf8b0

.malcolm

Review: added missing cmake NAME arg to add_test()

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

#185

- Added missing NAME arg to add_test() as this was failing to work on linux (worked on OSX, small cmake bug)

http://github.com/malcolmhumphreys/OpenColorIO/commit/8e80331866f003b99bbc2bfae627b730980cf8b0

.malcolm

Re: Review: renamed config.writeToStream to config.serialize

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

#183

LGTM,

Did you see my comment in one of the threads about also supporting the .__str__() method?

Show quoted text

On 20/10/2010, at 9:05 AM, Jeremy Selan wrote:

This makes the naming unified across C++ / python, and also gets rid
of "getXML" (which is no longer xml).

js/master
commits:
bd559608bdfbbc930634ef78eebd42cda83647da

Re: Review: Fixed malformed ocio profile header tag

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

#182

LGTM

Show quoted text

On 20/10/2010, at 9:04 AM, Jeremy Selan wrote:

* cleaned up malformed profile name (ocs_profile_version vs
ocio_profile_version)
* added better loading error checks
* updated raw profile to new yaml format

js/master
commits:
ecf370fdc3e872b4ec78e04bc7b1f13b65e95305
77493a8821d087a1352d6da31dd5507d3a34c07a

Review: renamed config.writeToStream to config.serialize

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

#177

This makes the naming unified across C++ / python, and also gets rid
of "getXML" (which is no longer xml).

js/master
commits:
bd559608bdfbbc930634ef78eebd42cda83647da

Review: Fixed malformed ocio profile header tag

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

#176

* cleaned up malformed profile name (ocs_profile_version vs
ocio_profile_version)
* added better loading error checks
* updated raw profile to new yaml format

js/master
commits:
ecf370fdc3e872b4ec78e04bc7b1f13b65e95305
77493a8821d087a1352d6da31dd5507d3a34c07a

Trunk is now OCIO 0.7 (dev branch)

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

#175

Folks,

I've deleted the OCIO yaml branch.   The main trunk now is up to date
with all recent commits.

-- Jeremy

Re: Review: fix some 'const char*' to 'char*' in pyglue

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

#172

LGTM.

This was a compilation warning though, right? (It shouldnt have been an error).

-- Jeremy

On Mon, Oct 18, 2010 at 11:42 PM, Malcolm Humphreys
<malcolmh...@mac.com> wrote:
PyOpenColorIO.so wouldn't compile on linux

http://github.com/malcolmhumphreys/OpenColorIO/commit/bb90284fa98161aae4ace74987246ae301ec1822
- Added some const_cast<char*>(..) ie. 'error: invalid conversion from ‘const char*’ to ‘char*’'

.malcolm

Re: Review: YAML serialization

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

#171

Works great on our linux distro now, thanks!

(LGTM)

I agree that it may make sense to include boost/shared_ptr.hpp, let me
look into this.

-- Jeremy

On Mon, Oct 18, 2010 at 11:27 PM, Malcolm Humphreys
<malcolmh...@mac.com> wrote:
Hi,

This should fix the link error on linux
http://github.com/malcolmhumphreys/OpenColorIO/commit/7b016f274aefd8a4ce2deb7c72639351db8de0b4

I always forget how to create these diffs so I guess other people might as well.
$> diff -Naur yaml-cpp-0.2.5 YAML_CPP_LOCAL > mynew.patch

I just edited the YAML_CPP_LOCAL src dir while running make in ext/build till OpenImageIO linked, then running the above command on a freshly extracted yaml-cpp-0.2.5.tar.gz which gives me the patch.

I found when building on a drd box we don't have any boost versions installed locally, these fix this issue.
http://github.com/malcolmhumphreys/OpenColorIO/commit/42c400a00f83a0e02734d9d216b6a07e1b7a4898
http://github.com/malcolmhumphreys/OpenColorIO/commit/8ec728181314d78be3a5d5b91b70793bb5b767b7

I'm not sure if we want to just include the boost/shared_ptr.hpp files or depend on boost.

.malcolm

On 19/10/2010, at 2:17 PM, Jeremy Selan wrote:

It does compile on osx, so I'll do further linux investigations tomorrow.

-- Jeremy

On Mon, Oct 18, 2010 at 7:18 PM, Jeremy Selan <jeremy...@gmail.com> wrote:
* Unfortunately, the compile issue I'm having does not appear to be
tied to the namespace stuff. Does your yaml branch currently build? On
my linux box, it does not.  What OS are you testing on?

Review: fix some 'const char*' to 'char*' in pyglue

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

#181

PyOpenColorIO.so wouldn't compile on linux

http://github.com/malcolmhumphreys/OpenColorIO/commit/bb90284fa98161aae4ace74987246ae301ec1822
- Added some const_cast<char*>(..) ie. 'error: invalid conversion from ‘const char*’ to ‘char*’'

.malcolm

Re: Review: YAML serialization

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

#179

Hi,

This should fix the link error on linux
http://github.com/malcolmhumphreys/OpenColorIO/commit/7b016f274aefd8a4ce2deb7c72639351db8de0b4

I always forget how to create these diffs so I guess other people might as well.
$> diff -Naur yaml-cpp-0.2.5 YAML_CPP_LOCAL > mynew.patch

I just edited the YAML_CPP_LOCAL src dir while running make in ext/build till OpenImageIO linked, then running the above command on a freshly extracted yaml-cpp-0.2.5.tar.gz which gives me the patch.

I found when building on a drd box we don't have any boost versions installed locally, these fix this issue.
http://github.com/malcolmhumphreys/OpenColorIO/commit/42c400a00f83a0e02734d9d216b6a07e1b7a4898
http://github.com/malcolmhumphreys/OpenColorIO/commit/8ec728181314d78be3a5d5b91b70793bb5b767b7

I'm not sure if we want to just include the boost/shared_ptr.hpp files or depend on boost.

.malcolm

Show quoted text

On 19/10/2010, at 2:17 PM, Jeremy Selan wrote:

It does compile on osx, so I'll do further linux investigations tomorrow.

-- Jeremy

On Mon, Oct 18, 2010 at 7:18 PM, Jeremy Selan <jeremy...@gmail.com> wrote:
* Unfortunately, the compile issue I'm having does not appear to be
tied to the namespace stuff. Does your yaml branch currently build? On
my linux box, it does not.  What OS are you testing on?