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

Support operations for render layers #13310

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

JoJoJet
Copy link
Member

@JoJoJet JoJoJet commented May 10, 2024

Objective

Allow combining render layers with a more-ergonomic syntax than RenderLayers::from_iter(a.iter().chain(b.iter())).

Solution

Add the or operation (and corresponding const method) to allow computing the union of a set of render layers. While we're here, also added and and xor operations. Someone might find them useful

Testing

Added a simple unit test.

Comment on lines 120 to 126
pub const fn and(self, other: Self) -> Self {
let mask = self.0 & other.0;
Self(mask)
}

/// Returns all [layers](Layer) included in either instance of [`RenderLayers`].
pub const fn or(self, other: Self) -> Self {
Copy link
Member Author

Choose a reason for hiding this comment

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

I considered calling these intersection and union for clarity, but I wasn't sure what to call xor using that naming convention

Copy link
Contributor

Choose a reason for hiding this comment

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

This would be the symmetric difference or disjunctive union. Pretty obscure so not sure it's worth switching. Note that Python does have a symmetric difference operation on sets. https://en.m.wikipedia.org/wiki/Symmetric_difference

Copy link
Member Author

Choose a reason for hiding this comment

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

It probably is worth switching, at least to avoid potential confusion with the and method. I imagine some people may think a.and(b) means "all the layers from a and b"

Copy link
Member

@james7132 james7132 left a comment

Choose a reason for hiding this comment

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

LGTM other than the renaming.

@james7132 james7132 added A-Rendering Drawing game state to the screen C-Usability A simple quality-of-life change that makes Bevy easier to use D-Straightforward Simple bug fixes and API improvements, docs, test and examples labels May 13, 2024
@tychedelia
Copy link
Contributor

tychedelia commented May 13, 2024

if we can do #13317 would mean these can't be const, not sure how const-ness affects their value for you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen C-Usability A simple quality-of-life change that makes Bevy easier to use D-Straightforward Simple bug fixes and API improvements, docs, test and examples
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants