From 2dca58b844d4e38c63a66917f5116da8aaae35df Mon Sep 17 00:00:00 2001 From: James Hamilton Date: Mon, 23 Aug 2021 01:47:14 +0900 Subject: [PATCH] some updates from review --- rollup.config.js | 4 ++-- rollup.config.style-spec.js | 8 ++----- src/data/bucket.ts | 4 ++-- src/data/bucket/fill_bucket.ts | 4 +--- src/data/bucket/symbol_bucket.ts | 36 ++++++++++++++++++++++++------- src/data/feature_index.ts | 2 +- src/geo/transform.ts | 1 - src/render/draw_fill.ts | 2 +- test/unit/util/primitives.test.js | 6 ++---- 9 files changed, 39 insertions(+), 28 deletions(-) diff --git a/rollup.config.js b/rollup.config.js index 770e8b265f..fba64210da 100644 --- a/rollup.config.js +++ b/rollup.config.js @@ -11,8 +11,8 @@ const outputFile = minified ? 'dist/maplibre-gl.js' : 'dist/maplibre-gl-unminified.js'; export default [{ - // Before rollup you should run build-tsc to transpile from typesript to javascript - // and to copy the shaders and convert them to js strings + // Before rollup you should run build-tsc to transpile from typescript to javascript + // and to copy the shaders and convert them to js strings // Rollup will use code splitting to bundle GL JS into three "chunks": // - rollup/build/maplibregl/index.js: the main module, plus all its dependencies not shared by the worker module // - rollup/build/maplibregl/worker.js: the worker module, plus all dependencies not shared by the main module diff --git a/rollup.config.style-spec.js b/rollup.config.style-spec.js index b13145eae3..4c69b76b48 100644 --- a/rollup.config.style-spec.js +++ b/rollup.config.style-spec.js @@ -4,14 +4,10 @@ import resolve from '@rollup/plugin-node-resolve'; import commonjs from '@rollup/plugin-commonjs'; import unassert from 'rollup-plugin-unassert'; import json from '@rollup/plugin-json'; -import { fileURLToPath } from 'url'; - -const __dirname = dirname(fileURLToPath(import.meta.url)); +import {fileURLToPath} from 'url'; const esm = 'esm' in process.env; -const ROOT_DIR = __dirname; - const config = [{ input: 'rollup/build/tsc/style-spec/style-spec.js', output: { @@ -29,7 +25,7 @@ const config = [{ // This check will cause the build to fail on CI allowing these issues to be caught. if (importer && !importer.includes('node_modules')) { const resolvedPath = path.join(importer, source); - const fromRoot = path.relative(ROOT_DIR, resolvedPath); + const fromRoot = path.relative(dirname(fileURLToPath(import.meta.url)), resolvedPath); if (fromRoot.length > 2 && fromRoot.slice(0, 2) === '..') { throw new Error(`Module ${importer} imports ${source} from outside the style-spec package root directory.`); } diff --git a/src/data/bucket.ts b/src/data/bucket.ts index fbc44aa6e1..516ab6fc98 100644 --- a/src/data/bucket.ts +++ b/src/data/bucket.ts @@ -114,8 +114,8 @@ export function deserialize(input: Array, style: Style): {[_: string]: B // look up StyleLayer objects from layer ids (since we don't // want to waste time serializing/copying them from the worker) (bucket as any).layers = layers; - if ((bucket as any).stateDependentLayerIds) { - (bucket as any).stateDependentLayers = (bucket as any).stateDependentLayerIds.map((lId) => layers.filter((l) => l.id === lId)[0]); + if (bucket.stateDependentLayerIds) { + (bucket as any).stateDependentLayers = bucket.stateDependentLayerIds.map((lId) => layers.filter((l) => l.id === lId)[0]); } for (const layer of layers) { output[layer.id] = bucket; diff --git a/src/data/bucket/fill_bucket.ts b/src/data/bucket/fill_bucket.ts index 56d1d5d770..4255d61d5e 100644 --- a/src/data/bucket/fill_bucket.ts +++ b/src/data/bucket/fill_bucket.ts @@ -104,9 +104,7 @@ class FillBucket implements Bucket { } if (sortFeaturesByKey) { - bucketFeatures.sort((a, b) => { - return (a.sortKey) - (b.sortKey); - }); + bucketFeatures.sort((a, b) => a.sortKey - b.sortKey); } for (const bucketFeature of bucketFeatures) { diff --git a/src/data/bucket/symbol_bucket.ts b/src/data/bucket/symbol_bucket.ts index b67ed57e9d..b600e22314 100644 --- a/src/data/bucket/symbol_bucket.ts +++ b/src/data/bucket/symbol_bucket.ts @@ -85,7 +85,7 @@ export type SymbolFeature = { sourceLayerIndex: number, geometry: Array>, properties: any, - type: "Point" | "LineString" | "Polygon", + type: 'Point' | 'LineString' | 'Polygon', id?: any }; @@ -107,7 +107,21 @@ const shaderOpacityAttributes = [ {name: 'a_fade_opacity', components: 1, type: 'Uint8' as ViewType, offset: 0} ]; -function addVertex(array, anchorX, anchorY, ox, oy, tx, ty, sizeVertex, isSDF: boolean, pixelOffsetX, pixelOffsetY, minFontScaleX, minFontScaleY) { +function addVertex( + array: StructArray, + anchorX: number, + anchorY: number, + ox: number, + oy: number, + tx: number, + ty: number, + sizeVertex: number, + isSDF: boolean, + pixelOffsetX: number, + pixelOffsetY: number, + minFontScaleX: number, + minFontScaleY: number +) { const aSizeX = sizeVertex ? Math.min(MAX_PACKED_SIZE, Math.round(sizeVertex[0])) : 0; const aSizeY = sizeVertex ? Math.min(MAX_PACKED_SIZE, Math.round(sizeVertex[1])) : 0; array.emplaceBack( @@ -615,7 +629,7 @@ class SymbolBucket implements Bucket { lineOffset: [number, number], alongLine: boolean, feature: SymbolFeature, - writingMode: any, + writingMode: WritingMode, labelAnchor: Anchor, lineStartIndex: number, lineLength: number, @@ -657,15 +671,21 @@ class SymbolBucket implements Bucket { } } - arrays.placedSymbolArray.emplaceBack(labelAnchor.x, labelAnchor.y, - glyphOffsetArrayStart, this.glyphOffsetArray.length - glyphOffsetArrayStart, vertexStartIndex, - lineStartIndex, lineLength, ((labelAnchor.segment as any)), - sizeVertex ? sizeVertex[0] : 0, sizeVertex ? sizeVertex[1] : 0, + arrays.placedSymbolArray.emplaceBack( + labelAnchor.x, labelAnchor.y, + glyphOffsetArrayStart, + this.glyphOffsetArray.length - glyphOffsetArrayStart, + vertexStartIndex, + lineStartIndex, + lineLength, + labelAnchor.segment, + sizeVertex ? sizeVertex[0] : 0, + sizeVertex ? sizeVertex[1] : 0, lineOffset[0], lineOffset[1], writingMode, // placedOrientation is null initially; will be updated to horizontal(1)/vertical(2) if placed 0, - (false as any), + false as unknown as number, // The crossTileID is only filled/used on the foreground for dynamic text anchors 0, associatedIconIndex diff --git a/src/data/feature_index.ts b/src/data/feature_index.ts index 2a0859765b..fe20970af1 100644 --- a/src/data/feature_index.ts +++ b/src/data/feature_index.ts @@ -47,7 +47,7 @@ class FeatureIndex { grid: Grid; grid3D: Grid; featureIndexArray: FeatureIndexArray; - promoteId: PromoteIdSpecification | undefined | null; + promoteId?: PromoteIdSpecification; rawTileData: ArrayBuffer; bucketLayerIDs: Array>; diff --git a/src/geo/transform.ts b/src/geo/transform.ts index e3f132f0b7..b47f7d887f 100644 --- a/src/geo/transform.ts +++ b/src/geo/transform.ts @@ -605,7 +605,6 @@ class Transform { } customLayerMatrix(): mat4 { - // I assume the previous 'slice' was just to trigger a copy? return mat4.clone(this.mercatorMatrix); } diff --git a/src/render/draw_fill.ts b/src/render/draw_fill.ts index 39740992ad..907e96fd97 100644 --- a/src/render/draw_fill.ts +++ b/src/render/draw_fill.ts @@ -28,7 +28,7 @@ function drawFill(painter: Painter, sourceCache: SourceCache, layer: FillStyleLa const pattern = layer.paint.get('fill-pattern'); const pass = painter.opaquePassEnabledForLayer() && - (!pattern.constantOr(((1 as any))) && + (!pattern.constantOr(1 as any) && color.constantOr(Color.transparent).a === 1 && opacity.constantOr(0) === 1) ? 'opaque' : 'translucent'; diff --git a/test/unit/util/primitives.test.js b/test/unit/util/primitives.test.js index 0789e88166..270cd99dd9 100644 --- a/test/unit/util/primitives.test.js +++ b/test/unit/util/primitives.test.js @@ -112,10 +112,8 @@ test('primitives', (t) => { t.test('frustum', (t) => { t.test('Create a frustum from inverse projection matrix', (t) => { - const proj = new Float64Array(16); // [] also passes test - const invProj = new Float64Array(16); // [] also passes test - // FAIL const proj = mat4.create(); // Float32Array also fails - // FAIL const invProj = mat4.create(); // Float32Array also fails + const proj = new Float64Array(16); + const invProj = new Float64Array(16); mat4.perspective(proj, Math.PI / 2, 1.0, 0.1, 100.0); mat4.invert(invProj, proj);