Display boundaries and borders

[YongGoose]

fixes #928

[YongGoose]

@bchapuis

I tried to use the simplify method of DouglasPeuckerSimplifier in the MultiPolygonDataType class for simplification, but I couldn’t figure out where the simplification should be applied, so I couldn't implement it.

The situations where I think simplification should occur are:

1. When the zoom level falls below a certain threshold.
2. When there are multiple islands.

What do you think? 🤔

[YongGoose]

@bchapuis

It seems that some parts of the lines are not visible, possibly due to incomplete data being saved in the database.
Could you please check this?

[bchapuis]

I tried to use the simplify method of DouglasPeuckerSimplifier in the MultiPolygonDataType class for simplification, but I couldn’t figure out where the simplification should be applied, so I couldn't implement it.

I think the boundaries should be simplified using materialized views in the sql database with st_simplifypreservetopology. For instance, here are the simplifications applied to the natural layer (i.e. polygons).

I think that on the java side, we should probably add logic to build complex geometries corresponding to Boundaries which are not necessarily MultiPolygons. For instance, these geometries may be MultiLineString or GeometryCollection. There is probably some openstreetmap documentation reading to be made as I'm not sure how communities create boundaries. According to the wiki, all types are involved (nodes, ways corresponding to lines, ways corresponding to polygons, and relations).

As the name suggests, the current implementation of the RelationMultiPolygonBuilder probably miss the lines which are parts of boundaries, hence the discrepency when looking at the raster map. Maybe, we should introduce a new RelationBoundaryBuilder` that specifically focuses on creating geometries for boundaries and skip the other entities. This would allow to focus solely on this case.

[YongGoose]

I tried to use the simplify method of DouglasPeuckerSimplifier in the MultiPolygonDataType class for simplification, but I couldn’t figure out where the simplification should be applied, so I couldn't implement it.

I think the boundaries should be simplified using materialized views in the sql database with st_simplifypreservetopology. For instance, here are the simplifications applied to the natural layer (i.e. polygons).

I think that on the java side, we should probably add logic to build complex geometries corresponding to Boundaries which are not necessarily MultiPolygons. For instance, these geometries may be MultiLineString or GeometryCollection. There is probably some openstreetmap documentation reading to be made as I'm not sure how communities create boundaries. According to the wiki, all types are involved (nodes, ways corresponding to lines, ways corresponding to polygons, and relations).

As the name suggests, the current implementation of the RelationMultiPolygonBuilder probably miss the lines which are parts of boundaries, hence the discrepency when looking at the raster map. Maybe, we should introduce a new RelationBoundaryBuilder` that specifically focuses on creating geometries for boundaries and skip the other entities. This would allow to focus solely on this case.

Thank you! 👍🏻

I’ll go ahead and implement it while reading through the document you provided.
If I have any questions, I’ll leave a comment on this PR.

[YongGoose]

@bchapuis

I’ve been quite busy lately, so this task has been delayed.
I’ll make sure to complete it by this weekend. 😊

[bchapuis]

@YongGoose No problem, same situation here with many deadlines. Looking forward for your contributions.

[YongGoose]

@bchapuis

After reading your comment, I tried adding ST_SimplifyPreserveTopology to tile.js. b0a5e91
However, I’m wondering if this aligns with your intended approach.

