Update WebGPU vertex/index buffer binding methods#15693
Merged
Conversation
makeU64ToNumberWithSentinelAsUndefined should consider that the high and low bits are passed in as signed 32-bit integers (per JavaScript behavior) - 0xFFFFFFFF expressed as an i32 is -1, current behavior expresses 0xFFFFFFFF as a JavaScript number in the signed double range. Update webgpu_cpp.h to reflect deprecation of size=0 default in Dawn: see https://dawn.googlesource.com/dawn/+/2be4b8483c745794971fad323f956d5c0392c878 Update webgpu_basic_rendering test to use defaults
sessamekesh
changed the title
sessamekesh
changed the title
kainino0x
approved these changes
Dec 2, 2021
Collaborator
kainino0x
left a comment
kainino0x
left a comment
There was a problem hiding this comment.
Thank you so much for this contribution! Sorry that I let the bug sit for so long.
Including just the changes from that one Dawn commit is a good idea. Next time we "roll" to match Dawn we'll certainly roll past that commit so it'll be no problem.
LGTM!
Collaborator
mmarczell-graphisoft
pushed a commit
to GRAPHISOFT/emscripten
that referenced
this pull request
Jan 5, 2022
) makeU64ToNumberWithSentinelAsUndefined should consider that the high and low bits are passed in as signed 32-bit integers (per JavaScript behavior) - 0xFFFFFFFF expressed as an i32 is -1, current behavior expresses 0xFFFFFFFF as a JavaScript number in the signed double range. Update webgpu_cpp.h to reflect deprecation of size=0 default in Dawn: see https://dawn.googlesource.com/dawn/+/2be4b8483c745794971fad323f956d5c0392c878 Update webgpu_basic_rendering test to use defaults
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
3 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.
I've also been running into #15237 - I'm using this fix on my development branch, and would like to merge it upstream.
I have a couple concerns, since this is my first contribution to Emscripten's code base and I want to make sure everything is in order:
I only included this commit from Dawn - there's probably a lot others that I didn't include, I didn't bother to check the rest (I'm not confident I would have been able to update all of them appropriately).
Thee bug in
makeU64ToNumberWithSentinelAsUndefinedis that0xFFFFFFFFin JavaScript is evaluated as a Number (not as an Int32) to 2^32-1, butlowNameandhighNamewere evaluated from that same bitset asInt32s. I could also compare as((${highName} === (0xFFFFFFFF | 0)))to force evaluation of0xFFFFFFFFas anInt32as well (compare apples to apples), but=== -1is a bit less crypitc IMO.Thanks!