-
Notifications
You must be signed in to change notification settings - Fork 303
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
MapBox VectorTile: bug fixes using an official MapBox stream #2469
base: master
Are you sure you want to change the base?
Changes from 16 commits
06804f1
5c62908
f903a1a
328cc2c
a96f21b
d756acb
a881fac
b77965b
7f27ef8
c7a1c32
6d568fa
5f63ee2
af2e0f1
049dd6a
efa1777
36bcd33
151e9af
bb8f57d
18b1d2c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -69,7 +69,6 @@ | |
} | ||
|
||
var kmlStyle = { | ||
zoom: { min: 10, max: 20 }, | ||
text: { | ||
field: '{name}', | ||
haloColor: 'black', | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -87,14 +87,18 @@ async function loadImage(source) { | |
return (await promise).image; | ||
} | ||
|
||
function cropImage(img, cropValues = { width: img.naturalWidth, height: img.naturalHeight }) { | ||
canvas.width = cropValues.width; | ||
canvas.height = cropValues.height; | ||
function cropImage(img, cropValues) { | ||
const x = cropValues.x || 0; | ||
const y = cropValues.y || 0; | ||
const width = cropValues.width || img.naturalWidth; | ||
const height = cropValues.height || img.naturalHeight; | ||
canvas.width = width; | ||
canvas.height = height; | ||
const ctx = canvas.getContext('2d', { willReadFrequently: true }); | ||
ctx.drawImage(img, | ||
cropValues.x || 0, cropValues.y || 0, cropValues.width, cropValues.height, | ||
0, 0, cropValues.width, cropValues.height); | ||
return ctx.getImageData(0, 0, cropValues.width, cropValues.height); | ||
x, y, width, height, | ||
0, 0, width, height); | ||
return ctx.getImageData(0, 0, width, height); | ||
} | ||
|
||
function replaceWhitePxl(imgd, color, id) { | ||
|
@@ -158,7 +162,7 @@ function defineStyleProperty(style, category, parameter, userValue, defaultValue | |
const dataValue = style.context.featureStyle?.[category]?.[parameter]; | ||
if (dataValue != undefined) { return readExpression(dataValue, style.context); } | ||
if (defaultValue instanceof Function) { | ||
return defaultValue(style.context.properties, style.context); | ||
return defaultValue(style.context.properties, style.context) ?? defaultValue; | ||
} | ||
return defaultValue; | ||
}, | ||
|
@@ -765,10 +769,11 @@ class Style { | |
* @param {Object} sprites vector tile layer. | ||
* @param {Number} [order=0] | ||
* @param {Boolean} [symbolToCircle=false] | ||
* @param {Set} warn Set storing all warnings encountered by the source. | ||
* | ||
* @returns {StyleOptions} containing all properties for itowns.Style | ||
*/ | ||
static setFromVectorTileLayer(layer, sprites, order = 0, symbolToCircle = false) { | ||
static setFromVectorTileLayer(layer, sprites, order = 0, symbolToCircle = false, warn) { | ||
const style = { | ||
fill: {}, | ||
stroke: {}, | ||
|
@@ -804,7 +809,8 @@ class Style { | |
style.stroke.color = color; | ||
style.stroke.opacity = opacity; | ||
style.stroke.width = 1.0; | ||
style.stroke.dasharray = []; | ||
} else { | ||
style.stroke.width = 0.0; | ||
} | ||
} else if (layer.type === 'line') { | ||
const prepare = readVectorProperty(layer.paint['line-color'], { type: 'color' }); | ||
|
@@ -820,6 +826,8 @@ class Style { | |
style.point.opacity = opacity; | ||
style.point.radius = readVectorProperty(layer.paint['circle-radius']); | ||
} else if (layer.type === 'symbol') { | ||
// if symbol we shouldn't draw stroke but defaut value is 1. | ||
style.stroke.width = 0.0; | ||
// overlapping order | ||
style.text.zOrder = readVectorProperty(layer.layout['symbol-z-order']); | ||
if (style.text.zOrder == 'auto') { | ||
|
@@ -861,6 +869,12 @@ class Style { | |
// additional icon | ||
const iconImg = readVectorProperty(layer.layout['icon-image']); | ||
if (iconImg) { | ||
const cropValueDefault = { | ||
x: 0, | ||
y: 0, | ||
width: 1, | ||
height: 1, | ||
}; | ||
try { | ||
style.icon.id = iconImg; | ||
if (iconImg.stops) { | ||
|
@@ -871,28 +885,59 @@ class Style { | |
if (stop[1].includes('{')) { | ||
cropValues = function _(p) { | ||
const id = stop[1].replace(/\{(.+?)\}/g, (a, b) => (p[b] || '')).trim(); | ||
cropValues = sprites[id]; | ||
if (cropValues === undefined) { | ||
const warning = `WARNING: "${id}" not found in sprite file`; | ||
if (!warn.has(warning)) { | ||
warn.add(warning); | ||
console.warn(warning); | ||
} | ||
sprites[id] = cropValueDefault;// or return cropValueDefault; | ||
} | ||
return sprites[id]; | ||
}; | ||
} else if (cropValues === undefined) { | ||
const warning = `WARNING: "${stop[1]}" not found in sprite file`; | ||
if (!warn.has(warning)) { | ||
warn.add(warning); | ||
console.warn(warning); | ||
} | ||
cropValues = cropValueDefault; | ||
} | ||
return [stop[0], cropValues]; | ||
}), | ||
}; | ||
style.icon.cropValues = iconCropValue; | ||
} else { | ||
style.icon.cropValues = sprites[iconImg]; | ||
if (iconImg[0].includes('{')) { | ||
if (iconImg.includes('{')) { | ||
style.icon.cropValues = function _(p) { | ||
const id = iconImg.replace(/\{(.+?)\}/g, (a, b) => (p[b] || '')).trim(); | ||
style.icon.cropValues = sprites[id]; | ||
if (sprites[id] === undefined) { | ||
const warning = `WARNING: "${id}" not found in sprite file`; | ||
if (!warn.has(warning)) { | ||
warn.add(warning); | ||
console.warn(warning); | ||
} | ||
sprites[id] = cropValueDefault;// or return cropValueDefault; | ||
} | ||
return sprites[id]; | ||
}; | ||
} else if (sprites[iconImg] === undefined) { | ||
const warning = `WARNING: "${iconImg}" not found in sprite file`; | ||
if (!warn.has(warning)) { | ||
warn.add(warning); | ||
console.warn(warning); | ||
} | ||
style.icon.cropValues = cropValueDefault; | ||
} | ||
} | ||
style.icon.source = sprites.source; | ||
style.icon.size = readVectorProperty(layer.layout['icon-size']) || 1; | ||
const { color, opacity } = rgba2rgb(readVectorProperty(layer.paint['icon-color'], { type: 'color' })); | ||
style.icon.color = color; | ||
// https://docs.mapbox.com/style-spec/reference/layers/#paint-symbol-icon-color | ||
if (iconImg.sdf) { | ||
style.icon.color = color; | ||
} | ||
style.icon.opacity = readVectorProperty(layer.paint['icon-opacity']) || (opacity !== undefined && opacity); | ||
} catch (err) { | ||
err.message = `VTlayer '${layer.id}': argument sprites must not be null when using layer.layout['icon-image']`; | ||
|
@@ -919,29 +964,28 @@ class Style { | |
* @param {Boolean} canBeFilled - true if feature.type == FEATURE_TYPES.POLYGON. | ||
*/ | ||
applyToCanvasPolygon(txtrCtx, polygon, invCtxScale, canBeFilled) { | ||
const context = this.context; | ||
// draw line or edge of polygon | ||
if (this.stroke) { | ||
if (this.stroke.width > 0) { | ||
// TO DO add possibility of using a pattern (https://github.com/iTowns/itowns/issues/2210) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should we check for stroke properties too, like for fill ? |
||
this._applyStrokeToPolygon(txtrCtx, invCtxScale, polygon, context); | ||
this._applyStrokeToPolygon(txtrCtx, invCtxScale, polygon); | ||
} | ||
|
||
// fill inside of polygon | ||
if (canBeFilled && this.fill) { | ||
if (canBeFilled && (this.fill.pattern || this.fill.color)) { | ||
// canBeFilled can be move to StyleContext in the later PR | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we remove canBeFilled ? Its value is feature.type == FEATURE_TYPES.POLYGON . We are in a method named |
||
this._applyFillToPolygon(txtrCtx, invCtxScale, polygon, context); | ||
this._applyFillToPolygon(txtrCtx, invCtxScale, polygon); | ||
} | ||
} | ||
|
||
_applyStrokeToPolygon(txtrCtx, invCtxScale, polygon) { | ||
if (txtrCtx.strokeStyle !== this.stroke.color) { | ||
txtrCtx.strokeStyle = this.stroke.color; | ||
} | ||
const width = (this.stroke.width || 2.0) * invCtxScale; | ||
const width = this.stroke.width * invCtxScale; | ||
if (txtrCtx.lineWidth !== width) { | ||
txtrCtx.lineWidth = width; | ||
} | ||
const alpha = this.stroke.opacity == undefined ? 1.0 : this.stroke.opacity; | ||
const alpha = this.stroke.opacity; | ||
if (alpha !== txtrCtx.globalAlpha && typeof alpha == 'number') { | ||
txtrCtx.globalAlpha = alpha; | ||
} | ||
|
@@ -957,7 +1001,7 @@ class Style { | |
// need doc for the txtrCtx.fillStyle.src that seems to always be undefined | ||
if (this.fill.pattern) { | ||
let img = this.fill.pattern; | ||
const cropValues = this.fill.pattern.cropValues; | ||
const cropValues = { ...this.fill.pattern.cropValues }; | ||
if (this.fill.pattern.source) { | ||
img = await loadImage(this.fill.pattern.source); | ||
} | ||
|
@@ -1032,7 +1076,7 @@ class Style { | |
if (!this.icon.cropValues && !this.icon.color) { | ||
icon.src = this.icon.source; | ||
} else { | ||
const cropValues = this.icon.cropValues; | ||
const cropValues = { ...this.icon.cropValues }; | ||
const color = this.icon.color; | ||
const id = this.icon.id || this.icon.source; | ||
const img = await loadImage(this.icon.source); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -116,10 +116,11 @@ function vtFeatureToFeatureGeometry(vtFeature, feature, classify = false) { | |
function readPBF(file, options) { | ||
options.out = options.out || {}; | ||
const vectorTile = new VectorTile(new Protobuf(file)); | ||
const sourceLayers = Object.keys(vectorTile.layers); | ||
const vtLayerNames = Object.keys(vectorTile.layers); | ||
|
||
if (sourceLayers.length < 1) { | ||
return; | ||
const collection = new FeatureCollection(options.out); | ||
if (vtLayerNames.length < 1) { | ||
return Promise.resolve(collection); | ||
} | ||
|
||
// x,y,z tile coordinates | ||
|
@@ -130,41 +131,33 @@ function readPBF(file, options) { | |
// Only if the layer.origin is top | ||
const y = options.in.isInverted ? options.extent.row : (1 << z) - options.extent.row - 1; | ||
|
||
const collection = new FeatureCollection(options.out); | ||
|
||
const vFeature = vectorTile.layers[sourceLayers[0]]; | ||
// TODO: verify if size is correct because is computed with only one feature (vFeature). | ||
const size = vFeature.extent * 2 ** z; | ||
const vFeature0 = vectorTile.layers[vtLayerNames[0]]; | ||
// TODO: verify if size is correct because is computed with only one feature (vFeature0). | ||
const size = vFeature0.extent * 2 ** z; | ||
const center = -0.5 * size; | ||
|
||
collection.scale.set(globalExtent.x / size, -globalExtent.y / size, 1); | ||
collection.position.set(vFeature.extent * x + center, vFeature.extent * y + center, 0).multiply(collection.scale); | ||
collection.position.set(vFeature0.extent * x + center, vFeature0.extent * y + center, 0).multiply(collection.scale); | ||
collection.updateMatrixWorld(); | ||
|
||
sourceLayers.forEach((layer_id) => { | ||
if (!options.in.layers[layer_id]) { return; } | ||
vtLayerNames.forEach((vtLayerName) => { | ||
if (!options.in.layers[vtLayerName]) { return Promise.resolve(collection); } | ||
|
||
const sourceLayer = vectorTile.layers[layer_id]; | ||
const vectorTileLayer = vectorTile.layers[vtLayerName]; | ||
|
||
for (let i = sourceLayer.length - 1; i >= 0; i--) { | ||
const vtFeature = sourceLayer.feature(i); | ||
for (let i = vectorTileLayer.length - 1; i >= 0; i--) { | ||
const vtFeature = vectorTileLayer.feature(i); | ||
vtFeature.tileNumbers = { x, y: options.extent.row, z }; | ||
const layers = options.in.layers[layer_id].filter(l => l.filterExpression.filter({ zoom: z }, vtFeature) && z >= l.zoom.min && z < l.zoom.max); | ||
let feature; | ||
// Find layers where this vtFeature is used | ||
const layers = options.in.layers[vtLayerName] | ||
.filter(l => l.filterExpression.filter({ zoom: z }, vtFeature)); | ||
|
||
for (const layer of layers) { | ||
if (!feature) { | ||
feature = collection.requestFeatureById(layer.id, vtFeature.type - 1); | ||
feature.id = layer.id; | ||
feature.order = layer.order; | ||
feature.style = options.in.styles[feature.id]; | ||
vtFeatureToFeatureGeometry(vtFeature, feature); | ||
} else if (!collection.features.find(f => f.id === layer.id)) { | ||
feature = collection.newFeatureByReference(feature); | ||
feature.id = layer.id; | ||
feature.order = layer.order; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems like some kind of optimization, can you explain in more details why you remove it please? |
||
feature.style = options.in.styles[feature.id]; | ||
} | ||
const feature = collection.requestFeatureById(layer.id, vtFeature.type - 1); | ||
feature.id = layer.id; | ||
feature.order = layer.order; | ||
feature.style = options.in.styles[feature.id]; | ||
vtFeatureToFeatureGeometry(vtFeature, feature); | ||
} | ||
} | ||
}); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -186,13 +186,15 @@ class Label2DRenderer { | |
if (!frustum.containsPoint(worldPosition.applyMatrix4(camera.matrixWorldInverse)) || | ||
// Check if globe horizon culls the label | ||
// Do some horizon culling (if possible) if the tiles level is small enough. | ||
label.horizonCullingPoint && GlobeLayer.horizonCulling(label.horizonCullingPoint) || | ||
// Check if content isn't present in visible labels | ||
this.grid.visible.some((l) => { | ||
// TODO for icon without text filter by position | ||
const textContent = label.content.textContent; | ||
return textContent !== '' && l.content.textContent.toLowerCase() == textContent.toLowerCase(); | ||
})) { | ||
label.horizonCullingPoint && GlobeLayer.horizonCulling(label.horizonCullingPoint) | ||
// Why do we might need this part ? | ||
// || // Check if content isn't present in visible labels | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it was to avoid overlapping but I'm not sure we need it too so let's remove it completely in this case. We still have git if we need it later. @Desplandis WDYT? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we can't reproduce the overlapping and it seems to be the case here. I think we could totally remove this. |
||
// this.grid.visible.some((l) => { | ||
// // TODO for icon without text filter by position | ||
// const textContent = label.content.textContent; | ||
// return textContent !== '' && l.content.textContent.toLowerCase() == textContent.toLowerCase(); | ||
// }) | ||
) { | ||
label.visible = false; | ||
} else { | ||
// projecting world position label | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -108,6 +108,7 @@ class Source extends InformationsData { | |
constructor(source) { | ||
super(source); | ||
this.isSource = true; | ||
this.warn = new Set(); | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Aggregating logs is a good idea and needed in itowns. I think we should not store aggregated logs in objects and pass them around as function parameters. An alternative implementation would be to implement a LogAggregator that stores logs, aggregates them and can log info, warn and errors. A global instance could be used by all itowns components then. If we do so, we need to be careful with memory usage and empty the log cache frequently. And if we don't want to implement it ourselves, I'm sure there are libraries available. However I think that's out of scope of this PR, so I'd be for removing this log aggregation for now and implement it differently later. @Desplandis what is your opinion on this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, we could have some kind of heap with a max size to aggregate those warnings. I think there should be some kind of library to do it. However, I agree that this is out of scope of this PR. |
||
if (!source.url) { | ||
throw new Error('New Source: url is required'); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need for this comment I think, the code is clear enough