Shading state uniform #1585
Shading state uniform #1585
Conversation
There was a problem hiding this comment.
What's not clear to me from these changes is whether we have to expose the opaque ssu pointer to renderers in the ShaderGlobals.
It looks like all the uses here could be replaced with an additional void* arg to the shadeops which was the ssu and we could code-gen submitting that pointer to the shadeop, and then it's never exposed on the ShaderGlobals.
An additional question is should it really live on the shading system? Or more specifically, would it be useful to have it vary across multiple shading contexts that are used with a single shading system? If the answer is "yes" or "maybe", then perhaps we should think of this as the portion of the shading context that must be guaranteed to be in shading device memory, and bifurcate the context into global/on-device?
|
@sfriedmapixar if a renderer wished to have a multiple ShadingStateUniform, it could. Intent is current "state" of the ShadingSystem could be exported/copied to a device buffer. When launching a kernel, that pointer would be passed through. Then up to the renderer to manage ShadingStateUniform copies and their association with ShadingNetwork kernels(or the device side code they produce). This PR is to make the change for the CPU side (so not GPU side only), and we don't quite have a "kernel launch" or even range based execution (although we left a hint of a new potential ranged based execute method in this PR). As we don't have a place to pass in a ShadingStateUniform pointer yet, and even if we did we still need to stuff it someplace where OSL library functions can access it, and today that answer is ShaderGlobals. ShaderGlobals just happens to be the top level pointer being passed into and through OSL shader functions, and most library functions. ShaderGlobals already contains alot of pointers that aren't really shader globals. What we "imagine" happening here is a future PR establishes a new way for a renderer to provide "real" shader global values (position, normal, etc.), and then the existing ShaderGlobals structure has those shader global values removed. This would leave ShaderGlobals with a mix of UniformState, PerShadeState, and RendererState poointers. It wouldn't really be ShaderGlobals anymore, but more of a "Context". Unfortunately ShadingContext already exists as a CPU side object (we are working on reducing its direct responsibilities). But imagine ShadingContext gets renamed LegacyShadingContext, and ShaderGlobals (after the globals are removed) becomes ExecContext or something similar. |
|
Is this still awaiting further changes from your side? |
steenax86
force-pushed
the
shading_state_uniform
branch
from
5484bf1
to
dd21486
Compare
for accessing the ShadingSystem and consequently the ColorSystem. In its current form it is not device-friendly. We introduce a self-contained pvt::ShadingStateUniform that maintains a proper state and is independent of the ShadingContext and ShadingSystem. We add an opaque pointer to ShadingStateUniform in the ShaderGlobals structure. Update uses of ColorSystem in opcolor.cpp to go through ShaderGlobals::shadingStateUniform. Also update uses of m_commonspace_synonym and m_unknown_coordsys_error in opmatrix.cpp to go through ShaderGlobals::shadingStateUniform. NOTE: The ShadingStateUniform is private and an OSL implementation detail and does not change the public interface other than adding an opaque pointer to ShaderGlobals. Signed-off-by: Steena Monteiro <steena.monteiro@intel.com>
…niform. Signed-off-by: Steena Monteiro <steena.monteiro@intel.com>
Signed-off-by: Steena Monteiro <steena.monteiro@intel.com>
Moved ShadingStateUniform closer to its use in osl_prepend_matrix_from Signed-off-by: Steena Monteiro <steena.monteiro@intel.com>
…obals in rend_lib.h. Needed for proper working. Signed-off-by: Steena Monteiro <steena.monteiro@intel.com>
Signed-off-by: Steena Monteiro <steena.monteiro@intel.com>
Adjusted batched struct data layout check in codegen. Signed-off-by: Steena Monteiro <steena.monteiro@intel.com>
Signed off-by: Steena Monteiro <steena.monteiro@intel.com> Signed-off-by: Steena Monteiro <steena.monteiro@intel.com>
…eaders from liboslexec Signed-off-by: Steena Monteiro <steena.monteiro@intel.com>
steenax86
force-pushed
the
shading_state_uniform
branch
from
dd21486
to
476f9ec
Compare
|
It has been ready but stuck on CI issues. It should be ok now. |
Description
Reduce dependency on ShadingContext and ShadingSystem objects for accessing the ShadingSystem and consequently the ColorSystem. In its current form it is not device-friendly.
We introduce a self-contained pvt::ShadingStateUniform that maintains a proper
state and is independent of the ShadingContext and ShadingSystem. We add an opaque pointer to ShadingStateUniform in the ShaderGlobals structure.
Update uses of ColorSystem in opcolor.cpp to go through ShaderGlobals::shadingStateUniform. Also update uses of m_commonspace_synonym and m_unknown_coordsys_error in opmatrix.cpp to go through ShaderGlobals::shadingStateUniform.
NOTE: The ShadingStateUniform is private and an OSL implementation detail and
does not change the public interface other than adding an opaque pointer to
ShaderGlobals.
Expect GPUs to have to export ShadingStateUniform to a GPU buffer to use with multiple kernel launches. Left for future PR.
Tests
All existing tests pass.
Checklist: