Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement Mappable Primary Buffers Extension #708

Merged
merged 1 commit into from
Jun 9, 2020

Conversation

cwfitzgerald
Copy link
Member

@cwfitzgerald cwfitzgerald commented Jun 8, 2020

Connections

#675 made MAP_WRITE | STORAGE on buffers not possible. This extension re-enables it.

Description

UMA systems rejoice everywhere.

Testing

Hopefully it didn't break since it was usable a week or so ago, so this shouldn't need testing...

Review Notes

The name could be changed, particularly if primary has a certain meaning wrt buffers. It just seemed a reasonable description.

Knowing my luck, I got the bitflags call wrong...

Copy link
Member

@kvark kvark left a comment

Choose a reason for hiding this comment

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

Thank you, the PR looks good!
Let's talk over matrix about how this will be used.

@kvark
Copy link
Member

kvark commented Jun 9, 2020

bors r+

bors bot added a commit that referenced this pull request Jun 9, 2020
708: Implement Mappable Primary Buffers Extension r=kvark a=cwfitzgerald

**Connections**

#675 made `MAP_WRITE | STORAGE` on buffers not possible. This extension re-enables it. 

**Description**

UMA systems rejoice everywhere.

**Testing**

Hopefully it didn't break since it was usable a week or so ago, so this shouldn't need testing...

**Review Notes**

The name could be changed, particularly if primary has a certain meaning wrt buffers. It just seemed a reasonable description.

Knowing my luck, I got the bitflags call wrong...

Co-authored-by: Connor Fitzgerald <[email protected]>
@kvark
Copy link
Member

kvark commented Jun 9, 2020

bors r-

@bors
Copy link
Contributor

bors bot commented Jun 9, 2020

Canceled.

@kvark
Copy link
Member

kvark commented Jun 9, 2020

One thing is missing: where and when is this actually going to be exposed? In the PR as is, the extension is not exposed at all, so it's not usable.

@cwfitzgerald
Copy link
Member Author

All this changes is the valid usages for buffers so the interface doesn't change at all. This worked fine before the assert was added, so]disabling the assert makes it work again.

@kvark
Copy link
Member

kvark commented Jun 9, 2020

Well, yes and no. The assert is gone when the new extension is enabled. But for it to be enabled, the adapter needs to expose it as available. I don't see where it exposes it.

@cwfitzgerald cwfitzgerald force-pushed the mappable-primary-buffers branch from ab4eea4 to 525c44f Compare June 9, 2020 02:10
@cwfitzgerald
Copy link
Member Author

D'oh, of course, fixed!

@cwfitzgerald cwfitzgerald force-pushed the mappable-primary-buffers branch from 525c44f to 4258c60 Compare June 9, 2020 02:33
Copy link
Member

@kvark kvark left a comment

Choose a reason for hiding this comment

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

Thank you!
bors r+

@bors
Copy link
Contributor

bors bot commented Jun 9, 2020

@bors bors bot merged commit f8a68cd into gfx-rs:master Jun 9, 2020
@cwfitzgerald cwfitzgerald deleted the mappable-primary-buffers branch June 26, 2020 23:11
Patryk27 pushed a commit to Patryk27/wgpu that referenced this pull request Nov 23, 2022
…s#708)

* Add a note about references and pointers to Expression::Load

* Formatting

* Remove comma

* Clearer English
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants