The "dynamic dispatch" is kind of ugly. I really don't like the idea of the
core kernel of each function needing to be expressed twice (once for the
templated specialization, once for dynamic dispatch). That seems like a
nightmare to maintain.
I'm very torn between the "IBA should work for any type of IB" principle, and
the idea of greatly simplifying by simply rejecting all but the very small
subset of types that we are likely to see in real life. If we restrict to
float/half/uint8/uint16, it already cuts 1331 cases to 64, with very little
chance of anybody encountering one of the unsupported types (oiiotool never
will -- it already enforces all-float via the ImageCache), and contrarian app
could very easily convert non-float IB's to float IB's before passing them on
to IBA functions.
So while I love the principle, I think it's a legit question to ask what the
chances are that IBA will be used by an app that (a) is not oiiotool, and (b)
does not use ImageCache (which already restricts to float/uint8, with the
option of restricting to only float), (c) would not be ok with implementing an
extra conversion step to float on its side for those oddball images that aren't
one of the 99% useful types.
Also, this is a correctable problem in the future. If our internals for IBA
look like:
template<class T> bool specialized_foo (...) { IB::Iterator<T,float>
blah... }
bool foo (ImageBuf &ib)
{
switch (ib.spec().format.basetype) {
case TypeDesc::FLOAT : return specialized_foo<float>(); break;
case TypeDesc::HALF : return specialized_foo<half>(); break;
case TypeDesc::UINT8 : return specialized_foo<unsigned char>();
break;
case TypeDesc::UINT16 : return specialized_foo<unsigned short>();
break;
default: return false; // reject
}
We can always go back and add more cases later, if we realize that we've made a
mistake and left out an important type.
The rejection of several types that, realistically, nobody is likely to ever
use for these IBA functions, may be a lesser evil than either over-specializing
(long compile times and code bloat) or having to implement each function twice
to also have the "dynamic" version (which, again, may never be called in the
real world).
This is the way I'm leaning tonight. Can I convince you? (At this point, I
could probably also be convinced to support the "IB for anything, IBA for float
IB's only" approach and radically simplify.)
-- lg
On Jun 20, 2012, at 3:28 PM, Chris Foster wrote:
> Hi Larry,
>
> On Thu, Jun 21, 2012 at 12:38 AM, Larry Gritz <[email protected]> wrote:
>> Chris, do you think this is also a severe problem with the
>> template-based method I advocated, which supports the cross-product of
>> all the basic data types?
>
> It's exactly the same problem: if you want a specialized version for
> each combination of types, that mandates 11^3 versions of the function.
> There's no getting around this basic fact I'm afraid.
>
>> An alternative would be to simply declare that even though IB's can
>> contain any of the basic types, the ImageBufAlgo image processing
>> functions only are expected to work on ImageBuf's that are a subset of
>> the most common formats (say: float, half, uint8, uint16), error if
>> the IBA's see an IB not one of those, and leave it up to the app to
>> convert. After all, there is probably no point in being able to do
>> "int32 over int8", considering that in practice, we basically never
>> see either of those formats. That would cut the cross product down
>> from 11^3 to 4^3.
>
> I strongly feel the user should be able to put what type they like into
> an IB and the IBAlgo functions should still work.
>
> However, that's not to say we can't use dynamic dispatch on the full
> cross-product of types, and only have specialized versions for rather
> specific and common ones.
>
> over() has three types T1, T2, T3. Here's some possible rules:
>
> 1) If T1, T2, T3 are all the same, use a specialized version
> (11 specializations)
>
> 2) If T1, T2, T3 are any combination of float, half, uint8, uint16, use
> a specialized version (an additional 4^3 - 4 = 60 specializations)
>
> 3) For all other cases, fall back to using dynamic dispatch to get
> pixels inside the inner loop - it will still work, but will be slower
> (1 special case dynamic version)
>
> That's a total of 71 specialized versions of over(), which I think is
> workable. (Much more so than 11^3 == 1331) If we did this carefully we
> can still have a single version of the implementation - would require an
> iterator type which does dynamic dispatch to get at the pixel values.
>
> Thoughts?
> ~Chris
>
>
> PS: Stefan, sorry for rethinking the design yet again at this late
> stage, but I think we will be in very good shape when we have this
> sorted out! I think you're certainly ready for a pull request - that
> just means you're ready for us to review your code, which we're already
> doing anyway. The way we usually do things is to submit pull requests
> early, and when people want things changed you commit additional patches
> into the pull request. To make this happen smoothly it's useful to make
> a new git branch in your fork of OIIO which is specific to the feature
> being worked on. If you do that, any further commits made to the branch
> in your fork will append to the pull request automatically. When we've
> agreed that the patch is good, we can just squash all the commits
> together (git rebase -i) before committing to the trunk.
> _______________________________________________
> Oiio-dev mailing list
> [email protected]
> http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org
--
Larry Gritz
[email protected]
_______________________________________________
Oiio-dev mailing list
[email protected]
http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org
