protomaps / basemaps

Basemap PMTiles generation and cartographic styles for OpenStreetMap data and more
https://maps.protomaps.com/
Other
347 stars 44 forks source link

More baselayers #224

Open pietervdvn opened 6 months ago

pietervdvn commented 6 months ago

Hi,

Because I didn't like the colour scheme of 'protomaps.light', I'm in the process of creating a new one - partly inspired by Mapnik/Osm-carto, partly by MapTiler. You can already browse it at https://dev.mapcomplete.org/cyclofix.html? (ignore the pins)

It uses the current hosted version of protomaps.

Would this be considered a nice addition for protomaps? If so, please point me where this theme should be put, I'll open a PR then.

bdon commented 6 months ago

In general, no, themes beyond the base 5 (light, dark, white, grayscale, black) we can't commit to maintaining indefinitely into the future.

The suggested theming approach is to create a set of named color variables like here https://github.com/protomaps/basemaps/blob/main/styles/src/themes.ts instead of writing a complete style, because that would let any color scheme take advantage of changes in the upstream style.

Otherwise you can suggest an improvement specific to what you don't like? Deriving things from OSM Carto is fine, but other styles will likely not be license-compatible with the styles in this repo.

Edefritz commented 6 months ago

Thanks for your suggestions about creating a theme instead of meddling with the styles themselves.

But as far as I can see, there is no way to pass a custom theme to any of the exported functions of the package. They basically only allow passing a key to existing themes:

export default function (source: string, key: string): LayerSpecification[] {
  const theme = themes[key];
  return nolabels_layers(source, theme).concat(labels_layers(source, theme));
}

export function noLabels(source: string, key: string): LayerSpecification[] {
  const theme = themes[key];
  return nolabels_layers(source, theme);
}

export function labels(source: string, key: string): LayerSpecification[] {
  const theme = themes[key];
  return labels_layers(source, theme);
}

I guess to do that we need something like this?

export function layersWithTheme(source: string, theme: Theme): LayerSpecification[] {
  return nolabels_layers(source, theme).concat(labels_layers(source, theme));
}

I'm happy to provide a PR if necessary. Or am I missing something?

bdon commented 6 months ago

@Edefritz yes, that looks like the right approach. It doesn't express a good way to separate labels/no labels for custom themes, or a way to inherit parts of a theme without re-defining every single theme property, but we can address those issues later. Can you open a PR?