Skip to content
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

Free functions for texturing, point clouds, and trace #1852

Merged

Conversation

curtisblack
Copy link
Contributor

@curtisblack curtisblack commented Aug 23, 2024

Description

Provides free function API for:

  • rs_texture
  • rs_texture3d
  • rs_environment
  • rs_get_texture_info
  • rs_get_texture_info_st
  • rs_pointcloud_search
  • rs_pointcloud_get
  • rs_pointcloud_write
  • rs_trace

Each of these match the existing signatures of the renderer services equivalents. A new function rs_trace_get is added to retrieve trace results and is intended to replace getmessage("trace", ...) which is currently not available on GPUs.

The fallback for each of these is to call the existing renderer services functions.

To achieve this, the helper structs TextureOpt, TraceOpt, NoiseOpt are removed from the host-only shading context, and are instead added into the LLVM generated code as stack allocations. These allocations only occur if the shader uses these features.

To allow these free functions to compile on GPUs, the texture per-thread context pointer is currently passed null. Future work will be needed to give this a proper home.

To make the pointcloud functions GPU friendly, they had to modified to not use variadic arguments and to avoid dynamic alloca.

Tests

Checklist:

  • I have read the contribution guidelines.
  • I have updated the documentation, if applicable.
  • I have ensured that the change is tested somewhere in the testsuite (adding new test cases if necessary).
  • My code follows the prevailing code style of this project. If I haven't
    already run clang-format v17 before submitting, I definitely will look at
    the CI test that runs clang-format and fix anything that it highlights as
    being nonconforming.

Signed-off-by: Curtis Black <[email protected]>
Signed-off-by: Curtis Black <[email protected]>
Signed-off-by: Curtis Black <[email protected]>
Signed-off-by: Curtis Black <[email protected]>
Signed-off-by: Curtis Black <[email protected]>
Signed-off-by: Curtis Black <[email protected]>
Signed-off-by: Curtis Black <[email protected]>
Signed-off-by: Curtis Black <[email protected]>
Signed-off-by: Curtis Black <[email protected]>
Signed-off-by: Curtis Black <[email protected]>
Signed-off-by: Curtis Black <[email protected]>
Signed-off-by: Curtis Black <[email protected]>
Signed-off-by: Curtis Black <[email protected]>
Signed-off-by: Curtis Black <[email protected]>
Signed-off-by: Curtis Black <[email protected]>
Signed-off-by: Curtis Black <[email protected]>
Signed-off-by: Curtis Black <[email protected]>
Signed-off-by: Curtis Black <[email protected]>
Signed-off-by: Curtis Black <[email protected]>
Signed-off-by: Curtis Black <[email protected]>
Signed-off-by: Curtis Black <[email protected]>
Signed-off-by: Curtis Black <[email protected]>
Signed-off-by: Curtis Black <[email protected]>
Signed-off-by: Curtis Black <[email protected]>
Signed-off-by: Curtis Black <[email protected]>
Signed-off-by: Curtis Black <[email protected]>
Signed-off-by: Curtis Black <[email protected]>
Signed-off-by: Curtis Black <[email protected]>
Signed-off-by: Curtis Black <[email protected]>
Signed-off-by: Curtis Black <[email protected]>
Signed-off-by: Curtis Black <[email protected]>
Signed-off-by: Curtis Black <[email protected]>
Signed-off-by: Curtis Black <[email protected]>
Signed-off-by: Curtis Black <[email protected]>
@lgritz
Copy link
Collaborator

lgritz commented Sep 18, 2024

@curtisblack Can you please resolve the merge conflict so we can run the CI?

Signed-off-by: Curtis Black <[email protected]>
Signed-off-by: Curtis Black <[email protected]>
float dsdy, float dtdy, int nchannels, float* result,
float* dresultds, float* dresultdt, OSL::ustringhash* errormessage)
{
#ifndef __CUDA_ARCH__
Copy link
Contributor

@AlexMWells AlexMWells Oct 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use OSL_HOST_RS_BITCODE macro to wrap casts to ShaderGlobals and accesss to RendererServices only happen on host bitcode.

{
#ifndef __CUDA_ARCH__
ShaderGlobals* sg = (ShaderGlobals*)oec;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we use get_sg(oec), means duplicating or moving get_sg.

{
#ifndef __CUDA_ARCH__
ShaderGlobals* sg = (ShaderGlobals*)oec;
ShadingSystemImpl& shadingsys(sg->context->shadingsys());
if (shadingsys.no_pointcloud()) // Debug mode to skip pointcloud expense
return 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does anyone need/use the no_pointcloud option?
We could support it via the ShadingStateUniform, but don't want to if there are no use cases for it.

@@ -379,89 +393,76 @@ RendererServices::pointcloud_write(ShaderGlobals* /*sg*/, ustringhash filename,

namespace pvt {

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extract osl_point_* to oppointcould.cpp so bitcode compilation doesn't need to see the implementation above.

#ifndef __CUDA_ARCH__
auto sg = (OSL::ShaderGlobals*)ec;
return sg->renderer->texture(filename, texture_handle, texture_thread_info,
options, sg, s, t, dsdx, dtdx, dsdy, dtdy,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's not have a customization point call a customization point, instead lets take the default implementation of RendererServices::texture that utilizes a texuresys() and put it here as example for host (or device when OIIO texture sys free functions are fully available).

@AlexMWells
Copy link
Contributor

@lgritz lets go ahead and get his merged. We have some cleanup ideas, but it all builds on top of this and CI is green!

Copy link
Contributor

@AlexMWells AlexMWells left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@lgritz lgritz merged commit 48c0b16 into AcademySoftwareFoundation:main Oct 8, 2024
21 of 22 checks passed
lgritz added a commit to lgritz/OpenShadingLanguage that referenced this pull request Oct 11, 2024
lgritz pushed a commit that referenced this pull request Oct 29, 2024
The recent free function change (#1852) put the OptiX paths in testrender and testshade in a non-working state (issue #1894). The bitcode for the free functions wasn't plumbed through for OptiX. This PR rolls the free function definitions into the existing "rendlib" bitcode and PTX. This is somewhat of a stopgap fix; eventually we'll want to handle the free function more explicitly.

Fixes #1894 

Signed-off-by: Tim Grant <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants