AcademySoftwareFoundation / AcademySoftwareFoundation/OpenShadingLanguage

OSL fails assert when updating shader group after executing shader when using BatchedRendererServices

Open
#1,800 1 comment 0 reactions 0 assignees View on GitHub
batch shading bug
Dominant language
C++
Stars
2.3k
Forks
414
Avg merge
3d 1h
Merged PRs (30d)
10

Description

In this minimal sample the following assert triggers at the end of main() but in my production code it triggered at the second call to ShaderGroupBegin(). When compiling OSL in Release mode instead of Debug mode, there's seemingly no issues.

### Problem

/src/liboslexec/instance.cpp:75: ~ShaderInstance: Assertion 'm_instops.size() == 0 && m_instargs.size() == 0' failed.

### Steps to Reproduce

g++ -g test.cpp -loslcomp -loslexec -loslnoise -loslquery -lOpenImageIO -lOpenImageIO_Util
./a.out

```
oslmaterial.hpp:

#pragma once
#if OSL_USE_BATCHED
#include
#include
#endif
#include

class OSLMaterial;

class OSLMaterialN : public OSL::BatchedRendererServices<8> {
public:

explicit OSLMaterialN(OSLMaterial& m);

//OIIO::ErrorHandler& errhandler() const { return *m_errhandler; }
/// Turn information at hitpoint into ShaderGlobals for OSL
void globals_from_hit(::OSL::BatchedShaderGlobals<8>& bsg)
{

// Uniform
auto& usg = bsg.uniform;
std::memset(&usg, 0, sizeof(::OSL::UniformShaderGlobals));
usg.raytype = 1; // 1 stands for camera ray?

// Varying
auto& vsg = bsg.varying;

using ::OSL::assign_all;
using ::OSL::Vec3;
}

bool is_overridden_get_inverse_matrix_WmWxWf() const override { return false; };
bool is_overridden_get_matrix_WmWsWf() const override { return false; };
bool is_overridden_get_inverse_matrix_WmsWf() const override { return false; };
bool is_overridden_get_inverse_matrix_WmWsWf() const override { return false; };
bool is_overridden_texture() const override { return false; };
bool is_overridden_texture3d() const override { return false; };
bool is_overridden_environment() const override { return false; };
bool is_overridden_pointcloud_search() const override { return false; };
bool is_overridden_pointcloud_get() const override { return false; };
bool is_overridden_pointcloud_write() const override { return false; };

OSLMaterial& m_sr;

private:

};

/// Custom RendererServices
class OSLMaterial : public OSL::RendererServices {
public:

OSLMaterial();

OIIO::ErrorHandler& errhandler() const { return *m_errhandler; }
/// Turn information at hitpoint into ShaderGlobals for OSL
void globals_from_hit(OSL::ShaderGlobals& sg)
{
sg.P = {0.0f,0.0f,0.0f}; // surface pos
sg.dPdx = {0.0f,0.0f,0.0f};
sg.dPdy = {0.0f,0.0f,0.0f};
sg.dPdz = {0.0f,0.0f,0.0f}; // for volume shading only

sg.I = {0.0f,0.0f,-1.0f}; // incident ray
sg.dIdx = {0.0f,0.0f,0.0f};
sg.dIdy = {0.0f,0.0f,0.0f};

sg.N = {0.0f,0.0f,1.0f}; // shading normal
sg.Ng = {0.0f,0.0f,1.0f}; // true geometric normal

sg.u = 0.5f; // 2D surface parameter u, and its differentials.
sg.dudx = 0.0f;
sg.dudy = 0.0f;
sg.v = 0.5f; // 2D surface parameter v, and its differentials.
sg.dvdx = 0.0f;
sg.dvdy = 0.0f;

// Surface tangents: derivative of P with respect to surface u and v.
sg.dPdu = {1.0f,0.0f,0.0f};
sg.dPdv = {0.0f,1.0f,0.0f};

sg.time = 0.0f;
sg.dtime = 0.001f;

// Velocity vector: derivative of position P with respect to time.
sg.dPdtime = {0.0f,0.0f,0.0f};

// For lights or light attenuation shaders: the point being illuminated (???)
sg.Ps = {0.0f,0.0f,0.0f};
sg.dPsdx = {0.0f,0.0f,0.0f};
sg.dPsdy = {0.0f,0.0f,0.0f};

// Renderer user pointers
sg.renderstate= NULL;
sg.tracedata=NULL;
sg.objdata=NULL;

sg.renderer = this;

sg.raytype = 1; // 1 stands for camera ray?
sg.flipHandedness = 0;
sg.backfacing = 0;

// output closure, needs to be null initialized
sg.Ci=NULL;
}

// ShaderGroupRef storage
std::vector& shaders() { return m_shaders; }
std::vector m_shaders;

OSL::BatchedRendererServices<8>* batched(OSL::WidthOf<8>) override
{
return &m_batch;
}

private:
OSLMaterialN m_batch;
std::unique_ptr m_errhandler;
};
```
```
test.cpp:

#include
#include
#include
#include
#include
#include
#include
#include
#include "oslmaterial.hpp"

using namespace OSL;

OSLMaterialN::OSLMaterialN(OSLMaterial& m) : OSL::BatchedRendererServices<8>(m.texturesys()), m_sr(m)
{
}

OSLMaterial::OSLMaterial() : m_batch(*this)
{
}

// Supported closures and parameters
struct EmptyParams {};

enum ClosureIDs {
EMISSION_ID,
BACKGROUND_ID,
DIFFUSE_ID,
};

struct DiffuseParams {
OSL::Vec3 N;
};

void register_closures(OSL::ShadingSystem* ss)
{
// "Describe the memory layout of each closure type to the OSL runtime"
constexpr int MaxParams = 32;
struct BuiltinClosures {
const char* name;
int id;
OSL::ClosureParam params[MaxParams]; // "upper bound"
};
// Closures with support built into OSL, connected by the 1st string
BuiltinClosures supported[] = {
{ "emission", EMISSION_ID, { CLOSURE_FINISH_PARAM(EmptyParams) } },
{ "background", BACKGROUND_ID, { CLOSURE_FINISH_PARAM(EmptyParams) } },
{ "diffuse", DIFFUSE_ID,
{ CLOSURE_VECTOR_PARAM(DiffuseParams, N),
CLOSURE_FINISH_PARAM(DiffuseParams) } },
};
// Closure registration here enables that type of closure, when executing or compiling a shader
for (const BuiltinClosures& c : supported)
ss->register_closure(c.name, c.id, c.params, nullptr, nullptr);
}

int main(void)
{
// Renderer for OSL
OSLMaterial* oslmat = new OSLMaterial();

OSL::TextureSystem* texturesys = TextureSystem::create();
OSL::ShadingSystem* ss = new OSL::ShadingSystem(oslmat, texturesys, &oslmat->errhandler());
register_closures(ss);

OSL::PerThreadInfo* thread_info;
OSL::ShadingContext* context;
OSL::ShaderGlobals globals;
thread_info = ss->create_thread_info();
context = ss->get_context(thread_info);

// Renderer hit a surface to be shaded, construct ShaderGlobals from information at hitpoint
oslmat->globals_from_hit(globals);

// Create one shader
OSLCompiler compiler;
std::vector options;

// Create a new shader group
oslmat->m_shaders.emplace_back();
oslmat->m_shaders[0] = ss->ShaderGroupBegin (std::to_string(0));
ShaderGroupRef group = oslmat->m_shaders[0];

{
OSL::OSLCompiler compiler;
compiler.compile("matte.osl", options);
}
ss->Shader(*group, "surface", "matte", "My Matte");
ss->ShaderGroupEnd(*group);

// Run the shader that was just created
ss->execute(context, *group, globals);

ShaderGroupRef group2;
{
// Replace shader group
oslmat->m_shaders[0] = ss->ShaderGroupBegin (std::to_string(0));
group2 = oslmat->m_shaders[0];
ss->Shader(*group2, "surface", "matte", "My Matte");
ss->ShaderGroupEnd(*group2);
}

// Run the shader again
oslmat->globals_from_hit(globals);
ss->execute(context, *group2, globals);

ss->release_context(context);
ss->destroy_thread_info(thread_info);

//delete ss;
delete oslmat;

std::cout << "next we fail assert" << std::endl;
return 0;
}
```

### Versions

* OSL branch/version: 1.13.18.0
* OS: Linux
* C++ compiler: g++ 10.2.1
* LLVM version: clang 11.0.1-2
* OIIO version: 2.5.4

Contributor guide

Open the contributing guide

Research direction

Start by reproducing the assertion with test.cpp and the provided oslmaterial.hpp sample on OSL 1.13.18.0. Inspect src/liboslexec/instance.cpp:75 and the ShaderGroupBegin/execute sequence, then verify that replacing and executing the shader group no longer leaves ShaderInstance state that triggers the assertion.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
computer-graphics
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.