Skip to content

Change mapAsync to also early-reject if already mapped#4787

Merged
kainino0x merged 1 commit intogpuweb:mainfrom
kainino0x:mapasync-early-reject2
Aug 29, 2024
Merged

Change mapAsync to also early-reject if already mapped#4787
kainino0x merged 1 commit intogpuweb:mainfrom
kainino0x:mapasync-early-reject2

Conversation

@kainino0x
Copy link
Copy Markdown
Contributor

@kainino0x kainino0x commented Jul 25, 2024 •
edited
Loading

Not just if already pending mapping. This makes the two cases more consistent with their timing which is probably good for application robustness.

PR is dependent on #4786. Rebased!

This is currently a proposal. Approved!

Related to #4690.

@kainino0x kainino0x added proposal needs-cts-issue This change requires tests (or would need tests if accepted), but may not have a CTS issue filed yet api WebGPU API labels Jul 25, 2024
@kainino0x kainino0x added this to the Milestone 0 milestone Jul 25, 2024
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Jul 25, 2024 •
edited
Loading

Previews, as seen when this build job started (2e52eaf):
WebGPU webgpu.idl | Explainer | Correspondence Reference
WGSL grammar.js | wgsl.lalr.txt

@kainino0x kainino0x added the potentially breaking Could require a breaking change to the API label Jul 26, 2024
@kainino0x kainino0x added api resolved Resolved - waiting for a change to the API specification and removed proposal labels Aug 28, 2024
mwyrzykowski
mwyrzykowski previously approved these changes Aug 28, 2024
View reviewed changes
@mwyrzykowski mwyrzykowski dismissed their stale review August 28, 2024 19:59

Need to look at this again

mwyrzykowski
mwyrzykowski approved these changes Aug 28, 2024
View reviewed changes
Copy link
Copy Markdown

@mwyrzykowski mwyrzykowski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is preferable, certainly with the change from #4786

@kainino0x
Change mapAsync to also early-reject if already mapped
2e52eaf 
Not just if already _pending_ mapping.
@kainino0x kainino0x force-pushed the mapasync-early-reject2 branch from 66be44e to 2e52eaf Compare August 28, 2024 22:02
@kainino0x
Copy link
Copy Markdown
Contributor Author

kainino0x commented Aug 28, 2024

Thanks!

(Rebased without change)

@kainino0x kainino0x marked this pull request as ready for review August 28, 2024 22:02
@kainino0x kainino0x requested a review from toji August 28, 2024 22:02
@kainino0x kainino0x merged commit edfffd9 into gpuweb:main Aug 29, 2024
@kainino0x kainino0x deleted the mapasync-early-reject2 branch August 29, 2024 17:58
@Kangz
Copy link
Copy Markdown
Contributor

Kangz commented Sep 3, 2024

GPU Web WG 2024-08-28 Atlantic-time 
* blocked on the other mapAsync one
* Makes it so we avoid lots of round-trips to the device timeline
* For both pending-map and already-mapped, reject immediately on the client side. We have the state on both client and device-timeline sides
* KG: if we can do it on the client timeline, let's do it on the client timeline
* MW: OK for WebKit too
* **Resolved to make this change**

@teoxoy teoxoy mentioned this pull request Jan 23, 2026
Merged
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@toji toji toji approved these changes

@mwyrzykowski mwyrzykowski mwyrzykowski approved these changes

Assignees

No one assigned

Labels

api resolved Resolved - waiting for a change to the API specification api WebGPU API needs-cts-issue This change requires tests (or would need tests if accepted), but may not have a CTS issue filed yet potentially breaking Could require a breaking change to the API

Projects

None yet

Milestone

Milestone 0

Development

Successfully merging this pull request may close these issues.

4 participants

@kainino0x @Kangz @toji @mwyrzykowski