[naga] Support dynamic indexing of by-value matrices #6362
Conversation
This comment was marked as duplicate.
This comment was marked as duplicate.
naga/src/back/spv/writer.rs
Outdated
| let element_pointer_type_id = match element_ty { | ||
| LookupType::Handle(handle) => { | ||
| self.get_pointer_id(ir_types, handle, spirv::StorageClass::Function)? | ||
| } | ||
| LookupType::Local(local_type) => { | ||
| self.get_local_pointer_id(local_type, spirv::StorageClass::Function) | ||
| } | ||
| }; |
There was a problem hiding this comment.
Should we pack this in one single get_pointer_id method?
BTW get_pointer_id never returns Err.
There was a problem hiding this comment.
I think we should update get_pointer_id and address the TODO that's currently in get_local_pointer_id.
There was a problem hiding this comment.
address the TODO that's currently in get_local_pointer_id
This is hard as we cannot pack LookupType in pointer (due to infinite struct size), except if we box it. Another alternative is to simply put u32 for local type, but this could get pretty chaotic.
I think boxing solution is the most nice one. What do you think?
There was a problem hiding this comment.
What I hand in mind is adding a new ValuePointer variant. Similar to:
Line 751 in 43cb730
There was a problem hiding this comment.
LocalType has pointers encoded in LocalType::Value field.
sagudev
changed the title
Signed-off-by: sagudev <16504129 [email protected]>
sagudev
force-pushed
the
dyn-idx-matrix
branch
from
a869bd5
to
a12a9ba
Compare
naga/src/back/spv/block.rs
Outdated
| TypeResolution::Handle(handle) => handle, | ||
| TypeResolution::Value(_) => { | ||
| return Err(Error::Validation( | ||
| "Array types should always be in the arena", |
There was a problem hiding this comment.
| "Array types should always be in the arena", | |
| "Matrix types should always be in the arena", |
I don't think this is always true:
wgpu/naga/src/proc/typifier.rs
Line 98 in 43cb730
There was a problem hiding this comment.
This was copy&paste from array, I will check and fix.
Signed-off-by: sagudev <16504129 [email protected]>
Signed-off-by: sagudev <16504129 [email protected]>
Connections
Fix #4337
Description
Add support for dynamic indexing of by-value matrices, by introducing
get_local_pointer_idand then follow #6188.Testing
New tests added and because there was no expectation changes in servo's CTS run I wrote new test: gpuweb/cts#3982 (which passes with this PR).
Checklist
cargo fmt.cargo clippy. If applicable, add:--target wasm32-unknown-unknown--target wasm32-unknown-emscriptencargo xtask testto run tests.CHANGELOG.md. See simple instructions inside file.