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

Use Bitshift Style for All Bitflag Consts #1737

Merged
merged 1 commit into from
Jul 28, 2021

Conversation

zicklag
Copy link
Contributor

@zicklag zicklag commented Jul 27, 2021

Connections
None exactly, but somewhat related to #1686 as I was going to put some of this change into that PR, but am splitting it out for ease of review.

Description
This is purely code-style related, but it makes it easier to read bitflag constants and much eaiser to add bitflags in the future.

Testing
Tested on Ubuntu 20.04

Copy link
Member

@cwfitzgerald cwfitzgerald left a comment

Choose a reason for hiding this comment

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

Minor comment, but otherwise good

/// Enables clear to zero for buffers & images.
///
/// Supported platforms:
/// - All
///
/// This is a native only feature.
const CLEAR_COMMANDS = 0x0000_0400_0000_0000;
const CLEAR_COMMANDS = 1 << 42;
Copy link
Member

Choose a reason for hiding this comment

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

Jump from 33 to 42?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was just copying what the existing flags were, I wasn't sure if there was a reason for the jump or not. If it's native only are these values not tied to the WebGPU spec?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah they aren't. I don't know why we would jump here. Likely just due to how difficult maintaining the old way was :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, pushed an update to fix.

This makes existing bitflags easier to read and makes it eaiser to add
new flags later.
@cwfitzgerald
Copy link
Member

Thanks!

bors r+

@bors
Copy link
Contributor

bors bot commented Jul 28, 2021

@bors bors bot merged commit efba648 into gfx-rs:master Jul 28, 2021
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