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

Fix ineffective FOG variant filter #6968

Merged
merged 3 commits into from
Jul 20, 2023
Merged

Fix ineffective FOG variant filter #6968

merged 3 commits into from
Jul 20, 2023

Conversation

bejado
Copy link
Member

@bejado bejado commented Jul 20, 2023

The FOG variant filter has no effect because UserVariantFilterBit::FOG != FOG. Thus, this has no effect:

            variant.key &= ~(filterMask & FOG);

I'm not sure why we needed to bitwise AND the filterMask with the variant bit. Maybe I'm missing something.

@github-actions
Copy link

Please add a release note line to NEW_RELEASE_NOTES.md. If this PR does not warrant a release note, add the 'internal' label to this PR.

@pixelflinger
Copy link
Collaborator

The FOG variant filter has no effect because UserVariantFilterBit::FOG != FOG. Thus, this has no effect:

            variant.key &= ~(filterMask & FOG);

I'm not sure why we needed to bitwise AND the filterMask with the variant bit. Maybe I'm missing something.

It was just stupid from my part. This is because I was thinking of doing something like variant.key &= ~filterMask and because the bits of the two enums don't match, I thought of "keeping" only the good ones -- but obviously this was completely wrong.

Good catch!

@pixelflinger pixelflinger added the internal Issue/PR does not affect clients label Jul 20, 2023
@bejado bejado merged commit 626577f into main Jul 20, 2023
8 checks passed
@bejado bejado deleted the bjd/fix-fog-variant-filter branch July 20, 2023 18:54
plepers pushed a commit to plepers/filament that referenced this pull request Dec 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal Issue/PR does not affect clients
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants