GafferAppleseed: New renderer

[est77]

This PR depends on ImageEngine/cortex#520.

• Replaced the old render.
• Added environment lights transforms.
• Added computation of smooth normals and tangents.
• Added logging options.
• Fixed Lat-long env visualizers.
• Fixed ShaderBall preview with new Renderer.
• Temporary removed auto-instancing support.

[johnhaddon]

Brilliant! I'm very much looking forward to trying this. I've marked it as on hold temporarily, because from the looks of things I need to update gafferDependencies to include the equivalent cortex PR before Travis will be happy.

We're actually about to push out a new Gaffer release for production use, so I'll hold off on doing a detailed review/merge until that is out of the way, as this seems big enough to warrant a bit of care.

[est77]

I agree, doing the release and merging later is a good plan.
I'm going to start moving the unit tests from the old renderer to the new one in the meantime.

[johnhaddon]

Hi Est,

I'm just finding some time to start to look at this now. The new IPR feels much more responsive and moving objects around seems to be working well - nice!

I've hit one problem, which is that the images that are auto-generated for the tutorial in the documentation now all come out black. This seems to be because I didn't use an AppleseedOptions node to specify which environment light to use, and the new backend doesn't pick one automatically in the same way the old one did. If I use an AppleseedOptions node to specify the right environment, it does work, but I have to restart the IPR for it to take effect. Since following the tutorial will often be the first experience people have of either Gaffer or Appleseed, it'd be great to get this sorted so it works smoothly - either without needing the option, or without needing a restart. Do you think one of those is doable?

Cheers...
John

[est77]

I didn't like the automatic environment light selection of the old backend. It seemed like a good idea
when I added it, but I think it's too random. If you selected an environment light in the options node then it would be used, if not, the first one specified to the renderer would be used. If you have two lights, then it's not clear which one would be selected.

I'm going to fix the environment updates. Should be easy.

Thanks for looking into the PR!

[est77]

Environment updates should be fixed now.

[johnhaddon]

Hi Est,
Thanks for that last fix. In trying to do my own testing I had to come up with a few more fixes, which you can see on the tip of the branch here :

https://github.com/johnhaddon/gaffer/commits/newAppleseedBackendWithTweaks

I've got the gafferDependencies project updated with the latest Cortex/Appleseed combo, and am in the process of uploading the binaries for them, so hopefully we'll be in a position to get this one merged soon too.
Cheers...
John

[est77]

Thanks for the fixes. I added a commit here including them.

[johnhaddon]

Thanks Est, and sorry it's taken me so long to look at this again. I've done a bit more testing and run into a crash which seems to be repeatable using the following steps :

• Merge this branch to master
• Build it against the gafferDependencies-0.29.0.0 package
• Open Gaffer
• Create an as_disney_material (it should show up in the shader viewer)
• Scrub the specular setting forwards and backwards using Control + Left Drag

Here's the stacktrace :

#0  0x00007fffd1dadfe5 in renderer::(anonymous namespace)::OSLMaterial::on_frame_end(renderer::Project const&, renderer::Assembly const&) () from /disk1/john/dev/build/gaffer/appleseed/lib/libappleseed.so
#1  0x00007fffd1e0ce57 in renderer::Assembly::on_frame_end(renderer::Project const&) () from /disk1/john/dev/build/gaffer/appleseed/lib/libappleseed.so
#2  0x00007fffd1e161a4 in renderer::Scene::on_frame_end(renderer::Project const&) () from /disk1/john/dev/build/gaffer/appleseed/lib/libappleseed.so
#3  0x00007fffd1c24ebe in renderer::MasterRenderer::render_frame_sequence(renderer::IFrameRenderer&, foundation::IAbortSwitch&) () from /disk1/john/dev/build/gaffer/appleseed/lib/libappleseed.so
#4  0x00007fffd1c25526 in renderer::MasterRenderer::initialize_and_render_frame_sequence() () from /disk1/john/dev/build/gaffer/appleseed/lib/libappleseed.so
#5  0x00007fffd1c256b6 in renderer::MasterRenderer::do_render() () from /disk1/john/dev/build/gaffer/appleseed/lib/libappleseed.so
#6  0x00007fffd1c25706 in renderer::MasterRenderer::render() () from /disk1/john/dev/build/gaffer/appleseed/lib/libappleseed.so
#7  0x00007fffd2345a4f in (anonymous namespace)::AppleseedRenderer::interactiveRenderThreadFun() () from /disk1/john/dev/build/gaffer/lib/libGafferAppleseed.so
#8  0x00007ffff0a7baa3 in thread_proxy () from /disk1/john/dev/build/gaffer/lib/libboost_thread.so.1.55.0
#9  0x0000003eda607a51 in start_thread () from /lib64/libpthread.so.0
#10 0x0000003ed9ee893d in clone () from /lib64/libc.so.6

Any ideas?

[est77]

I couldn't reproduce this crash, using my own dev build or building gaffer using the prebuilt dependencies. I tried in two different computers, one using Fedora 23 and another Centos 7.

I can't see anything in the stacktrace that could cause a crash.

[johnhaddon]

Hi Est,
I saw your on_frame_begin fixes go into Appleseed master and gave them a go - it all seems to be working fine now. Were you planning on doing more or making a new Appleseed release soon, or should I update the dependencies project with the current state of master? By the way, ObjectInterface::attributes() was updated to return a bool since this PR started - I just updated everything to return true locally. You can return false if an edit to an attribute is impossible for any reason - in Arnold we use that when the subdivision settings change for instance, and then Gaffer will resupply the geometry from scratch.
Cheers...
John

[est77]

Yes, that fixed the crash.

