Skip to content
This repository has been archived by the owner on Jun 18, 2021. It is now read-only.

Fix generic bounds on buffer mapping #126

Merged
merged 1 commit into from
Nov 21, 2019
Merged

Fix generic bounds on buffer mapping #126

merged 1 commit into from
Nov 21, 2019

Conversation

kvark
Copy link
Member

@kvark kvark commented Nov 21, 2019

Smaller (and incomplete!) alternative to #119 while it's still discussed.
One missing piece here is alignment. cc @Coder-256

Also, importing wgpu-core as just "core" wasn't the best idea 😅 : it collides with the actual core.

@kvark kvark requested a review from grovesNL November 21, 2019 02:27
Copy link
Collaborator

@grovesNL grovesNL left a comment

Choose a reason for hiding this comment

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

bors r+

bors bot added a commit that referenced this pull request Nov 21, 2019
126: Fix generic bounds on buffer mapping r=grovesNL a=kvark

Smaller (and incomplete!) alternative to #119 while it's still discussed.
One missing piece here is alignment. cc @Coder-256

Also, importing `wgpu-core` as just "core" wasn't the best idea 😅 : it collides with the actual `core`.

Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
@Coder-256
Copy link
Contributor

I think this solves part of the issue, but why not just return a &[u8]? I'm not convinced that would be the best long-term solution, but it would partially fix the problem of alignment (since [u8] has an alignment of 1). Users can initialize their data from manual offsets with from_ne_bytes, which was recently stabilized for floats (so it should be available on 1.40).

@bors
Copy link
Contributor

bors bot commented Nov 21, 2019

Build succeeded

@bors bors bot merged commit 3609e52 into gfx-rs:master Nov 21, 2019
@kvark kvark deleted the lifetime branch November 21, 2019 05:06
@kvark
Copy link
Member Author

kvark commented Nov 21, 2019

@Coder-256 that would address the alignment flaw, yes, but it seems super inconvenient for the user, which I am currently being while dogfooding the library. It might be the way to proceed, I'm just not in a rush to call that while the very question of "Should we have mapping in WebGPU?" is being actively discussed by the working group.

If you want to help by making a PR that removes the generic stuff from mapping altogether (as well as drops zerocopy dependency), while providing some way or example for the users to operate (!), I'd be happy to merge it.

@Coder-256
Copy link
Contributor

@kvark I agree 100%, it's very inconvenient, but sadly I think it might be the only option. See my latest comment on #119. I'll start work on a PR. To be honest, I'm not 100% sure how it will turn out but I'll give it a shot and see how it affects the examples.

bors bot added a commit that referenced this pull request Nov 22, 2019
127: Use u8 for buffer mapping r=kvark a=Coder-256

cc @kvark @grovesNL

This is a temporary solution for #119, and a follow-up for #126.

Co-authored-by: Jacob Greenfield <jacob@jacobgreenfield.me>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants