[Shaders] `stage compose` supplied from a nested composition: which rule for the mixer?
Nobody has claimed this yet.
- Dominant language
- C#
- Stars
- 7.8k
- Forks
- 1.2k
- Avg merge
- 2d 17h
- Merged PRs (30d)
- 49
Description
Is your feature request related to a problem? Please describe.
The new mixer promotes any shader declaring stage members to the root, stage compose included. The value supplied for that composition does not follow: it stays in the Compositions dictionary of the node that supplied it. Supplied at the root it works (CompositionArray1). Supplied by a nested effect, the merge fails with No composition was supplied for 'AttributeSamplers' ... while merging the mixin node '<root>' (the named error added in #3382).
Case in the engine: Stride.Voxels. MarchAttributes declares stage compose IVoxelSampler AttributeSamplers[]. LightVoxelEffect supplies it, and is itself composed by the forward effect at environmentLights[i]. The marchers that sample through it inherit MarchAttributes two levels further down (Marcher.diffuseMarcher.environmentLights[i], specularMarcher.environmentLights[i]). A light renderer never controls the root effect, so "supply it at the root" is not something a light can do.
#3382 carried two commits that made this work (4df2f3d: hoist the value with the declaring shader; 6a90900: keep the supply path so resource keys get the names the engine composes). They were removed from the PR at @xen2's request so the rest can merge; this issue is the follow-up.
Describe the solution you'd like
Rules proposed by @xen2 in the #3382 review:
- a
stage composecan be declared anywhere (as today); - its value can only be supplied at the root. Supplying it from a nested node is an error, in
ProcessCompositions:
if (isSupplied && (variable.Flags & VariableFlagsMask.Stage) != 0 && !isRoot)
log.Error($"'{variableName}' is a `stage compose` declared by '{shaderName}', so it is one slot "
+ $"for the whole effect and must be supplied at the root. It was supplied at '{compositionPath}'.");
Consequences:
Stride.VoxelsdropsstageonAttributeSamplersand supplies the sampler to every marcher composition that uses it (each gets its own resource bindings, so the C# applies the sampler parameters once per marcher path).- Other
stage composeusers to check:LightShaftsShader.lightGroup,DirectLightGroupArray.directLightGroups[],EnvironmentLightArray.environmentLights[]. Those are supplied by the root effect today.
Describe alternatives you've considered
Hoist the value with the declaring shader (the two dropped commits). It needs a rule for two different values supplied for one stage slot (error, or first wins), and the supply path has to travel with the value so the resource keys keep the names the engine composes (...AttributeSamplers[0].environmentLights[2], not ...AttributeSamplers[0]). It is more state in the mixer, and the open points in review were the ReferenceEquals check and whether to fail or concatenate.
Additional context
- Review thread: #3382,
ShaderMixer.ShaderSourceEvaluator.cs. - Tests that covered the hoist and were removed with the commits:
CompositionArrayStageFromNested,StageCompositionSuppliedFromNestedKeepsItsSupplyPath(both in the PR history). - Question for the rule: is a shared composition below a light, which is always nested, meant to be impossible with
stage compose? If so, what is the intended way for a light's nested compositions to share one sampler?
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in ShaderMixer.ShaderSourceEvaluator.cs, specifically ProcessCompositions, and review the #3382 discussion and its removed commits. Check the Stride.Voxels usages and the other listed stage compose values, then inspect the removed CompositionArrayStageFromNested and StageCompositionSuppliedFromNestedKeepsItsSupplyPath tests. Done means the agreed nested-supply rule is implemented and affected compositions follow it without breaking resource paths.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp
- Domain
- computer-graphics, game-dev
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100