I am in favor of keeping GroupTransform as is. Having a group of transforms (at the API level) be interchangeable with a single transform is a very powerful concept. The allows the API to be pretty flexible, as class methods can seamlessly accept either a single transform, or a group of transforms, using the same interface.
A good example of this is config.applyTransform(Transform *), which can take either a single transform or a group of transforms. If we were to create a separate object - TransformVec - we would need to have 2 overloaded calls to expose the same functionality.
Another example of this is DisplayTransform.setDisplayCC(Transform *), which controls the 'post display transform' color correction. (Think the gamma slider in the nuke monitor).
In our internal compositing package, we actually let the user adjust the white point, black point, and gamma. This is internally handled by creating a group transform (at runtime), adding the appropriate children, and then calling setDisplayCC. There is public code analogous to this in nuke/Display/Display.cpp, Display::_validate.
In fact, I'd probably want to even go a step further and have the ColorSpace.setTransform call take a Transform * rather than a GroupTransform *. (If you want it to be a group, great! Otherwise, why force it?) There's no reason to force a grouptransform anywhere in the public interface, as far as I can tell.)
+1 this would clean the whole thing up for me.
The potentially nested nature of groupTransforms / subgroups does not bother me. If a group - containing a group - happens to be the natural way to express a particular transform to the user, why not allow it?
Yeah thats sounds cool, ColorSpace.setTransform(Transform *) clearly makes a GroupTransform just another Transform which I like a lot more than it being special in the ColorSpace() context.
.malcolm
-- Jeremy
* You mention in the code, "GroupTransform seems to be a mixed concept". And, you dont allow for it to be serialized. Could you elaborate your thinking behind this?
With the current structure it would be possible to have GroupTransform()s containing GroupTransform()s and possibly more GroupTransform()s and so on..
On a colorspace you can set ColorSpace::setTransform(GroupTransform, dir), to me this seems ok and I do serialize it in this case, but if I support the GroupTransform where I have support for all the other Transform()s then you could build multiple layer deep GroupTransform()s which doesn't seem necessary.
I would prefer to create a new type TransformList eg. ColorSpace::setTransform(TransformList, dir) which would replace GroupTransform and it would different than the base Transform()
