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

GPU Aggregation (4/8): ScreenGridLayer #8942

Merged
merged 9 commits into from
Jun 24, 2024
Merged

Conversation

Pessimistress
Copy link
Collaborator

For #7457

Change List

  • Move ScreenGridLayer to use new AggregationLayer and aggregator classes
  • Update tests

},
// Evaluate domain at draw() time
colorDomain: () => this.props.colorDomain || aggregator.getResultDomain(0),
// Extensions are already handled by the GPUAggregator, do not pass it down
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this always going to be true? Could there not be extensions which CellLayer will need?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For ScreenGridLayer there's no existing extension that should be applied to the cells directly, and unlikely for any custom extensions.

This will be tricky for GridLayer/HexagonLayer. FillStyleExtension and TerrainExtension should affect the screen pass only, and *FilterExtension should affect the aggregation pass only.

We could add some flag to the extensions to categorize them?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Would it be sufficient for the extension to signal which stage of the GPU pipeline it affects? E.g. 'vertex'/'fragment'. The aggregation pass should only use the 'vertex' extensions and the screen pass the 'fragment'.

Even so, how does this work for DataFilterExtension? There we would like the vertex shader injections to hide the filtered data points before aggregation, but in the screen pass the dataFilter_value needs to both shrink the geometry in the vertex stage & fade in the fragment stage

An alternative would be to have the extensions understand the concept of what context they are being invoked in, by looking at what type of Pass they are being rendered in. The DataFilterExtension could then do one thing in the AggregationPass and another in the ScreenPass

Base automatically changed from x/aggregation-3 to master June 22, 2024 18:07
@coveralls
Copy link

Coverage Status

coverage: 89.222% (-0.2%) from 89.454%
when pulling 122449b on x/aggregation-4
into b8f9181 on master.

@coveralls
Copy link

Coverage Status

coverage: 89.222% (-0.2%) from 89.454%
when pulling 9127f6d on x/aggregation-4
into b8f9181 on master.

@coveralls
Copy link

Coverage Status

coverage: 89.281% (-0.2%) from 89.454%
when pulling 7dcb805 on x/aggregation-4
into b8f9181 on master.

@Pessimistress Pessimistress merged commit 1c286d0 into master Jun 24, 2024
4 checks passed
@Pessimistress Pessimistress deleted the x/aggregation-4 branch June 24, 2024 05:26
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