Shield rotation #716
Shield rotation #716
Conversation
|
To get started, run
It is good that the shields are grouped, but their orientation should be vertical. |
|
The code is copy-pasted from https://github.com/ZeLonewolf/maplibre-shield-rotation-sample and was written by @ZeLonewolf |
|
Bundle size report: Size Change: +42 B
ℹ️ View Details
|
|
If maplibre-gl-js/src/symbol/quads.ts Lines 316 to 325 in d00f754 |
|
The glyph quads will rotate more later to align with the orientation of the line. |
|
I think I found a way to achieve what we want:
For this what I changed was this: Which I hard-coded to zero, Of course, this is not sustainable as it basically breaks the rest of the library but at least I found a line in the code-base where a tweak will stop symbols and text from being rotated. |
|
That preview looks awesome! It would have taken me forever just to find the spot in the codebase where that code even happens. |
|
Doing this in the shaders as suggested above is probably the worst option. We can achieve the same result by hard-coding something in a TypeScript function called maplibre-gl-js/src/symbol/projection.ts Lines 336 to 427 in d00f754 If I make the return statement this: return {
point: p,
angle: 0.0, // segmentAngle,
path: pathVertices
};I also get upright shields. |
|
This hack does not work with rotated or pitched maps. What would be your expectation if the map is rotated or pitched? |
|
For a rotated or tilted map, I would expect the shields to tilt while maintaining a vertical orientation. What does ordinary text currently do on rotation? Does it maintain an upright bias like it does on an unrotated/untilted map? |
|
This is probably quite close to what we want. It is just a bit strange that at zero pitch, i.e., default view which is perpendicular to the map, the shield looks quite bad (worse than without the text-pitch-alignment). Without
With
With
|
|
I added a new boolean style property called @ZeLonewolf can you try it out and give me some feedback? |
|
Thanks @wipfli I would be thrilled to test this out. I'll let you know how it goes. |
|
|
|
Wow! This looks awesome in 2D in Indiana:
Pan and tilt:
We might want to change the orientation on the pan and tilt version, but this is already better than the status quo. |
|
One thing I've noticed is that the corners of icons overlap when they're placed along a diagonal, but there's space between them when they're arranged vertically or horizontally. In the screenshots below, there is a single space character between each icon pair:
|
|
I wonder if MapLibre handles this correctly for collisions that have nothing to do with my change. Can you check what happens if you set |
|
By the way I love all these little shields with their strange shapes. Some of them look like a map on the map. |
|
With On a 45-degree road, I get the expected spacing: |
|
If you want to build the library from source code and produce the files in the nvm use 14
npm ci
npm run build-prod
ls dist
# maplibre-gl.css maplibre-gl-dev.js maplibre-gl.d.ts maplibre-gl-unminified.js maplibre-gl-unminified.js.map package.json |
|
Since |
I agree. This is an issue for letters also, which overlap when following lines with too tight of a radius. |
|
Hey - quick question here - for onthegomap.com, I implemented highway shields by using: 'layout': {
'icon-image': [
'concat',
['get', 'network'],
'-',
['to-string', ['max', 2, ['get', 'ref_length']]],
],
'icon-rotation-alignment': 'viewport',
'symbol-placement': 'line',
'symbol-spacing': 500,
'text-field': ['to-string', ['get', 'ref']],
'text-font': ['Robonoto Regular'],
'text-offset': [0, 0.1],
'text-rotation-alignment': 'viewport',
'text-size': 10,
'icon-size': 0.9,
},2 questions:
Also, I like the highway shield images! Are they available anywhere for other sites to use? |
|
This is my MO basically:
Here's my initial code, far from complete at it fails somewhere in the middle of the code. import {SymbolLayerSpecification} from '../style-spec/types.g';
import EvaluationParameters from '../style/evaluation_parameters';
import SymbolStyleLayer from '../style/style_layer/symbol_style_layer';
import ZoomHistory from '../style/zoom_history';
import drawSymbol from './draw_symbol';
import Painter from './painter';
jest.mock('./painter');
describe('drawSymbol', () => {
test('should not do anything', () => {
const mockPainter = new Painter(null, null);
mockPainter.renderPass = 'opaque';
drawSymbol(mockPainter, null, null, null, null);
expect(mockPainter.colorModeForRenderPass).not.toHaveBeenCalled();
});
test('should draw and rotate glyphs according to the line', () => {
const mockPainter = new Painter(null, null);
mockPainter.context = {
gl: jest.fn() as any
} as any;
mockPainter.renderPass = 'translucent';
const layerSpec = {
id: 'mock-layer',
source: 'empty-source',
type: 'symbol',
layout: {
'text-variable-anchor': ['center']
},
paint: {}
} as SymbolLayerSpecification;
const layer = new SymbolStyleLayer(layerSpec);
layer.recalculate({zoom: 0, zoomHistory: {} as ZoomHistory} as EvaluationParameters, []);
// a lot more mocking is needed here, I copy it from different tests until it passes the line of code I'm not interested in, in the production code
drawSymbol(mockPainter, null, layer, [], null);
// expect program.draw to be called with specific parameters
});
}); |
|
The following is the entire test file that gets to the program.draw method: import { mat4 } from 'gl-matrix';
import SymbolBucket from '../data/bucket/symbol_bucket';
import SourceCache from '../source/source_cache';
import Tile from '../source/tile';
import {OverscaledTileID} from '../source/tile_id';
import {SymbolLayerSpecification} from '../style-spec/types.g';
import EvaluationParameters from '../style/evaluation_parameters';
import SymbolStyleLayer from '../style/style_layer/symbol_style_layer';
import ZoomHistory from '../style/zoom_history';
import drawSymbol from './draw_symbol';
import Painter from './painter';
import Program from './program';
jest.mock('./painter');
jest.mock('./program');
jest.mock('../source/source_cache');
jest.mock('../source/tile');
jest.mock('../data/bucket/symbol_bucket');
describe('drawSymbol', () => {
test('should not do anything', () => {
const mockPainter = new Painter(null, null);
mockPainter.renderPass = 'opaque';
drawSymbol(mockPainter, null, null, null, null);
expect(mockPainter.colorModeForRenderPass).not.toHaveBeenCalled();
});
test('should update variable anchors', () => {
const mockPainter = new Painter(null, null);
mockPainter.context = {
gl: {},
activeTexture: {
set: () => {}
}
} as any;
mockPainter.renderPass = 'translucent';
mockPainter.transform = { pitch: 0, labelPlaneMatrix: mat4.create() } as any;
mockPainter.options = {} as any;
const layerSpec = {
id: 'mock-layer',
source: 'empty-source',
type: 'symbol',
layout: {
//'text-variable-anchor': ['center']
},
paint: {
'text-opacity': 1
}
} as SymbolLayerSpecification;
const layer = new SymbolStyleLayer(layerSpec);
layer.recalculate({zoom: 0, zoomHistory: {} as ZoomHistory} as EvaluationParameters, []);
const tileId = new OverscaledTileID(1, 0, 1, 0, 0);
tileId.posMatrix = mat4.create();
const programMock = new Program(null, null, null, null, null, null);
(mockPainter.useProgram as jest.Mock).mockReturnValue(programMock);
const bucket = new SymbolBucket(null);
bucket.icon = {
programConfigurations: {
get: () => {}
},
segments: {
get: () => [1]
}
} as any;
bucket.iconSizeData = {
kind: 'constant',
layoutSize: 1
}
const tile = new Tile(tileId, 256);
tile.tileID = tileId;
tile.imageAtlasTexture = {
bind: () => {}
} as any;
(tile.getBucket as jest.Mock).mockReturnValue(bucket);
const sourceCacheMock = new SourceCache(null, null, null);
(sourceCacheMock.getTile as jest.Mock).mockReturnValue(tile);
sourceCacheMock.map = { showCollisionBoxes: false } as any;
drawSymbol(mockPainter, sourceCacheMock, layer, [tileId], null);
expect(programMock.draw).toHaveBeenCalledTimes(1);
});
});I have added a lot of typings to the |
|
This should conflict, shouldn't it? |
|
Merged main into shield-rotation without conflicts. Where did you expect a collision? |
|
In the draw symbol file. But I might have underestimated the merge engine :-) |
|
Are you now able to write the relevant test(s) or is my assistance still needed? |
|
Thanks for adding that test @HarelM. Sorry if I am a bit slow to understand here, but which part of 70,114-235,240,242,306-313,334,346-351,374-377,393,402-404,409-414 |
|
I didn't cover everything in that PR. I suggest only to add a test for this logic that was changed as part of this PR. |
|
In general, we should cover as much as possible. But I don't want to hold this PR due to low coverage. |
|
Overall very nice! |
|
I think this PR should wait until #1022 will be merges since I think solving conflicts in this PR will be easier... |
|
Thanks for the |
|
Cool, I'll approve, you can decide if you want to merge this into main and then merge to terrain3d or wait, your call. |
Image symbols placed along a line currently rotate with the line, similar to what text along a river or street does.
We would like to place multiple images along a line that are always upright.
The images are highway shields and highways in the US can have multiple shields.
This pull requests is work in progress and facilitates collaboration on the new feature.
## Launch Checklist
maplibre-gl-jschangelog:<changelog></changelog>.