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

added id32 feature #2464

Merged
merged 1 commit into from
Feb 14, 2022
Merged

added id32 feature #2464

merged 1 commit into from
Feb 14, 2022

Conversation

caiiiycuk
Copy link
Contributor

Connections
Related to gfx-rs/wgpu-native#171. Added feature to enable 32 bit ids.

Description
If you activate --features id32bits, then wgpu will generate 32 bits ids, and so wgpu-native can be used on 32bit systems like emscripten.

Testing
Added unit test to check corner cases.

@caiiiycuk
Copy link
Contributor Author

caiiiycuk commented Feb 8, 2022

With current code max values are:
id 65535
epoch 8191
backend 7

@caiiiycuk
Copy link
Contributor Author

@kvark can you take a look? I didn't understand why rust complains if I don't specify repr(C) in 64bit case, and why deserialize fails...

wgpu-core/src/id.rs Outdated Show resolved Hide resolved
wgpu-core/src/id.rs Outdated Show resolved Hide resolved
@kvark
Copy link
Member

kvark commented Feb 10, 2022

Once we do these changes, the error will likely go away. It occurred because you had 2 definitions of Id and one didn't have all of the configuration/attributes of the original.

@caiiiycuk caiiiycuk force-pushed the wgpu-native-emscripten branch from 2a3640a to e72a728 Compare February 14, 2022 03:20
@caiiiycuk
Copy link
Contributor Author

Fixed

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.

Nice work!

@kvark kvark merged commit d586d6d into gfx-rs:master Feb 14, 2022
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.

2 participants