search

RenderKit / ospray

An Open, Scalable, Portable, Ray Tracing Based Rendering Engine for High-Fidelity Visualization
http://ospray.org
Apache License 2.0
1k stars 182 forks source link

Segfault in cornell box example #370

Closed paulmelis closed 4 years ago

paulmelis commented 4 years ago

With the new ospExamples I get a segfault when run with -s cornell_box:

paulm@cmstorm 11:36:~/c/ospray-git/apps/ospray_testing/builders$ gdb ~/software/ospray-superbuild-git/bin/ospExamples 
GNU gdb (GDB) 8.3
...
Reading symbols from /home/paulm/software/ospray-superbuild-git/bin/ospExamples...
(gdb) run -s cornell_box
Starting program: /home/paulm/software/ospray-superbuild-git/bin/ospExamples -s cornell_box
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/usr/lib/libthread_db.so.1".
[New Thread 0x7fffeb189700 (LWP 26170)]
[New Thread 0x7fffead88700 (LWP 26171)]
[New Thread 0x7fffea987700 (LWP 26172)]
[New Thread 0x7fffea586700 (LWP 26173)]
[New Thread 0x7fffea185700 (LWP 26174)]
[New Thread 0x7fffe9d84700 (LWP 26175)]
[New Thread 0x7fffe9983700 (LWP 26176)]

Thread 8 "ospExamples" received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fffe9983700 (LWP 26176)]
0x00007ffff6ca5695 in samplePath___REFs_5B__c_vyPathContext_5D_REFs_5B_vyPathState_5D_REFs_5B_vyRay_5D_REFs_5B_vyScreenSample_5D_avx2 (pathContext=..., pathState=..., ray=..., sample=...) at PathSampler.ispc:191
191           pathVertex.bsdf = m->getBSDF(m, &ctx, pathVertex.dg, ray, pathState.currentMedium);
(gdb) bt
#0  0x00007ffff6ca5695 in samplePath___REFs_5B__c_vyPathContext_5D_REFs_5B_vyPathState_5D_REFs_5B_vyRay_5D_REFs_5B_vyScreenSample_5D_avx2 (pathContext=..., pathState=..., ray=..., sample=...) at PathSampler.ispc:191
#1  0x00007ffff6c785a6 in PathTraceIntegrator_Li___un_3C_s_5B__c_unPathTracer_5D__3E_un_3C_s_5B__c_unWorld_5D__3E_un_3C_s_5B_unFrameBuffer_5D__3E_CvyuCvyuREFs_5B__c_vyvec2f_5D_REFs_5B_vyRay_5D_un_3C_s_5B_vyLDSampler_5D__3E_un_3C_s_5B_vyRandomSampler_5D__3E_avx2 (self=<optimized out>, world=<optimized out>, fb=<optimized out>, ix=..., iy=..., pixel=..., ray=..., ldSampler=0x7fffe99555c0, randomSampler=0x7fffe99551c0) at PathTracer.ispc:86
#2  0x00007ffff6c7a573 in PathTracer_renderPixel___un_3C_s_5B_unPathTracer_5D__3E_un_3C_s_5B_unFrameBuffer_5D__3E_un_3C_s_5B_unCamera_5D__3E_un_3C_s_5B_unWorld_5D__3E_CvyuCvyuCvyuavx2 (self=<optimized out>, 
    fb=<optimized out>, camera=<optimized out>, world=<optimized out>, ix=..., iy=..., accumID=...) at PathTracer.ispc:135
#3  PathTracer_renderTileJob___un_3C_s_5B_unPathTracer_5D__3E_un_3C_s_5B_unFrameBuffer_5D__3E_un_3C_s_5B_unCamera_5D__3E_un_3C_s_5B_unWorld_5D__3E_REFs_5B_unTile_5D_uniavx2 (self=<optimized out>, fb=<optimized out>, 
    camera=<optimized out>, world=<optimized out>, tile=..., taskIndex=<optimized out>) at PathTracer.ispc:174
#4  0x00007ffff6bdbf24 in Renderer_renderTile () from /home/paulm/software/ospray-superbuild-git/bin/../lib64/libospray_module_ispc.so
#5  0x00007ffff6aa10a8 in ospray::Renderer::renderTile (this=0x5555555a89f0, fb=0x555555814e10, camera=0x5555559a4470, world=0x5555555ab8e0, perFrameData=0x0, tile=..., jobID=0)
    at /home/paulm/c/ospray-git/ospray/render/Renderer.cpp:83
#6  0x00007ffff6a9e0c9 in ospray::LocalTiledLoadBalancer::<lambda(int)>::<lambda(size_t)>::operator()(size_t) const (__closure=0x7fffe9956b10, tIdx=0) at /home/paulm/c/ospray-git/ospray/render/LoadBalancer.cpp:63
#7  0x00007ffff6a9fd88 in tbb::internal::parallel_for_body<ospray::LocalTiledLoadBalancer::renderFrame(ospray::FrameBuffer*, ospray::Renderer*, ospray::Camera*, ospray::World*)::<lambda(int)>::<lambda(size_t)>, long unsigned int>::operator()(const tbb::blocked_range<unsigned long> &) const (this=0x7fffeb737a60, r=...) at /home/paulm/software/ospray-superbuild-git/include/tbb/parallel_for.h:174
#8  0x00007ffff6a9faed in tbb::interface9::internal::start_for<tbb::blocked_range<long unsigned int>, tbb::internal::parallel_for_body<ospray::LocalTiledLoadBalancer::renderFrame(ospray::FrameBuffer*, ospray::Renderer*, ospray::Camera*, ospray::World*)::<lambda(int)>::<lambda(size_t)>, long unsigned int>, const tbb::auto_partitioner>::run_body(tbb::blocked_range<unsigned long> &) (this=0x7fffeb737a40, r=...)
    at /home/paulm/software/ospray-superbuild-git/include/tbb/parallel_for.h:112
