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

[Feat] upgrade to deck.gl@8 #889

Merged
merged 10 commits into from Feb 18, 2020
Merged

[Feat] upgrade to deck.gl@8 #889

merged 10 commits into from Feb 18, 2020

Conversation

heshan0131
Copy link
Contributor

Signed-off-by: Shan He heshan0131@gmail.com

@heshan0131 heshan0131 changed the base branch from gpu-data-filter-6 to master February 6, 2020 21:25
@heshan0131 heshan0131 mentioned this pull request Feb 17, 2020
@heshan0131 heshan0131 changed the title [WIP] upgrade to deck.gl@8 [Feat] upgrade to deck.gl@8 Feb 18, 2020
add @luma.gl/test-utils

fix h3 layer color, update aggregation with latest aPI

Signed-off-by: Shan He <heshan0131@gmail.com>
Signed-off-by: Shan He <heshan0131@gmail.com>
Signed-off-by: Shan He <heshan0131@gmail.com>
Signed-off-by: Shan He <heshan0131@gmail.com>
Signed-off-by: Shan He <heshan0131@gmail.com>
Signed-off-by: Shan He <heshan0131@gmail.com>
Signed-off-by: Shan He <heshan0131@gmail.com>
Signed-off-by: Shan He <heshan0131@gmail.com>
@@ -240,6 +240,7 @@ export default class ArcLayer extends Layer {
getFilterValue: gpuFilter.filterValueUpdateTriggers,
getWidth: {
sizeField: this.config.sizeField,
sizeScale: this.config.sizeScale,
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 property now available on the layer side panel?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes

pointRadiusScale: radiusScale,
lineMiterLimit: 4
};

const updateTriggers = {
getElevation: {
heightField: this.config.heightField,
heightScale: this.config.heightScale,
heightScaleType: this.config.heightScale,
Copy link
Collaborator

Choose a reason for hiding this comment

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

new naming convention?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is just to avoid conflict with deck.gl layer props, for updateTriggers it doesn't really matter of the name, becuase it's a shallow comparison between newUpdateTriggers. heightScaleType and oldUpdateTriggers. heightScaleType

@@ -126,7 +128,7 @@ export default class HexagonIdLayer extends Layer {

for (let i = 0; i < filteredIndex.length; i++) {
const index = filteredIndex[i];
const id = getHexId(allData[index]);
const id = getHexId({data: allData[index]});
Copy link
Collaborator

Choose a reason for hiding this comment

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

@macrigiuseppe update s2 layer


// save a reference of centroids to dataToFeature
// so we don't have to re calculate it again
centroids[index] = getCentroid({id});
return getCentroid({id});
});

const bounds = this.getPointsBounds(Object.values(centroids), d => d);
Copy link
Collaborator

Choose a reason for hiding this comment

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

isn't centroids an array already?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

: textLabel.size;

const pixelRadius = radiusScale * distanceScale.pixelsPerMeter[0];
const padding = 20;
Copy link
Collaborator

Choose a reason for hiding this comment

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

constant, define it outside the function

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Collaborator

@macrigiuseppe macrigiuseppe left a comment

Choose a reason for hiding this comment

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

I tested out the few use cases:

  • arcs
  • points, including text labels (zooming in/out)
  • filters

Make sure you re-rerun yarn
src/layers/layer-text-label.js needs to be updated

Signed-off-by: Shan He <heshan0131@gmail.com>
Signed-off-by: Shan He <heshan0131@gmail.com>
@heshan0131 heshan0131 merged commit de49061 into master Feb 18, 2020
@delete-merged-branch delete-merged-branch bot deleted the deck-8 branch February 18, 2020 23:15
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

2 participants