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 countItems in DataFilterExtension #9158

Merged
merged 9 commits into from
Sep 26, 2024
Merged

Conversation

felixpalmer
Copy link
Collaborator

Background

countItems has not been working since the upgrade to v9

Change List

  • Remove deprecated luma APIs
  • Set buffer layout on filterModel creation
  • Update test to catch this regression in the future

@@ -19,7 +18,8 @@ const float component = 1.0 / 255.0;

void main() {
#ifdef FLOAT_TARGET
dataFilter_value *= float(filterIndices != filterPrevIndices);
// BUG: always 0 for some reason
// dataFilter_value *= float(filterIndices != filterPrevIndices);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@Pessimistress any idea why this is broken? I've tried different variants of the check and it always evaluates to 0. Perhaps there is something wrong with using vertexOffset?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Subtle catch of v9 - when an attribute's descriptor contains shaderAttributes, the sub-attributes cannot use the same name.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That is subtle indeed :) would be nice if the types could catch this

@coveralls
Copy link

coveralls commented Sep 17, 2024

Coverage Status

coverage: 91.739% (+0.002%) from 91.737%
when pulling 0de113c on felix/data-filter-count-v9
into aa2537b on master.

Copy link
Collaborator

@ibgreen ibgreen left a comment

Choose a reason for hiding this comment

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

Thanks for this, LGTM.

@felixpalmer felixpalmer merged commit 230104f into master Sep 26, 2024
4 checks passed
@felixpalmer felixpalmer deleted the felix/data-filter-count-v9 branch September 26, 2024 08:02
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.

4 participants