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

HeatmapLayer/GPUAggregator: fix WebGL feature checking #3483

Merged
merged 2 commits into from Sep 11, 2019

Conversation

1chandu
Copy link
Contributor

@1chandu 1chandu commented Aug 23, 2019

For #3480

Requires: visgl/luma.gl#1210

Background

Fix feature checking in Heatmap Layer and GPUGridAggregator.

Change List

  • HeatmapLayer/GPUAggregator: fix WebGL feature checking

if (!isWebGL2(gl)) {
log.error(`HeatmapLayer ${this.id} is not supported on this browser, requires WebGL2`)();
if (!hasFeatures(gl, REQUIRED_FEATURES)) {
const info = REQUIRED_FEATURES.reduce(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Call GPUAggregator.isSupported?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Though the checks are same, HeatmapLayer doesn't use GPUGridAggregator, in future it is possible these feature list will change.

if (!hasFeatures(gl, REQUIRED_FEATURES)) {
const info = REQUIRED_FEATURES.reduce(
(acc, feature) =>
`${acc} ${feature}: ${hasFeatures(gl, feature) ? 'SUPPORTED' : 'NOT SUPPORTED'} `,
Copy link
Collaborator

Choose a reason for hiding this comment

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

  • this is technically debug info. I suggest moving this into hasFeature and use log.log(NON_ZERO_PRIORITY, ...)
  • map and then join will be more readable.

@coveralls
Copy link

Coverage Status

Coverage increased (+2.8%) to 82.777% when pulling 9f42694 on HeatmapLayerCrash into 05cb2d9 on master.

@1chandu
Copy link
Contributor Author

1chandu commented Sep 10, 2019

luma.gl now logs, when feature is checked and not supported. Updated HeatmapLayer to just log single statement when not supported.

@1chandu 1chandu merged commit 9431af7 into master Sep 11, 2019
@1chandu 1chandu deleted the HeatmapLayerCrash branch September 11, 2019 01:52
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.

None yet

3 participants