...

I don't see something obviously wrong in apps/ospray_testing/builders/CornellBox.cpp (but what do I know about using the API correctly :wink:). It works with the scivis renderer, so perhaps it's materials related? The other examples work for me, e.g. random spheres.

paulmelis commented 4 years ago

Hmm, so even though ospExamples -s boxes is working, when using the code below to construct the same boxes scene in my own code I get the same segfault, but only with the pathtracer (the scivis renderer works)

    auto builder = ospray::testing::newBuilder("boxes");
    ospray::testing::setParam(builder, "rendererType", state->renderer.c_str());
    ospray::testing::commit(builder);

    auto cpp_group = ospray::testing::buildGroup(builder);
    ospray::testing::release(builder);
    cpp_group.commit();

    OSPGroup group = cpp_group.handle();
    ospRetain(group);
    instances.push_back(std::make_pair(group, glm::mat4(1.0f)));
paulmelis commented 4 years ago

Shouldn't this

https://github.com/ospray/ospray/blob/2da4288ee71328c7cea18f60e25d442817f68d20/apps/ospray_testing/builders/Boxes.cpp#L86

be similar to this (i.e. set as data array instead of object)?

https://github.com/ospray/ospray/blob/2da4288ee71328c7cea18f60e25d442817f68d20/apps/ospray_testing/builders/Boxes.cpp#L79-L81

paulmelis commented 4 years ago

Guess not, as there's also https://github.com/ospray/ospray/blob/2da4288ee71328c7cea18f60e25d442817f68d20/apps/ospray_testing/builders/Boxes.cpp#L91

jeffamstutz commented 4 years ago

Yes, I caught this yesterday as I've been getting started porting our regression tests to the new ospray_testing: the cornell_box builder didn't call the base builder commit(), so it didn't get the right renderer type to make its materials.

I also made some more improvements to the C++ wrappers, so I'll just get that merged/pushed today without waiting on the regression test port (which will catch these things!).

jeffamstutz commented 4 years ago

Latest fixes/updates are now on release-2.0.x.

jeffamstutz commented 4 years ago

Also note that cpp::Data() now has additional constructors for std::vector and std::array, where the element type (OSPDataType) is inferred at compile-time transparently.

So this: https://github.com/ospray/ospray/blob/2da4288ee71328c7cea18f60e25d442817f68d20/apps/ospray_testing/builders/Boxes.cpp#L79-L81

...becomes just this:

https://github.com/ospray/ospray/blob/b1531f8efc09e05fc65a84a7a717d2ea04675eeb/apps/ospray_testing/builders/Boxes.cpp#L78

paulmelis commented 4 years ago

Great, I'll update locally!

With respect to the C++ wrappers, I'm still using the C API at the moment, but it's starting to become more and more attractive to switch to the C++ one it seems.

jeffamstutz commented 4 years ago

With respect to the C++ wrappers, I'm still using the C API at the moment, but it's starting to become more and more attractive to switch to the C++ one it seems.

That's the goal! ;)

paulmelis commented 4 years ago

Is there anything specific to watch out for when mixing the C++ and C APIs? Would something like this work as expected?

OSPGroup group = cpp_group.handle();
ospRetain(group);

I'm still seeing the original segfault from above in my code, even though it is now gone in ospExamples

jeffamstutz commented 4 years ago

Retaining a group created by ospray_testing should be all that's needed to keep it alive. I wonder if there's something else going on?

jeffamstutz commented 4 years ago

And is it still only a segfault with the pathtracer? Or are there other situations that it fails?

paulmelis commented 4 years ago

I only see it with the path tracer. Here's a reproduction based on ospTutorial.cpp, replacing the scene geometry setup with the piece above for the builder and "boxes": builder.zip

jeffamstutz commented 4 years ago

Well this is a bit embarrassing: the issue isn't the C++ wrappers, but rather that the rendererType parameter is set as a const char * and not a std::string. We didn't implement quite the exhaustive type compatibility with ospSetParam() in ospray_testing. Wrapping the value you set in a string constructor works: std::string(renderer_type).

jeffamstutz commented 4 years ago

And by embarrassing, I meant us! The ospray_testing API silently doing the wrong thing isn't good here.

jeffamstutz commented 4 years ago

However, I'm not sure how to better address it. Basically the T value passed as a parameter must exactly match what the underlying Builder queries: a trade off for having flexible parameter types is that it is easier to get them wrong. :/

paulmelis commented 4 years ago

It gets worse, the original line I use in my code is this :grin:

ospray::testing::setParam(builder, "rendererType", state->renderer.c_str());

But okay, that's an easy fix