I tagged a version that we can use for the gaffer releases: https://github.com/appleseedhq/appleseed/releases/tag/1.5.2-beta.

[johnhaddon]

Nice one - thanks! I'll start getting things together.

[est77]

Great, thanks!

I didn't do the ObjectInterface changes yet. I was planning to do the same you did, returning true
and later, in another PR, do the right thing for the smooth normals and tangents attribute edits.

[johnhaddon]

That sounds like a plan. I'm aiming to get this PR merged this afternoon - just uploading a new dependencies tarball now, then I need to update the docs to include the extra step for the environment light, and then I think we're good to go?

[johnhaddon]

Actually, I'm seeing 3 failing tests :

======================================================================
FAIL: test (GafferAppleseedUITest.DocumentationTest.DocumentationTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/disk1/john/dev/build/gaffer/python/GafferAppleseedUITest/DocumentationTest.py", line 50, in test
    additionalTerminalPlugTypes = ( GafferScene.ScenePlug, )
  File "/disk1/john/dev/build/gaffer/python/GafferTest/TestCase.py", line 233, in assertNodesAreDocumented
    self.assertEqual( undocumentedPlugs, [] )
AssertionError: Lists differ: ['AppleseedOptions.options.int... != []

First list contains 1 additional elements.
First extra element 0:
AppleseedOptions.options.interactiveRenderMaxSamples

- ['AppleseedOptions.options.interactiveRenderMaxSamples']
+ []

======================================================================
FAIL: testImageOutput (GafferAppleseedTest.AppleseedRenderTest.AppleseedRenderTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/disk1/john/dev/build/gaffer/python/GafferAppleseedTest/AppleseedRenderTest.py", line 170, in testImageOutput
    self.failUnless( os.path.exists( self.temporaryDirectory() + "/test.%04d.exr" % i ) )
AssertionError: False is not true

======================================================================
FAIL: testWaitForImage (GafferAppleseedTest.AppleseedRenderTest.AppleseedRenderTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/disk1/john/dev/build/gaffer/python/GafferAppleseedTest/AppleseedRenderTest.py", line 111, in testWaitForImage
    self.failUnless( os.path.exists( self.temporaryDirectory() + "/test.exr" ) )
AssertionError: False is not true

On the subject of tests, I did the testing for the InteractiveArnoldRender node using a base class for the test so it could be reused for other interactive renderers - it'd be nice to use that for Appleseed too at some point.

[est77]

I'll look into the failing tests after work.
I'll push the ObjectInterface changes and add a small fix I wanted to include in this PR so that everything is together in only one branch.

[johnhaddon]

Great - thanks. I also just noticed that the interactiveRenderMaxSamples option isn't assigned to any layout section, so it appears at the top of the UI. Would be nice to tuck that away somewhere appropriate.

[est77]

I'll fix that too.

I wanted to hide interactiveRenderMaxSamples from the UI. For now it would be an internal option
for use in the shader viewer. I tought I could do that by not adding it to any layout and iirc it was not
showing in the options node before.

What is the best way to hide an option?

[johnhaddon]

The best way to hide something from the UI is to disable the widget with a "plugValueWidget:type", "" metadata registration.

[johnhaddon]

I've pushed a branch which has my tweaks for the attributes() method and the updated documentation here :

https://github.com/johnhaddon/gaffer/commits/newAppleseedRendererTweaks

If you could cherry-pick them over to your PR that would be handy, but I can deal with it if not...
Cheers...
John

[est77]

Yes, I'll merge your commits, no problem.

Hiding the widgets does not seems to work:

File "/home/lemans/Devel/gaffer/gafferDependencies-0.29.0.0-linux/python/GafferUI/PlugLayout.py", line 372, in createPlugWidget
result = GafferUI.PlugValueWidget.create( plug )
File "/home/lemans/Devel/gaffer/gafferDependencies-0.29.0.0-linux/python/GafferUI/PlugValueWidget.py", line 390, in create
return creator( plug )
File "/home/lemans/Devel/gaffer/gafferDependencies-0.29.0.0-linux/python/GafferUI/CompoundDataPlugValueWidget.py", line 172, in __init
self.__row.append( GafferUI.PlugValueWidget.create( childPlug["value"] ), expand = True )
File "/home/lemans/Devel/gaffer/gafferDependencies-0.29.0.0-linux/python/GafferUI/ListContainer.py", line 76, in append
assert( isinstance( child, GafferUI.Widget ) )
AssertionError

[johnhaddon]

It sounds like you're applying the metadata to the child (value) plug rather than the parent plug? I think this commit is what you want :

johnhaddon@f94bf25

[est77]

All tests pass now and I think the other issues should be fixed too.

[johnhaddon]

Thanks Est, could you cherry pick over johnhaddon@57282c1 as well? I think that should get the travis tests passing...
Cheers...
John

[est77]

Done

[johnhaddon]

Thanks!

[est77]

I just realised that I've added in this PR and in the Cortex PR the -DAPPLESEED_USE_SSE
define in SConstruct but appleseed in the dependencies package is built without SSE support.

The flag can probably be removed from both PRs, it does not seem to change anything in appleseed's ABI (I'll double check that), but it would be nice if we could have SIMD optimizations.

Would it be possible to update the appleseed build to use the SSE flag?
It should be safe, the flag enables SSE2 optimizations, supported by all intel CPUs since 2001.

Here's the cmake option needed:
https://github.com/appleseedhq/appleseed/blob/master/travis-ci-build.sh#L64

Thanks!

[johnhaddon]

I've rebased and merged via the command line to get rid of the Merge remote-tracking branch 'upstream/master'. Just doing a rebuild of the dependencies using -D USE_SSE=ON now.
Thanks!
John

[est77]

Great!

Thanks!!!