Tint: extend kAllowStructMemberSizeMismatch to the struct total-size check - #4
Open
octopusburrow wants to merge 1 commit into
Open
Tint: extend kAllowStructMemberSizeMismatch to the struct total-size check#4octopusburrow wants to merge 1 commit into
octopusburrow wants to merge 1 commit into
Conversation
…check Vendored as patches/0007 per the thirdparty/tint/patches/ convention. This is a Group B (Specialization Constants) patch, completing the capability this fork introduced in 0002 and extended in 0003/0005: Godot's specialization constants change effective struct and array sizes at runtime, so a spec-constant-sized array leaves the containing struct's Size() at its unfolded value and the validator's TOTAL-size check (str->Size() < cur_offset) fires on IR that the fork's own 0002 rationale already classifies as valid-by-construction. 0002 relaxed the per-MEMBER size check for this exact producer; this patch applies the same capability to the struct-total check a few lines below it. Without this, the Mobile renderer's scene shader (depth shadow atlas + sampler UBO with spec-constant-sized arrays) fails Tint validation and every lit 3D scene renders black on Dawn/Windows, including this fork's own sample projects. Unshaded scenes are unaffected, which is why the gap survived: the fork's Windows TODO note and this failure are the same bug. Layer choice: an alternative would be changing Godot's shader generation so emitted SPIR-V never carries the mismatch, touching no vendored code. This fork already faced that choice for the member-size case and chose validator relaxation, three times (0002, 0003, 0005, documented as Group B in patches/README.md); this patch follows the established architecture rather than opening a second front. If Group B is ever re-architected toward shader-gen fixes, this check belongs in that migration with the rest of the group. Co-Authored-By: Claude Fable 5 <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
1 participant
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.
Without this, SPIR-V→WGSL conversion rejects the Mobile renderer's scene shaders and the
WebGPU rendering path produces no pixels. This completes the exemption your patch 0002
introduced:
kAllowStructMemberSizeMismatch(your Group B — specialization constants)exempts the per-member size check for Godot's runtime-specialized shaders, and this
extends the same capability to the whole-struct total-size check that still rejected the
same shaders.
Same producer, measured: instrumenting both check sites shows one struct,
InstanceDataBuffer_1_1_a64(declared size 16, member extent 208 — a spec-constant-sizedtrailing array folded at conversion), tripping the member-size check your 0002 rescues
AND the total-size check this patch rescues. The two checks reject the identical
reflection pattern; 0002 without this leaves the conversion failing one validator later.
Scope disclosure: like patch 0002, the capability is granted to all SPIR-V-read modules,
so this widens what that already-global flag suppresses — a struct whose declared total
size disagrees with its members' extent now converts instead of failing validation. The
check is validator-only (no codegen change), so correct shaders are unaffected; the cost
is a later, less-obvious failure for genuinely malformed reflection data. A narrower
form (exempt only specialization-constant-sized trailing arrays) would be preferable if
Tint's IR exposes enough to detect it at that point; happy to iterate if you'd rather
gate it tighter.
Verified by reverting the patch: zero pixels on the RD/WebGPU renderers (forward_plus
and mobile) on desktop Chrome; with it, both render correctly.