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

Current workgraph recursion restrictions are too limiting #342

Open
DBouma opened this issue Oct 17, 2024 · 0 comments
Open

Current workgraph recursion restrictions are too limiting #342

DBouma opened this issue Oct 17, 2024 · 0 comments

Comments

@DBouma
Copy link

DBouma commented Oct 17, 2024

From the workgraph spec:
The graph is acyclic, with one exception: a node can output to itself. There is a depth limit of 32 including recursion.

When writing a path tracer for example in a workgraph, these limitations quickly turn into a substantial issue as it now forces the user to immediately worry about how to get around these limitations.

Consider the following simple pattern:

  1. Primary rays origins and directions are generated from broadcast node A
  2. Rays are traced in thread node B
  3. Results are shaded, and new ray origins and directions are generated in thread node C
  4. Next event estimation is traced in thread node D
  5. New rays are traced in thread node B
  • repeat steps 3 to 5 for every bounce

In this simple case, doing multiple bounces in a single workgraph requires the user to manually write out or generate a lot of duplicate code.
For example just generating the hit shaders for every bounce with a macro results in something along the lines of this;

#define STRINGIZE_NX(A) #A
#define STRINGIZE(A) STRINGIZE_NX(A)

#define HIT_SHADING_VARIATION(NodeIndex, functionName, neeIndex, bounceIndex, nextBounceIndex)  \                                                          
[Shader("node")]                 \
[NodeLaunch("thread")]                      \
[NodeID("hitShading" STRINGIZE(bounceIndex), NodeIndex)]                   \
void functionName##NodeIndex##neeIndex##bounce##bounceIndex##hitShading(                  \
        ThreadNodeInputRecord<HitShadingInput> inputData,                                           \
        [NodeID("traceNee", 0)]                          \
        [MaxRecords(1)]                              \
        [NodeArraySize(2)]                                 \
        NodeOutputArray<NeeTraceInput> neeNodes,        \
        [NodeID("killPath", 0)]                    \
        [MaxRecords(1)]         \
        NodeOutput<KillPathInput> killedPaths,                   \
        [NodeID("tracePathSegment" STRINGIZE(nextBounceIndex), 0)]            \
        [MaxRecords(1)]                \
        NodeOutput<TraceRaysInput> newSegmentToTrace               \
    ) {  \
    .... shading code              \                                            
}

#define ALL_BOUNCES_HIT_SHADER_DECL() \
    HIT_SHADING_VARIATION(DEFAULT_SHADING_RECORD, shadeHitDefault, DEFAULT_INDEX, 0, 1)\
    HIT_SHADING_VARIATION(DEFAULT_SHADING_RECORD, shadeHitDefault, DEFAULT_INDEX, 1, 2)\ 
    HIT_SHADING_VARIATION(DEFAULT_SHADING_RECORD, shadeHitDefault, DEFAULT_INDEX, 2, 3)\ 
    HIT_SHADING_VARIATION(DEFAULT_SHADING_RECORD, shadeHitDefault, DEFAULT_INDEX, 3, 4) 
... etc

You can quickly see how this is not elegant at all, generating the code externally isn't really ideal either as it feels like there should be something possible here in order to reuse nodes / allow recursion from node C > B for example.

Additionally writing a path tracer this way also quickly hits the current limit of 32, as each bounce already visits a minimum of 3 nodes, which quickly limits how many bounces a path can do, especially when additional logic is inserted between stages.

So to summerize;

  • We'd like the ability to do recursion to different nodes
  • The depth limit of 32 is too low
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Triaged
Development

No branches or pull requests

3 participants