Camera IPR

[johnhaddon]

This implements camera changes during IPR renders, as per #190. It requires ImageEngine/cortex#296.

I'm putting this up here in case anyone wants to give it a play, and for an early review of the code, but I'm afraid I don't think it's a good idea to merge yet. Although I've yet to have a problem with it interactively, the unit tests currently crash every time. I've reproduced this in a simple standalone program, and have an open ticket for it on the 3delight tracker (0005430 for anyone who'd like to follow along). Hopefully it's something stupid I'm doing or something that can be fixed quickly.

[johnhaddon]

Quick update to say that the crash has been fixed in 3delight 11.0.84 - so once we get our grubby mits on that we should be able to test and merge this branch.

[davidsminor]

I've just plugged it into caribou. Is nice, I like.

I guess the crash was inside RiCameraV()? I was seeing it every so often in maya, largely when doing things like disconnecting and reconnecting nodes while the IPR was running. It went a little something like this:

Signal: 11 (Unknown Signal)
Stack trace:
  /lib64/libc.so.6() [0x36d5032920]
  /software/apps/3delight/11.0.68/cent6.x86_64/lib/lib3delight.so(+0x77798) [0x7fd1c4014798]
  RiCameraV
  RiCamera
  IECoreRI::RendererImplementation::camera(std::string const&, std::map<IECore::InternedString, IECore::IntrusivePtr<IECore::Data>, std::less<IECore::InternedString>, std::allocator<std::pair<IECore::InternedString const, IECore::IntrusivePtr<IECore::Data> > > > const&)
  IECore::Camera::render(IECore::Renderer*) const
  GafferScene::outputCamera(GafferScene::ScenePlug const*, IECore::CompoundObject const*, IECore::Renderer*)
  GafferScene::InteractiveRender::updateCamera()
  GafferScene::InteractiveRender::update()
  boost::signal1<void, Gaffer::Plug*, boost::last_value<void>, int, std::less<int>, boost::function<void ()(Gaffer::Plug*)> >::operator()(Gaffer::Plug*)
  Gaffer::DependencyNode::propagateDirtiness(Gaffer::Plug*)
  Gaffer::Plug::setInputInternal(IECore::IntrusivePtr<Gaffer::Plug>, bool)

I'll see if it still happens once we get 11.0.84.

[johnhaddon]

Glad you like it. It must be much handier in a system where you actually have manipulators - I'm really looking forward to seeing Caribou.

The crashes I saw were identical to the one you posted, and I also managed to reproduce them in a small test program without Gaffer. Looks like 11.0.84 is available now - I'll give it a go this end if you could do some quick tests on Linux as well.

[johnhaddon]

OK. I've done the following :

• Rebased on top of the current master
• Fixed Strange UI behaviour when disconnecting Group during IPR #898 and Handle invalid camera errors better #371
• Fixed bugs where exceptions could cause incomplete edit blocks to be emitted (needs Added an EditBlock class for exception-safe rerendering edits. ImageEngine/cortex#303)
• Added a unit test for the camera edits
• Tested with 3delight 11.0.84 on OS X (all good)

There are some error messages (not test failures) printed by the unit tests now - these occur because as the script is deleted when the test exits, it deletes all its nodes one by one, and the renderer is trying to represent these with edits. If the camera node is deleted before the StandardOptions node then errors about missing cameras will get output. I have some ideas about how to prevent this, but since it's harmless I'd like to give those ideas a bit more time to develop before doing them. I reckon as long as everything tests out OK with 11.0.84 at your end, we should be good to merge this now.

[johnhaddon]

Any chance of getting this and my other Gaffer pull requests reviewed/merged today?

[davidsminor]

Haven't been able to break this since I installed 3delight 11.0.84. I'm getting two test failures for GafferRenderMan though - this is using the latest cortex trunk (cortex 5c16fde47f40dc6c521ed04caf676c3a7b56a7e0) and basing this pull request off the latest gaffer trunk (2987bc7).

GafferRenderManTest.RenderManLightTest.testRender: this one's expected actually isn't it...
GafferRenderManTest.RenderManRenderTest.testCameraMotionBlur: the last couple of cases where it checks the shutter times fail - this seems to be because you've used RiMotionBegin() rather than RiMotionBeginV() on line 461 of IECoreRI/RendererImplementation.cpp

The failures don't seem to be related to this pull request, so I think I'll just pull it...