GafferCycles : Remove problematic mutex lock

[ericmehl]

When using SVM shader mode on Windows, this lock was causing Gaffer to hang after any scene change. @boberfly, in #4951, you said that we're now flagging a warning to the user about resetting instead of trying to manage it here. I'm hoping that means that this lock is ok to remove entirely. Do you know of anything that would make that a bad idea?

In limited interactive testing it seemed to work fine.

Checklist

• I have read the contribution guidelines.
• I have updated the documentation, if applicable.
• I have tested my change(s) in the test suite, and added new test cases where necessary.
• My code follows the Gaffer project's prevailing coding style and conventions.

[boberfly]

Ahh here be dragons, I don't think we can get rid of the mutex lock for interactive renders rapidly changing the scene/shaders/etc. will eventually tread on some race conditions here.

There was talks/suggestions before that we could defer Cycles' scene/session creation to the first call to render() so we don't have the reset at all, but the current code as it stands needs the first reset so that the desired scene/session parameters can be set before rendering starts. I've not started this yet and the annoying thing is any calls to option() needs to be caching all of these somewhere without scene/session existing yet, and maybe other things I'm not thinking of.

Anyways for a quicker/easier fix here to satisfy windows, could you try doing an unlock here before the reset() call?: https://github.com/GafferHQ/gaffer/blob/main/src/GafferCycles/IECoreCyclesPreview/Renderer.cpp#L3453

Because the mutex lock is a scope now, it's unfortunate as it'll try to free it a second time once we go out of scope which will probably cause the problem still, but I'm not too sure. Maybe because we know that we only want this first call to render() to be able to do the hard reset, we can use the scoped mutex for when if( m_renderState == RENDERSTATE_RENDERING ) is set and omitted when this is not set?

[boberfly]

Hey @ericmehl I've made a PR which cleans up and refactors this reset code, could you test it on windows for me?
#5101

Cheers

[johnhaddon]

I don't think we can get rid of the mutex lock for interactive renders rapidly changing the scene/shaders/etc. will eventually tread on some race conditions here.

While this may be true at present, I suspect it is only true because CyclesRenderer::pause() doesn't actually wait for Cycles to be done before returning. The intention of pause() is that the renderer should be hands off after that, and we should be free to do anything we want with the scene.

[ericmehl]

I'm closing this in favor of the solution in #5101