AcademySoftwareFoundation / OpenShadingLanguage

Advanced shading language for production GI renderers
BSD 3-Clause "New" or "Revised" License
2.07k stars 350 forks source link

Transforms not working (everything is identity) in testshade --optix #1264

Open anderslanglands opened 3 years ago

anderslanglands commented 3 years ago

Problem

Transforms don't appear to work in optix. I'm running the following shader through testshade:

surface test_add(output color Cout = 0) {
    point Po = transform("myspace", vector(1, 1, 1));
    if (u < 0.1 && v < 0.1) {
        printf("Po = %f %f %f\n", Po.x, Po.y, Po.z);
    }
    Ci = diffuse(N) * color(Po) + emission() * u;
}

EDIT: I've just realised a my shader is passing a vector to transform() and expecting a point back again, which seems wrong. In any case, if I change that to point(1,1,1) instead of vector the behaviour on both CPU and GPU is unchanged.

Expected behavior: This should print out:

Po = 1.0 0.5 1.0

and indeed it does when running on the CPU

Actual behavior:
Prints:

Po = 1.0 1.0 1.0

when running with --optix

Steps to Reproduce

  1. Compile the shader above
  2. Run testshade test_add --res 16 16
  3. Run testshade test_add --res 16 16 --optix

Versions

anderslanglands commented 3 years ago

Based on today's discussion threads I assume this simply has not been implemented yet. If someone can give me a broad overview of what needs to happen to make this work I'm more than happy to jump in and do it myself.

lgritz commented 3 years ago

On the CPU side, this kind of thing turns into a call to RendererServices::get_matrix() and get_inverse_matrix to retrieve the named matrix. In essence, the renderer supplies the implementation of retrieving named matrices in the form of a RendererServices subclass whose methods are functions to call to perform a number of tasks that are renderer specific or that require knowledge that's internal to the renderer.

On the GPU side... currently... this is implemented by the renderer here as well, but by supplying a Cuda implementation of osl_get_matrix and osl_get_inverse_matrix functions that will be called. You can see the implementation used by testrender and testshade, which is at this moment just a placeholder that doesn't do anything useful.

So your renderer would need to have its own rend_lib.cu that it supplies, with a more complete implementation.

You're running right up against the bleeding edge right now. I think that where we want to end up eventually is that instead of having RendererServices on the GPU side, each of the methods has an equivalent that the renderer supplies to OSL as either Cuda source or precompiled (?), via some API we have not yet established.

Each and every bit of functionality currently implemented in RendererServices will need some analysis and plan for how to implement its equivalent for the OptiX/Cuda path.

This is much more than I can do alone, and if you want to take a stab at this, it would be great. I think the transform / matrix retrieval bits are not super hard and are as reasonable as anyplace to start fleshing this out. There are a few decisions we should hash out. For example, is the right approach very closely analogous to RendererServices, i.e., should the renderer give OSL the Cuda function as a black box and OSL itself just needs to make the call when it needs a named matrix? Or should OSL house the guts of the implementation but maybe what the renderer supplies to OSL is just a list of the name/matrix pairs, which are downloaded to the Cuda side as some kind of global data? Or something else? And what should the API look like to communicate this to the ShadingSystem?

You're more than welcome to join our fortnightly call on Thursday, we can make this one of the main topics to discuss and maybe can emerge with a plan for how to tackle it. I think everybody has been hesitant to attack the RendererServices aspects of things on the GPU, but maybe if we just pick one bit of functionality to try to prototype first, a bunch of our design questions will get answered in the process and show the way to how the rest of them should be done. The whole GPU path is currently designated as "experimental", so I think it's fine to plow ahead and prototype things even knowing that we may decide to rewrite it a couple times as we get more experience. We are not beholden to any promises of ongoing compatibility quite yet, the stakes are low and even small steps ahead on the GPU side is still positive motion.

anderslanglands commented 3 years ago

Hi Larry, it will be difficult for me to make a call this Thursday but I should be able to do the next one. What time do you do them (I’m in NZ)?

I’ll take a deeper look into the current structure in the intervening time.

On Wed, 14 Oct 2020 at 20:31, Larry Gritz notifications@github.com wrote:

On the CPU side, this kind of thing turns into a call to RendererServices::get_matrix() and get_inverse_matrix to retrieve the named matrix. In essence, the renderer supplies the implementation of retrieving named matrices in the form of a RendererServices subclass whose methods are functions to call to perform a number of tasks that are renderer specific or that require knowledge that's internal to the renderer.

On the GPU side... currently... this is implemented by the renderer here as well, but by supplying a Cuda implementation of osl_get_matrix and osl_get_inverse_matrix functions that will be called. You can see the implementation used by testrender and testshade https://github.com/imageworks/OpenShadingLanguage/blob/master/src/testrender/cuda/rend_lib.cu#L357, which is at this moment just a placeholder that doesn't do anything useful.

So your renderer would need to have its own rend_lib.cu that it supplies, with a more complete implementation.

You're running right up against the bleeding edge right now. I think that where we want to end up eventually is that instead of having RendererServices on the GPU side, each of the methods has an equivalent that the renderer supplies to OSL as either Cuda source or precompiled (?), via some API we have not yet established.

Each and every bit of functionality currently implemented in RendererServices will need some analysis and plan for how to implement its equivalent for the OptiX/Cuda path.

This is much more than I can do alone, and if you want to take a stab at this, it would be great. I think the transform / matrix retrieval bits are not super hard and are as reasonable as anyplace to start fleshing this out. There are a few decisions we should hash out. For example, is the right approach very closely analogous to RendererServices, i.e., should the renderer give OSL the Cuda function as a black box and OSL itself just needs to make the call when it needs a named matrix? Or should OSL house the guts of the implementation but maybe what the renderer supplies to OSL is just a list of the name/matrix pairs, which are downloaded to the Cuda side as some kind of global data? Or something else? And what should the API look like to communicate this to the ShadingSystem?

You're more than welcome to join our fortnightly call on Thursday, we can make this one of the main topics to discuss and maybe can emerge with a plan for how to tackle it. I think everybody has been hesitant to attack the RendererServices aspects of things on the GPU, but maybe if we just pick one bit of functionality to try to prototype first, a bunch of our design questions will get answered in the process and show the way to how the rest of them should be done. The whole GPU path is currently designated as "experimental", so I think it's fine to plow ahead and prototype things even knowing that we may decide to rewrite it a couple times as we get more experience. We are not beholden to any promises of ongoing compatibility quite yet, the stakes are low and even small steps ahead on the GPU side is still positive motion.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/imageworks/OpenShadingLanguage/issues/1264#issuecomment-708218057, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAOYQXNO7L5ZPVV2J7C6YJTSKVHWRANCNFSM4SK5FPPA .

lgritz commented 3 years ago

2pm Pacific time, every second Thursday