-
Notifications
You must be signed in to change notification settings - Fork 124
Optimized the LensDistortOp, improving the performace 4x #1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…mple look-up from within the warp() method instead of recomputing it per channel. Removed all conditional statements from the tight loops.
andrewkaufman
added a commit
that referenced
this pull request
Jun 17, 2013
Optimized the LensDistortOp, improving the performace 4x
johnhaddon
added a commit
that referenced
this pull request
Jan 9, 2020
`DisplayTileCallbackFactory::create()` returned the same DisplayTileCallback instance every time, and up until now took responsibility for deleting it. This has always been sketchy because Appleseed holds the returned DisplayTileCallback's with `auto_release_ptr`, so `DisplayTileCallback::release()` must avoid double deletes by just doing nothing. This worked if `DisplayTileCallback::release()` was called before `DisplayTileCallbackFactory::release()`, but Appleseed 2.1 has reversed the call order so that the factory is destroyed before the callback is. This means `DisplayTileCallback::release()` is called on an already-deleted instance, and crashes, with a stack trace like so : ``` #0 0x0000000000000000 in ?? () #1 0x00007fffe18153a6 in foundation::auto_release_ptr<renderer::ITileCallback>::reset (this=0x7fff8c012148, ptr=0x0) at /disk1/john/dev/gafferDependencies/Appleseed/working/appleseed-2.1.0-beta/src/appleseed/foundation/utility/autoreleaseptr.h:187 #2 0x00007fffe181137f in renderer::(anonymous namespace)::ProgressiveFrameRenderer::~ProgressiveFrameRenderer (this=0x7fff8c0120a0, __in_chrg=<optimized out>) at /disk1/john/dev/gafferDependencies/Appleseed/working/appleseed-2.1.0-beta/src/appleseed/renderer/kernel/rendering/progressive/progressiveframerenderer.cpp:178 #3 0x00007fffe1811592 in renderer::(anonymous namespace)::ProgressiveFrameRenderer::~ProgressiveFrameRenderer (this=0x7fff8c0120a0, __in_chrg=<optimized out>) at /disk1/john/dev/gafferDependencies/Appleseed/working/appleseed-2.1.0-beta/src/appleseed/renderer/kernel/rendering/progressive/progressiveframerenderer.cpp:187 #4 0x00007fffe18115ca in renderer::(anonymous namespace)::ProgressiveFrameRenderer::release (this=0x7fff8c0120a0) at /disk1/john/dev/gafferDependencies/Appleseed/working/appleseed-2.1.0-beta/src/appleseed/renderer/kernel/rendering/progressive/progressiveframerenderer.cpp:191 #5 0x00007fffe1712fb7 in foundation::auto_release_ptr<renderer::IFrameRenderer>::~auto_release_ptr (this=0x7fff8c01f928, __in_chrg=<optimized out>) at /disk1/john/dev/gafferDependencies/Appleseed/working/appleseed-2.1.0-beta/src/appleseed/foundation/utility/autoreleaseptr.h:124 #6 0x00007fffe17121be in renderer::RendererComponents::~RendererComponents (this=0x7fff8c01f890, __in_chrg=<optimized out>) at /disk1/john/dev/gafferDependencies/Appleseed/working/appleseed-2.1.0-beta/src/appleseed/renderer/kernel/rendering/renderercomponents.h:73 #7 0x00007fffe1712288 in std::default_delete<renderer::RendererComponents>::operator() (this=0x7fff8c000ae0, __ptr=0x7fff8c01f890) at /opt/rh/devtoolset-6/root/usr/include/c++/6.3.1/bits/unique_ptr.h:76 #8 0x00007fffe1711e0d in std::unique_ptr<renderer::RendererComponents, std::default_delete<renderer::RendererComponents> >::reset (this=0x7fff8c000ae0, __p=0x7fff8c01f890) at /opt/rh/devtoolset-6/root/usr/include/c++/6.3.1/bits/unique_ptr.h:347 #9 0x00007fffe1710632 in renderer::CPURenderDevice::~CPURenderDevice (this=0x7fff8c0009b0, __in_chrg=<optimized out>) at /disk1/john/dev/gafferDependencies/Appleseed/working/appleseed-2.1.0-beta/src/appleseed/renderer/device/cpu/cpurenderdevice.cpp:112 #10 0x00007fffe1710988 in renderer::CPURenderDevice::~CPURenderDevice (this=0x7fff8c0009b0, __in_chrg=<optimized out>) at /disk1/john/dev/gafferDependencies/Appleseed/working/appleseed-2.1.0-beta/src/appleseed/renderer/device/cpu/cpurenderdevice.cpp:129 #11 0x00007fffe181f21c in std::default_delete<renderer::IRenderDevice>::operator() (this=0x3093868, __ptr=0x7fff8c0009b0) at /opt/rh/devtoolset-6/root/usr/include/c++/6.3.1/bits/unique_ptr.h:76 #12 0x00007fffe181eafd in std::unique_ptr<renderer::IRenderDevice, std::default_delete<renderer::IRenderDevice> >::~unique_ptr (this=0x3093868, __in_chrg=<optimized out>) at /opt/rh/devtoolset-6/root/usr/include/c++/6.3.1/bits/unique_ptr.h:239 #13 0x00007fffe181d545 in renderer::MasterRenderer::Impl::~Impl (this=0x3093820, __in_chrg=<optimized out>) at /disk1/john/dev/gafferDependencies/Appleseed/working/appleseed-2.1.0-beta/src/appleseed/renderer/kernel/rendering/masterrenderer.cpp:168 #14 0x00007fffe181cfdb in renderer::MasterRenderer::~MasterRenderer (this=0x3088d70, __in_chrg=<optimized out>) at /disk1/john/dev/gafferDependencies/Appleseed/working/appleseed-2.1.0-beta/src/appleseed/renderer/kernel/rendering/masterrenderer.cpp:584 #15 0x00007fffc67d0ae0 in (anonymous namespace)::AppleseedRenderer::~AppleseedRenderer() () from /disk1/john/dev/build/gaffer/lib/libGafferAppleseed.so #16 0x00007fffd8a734b6 in GafferScene::InteractiveRender::stop() () from /disk1/john/dev/build/gaffer/lib/libGafferScene.so #17 0x00007fffd8a735bf in GafferScene::InteractiveRender::update() () from /disk1/john/dev/build/gaffer/lib/libGafferScene.so ``` The solution here is to return a new DisplayTileCallback every time `DisplayTileCallbackFactory::create()` is called, and to destroy those the regular way in `DisplayTileCallback::release()`. This works fine for interactive renders, because Appleseed will only ever make a single DisplayTileCallback at a time. For batch renders it is completely broken though, because Appleseed will call `DisplayTileCallbackFactory::create()` once per thread, and every thread will end up writing to a different display driver. So far we've only used the display driver for interactive renders though, so maybe this is OK.
lucienfostier
added a commit
to lucienfostier/cortex
that referenced
this pull request
Jul 20, 2023
…r is computing.
I noticed crashes when the SceneCacheReader property panel was displayed and you would change frame or
filter the scene tag.
The following traceback indicates that it was related with refresh the SceneView widget.
This is likely something fundamentally flawed with the node ( as Nuke often complains
we are updating knob inside the _validate).
Futhermore, we should be able to use native Nuke node once we start using Nuke 14+ and
its USD support ( using our SceneCacheDataFormat AbstractFormat USD plugin ).
The trick is to avoid updating the widget when it's already up to date.
While testing,I couldnt reproduce the crash I saw when changing the filtering
and the "loading" of location in the Nuke 3d system.
```
0 0x0000000080000017 in ?? ()
1 0x00007fa26c59fee2 in QObject::disconnect(QObject const*, char const*, QObject const*, char const*) () from /software/apps/nuke/13.2v8/cent7.x86_64/libQt5Core.so.5
2 0x00007fa26d341e90 in QTreeView::setModel(QAbstractItemModel*) () from /software/apps/nuke/13.2v8/cent7.x86_64/libQt5Widgets.so.5
3 0x00007fa25b1a089c in ?? () from /software/apps/nuke/13.2v8/cent7.x86_64/libnuke-13.2.8.so
4 0x00007fa25aae2656 in ?? () from /software/apps/nuke/13.2v8/cent7.x86_64/libnuke-13.2.8.so
5 0x00007fa25aaec3f9 in ?? () from /software/apps/nuke/13.2v8/cent7.x86_64/libnuke-13.2.8.so
6 0x00007fa175dc56e9 in IECoreNuke::SceneCacheReader::filterScene(std::string const&, std::string const&, bool) ()
from /software/apps/cortex/10.4.10.0/cent7.x86_64/nuke/13.2/lib/libIECoreNuke-10.4.so
7 0x00007fa175dc6065 in IECoreNuke::SceneCacheReader::_validate(bool) () from /software/apps/cortex/10.4.10.0/cent7.x86_64/nuke/13.2/lib/libIECoreNuke-10.4.so
8 0x00007fa2597faf3b in DD::Image::Op::force_validate(bool) () from /software/apps/nuke/13.2v8/cent7.x86_64/libDDImage.so
9 0x00007fa175da9794 in IECoreNuke::LiveScene::geometryList(DD::Image::Op*, double const&) const ()
from /software/apps/cortex/10.4.10.0/cent7.x86_64/nuke/13.2/lib/libIECoreNuke-10.4.so
10 0x00007fa175da9dc9 in IECoreNuke::LiveScene::geometryList(double const&) const () from /software/apps/cortex/10.4.10.0/cent7.x86_64/nuke/13.2/lib/libIECoreNuke-10.4.so
11 0x00007fa175daa860 in IECoreNuke::LiveScene::cacheGeometryList(double const&) const () from /software/apps/cortex/10.4.10.0/cent7.x86_64/nuke/13.2/lib/libIECoreNuke-10.4.so
12 0x00007fa175dab8f2 in IECoreNuke::LiveScene::objectNum(double const*) const () from /software/apps/cortex/10.4.10.0/cent7.x86_64/nuke/13.2/lib/libIECoreNuke-10.4.so
13 0x00007fa175dacedb in IECoreNuke::LiveScene::childNames(std::vector<IECore::InternedString, std::allocator<IECore::InternedString> >&) const ()
from /software/apps/cortex/10.4.10.0/cent7.x86_64/nuke/13.2/lib/libIECoreNuke-10.4.so
14 0x00007fa1298fbca9 in CaribouNuke::CaribouSceneReader::computeChildNames(std::vector<IECore::InternedString, std::allocator<IECore::InternedString> > const&, Gaffer::Context const*, GafferScene::ScenePlug const*) const () from /home/lucienf/apps/caribou/4.2.2dev/gaffer/1.2/tools/nuke/13.2/python/cent7.x86_64/cortex/10.4/CaribouNuke/_CaribouNuke.so
15 0x00007fa13530d444 in GafferScene::SceneNode::compute(Gaffer::ValuePlug*, Gaffer::Context const*) const ()
from /software/apps/gaffer/1.2.9.0/cent7.x86_64/cortex/10.4/nuke/13.2/lib/libGafferScene.so
16 0x00007fa148ec5340 in Gaffer::ValuePlug::ComputeProcess::ComputeProcess((anonymous namespace)::ComputeProcessKey const&) ()
from /software/apps/gaffer/1.2.9.0/cent7.x86_64/cortex/10.4/nuke/13.2/lib/libGaffer.so
17 0x00007fa148ece618 in Gaffer::ValuePlug::ComputeProcess::value(Gaffer::ValuePlug const*, IECore::MurmurHash const*) ()
from /software/apps/gaffer/1.2.9.0/cent7.x86_64/cortex/10.4/nuke/13.2/lib/libGaffer.so
18 0x00007fa148ec7f6a in Gaffer::ValuePlug::getValueInternal(IECore::MurmurHash const*) const () from /software/apps/gaffer/1.2.9.0/cent7.x86_64/cortex/10.4/nuke/13.2/lib/libGaffer.so
19 0x00007fa148eba781 in boost::intrusive_ptr<IECore::TypedData<std::vector<IECore::InternedString, std::allocator<IECore::InternedString> > > const> Gaffer::ValuePlug::getObjectValue<IECore::TypedData<std::vector<IECore::InternedString, std::allocator<IECore::InternedString> > > >(IECore::MurmurHash const*) const ()
from /software/apps/gaffer/1.2.9.0/cent7.x86_64/cortex/10.4/nuke/13.2/lib/libGaffer.so
20 0x00007fa148ebab7a in Gaffer::TypedObjectPlug<IECore::TypedData<std::vector<IECore::InternedString, std::allocator<IECore::InternedString> > > >::getValue(IECore::MurmurHash const*) const () from /software/apps/gaffer/1.2.9.0/cent7.x86_64/cortex/10.4/nuke/13.2/lib/libGaffer.so
21 0x00007fa148ebabbe in Gaffer::TypedObjectPlug<IECore::TypedData<std::vector<IECore::InternedString, std::allocator<IECore::InternedString> > > >::setFrom(Gaffer::ValuePlug const*)
() from /software/apps/gaffer/1.2.9.0/cent7.x86_64/cortex/10.4/nuke/13.2/lib/libGaffer.so
22 0x00007fa148e36256 in Gaffer::NameSwitch::compute(Gaffer::ValuePlug*, Gaffer::Context const*) const ()
from /software/apps/gaffer/1.2.9.0/cent7.x86_64/cortex/10.4/nuke/13.2/lib/libGaffer.so
23 0x00007fa148ec5340 in Gaffer::ValuePlug::ComputeProcess::ComputeProcess((anonymous namespace)::ComputeProcessKey const&) ()
from /software/apps/gaffer/1.2.9.0/cent7.x86_64/cortex/10.4/nuke/13.2/lib/libGaffer.so
24 0x00007fa148ece618 in Gaffer::ValuePlug::ComputeProcess::value(Gaffer::ValuePlug const*, IECore::MurmurHash const*) ()
from /software/apps/gaffer/1.2.9.0/cent7.x86_64/cortex/10.4/nuke/13.2/lib/libGaffer.so
25 0x00007fa148ec7f6a in Gaffer::ValuePlug::getValueInternal(IECore::MurmurHash const*) const () from /software/apps/gaffer/1.2.9.0/cent7.x86_64/cortex/10.4/nuke/13.2/lib/libGaffer.so
26 0x00007fa148eba781 in boost::intrusive_ptr<IECore::TypedData<std::vector<IECore::InternedString, std::allocator<IECore::InternedString> > > const> Gaffer::ValuePlug::getObjectValue<IECore::TypedData<std::vector<IECore::InternedString, std::allocator<IECore::InternedString> > > >(IECore::MurmurHash const*) const ()
from /software/apps/gaffer/1.2.9.0/cent7.x86_64/cortex/10.4/nuke/13.2/lib/libGaffer.so
27 0x00007fa148ebab7a in Gaffer::TypedObjectPlug<IECore::TypedData<std::vector<IECore::InternedString, std::allocator<IECore::InternedString> > > >::getValue(IECore::MurmurHash const*) const () from /software/apps/gaffer/1.2.9.0/cent7.x86_64/cortex/10.4/nuke/13.2/lib/libGaffer.so
28 0x00007fa1352c50ca in GafferScene::RenderController::SceneGraph::updateChildren(Gaffer::TypedObjectPlug<IECore::TypedData<std::vector<IECore::InternedString, std::allocator<IECore::InternedString> > > > const*) () from /software/apps/gaffer/1.2.9.0/cent7.x86_64/cortex/10.4/nuke/13.2/lib/libGafferScene.so
29 0x00007fa1352d9754 in GafferScene::RenderController::SceneGraph::update(std::vector<IECore::InternedString, std::allocator<IECore::InternedString> > const&, unsigned int, GafferScene::RenderController::SceneGraph::Type, GafferScene::RenderController*) () from /software/apps/gaffer/1.2.9.0/cent7.x86_64/cortex/10.4/nuke/13.2/lib/libGafferScene.so
30 0x00007fa1352d9e1b in GafferScene::RenderController::SceneGraphUpdateTask::execute() () from /software/apps/gaffer/1.2.9.0/cent7.x86_64/cortex/10.4/nuke/13.2/lib/libGafferScene.so
31 0x00007fa2568544ef in ?? () from /software/apps/nuke/13.2v8/cent7.x86_64/libtbb.so.2
---Type <return> to continue, or q <return> to quit---
32 0x00007fa256851b00 in ?? () from /software/apps/nuke/13.2v8/cent7.x86_64/libtbb.so.2
33 0x00007fa1352bb36e in GafferScene::RenderController::updateInternal(std::function<void (Gaffer::BackgroundTask::Status)> const&, IECore::PathMatcher const*, bool) ()
from /software/apps/gaffer/1.2.9.0/cent7.x86_64/cortex/10.4/nuke/13.2/lib/libGafferScene.so
34 0x00007fa148e3b47b in std::_Function_handler<void (IECore::Canceller const&), Gaffer::ParallelAlgo::callOnBackgroundThread(Gaffer::Plug const*, std::function<void ()>)::{lambda(IECore::Canceller const&)ImageEngine#1}>::_M_invoke(std::_Any_data const&, IECore::Canceller const&) () from /software/apps/gaffer/1.2.9.0/cent7.x86_64/cortex/10.4/nuke/13.2/lib/libGaffer.so
35 0x00007fa148d83dd5 in _ZN3tbb8internal13function_taskIZN6Gaffer14BackgroundTaskC4EPKNS2_4PlugERKSt8functionIFvRKN6IECore9CancellerEEEEUlvE_E7executeEv ()
from /software/apps/gaffer/1.2.9.0/cent7.x86_64/cortex/10.4/nuke/13.2/lib/libGaffer.so
36 0x00007fa2568544ef in ?? () from /software/apps/nuke/13.2v8/cent7.x86_64/libtbb.so.2
37 0x00007fa25684dae4 in ?? () from /software/apps/nuke/13.2v8/cent7.x86_64/libtbb.so.2
38 0x00007fa25684c603 in ?? () from /software/apps/nuke/13.2v8/cent7.x86_64/libtbb.so.2
39 0x00007fa256848846 in ?? () from /software/apps/nuke/13.2v8/cent7.x86_64/libtbb.so.2
40 0x00007fa256848a79 in ?? () from /software/apps/nuke/13.2v8/cent7.x86_64/libtbb.so.2
41 0x00007fa26bed3ea5 in start_thread () from /lib64/libpthread.so.0
42 0x00007fa26b308b0d in clone () from /lib64/libc.so.6
```
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Optimized the LensDistortOp by caching the distortion and doing a simple look-up from within the warp() method instead of recomputing it per channel.
Removed all conditional statements from the tight loops.