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

Rewrite road layer #995

Draft
wants to merge 11 commits into
base: main
Choose a base branch
from
36 changes: 33 additions & 3 deletions src/js/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,24 @@ export function layerClone(def, id) {
//Make a clone of a layer definition, with a filter added
export function filteredClone(def, filterStep, idSuffix) {
var clone = layerClone(def, def.id + idSuffix);
if (!["all", "any"].includes(clone.filter[0])) {
throw new TypeError("Unlikely filter");
switch (clone.filter[0]) {
case "all":
case "any":
clone.filter.push(filterStep);
break;
case "interpolate":
case "interpolate-hcl":
case "interpolate-lab":
case "step":
clone.filter = mapRampExpression(clone.filter, (input, output) => [
"all",
output,
filterStep,
]);
break;
default:
throw new TypeError("Unlikely filter");
}
clone.filter.push(filterStep);
return clone;
}

Expand All @@ -38,3 +52,19 @@ export function zoomMultiply(arr, multiplier) {
}
return transformedArray;
}

/**
* Returns a copy of the interpolate or step expression with each output replaced with the return value of the callback.
*
* @param expression An interpolate or step expression.
* @param callback A function that takes the expression's input value and output expression and returns a replacement expression.
* @returns A copy of the interpolate or step expression with the output expressions replaced.
*/
export function mapRampExpression(expression, callback) {
let copy = [...expression];
let start = copy[0] === "step" ? 1 : 3;
for (var i = start; i + 1 < copy.length; i += 2) {
copy[i + 1] = callback(copy[i], copy[i + 1]);
}
return copy;
}
17 changes: 9 additions & 8 deletions src/layer/construction.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
"use strict";

const majorConstruction = [
"match",
["get", "class"],
["motorway_construction", "trunk_construction"],
];
const motorwayHue = 218;
const majorConstruction = ["match", ["get", "class"], "motorway_construction"];

export const road = {
id: "highway-construction",
Expand Down Expand Up @@ -34,11 +31,15 @@ export const road = {
["exponential", 2],
["zoom"],
10,
[...majorConstruction, "lightcoral", "lightslategray"],
[...majorConstruction, `hsl(${motorwayHue}, 60%, 70%)`, "lightslategray"],
13,
[...majorConstruction, "maroon", "lightslategray"],
[
...majorConstruction,
`hsl(${motorwayHue}, 100%, 45%)`,
"lightslategray",
],
15,
[...majorConstruction, "maroon", "slategray"],
[...majorConstruction, `hsl(${motorwayHue}, 100%, 35%)`, "slategray"],
],
"line-opacity": ["interpolate", ["exponential", 2], ["zoom"], 10, 0, 11, 1],
"line-blur": 0.75,
Expand Down
78 changes: 5 additions & 73 deletions src/layer/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,7 @@ export function build(locales) {

lyrConstruction.road,

lyrRoad.roadTunnel.casing(),

lyrRoad.roadTunnel.fill(),
lyrRoad.roadTunnel,

lyrOneway.tunnel,

Expand All @@ -88,41 +86,8 @@ export function build(locales) {
lyrAeroway.taxiway,
lyrAeroway.taxiwayArea,

lyrRoad.motorwayLink.casing(),
lyrRoad.trunkLink.casing(),

lyrRoad.roadLinkSimpleCasing.casing(),

lyrRoad.motorway.casing(),
lyrRoad.trunk.casing(),
lyrRoad.primaryExpressway.casing(),
lyrRoad.secondaryExpressway.casing(),
lyrRoad.tertiaryExpressway.casing(),

lyrRoad.roadSimpleCasing.casing(),

lyrRoad.motorwayLink.fill(),
lyrRoad.roadLinkSimpleFill.fill(),
lyrRoad.primaryLink.fill(),
lyrRoad.primaryLinkToll.fill(),
lyrRoad.secondaryLink.fill(),
lyrRoad.secondaryLinkToll.fill(),
lyrRoad.tertiaryLink.fill(),
lyrRoad.tertiaryLinkToll.fill(),

lyrRoad.minor.fill(),
lyrRoad.minorToll.fill(),
lyrRoad.busway.fill(),
lyrRoad.tertiary.fill(),
lyrRoad.tertiaryToll.fill(),
lyrRoad.secondary.fill(),
lyrRoad.secondaryToll.fill(),
lyrRoad.primary.fill(),
lyrRoad.primaryToll.fill(),
lyrRoad.roadSimpleFill.fill(),
lyrRoad.motorway.fill(),

lyrRoad.road.surface(),
lyrRoad.roadBridgeCasing,
lyrRoad.road,

lyrRail.rail.dashes(),
lyrRail.railService.dashes(),
Expand All @@ -147,41 +112,8 @@ export function build(locales) {
var bridgeLayers = [
lyrRail.bridgeCasing,

lyrRoad.trunkLinkBridge.casing(),
lyrRoad.motorwayLinkBridge.casing(),

lyrRoad.roadLinkSimpleCasingBridge.casing(),

lyrRoad.tertiaryExpresswayBridge.casing(),
lyrRoad.secondaryExpresswayBridge.casing(),
lyrRoad.primaryExpresswayBridge.casing(),
lyrRoad.trunkBridge.casing(),
lyrRoad.motorwayBridge.casing(),

lyrRoad.roadSimpleCasingBridge.casing(),

lyrRoad.tertiaryLinkBridge.fill(),
lyrRoad.tertiaryLinkTollBridge.fill(),
lyrRoad.secondaryLinkBridge.fill(),
lyrRoad.secondaryLinkTollBridge.fill(),
lyrRoad.primaryLinkBridge.fill(),
lyrRoad.primaryLinkTollBridge.fill(),
lyrRoad.roadLinkSimpleFillBridge.fill(),
lyrRoad.motorwayLinkBridge.fill(),

lyrRoad.minorBridge.fill(),
lyrRoad.minorTollBridge.fill(),
lyrRoad.buswayBridge.fill(),
lyrRoad.tertiaryBridge.fill(),
lyrRoad.tertiaryTollBridge.fill(),
lyrRoad.secondaryBridge.fill(),
lyrRoad.secondaryTollBridge.fill(),
lyrRoad.primaryBridge.fill(),
lyrRoad.primaryTollBridge.fill(),
lyrRoad.roadSimpleFillBridge.fill(),
lyrRoad.motorwayBridge.fill(),

lyrRoad.roadBridge.surface(),
lyrRoad.roadBridgeKnockout,
lyrRoad.road,

lyrRail.railBridge.dashes(),
lyrRail.railServiceBridge.dashes(),
Expand Down
16 changes: 3 additions & 13 deletions src/layer/oneway.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,19 +53,7 @@ export const surface = {
0.6,
],
],
"icon-image": [
"match",
["get", "brunnel"],
"tunnel",
"oneway_black",
[
"match",
["get", "toll"],
1,
"oneway_black",
[...highwaySelector, "motorway", "oneway_white", "oneway_black"],
],
],
"icon-image": "oneway_black",
visibility: "visible",
"icon-padding": 2,
"symbol-spacing": [
Expand All @@ -79,6 +67,8 @@ export const surface = {
],
"symbol-placement": "line",
"icon-rotation-alignment": "map",
// Assumes driving on the right.
"icon-offset": [0, 10],
Comment on lines +70 to +71
Copy link
Member

Choose a reason for hiding this comment

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

I like the offset effect, but it looks really clunky in left-hand-drive localities, including the US Virgin Islands. Perhaps an asymmetric oneway icon could make it look a little better.

Copy link
Member Author

@1ec5 1ec5 Apr 1, 2024

Choose a reason for hiding this comment

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

I was thinking this might fall under the notion that we’re primarily optimizing for North America, but a regression in left-hand driving countries would be far from ideal, and you have a point about USVI. We could bundle a GeoJSON of left-hand driving countries and reference it within a within expression, similar to #749. Performance might be an issue, however.

Copy link
Member Author

@1ec5 1ec5 Apr 1, 2024

Choose a reason for hiding this comment

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

Really the effect I’m going for is how printed maps traditionally would append an arrow to the (offset) label of each one-way street, rather than treating it as a separately placed repeating element. That would have the benefit of less repetition and probably handling divided highways better in left-hand driving countries. But I don’t think that’s feasible here, because the oneway field is only present on the transportation layer, not the transportation_name layer.

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess 25.7 kilobytes or 57.2 kB isn’t too bad, though it sure would be nice if every road just had its country code on it.

},
paint: {
"icon-opacity": 0.5,
Expand Down
Loading