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.

Sign up for GitHub

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

Jump to bottom

add symbol-sort-key style property #7678

Merged
merged 4 commits into from
Dec 12, 2018
Merged

add symbol-sort-key style property #7678

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a0ecd68
add `symbol-sort-key` style property
ansis Dec 6, 2018
c4431e4
fixup
ansis Dec 6, 2018
2ce2118
have one render path instead of two
ansis Dec 10, 2018
71fd325
add render tests
ansis Dec 11, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 22 additions & 4 deletions src/data/bucket/symbol_bucket.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ export type CollisionArrays = {
};

export type SymbolFeature = {|
sortKey: number | void,
text: Formatted | void,
icon: string | void,
index: number,
Expand Down Expand Up @@ -101,7 +102,7 @@ function addDynamicAttributes(dynamicLayoutVertexArray: StructArray, p: Point, a
dynamicLayoutVertexArray.emplaceBack(p.x, p.y, angle);
}

class SymbolBuffers {
export class SymbolBuffers {
layoutVertexArray: SymbolLayoutArray;
layoutVertexBuffer: VertexBuffer;

Expand Down Expand Up @@ -261,6 +262,7 @@ class SymbolBucket implements Bucket {
tilePixelRatio: number;
compareText: {[string]: Array<Point>};
fadeStartTime: number;
sortFeaturesByKey: boolean;
sortFeaturesByY: boolean;
sortedAngle: number;
featureSortOrder: Array<number>;
Expand Down Expand Up @@ -291,7 +293,10 @@ class SymbolBucket implements Bucket {
this.iconSizeData = getSizeData(this.zoom, unevaluatedLayoutValues['icon-size']);

const layout = this.layers[0].layout;
const zOrderByViewportY = layout.get('symbol-z-order') === 'viewport-y';
const sortKey = layout.get('symbol-sort-key');
const zOrder = layout.get('symbol-z-order');
this.sortFeaturesByKey = zOrder !== 'viewport-y' && sortKey.constantOr(1) !== undefined;
const zOrderByViewportY = zOrder === 'viewport-y' || (zOrder === 'auto' && !this.sortFeaturesByKey);
this.sortFeaturesByY = zOrderByViewportY && (layout.get('text-allow-overlap') || layout.get('icon-allow-overlap') ||
layout.get('text-ignore-placement') || layout.get('icon-ignore-placement'));

Expand Down Expand Up @@ -333,6 +338,7 @@ class SymbolBucket implements Bucket {
(textField.value.kind !== 'constant' || textField.value.value.toString().length > 0) &&
(textFont.value.kind !== 'constant' || textFont.value.value.length > 0);
const hasIcon = iconImage.value.kind !== 'constant' || iconImage.value.value && iconImage.value.value.length > 0;
const symbolSortKey = layout.get('symbol-sort-key');

this.features = [];

Expand Down Expand Up @@ -370,14 +376,19 @@ class SymbolBucket implements Bucket {
continue;
}

const sortKey = this.sortFeaturesByKey ?
symbolSortKey.evaluate(feature, {}) :
undefined;

const symbolFeature: SymbolFeature = {
text,
icon,
index,
sourceLayerIndex,
geometry: loadGeometry(feature),
properties: feature.properties,
type: vectorTileFeatureTypes[feature.type]
type: vectorTileFeatureTypes[feature.type],
sortKey
};
if (typeof feature.id !== 'undefined') {
symbolFeature.id = feature.id;
Expand Down Expand Up @@ -405,6 +416,13 @@ class SymbolBucket implements Bucket {
// It's better to place labels on one long line than on many short segments.
this.features = mergeLines(this.features);
}

if (this.sortFeaturesByKey) {
this.features.sort((a, b) => {
// a.sortKey is always a number when sortFeaturesByKey is true
return ((a.sortKey: any): number) - ((b.sortKey: any): number);
});
}
}

update(states: FeatureStates, vtLayer: VectorTileLayer, imagePositions: {[string]: ImagePosition}) {
Expand Down Expand Up @@ -481,7 +499,7 @@ class SymbolBucket implements Bucket {
const layoutVertexArray = arrays.layoutVertexArray;
const dynamicLayoutVertexArray = arrays.dynamicLayoutVertexArray;

const segment = arrays.segments.prepareSegment(4 * quads.length, arrays.layoutVertexArray, arrays.indexArray);
const segment = arrays.segments.prepareSegment(4 * quads.length, arrays.layoutVertexArray, arrays.indexArray, feature.sortKey);
const glyphOffsetArrayStart = this.glyphOffsetArray.length;
const vertexStartIndex = segment.vertexLength;

Expand Down
9 changes: 6 additions & 3 deletions src/data/segment.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import type VertexArrayObject from '../render/vertex_array_object';
import type {StructArray} from '../util/struct_array';

export type Segment = {
sortKey: number | void,
vertexOffset: number,
primitiveOffset: number,
vertexLength: number,
Expand All @@ -23,16 +24,17 @@ class SegmentVector {
this.segments = segments;
}

prepareSegment(numVertices: number, layoutVertexArray: StructArray, indexArray: StructArray): Segment {
prepareSegment(numVertices: number, layoutVertexArray: StructArray, indexArray: StructArray, sortKey?: number): Segment {
let segment: Segment = this.segments[this.segments.length - 1];
if (numVertices > SegmentVector.MAX_VERTEX_ARRAY_LENGTH) warnOnce(`Max vertices per segment is ${SegmentVector.MAX_VERTEX_ARRAY_LENGTH}: bucket requested ${numVertices}`);
if (!segment || segment.vertexLength + numVertices > SegmentVector.MAX_VERTEX_ARRAY_LENGTH) {
if (!segment || segment.vertexLength + numVertices > SegmentVector.MAX_VERTEX_ARRAY_LENGTH || segment.sortKey !== sortKey) {
segment = ({
vertexOffset: layoutVertexArray.length,
primitiveOffset: indexArray.length,
vertexLength: 0,
primitiveLength: 0
}: any);
if (sortKey !== undefined) segment.sortKey = sortKey;
this.segments.push(segment);
}
return segment;
Expand All @@ -56,7 +58,8 @@ class SegmentVector {
primitiveOffset,
vertexLength,
primitiveLength,
vaos: {}
vaos: {},
sortKey: 0
}]);
}
}
Expand Down
120 changes: 104 additions & 16 deletions src/render/draw_symbol.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import drawCollisionDebug from './draw_collision_debug';

import SegmentVector from '../data/segment';
import pixelsToTileUnits from '../source/pixels_to_tile_units';
import * as symbolProjection from '../symbol/projection';
import * as symbolSize from '../symbol/symbol_size';
Expand All @@ -20,38 +21,62 @@ import {
import type Painter from './painter';
import type SourceCache from '../source/source_cache';
import type SymbolStyleLayer from '../style/style_layer/symbol_style_layer';
import type SymbolBucket from '../data/bucket/symbol_bucket';
import type SymbolBucket, {SymbolBuffers} from '../data/bucket/symbol_bucket';
import type Texture from '../render/texture';
import type {OverscaledTileID} from '../source/tile_id';
import type {UniformValues} from './uniform_binding';
import type {SymbolSDFUniformsType} from '../render/program/symbol_program';

export default drawSymbols;

type SymbolTileRenderState = {
buffers: SymbolBuffers,
program: any,
depthMode: DepthMode,
uniformValues: any,
atlasTexture: Texture,
atlasInterpolation: any,
isSDF: boolean,
hasHalo: boolean
};

function drawSymbols(painter: Painter, sourceCache: SourceCache, layer: SymbolStyleLayer, coords: Array<OverscaledTileID>) {
if (painter.renderPass !== 'translucent') return;

// Disable the stencil test so that labels aren't clipped to tile boundaries.
const stencilMode = StencilMode.disabled;
const colorMode = painter.colorModeForRenderPass();

const sortFeaturesByKey = layer.layout.get('symbol-sort-key').constantOr(1) !== undefined;

if (layer.paint.get('icon-opacity').constantOr(1) !== 0) {
const tileRenderState = sortFeaturesByKey ? [] : undefined;
drawLayerSymbols(painter, sourceCache, layer, coords, false,
layer.paint.get('icon-translate'),
layer.paint.get('icon-translate-anchor'),
layer.layout.get('icon-rotation-alignment'),
layer.layout.get('icon-pitch-alignment'),
layer.layout.get('icon-keep-upright'),
stencilMode, colorMode
stencilMode, colorMode, tileRenderState
);
if (sortFeaturesByKey) {
drawSymbolsSorted(painter, ((tileRenderState: any): Array<SymbolTileRenderState>), layer, colorMode, stencilMode);
}
}

if (layer.paint.get('text-opacity').constantOr(1) !== 0) {
const tileRenderState = sortFeaturesByKey ? [] : undefined;
drawLayerSymbols(painter, sourceCache, layer, coords, true,
layer.paint.get('text-translate'),
layer.paint.get('text-translate-anchor'),
layer.layout.get('text-rotation-alignment'),
layer.layout.get('text-pitch-alignment'),
layer.layout.get('text-keep-upright'),
stencilMode, colorMode
stencilMode, colorMode, tileRenderState
);
if (sortFeaturesByKey) {
drawSymbolsSorted(painter, ((tileRenderState: any): Array<SymbolTileRenderState>), layer, colorMode, stencilMode);
}
}

if (sourceCache.map.showCollisionBoxes) {
Expand All @@ -60,12 +85,13 @@ function drawSymbols(painter: Painter, sourceCache: SourceCache, layer: SymbolSt
}

function drawLayerSymbols(painter, sourceCache, layer, coords, isText, translate, translateAnchor,
rotationAlignment, pitchAlignment, keepUpright, stencilMode, colorMode) {
rotationAlignment, pitchAlignment, keepUpright, stencilMode, colorMode, tileRenderState) {

const context = painter.context;
const gl = context.gl;
const tr = painter.transform;

const sortByFeature = Boolean(tileRenderState);
const rotateWithMap = rotationAlignment === 'map';
const pitchWithMap = pitchAlignment === 'map';
const alongLine = rotateWithMap && layer.layout.get('symbol-placement') !== 'point';
Expand Down Expand Up @@ -99,19 +125,28 @@ function drawLayerSymbols(painter, sourceCache, layer, coords, isText, translate
context.activeTexture.set(gl.TEXTURE0);

let texSize: [number, number];
let atlasTexture;
let atlasInterpolation;
if (isText) {
tile.glyphAtlasTexture.bind(gl.LINEAR, gl.CLAMP_TO_EDGE);
atlasTexture = tile.glyphAtlasTexture;
atlasInterpolation = gl.LINEAR;
texSize = tile.glyphAtlasTexture.size;

} else {
const iconScaled = layer.layout.get('icon-size').constantOr(0) !== 1 || bucket.iconsNeedLinear;
const iconTransformed = pitchWithMap || tr.pitch !== 0;

tile.imageAtlasTexture.bind(isSDF || painter.options.rotating || painter.options.zooming || iconScaled || iconTransformed ?
gl.LINEAR : gl.NEAREST, gl.CLAMP_TO_EDGE);

atlasTexture = tile.imageAtlasTexture;
atlasInterpolation = isSDF || painter.options.rotating || painter.options.zooming || iconScaled || iconTransformed ?
gl.LINEAR :
gl.NEAREST;
texSize = tile.imageAtlasTexture.size;
}

if (!sortByFeature) {
atlasTexture.bind(atlasInterpolation, gl.CLAMP_TO_EDGE);
}

const s = pixelsToTileUnits(tile, 1, painter.transform.zoom);
const labelPlaneMatrix = symbolProjection.getLabelPlaneMatrix(coord.posMatrix, pitchWithMap, rotateWithMap, painter.transform, s);
const glCoordMatrix = symbolProjection.getGlCoordMatrix(coord.posMatrix, pitchWithMap, rotateWithMap, painter.transform, s);
Expand All @@ -124,36 +159,89 @@ function drawLayerSymbols(painter, sourceCache, layer, coords, isText, translate
uLabelPlaneMatrix = alongLine ? identityMat4 : labelPlaneMatrix,
uglCoordMatrix = painter.translatePosMatrix(glCoordMatrix, tile, translate, translateAnchor, true);

const hasHalo = isSDF && layer.paint.get(isText ? 'text-halo-width' : 'icon-halo-width').constantOr(1) !== 0;

let uniformValues;
if (isSDF) {
const hasHalo = layer.paint.get(isText ? 'text-halo-width' : 'icon-halo-width').constantOr(1) !== 0;

uniformValues = symbolSDFUniformValues(sizeData.functionType,
size, rotateInShader, pitchWithMap, painter, matrix,
uLabelPlaneMatrix, uglCoordMatrix, isText, texSize, true);

if (hasHalo) {
drawSymbolElements(buffers, layer, painter, program, depthMode, stencilMode, colorMode, uniformValues);
if (!sortByFeature) {
if (hasHalo) {
drawSymbolElements(buffers, buffers.segments, layer, painter, program, depthMode, stencilMode, colorMode, uniformValues);
}
uniformValues['u_is_halo'] = 0;
}

uniformValues['u_is_halo'] = 0;

} else {
uniformValues = symbolIconUniformValues(sizeData.functionType,
size, rotateInShader, pitchWithMap, painter, matrix,
uLabelPlaneMatrix, uglCoordMatrix, isText, texSize);
}

drawSymbolElements(buffers, layer, painter, program, depthMode, stencilMode, colorMode, uniformValues);
if (sortByFeature) {
((tileRenderState: any): Array<SymbolTileRenderState>).push({
buffers,
program,
depthMode,
uniformValues,
atlasTexture,
atlasInterpolation,
isSDF,
hasHalo
});
} else {
atlasTexture.bind(atlasInterpolation, gl.CLAMP_TO_EDGE);
drawSymbolElements(buffers, buffers.segments, layer, painter, program, depthMode, stencilMode, colorMode, uniformValues);
}
}
}

function drawSymbolElements(buffers, layer, painter, program, depthMode, stencilMode, colorMode, uniformValues) {
function drawSymbolElements(buffers, segments, layer, painter, program, depthMode, stencilMode, colorMode, uniformValues) {
const context = painter.context;
const gl = context.gl;
program.draw(context, gl.TRIANGLES, depthMode, stencilMode, colorMode, CullFaceMode.disabled,
uniformValues, layer.id, buffers.layoutVertexBuffer,
buffers.indexBuffer, buffers.segments, layer.paint,
buffers.indexBuffer, segments, layer.paint,
painter.transform.zoom, buffers.programConfigurations.get(layer.id),
buffers.dynamicLayoutVertexBuffer, buffers.opacityVertexBuffer);
}

function drawSymbolsSorted(painter: Painter, renderData: Array<SymbolTileRenderState>, layer, colorMode, stencilMode) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we refactor this so it's "prepare draw state per tile, then draw once per segment" whether we're sorting or not? I think it would be easier to follow that way -- we wouldn't have to deal with the ambiguous naming that drawLayerSymbols is sometimes a preparation step and sometimes an actual draw step.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think my thinking here was to avoid the extra overhead for symbol layers that don't need this but I should probably benchmark to see if this is valid. It looks like it should be pretty cheap to do what you are suggesting.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ChrisLoer I've pushed a commit to always use the preparation step. Can you review?

const symbols = [];

for (const data of renderData) {

const segments = data.buffers.segments.get();
for (const segment of segments) {
symbols.push({
data,
segment
});
}
}

symbols.sort((a, b) => {
return ((a.segment.sortKey: any): number) - ((b.segment.sortKey: any): number);
});

for (const symbol of symbols) {
const data = symbol.data;
const segments = new SegmentVector([symbol.segment]);

const gl = painter.context.gl;
data.atlasTexture.bind(data.atlasInterpolation, gl.CLAMP_TO_EDGE);

if (data.isSDF) {
const uniformValues = ((data.uniformValues: any): UniformValues<SymbolSDFUniformsType>);
if (data.hasHalo) {
uniformValues['u_is_halo'] = 1;
drawSymbolElements(data.buffers, segments, layer, painter, data.program, data.depthMode, stencilMode, colorMode, uniformValues);
}
data.uniformValues['u_is_halo'] = 0;
}
drawSymbolElements(data.buffers, segments, layer, painter, data.program, data.depthMode, stencilMode, colorMode, data.uniformValues);
}
}
20 changes: 19 additions & 1 deletion src/style-spec/reference/v8.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -958,17 +958,35 @@
},
"property-type": "data-constant"
},
"symbol-sort-key": {
"type": "number",
"doc": "Sorts features in ascending order based on this value. Features with a higher sort key will appear above features with a lower sort key wehn they overlap. Features with a lower sort key will have priority over other features when doing placement.",
"sdk-support": {
"js": "0.53.0"
},
"expression": {
"interpolated": false,
"parameters": [
"zoom",
"feature"
]
},
"property-type": "data-driven"
},
"symbol-z-order": {
"type": "enum",
"values": {
"auto": {
"doc": "If `symbol-sort-key` is set, sort based on that. Otherwise sort symbols by their position relative to the viewport."
},
"viewport-y": {
"doc": "Symbols will be sorted by their y-position relative to the viewport."
},
"source": {
"doc": "Symbols will be rendered in the same order as the source data with no sorting applied."
}
},
"default": "viewport-y",
"default": "auto",
"doc": "Controls the order in which overlapping symbols in the same layer are rendered",
"sdk-support": {
"basic functionality": {
Expand Down
3 changes: 2 additions & 1 deletion src/style-spec/types.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,8 @@ export type SymbolLayerSpecification = {|
"symbol-placement"?: PropertyValueSpecification<"point" | "line" | "line-center">,
"symbol-spacing"?: PropertyValueSpecification<number>,
"symbol-avoid-edges"?: PropertyValueSpecification<boolean>,
"symbol-z-order"?: PropertyValueSpecification<"viewport-y" | "source">,
"symbol-sort-key"?: DataDrivenPropertyValueSpecification<number>,
"symbol-z-order"?: PropertyValueSpecification<"auto" | "viewport-y" | "source">,
"icon-allow-overlap"?: PropertyValueSpecification<boolean>,
"icon-ignore-placement"?: PropertyValueSpecification<boolean>,
"icon-optional"?: PropertyValueSpecification<boolean>,
Expand Down
Loading