Maybe, we should introduce a new RelationBoundaryBuilder` that specifically focuses on creating geometries for boundaries and skip the other entities. This would allow to focus solely on this case.

Could you provide a more detailed explanation?

I’ve been spending some personal time studying the documentation for MapLibre and OpenStreetMap.
I also plan to read the paper you’ve written in the future.

Thank you, as always!

PS. I think I'll be able to dedicate more time next week.

[bchapuis]

No problem. I hope the following will help clarify the steps. The natural layer is taken as an example.

Geometry Validation and Storage: The current Java code is responsible for processing OpenStreetMap data to create valid polygons geometries for the ways and relations tagged with natural. These geometries are then stored in the PostGIS database without further simplification. This is the java class we discussed.

https://github.com/apache/incubator-baremaps/blob/main/baremaps-openstreetmap/src/main/java/org/apache/baremaps/openstreetmap/function/RelationMultiPolygonBuilder.java

Simplification in the Database: Instead of simplifying the geometries in the Java code, this is currently handled within the database. Materialized views are used to store simplified geometries. This allows to perform advanced operations (e.g. filtering, clustering, buffering, etc.) to merge nearby natural polygons of the same type (e.g. forest, etc.).

incubator-baremaps/basemap/layers/natural/create.sql

Line 156 in 28e5bb4

CREATE

Vector Tile Querying: The materialized views containing the simplified geometries are finally queried in the tileset.js file to generate vector tiles. The variable $zoom can be used in queries to select the right table or materialized view (e.g. osm_natural_z20).

incubator-baremaps/basemap/layers/natural/tileset.js

Line 23 in 28e5bb4

"sql": "SELECT id, tags, geom FROM osm_natural_z$zoom"

I hope this help. Do not hesitate to ask if you need further details, it really helps at figuring out where additional documentation is needed.

[YongGoose]

@bchapuis

First, I decided to implement the Java code first. d17166d

As a result, I introduced RelationBoundaryBuilder, as you suggested.
It processes nodes, ways corresponding to lines, ways corresponding to polygons, and relations, following the OpenStreetMap wiki.

This class is still in its early stages and will likely require many modifications.
If my implementation is heading in the wrong direction, I would appreciate any feedback.

[bchapuis]

@YongGoose I think it is a great start. I havn't used the GeometryCombiner yet, but according to the javadoc it looks like the right tool.

From what I understand, the RelationBoundaryBuilder consumer handles all relation regardless of their tags. It may be a good idea to add an early return in the accept method for relations that don't have the boundary tag. Similarly, it may be a good to add an early return for the relations that have the boundary tag in the RelationMultiPolygonBuilder. Using this approach, we should then be able to chain the Consumers with the .andThen method while ensuring that they don't interfer with each others.

[bchapuis]

Regarding the build, you may want to run ./mvnw spotless:apply to ensure that the code is well formatted. You may also want to add the Apache License at the begining of the file to ensure that it passes the apache rat check in the CI.

[YongGoose]

Regarding the build, you may want to run ./mvnw spotless:apply to ensure that the code is well formatted. You may also want to add the Apache License at the begining of the file to ensure that it passes the apache rat check in the CI.

Thank you!

I followed your advice by adding the license and applying Spotless, and it worked perfectly.
Since I'm more familiar with Gradle, I don't use Maven often, so I struggled a bit.

I really appreciate your help! 🙂

[YongGoose]

@YongGoose I think it is a great start. I havn't used the GeometryCombiner yet, but according to the javadoc it looks like the right tool.

From what I understand, the RelationBoundaryBuilder consumer handles all relation regardless of their tags. It may be a good idea to add an early return in the accept method for relations that don't have the boundary tag. Similarly, it may be a good to add an early return for the relations that have the boundary tag in the RelationMultiPolygonBuilder. Using this approach, we should then be able to chain the Consumers with the .andThen method while ensuring that they don't interfer with each others.

Done in d128e7b !

[YongGoose]

@bchapuis

Additionally, this is unrelated to the PR, but are you able to see the demo map on the Baremaps site? It doesn’t seem to be displaying properly for me.

If you're experiencing the same issue, I'll investigate it further tomorrow or the day after and create an issue if necessary.

I was about to open an issue in the baremaps-site repository but thought I should check first, as this might be a problem on my end.

[bchapuis]

@YongGoose I saw a couple of emails related to the deployment of strict CSP headers on the server of the apache software foundation. Let me know what you find tomorrow, I may have time to have a look at this as well on thursday.

[bchapuis]

I think it's good idea to open an issue to track this.

[YongGoose]

I think it's good idea to open an issue to track this.

I’ve created an issue to track this problem.

• Content Security Policy (CSP) Violation Preventing API and Script Loading incubator-baremaps-site#55

I’m not quite sure how to fix it yet, so it would be great to discuss possible solutions together. 🙂

[sonarqubecloud]

Quality Gate passed

Issues
6 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[YongGoose]

@YongGoose I think it is a great start. I havn't used the GeometryCombiner yet, but according to the javadoc it looks like the right tool.

From what I understand, the RelationBoundaryBuilder consumer handles all relation regardless of their tags. It may be a good idea to add an early return in the accept method for relations that don't have the boundary tag. Similarly, it may be a good to add an early return for the relations that have the boundary tag in the RelationMultiPolygonBuilder. Using this approach, we should then be able to chain the Consumers with the .andThen method while ensuring that they don't interfer with each others.

@bchapuis

Thanks for the feedback!

Based on your suggestion, I implemented chaining multiple Consumers using .andThen to ensure they don’t interfere with each other.

Now, the test is failing in OsmDataTest, and I'm not sure how to modify the code to fix it yet. I would really appreciate your help.

org.opentest4j.AssertionFailedError: 950: Multipolygon with one outer and one inner ring, tags on relation, type boundary.
Expected:
POLYGON ((9.01 1.51, 9.01 1.59, 9.09 1.59, 9.09 1.51, 9.01 1.51), (9.03 1.53, 9.03 1.57, 9.07 1.57, 9.07 1.53, 9.03 1.53))
Actual:
GEOMETRYCOLLECTION (GEOMETRYCOLLECTION (POLYGON ((9.01 1.51, 9.01 1.59, 9.09 1.59, 9.09 1.51, 9.01 1.51)), LINESTRING (9.03 1.53, 9.03 1.57, 9.07 1.57), LINESTRING (9.07 1.57, 9.07 1.53, 9.03 1.53))) ==> 
Expected :true
Actual   :false
<Click to see difference>


	at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
	at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
	at org.junit.jupiter.api.AssertTrue.failNotTrue(AssertTrue.java:63)
	at org.junit.jupiter.api.AssertTrue.assertTrue(AssertTrue.java:36)
	at org.junit.jupiter.api.Assertions.assertTrue(Assertions.java:214)
	at org.apache.baremaps.openstreetmap.OsmDataTest.runTest(OsmDataTest.java:132)
	at org.apache.baremaps.openstreetmap.OsmDataTest.lambda$createDynamicTest$1(OsmDataTest.java:72)
	at java.base/java.util.Optional.ifPresent(Optional.java:178)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)

[bchapuis]

@YongGoose Sorry, I missed this notification. Do not hesitate to ping me when this happen.

I believe that the openstreetmap relation of the test case 950 is not handled by the default consumer anymore. Maybe we should limit the tests to this consumer for now and ensure that the new one is tested independently.

https://github.com/apache/incubator-baremaps/blob/28e5bb4b9d1693d297dc849074b9126dd392ce4d/baremaps-testing/data/osm-testdata/9/950/data.osm

[YongGoose]

@YongGoose Sorry, I missed this notification. Do not hesitate to ping me when this happen.

I believe that the openstreetmap relation of the test case 950 is not handled by the default consumer anymore. Maybe we should limit the tests to this consumer for now and ensure that the new one is tested independently.

https://github.com/apache/incubator-baremaps/blob/28e5bb4b9d1693d297dc849074b9126dd392ce4d/baremaps-testing/data/osm-testdata/9/950/data.osm

It’s a common occurrence, so no need to worry!
I also miss notifications from time to time. 😅

As you mentioned, I’ll add test cases for the newly added consumer.