fix(glsl-in): support combined sampler2D uniforms#9558
Open
robertream-m42 wants to merge 5 commits into
Open
Conversation
GLSL combined image-sampler types (`sampler2D`, `isampler3D`, `sampler2DShadow`,
…) were previously unrecognized by naga's GLSL frontend, causing a
`NotImplemented('variable qualifier')` error when shaders declared uniforms
with these types.
Changes:
- `types.rs`: add `sampler_parse` to `parse_type`, mirroring `texture_parse`
and `image_parse`. Combined samplers map to the same `TypeInner::Image` as
their `texture*` counterparts. Add `is_combined_sampler` predicate used by
the lexer to emit `CombinedSamplerTypeName` tokens.
- `lex.rs`: emit `CombinedSamplerTypeName(t)` for combined sampler words so the
parser routes them correctly through the type-name path.
- `functions.rs`: handle `sampler2D(tex, samp)` constructor syntax in
`constructor_many`. Records the tex/sampler association via `ctx.samplers`;
shadow variant additionally promotes the image to `ImageClass::Depth`.
- `builtins.rs`: remove the old `MacroCall::Sampler` / `MacroCall::SamplerShadow`
overloads and their `inject_standard_builtins` branches — these are now
unreachable since combined sampler names are lexed as type names, not function
identifiers.
Fixes shader compilation for shaders that declare `uniform sampler2D` variables
and call `texture(sampler, uv)` or `texelFetch(sampler, coord, lod)`.
- Collapse imports and long lines to satisfy rustfmt - Use explicit &(_, expr) pattern in find() closures to avoid match ergonomics flagged by clippy::pattern_type_mismatch Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
find() passes &Item to predicate but returns Option<Item>; map() receives the item directly, not a reference. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
[`Context::new`] is private and not visible to rustdoc from mod.rs; use backtick code instead to pass -D warnings doc check. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
GLSL combined image-sampler types (
sampler2D,isampler3D,sampler2DShadow, …) were unrecognized by naga's GLSL frontend, causing aNotImplemented('variable qualifier')error when shaders declared uniforms with these types.Root cause
parse_typehandledtexture*andimage*type names but had no branch forsampler*combined types. The existinginject_standard_builtinscode tried to handle them as function-call overloads (MacroCall::Sampler/MacroCall::SamplerShadow), which was never reachable from the variable-declaration path.Changes
types.rs: addsampler_parsetoparse_type, mirroringtexture_parseandimage_parse. Combined samplers map to the sameTypeInner::Imageas theirtexture*counterparts. Addis_combined_samplerpredicate used by the lexer.lex.rs: emitCombinedSamplerTypeName(t)for combined sampler words so they route through the type-name path.functions.rs: handlesampler2D(tex, samp)constructor syntax inconstructor_many. Records the association viactx.samplers; shadow variant additionally promotes the image toImageClass::Depth.builtins.rs: remove the now-unreachableMacroCall::Sampler/MacroCall::SamplerShadowoverloads.Test fixtures
sampler-combined-types.frag— uniform declarations for all[i|u]sampler*variantssampler-combined-texture.frag—texture()andtexelFetch()calls through asampler2Duniform🤖 Generated with Claude Code