Sx renderer fixes2

[davidsminor]

Fixed thread safety bug in SXRenderer::shade(), and implemented some transformation matrix functionality, including coordinateSystem()

[johnhaddon]

Hi David,
Could you explain a bit about the thread safety change? If I recall correctly we've been using the Sx stuff in a threading heavy environment for years without issues - has something changed in our usage of SxExecutor or has something changed on the 3delight side?
Cheers…
John

[davidsminor]

Hmm. Well I've added a comment, and I wish I could reproduce this in a simple test case... I've got a production case where I've got a load of threads calling shade() on the same SXRenderer object, and it appears that unless I copy the context in SXExecutor::execute(), SxGetParameter() returns no points for Ci and Oi in most cases (seems like only one thread's successful).

Here's roughly what I'm doing:

IECoreRI::SXRendererPtr sxRenderer = new IECoreRI::SXRenderer();

...
set a bunch of coord systems and options,
and load a large coshader network
...

SXEval s;
s.sxRenderer = sxRenderer.get();
...

tbb::parallel_for( tbb::blocked_range<int>( 0, n ), s );

The SXEval::operator() is doing this kind of thing:

void operator()( const tbb::blocked_range<int>& range ) const
{
    for( int i=range.begin(); i!=range.end(); ++i )
    {
        // call shade() with a grid on a shared sx renderer:
        CompoundDataPtr shadingResult = sxRenderer->shade( patch[i], gridSize );

        // this fails most of the time because SxGetParameter is returning a zero point count for Ci
        std::vector<Imath::Color3f>& ci = runTimeCast< Color3fVectorData >( shadingResult->writable()["Ci"] )->writable();
        std::vector<Imath::Color3f>& oi = runTimeCast< Color3fVectorData >( shadingResult->writable()["Oi"] )->writable();
        ...

    }
}

It is indeed kind of odd that we haven't seen this in IEFur because it does something very similar, although the shader networks probably aren't quite as large and expensive as the ones I'm using and it doesn't use grids...

It is a bit of a strange one, but there definitely seems to be a problem when multiple threads create parameter lists in the same context and call SxCallShader().

[johnhaddon]

Sounds nasty - must have been fun to debug. Seems like it'd be a good idea to bring this up with the 3delight chaps, to check that our usage of the API is as expected, and see if they think there could be anything needing fixing on the Sx side of things. It seems like this may just be a workaround on our end of things, and since it's a fun hard-to-reproduce-threading bug it'd be nice to have additional confidence that this will at least work in all situations...

Have you got any measurements of the overhead of the extra context create/destroy? Is it worth trying to limit it to the cases where it's needed or is it fairly negligible?

[davidsminor]

You know what? The latest version of my tool doesn't share SX renderers between threads anyway, and seems to be working ok. I think I'd rather get this stuff rolled out and try and get to the bottom of it later, so I'm removing the thread safety commit.