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

water matcher #416

Open
wants to merge 24 commits into
base: main
Choose a base branch
from
Open

water matcher #416

wants to merge 24 commits into from

Conversation

wipfli
Copy link
Collaborator

@wipfli wipfli commented Mar 9, 2025

Copied filter rules from tilezen's water.yaml file. Each copied filter rule was marked with "done". Each skipped item was marked with "skip" in wipfli/vector-datasource#1

Skipped are the following

  • NE coast lines
  • intermittent
  • boat
  • reservoir
  • alkaline

Rules in tilezen were evaluated top to bottom, we do pick the last. So I reversed the order of the filter rules, e.g., covered=yes appears last here, in tilezen it was first...

@wipfli wipfli marked this pull request as draft March 9, 2025 18:23
@wipfli
Copy link
Collaborator Author

wipfli commented Mar 10, 2025

@wipfli
Copy link
Collaborator Author

wipfli commented Mar 11, 2025

Since the water layer is closely related to the earth layer, I also touched the Earth.java file. Islands in the Mediterran for example appear now at the same zoom level as the corresponding hole appears in the water layer. At least mostly.

I found that if we use FeatureMerge.mergeNearbyPolygons without any buffer, i.e., buffer=0, then there are sometimes artifacts like spikes in earth and water polygons around zoom levels 6, 7, 8. I chose therefore to buffer but if the buffer is too large, e.g., buffer=0.5, then islands don't get corresponding holes in the water layer. The smallest buffer I saw had an effect on removing the spikes artifacts was the tile resolution, i.e., 0.0625.

Since I was touching the Earth.java file, I saw that the Antarctica fix is there but it should really be in the Landcover.java file. So I moved it over...

@wipfli
Copy link
Collaborator Author

wipfli commented Mar 11, 2025

There are also some minor tweaks to the Landcover.java polygon merging. I can also revert those. I think if we want to go deeper into landcover I would do it in a separate pull request. Google maps buffers landcover by the way, I guess to avoid the gaps that you always get due to feature dropping and simplification:

image

@wipfli
Copy link
Collaborator Author

wipfli commented Mar 11, 2025

Uploaded a new demo file to protomaps.dev with the layers water, earth, and landcover:

https://maps.protomaps.com/#map=2.08/-1.83/20.53&theme=light&lang=en&tiles=https://protomaps.dev/~wipfli/water-matcher.pmtiles&flavorName=light

I just noticed that the Atlantic Ocean label and similar ones are missing below zoom 6. Need to fix this...

@wipfli
Copy link
Collaborator Author

wipfli commented Mar 11, 2025

The last commit introduces minzooms for some place=sea labels.

image

@wipfli
Copy link
Collaborator Author

wipfli commented Mar 11, 2025

Closes #229

@wipfli
Copy link
Collaborator Author

wipfli commented Mar 11, 2025

Some place=sea come at z3, all others at z6. Looks like this for the less important sea labels at z6 in the Mediterranean:

image

@wipfli
Copy link
Collaborator Author

wipfli commented Mar 12, 2025

@wipfli
Copy link
Collaborator Author

wipfli commented Mar 12, 2025

MinZoom for points should be 15, and not 12. Otherwise we get all these fountains etc on the map at z12

@wipfli
Copy link
Collaborator Author

wipfli commented Mar 12, 2025

Also these bays from nodes in Croatia are a bit too much at z12. Better to show them only from z15 on...
image
https://maps.protomaps.com/#map=12.36/44.37243/15.25533&theme=light&lang=en&tiles=https://protomaps.dev/~wipfli/water-matcher.pmtiles&flavorName=light

@wipfli wipfli marked this pull request as ready for review March 12, 2025 12:47
@wipfli
Copy link
Collaborator Author

wipfli commented Mar 12, 2025

I found that it is important to use MultiExpression.ofOrdered() instead of MultiExpression.of() because we rely on the order in which rules appear...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant