GeoJson structure breaks (#1089)

commons/src/commonMain/kotlin/org/jetbrains/letsPlot/commons/intern/typedGeometry/algorithms/Geometry.kt

@@ -58,45 +58,52 @@ private fun <T> findRingIntervals(path: List<T>, eq: (T, T) -> Boolean): List<In
  return intervals
 }
 
-fun <T> isRingTrimmed(ring: List<T>, eq: (T, T) -> Boolean): Boolean {
- if (ring.size < 3) {
- return true
- }
-
- if (!eq(ring[0], ring[1]) && !eq(ring[ring.lastIndex], ring[ring.lastIndex - 1])) {
- return true
+fun <T> isRingNormalized(ring: List<T>, eq: (T, T) -> Boolean): Boolean {
+ if (ring.isEmpty()) return true
+ var isRingOpened = true
+ ring.asSequence().drop(1).forEach {
+ if (!eq(it, ring.first())) {
+ if (!isRingOpened) return false
+ } else {
+ isRingOpened = !isRingOpened
+ }
  }
+ if (isRingOpened) return false
 
- return false
+ return true
 }
 
-// Remove the same points from the beginning and the end of the ring. If trim is not needed, return the original ring.
-fun <T> trimRing(ring: List<T>, eq: (T, T) -> Boolean): List<T> {
- if (isRingTrimmed(ring, eq)) {
- return ring
+// Normalized ring means that it is possible to draw it without artifacts.
+// Artifacts can be caused by the ring not being closed or containing some unclosed subrings.
+// This function normalizes the ring by adding the missing elements to close the subrings.
+fun <T> normalizeRing(ring: List<T>, eq: (T, T) -> Boolean): List<T> {
+ if (isRingNormalized(ring, eq)) return ring
+
+ val normalizedRing = mutableListOf<T>()
+ normalizedRing.add(ring.first())
+ var isRingOpened = true
+ ring.asSequence().drop(1).forEach {
+ if (!eq(it, ring.first())) {
+ if (!isRingOpened) {
+ normalizedRing.add(ring.first())
+ isRingOpened = true
+ }
+ } else {
+ isRingOpened = !isRingOpened
+ }
+ normalizedRing.add(it)
  }
-
- val firstElement = ring.first()
- val lastElement = ring.last()
-
- val inner = ring.subList(1, ring.lastIndex)
-
- val startSkipCount = inner.indexOfFirst { !eq(it, firstElement) }
- val endSkipCount = inner.asReversed().indexOfFirst { !eq(it, lastElement) }
-
- // All items are the same - trim to two items
- if (startSkipCount == -1 || endSkipCount == -1) {
- return listOf(firstElement, lastElement)
+ if (isRingOpened) {
+ normalizedRing.add(ring.first())
  }
 
- return ring.subList(startSkipCount, ring.lastIndex - endSkipCount + 1)
+ return normalizedRing
 }
 
 private fun <T> List<T>.sublist(range: IntSpan): List<T> {
  return this.subList(range.lowerEnd, range.upperEnd)
 }
 
-
 fun calculateArea(ring: List<DoubleVector>): Double {
  return calculateArea(ring, DoubleVector::x, DoubleVector::y)
 }

commons/src/jvmTest/kotlin/org/jetbrains/letsPlot/commons/intern/typedGeometry/algorithms/GeometryTests.kt

@@ -11,57 +11,94 @@ import kotlin.test.Test
 class GeometryTests {
 
  @Test
- fun `trim not repeating not closed ring with 2 elements`() {
- assertThat(trimRing(listOf(1, 2), Int::equals))
- .isEqualTo(listOf(1, 2))
+ fun `normalize empty ring`() {
+ assertThat(normalizeRing(emptyList(), Int::equals))
+ .isEqualTo(emptyList<Int>())
+ }
+
+ @Test
+ fun `normalize ring with one element`() {
+ assertThat(normalizeRing(listOf(1), Int::equals))
+ .isEqualTo(listOf(1, 1))
+ }
+
+ @Test
+ fun `normalize not repeating not closed ring with 2 elements`() {
+ assertThat(normalizeRing(listOf(1, 2, 1), Int::equals))
+ .isEqualTo(listOf(1, 2, 1))
  }
 
  @Test
- fun `trim not repeating closed ring with 4 elements`() {
- assertThat(trimRing(listOf(1, 2, 3, 1), Int::equals))
+ fun `normalize not repeating closed ring with 4 elements`() {
+ assertThat(normalizeRing(listOf(1, 2, 3, 1), Int::equals))
  .isEqualTo(listOf(1, 2, 3, 1))
  }
 
  @Test
- fun `trim not repeating not closed ring`() {
- assertThat(trimRing(listOf(1, 2, 3, 4), Int::equals))
- .isEqualTo(listOf(1, 2, 3, 4))
+ fun `normalize not repeating not closed ring`() {
+ assertThat(normalizeRing(listOf(1, 2, 3, 4), Int::equals))
+ .isEqualTo(listOf(1, 2, 3, 4, 1))
  }
 
  @Test
- fun `trim not closed simple ring`() {
- assertThat(trimRing(listOf(1, 1, 2, 3, 4, 4), Int::equals))
- .isEqualTo(listOf(1, 2, 3, 4))
+ fun `normalize not closed simple ring`() {
+ assertThat(normalizeRing(listOf(1, 1, 2, 3, 4, 4), Int::equals))
+ .isEqualTo(listOf(1, 1, 1, 2, 3, 4, 4, 1))
  }
 
  @Test
- fun `trim ring with 2 equal elements`() {
- assertThat(trimRing(listOf(1, 1), Int::equals))
+ fun `normalize ring with 2 equal elements`() {
+ assertThat(normalizeRing(listOf(1, 1), Int::equals))
  .isEqualTo(listOf(1, 1))
  }
 
  @Test
- fun `trim ring with 3 equal elements`() {
- assertThat(trimRing(listOf(1, 1, 1), Int::equals))
- .isEqualTo(listOf(1, 1))
+ fun `normalize ring with 2 different elements`() {
+ assertThat(normalizeRing(listOf(1, 2), Int::equals))
+ .isEqualTo(listOf(1, 2, 1))
  }
 
  @Test
- fun `trim ring with 4 equal elements`() {
- assertThat(trimRing(listOf(1, 1, 1, 1), Int::equals))
- .isEqualTo(listOf(1, 1))
+ fun `normalize ring with 3 equal elements`() {
+ assertThat(normalizeRing(listOf(1, 1, 1), Int::equals))
+ .isEqualTo(listOf(1, 1, 1, 1))
  }
 
  @Test
- fun `trim ring with last different element`() {
- assertThat(trimRing(listOf(1, 1, 1, 1, 2), Int::equals))
- .isEqualTo(listOf(1, 2))
+ fun `normalize ring with 4 equal elements`() {
+ assertThat(normalizeRing(listOf(1, 1, 1, 1), Int::equals))
+ .isEqualTo(listOf(1, 1, 1, 1))
  }
 
  @Test
- fun `trim empty ring`() {
- assertThat(trimRing(emptyList(), Int::equals))
- .isEqualTo(emptyList<Int>())
+ fun `normalize ring with last different element`() {
+ assertThat(normalizeRing(listOf(1, 1, 1, 1, 2), Int::equals))
+ .isEqualTo(listOf(1, 1, 1, 1, 1, 2, 1))
+ }
+
+
+ @Test
+ fun `normalize ring with last same elements`() {
+ assertThat(normalizeRing(listOf(1, 2, 3, 4, 1, 1, 1), Int::equals))
+ .isEqualTo(listOf(1, 2, 3, 4, 1, 1, 1))
+ }
+
+ @Test
+ fun `normalize ring with two last same elements`() {
+ assertThat(normalizeRing(listOf(1, 2, 3, 4, 1, 1), Int::equals))
+ .isEqualTo(listOf(1, 2, 3, 4, 1, 1, 1))
+ }
+
+ @Test
+ fun `normalize ring with start element in body`() {
+ assertThat(normalizeRing(listOf(1, 2, 3, 1, 4, 5, 6, 1), Int::equals))
+ .isEqualTo(listOf(1, 2, 3, 1, 1, 4, 5, 6, 1))
+ }
+
+ @Test
+ fun `normalize ring with few start element in body`() {
+ assertThat(normalizeRing(listOf(1, 2, 3, 1, 1, 4, 5, 6, 1, 1, 1, 7, 8, 1, 1, 1, 9, 10), Int::equals))
+ .isEqualTo(listOf(1, 2, 3, 1, 1, 4, 5, 6, 1, 1, 1, 1, 7, 8, 1, 1, 1, 1, 9, 10, 1))
  }
 
 }

future_changes.md

@@ -5,6 +5,6 @@
 ### Changed
 
 ### Fixed
-
 - Livemap: improve "tiles" documentation [[#1093](https://github.com/JetBrains/lets-plot/issues/1093)].
 - Undesired vertical scroller when displaying `gggrid` in Jupyter notebook.
+- GeoJson structure breaks if the ring start label occurs several times [[#1086](https://github.com/JetBrains/lets-plot/issues/1086)].

plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/geom/util/LinesHelper.kt

@@ -7,12 +7,9 @@ package org.jetbrains.letsPlot.core.plot.base.geom.util
 
 import org.jetbrains.letsPlot.commons.geometry.DoubleVector
 import org.jetbrains.letsPlot.commons.intern.splitBy
+import org.jetbrains.letsPlot.commons.intern.typedGeometry.algorithms.*
 import org.jetbrains.letsPlot.commons.intern.typedGeometry.algorithms.AdaptiveResampler.Companion.PIXEL_PRECISION
 import org.jetbrains.letsPlot.commons.intern.typedGeometry.algorithms.AdaptiveResampler.Companion.resample
-import org.jetbrains.letsPlot.commons.intern.typedGeometry.algorithms.isClosed
-import org.jetbrains.letsPlot.commons.intern.typedGeometry.algorithms.isRingTrimmed
-import org.jetbrains.letsPlot.commons.intern.typedGeometry.algorithms.splitRings
-import org.jetbrains.letsPlot.commons.intern.typedGeometry.algorithms.trimRing
 import org.jetbrains.letsPlot.commons.values.Colors.withOpacity
 import org.jetbrains.letsPlot.core.commons.geometry.PolylineSimplifier.Companion.DOUGLAS_PEUCKER_PIXEL_THRESHOLD
 import org.jetbrains.letsPlot.core.commons.geometry.PolylineSimplifier.Companion.douglasPeucker
@@ -392,9 +389,7 @@ class PolygonData private constructor(
  // Force the invariants
  val processedRings = rings
  .filter { it.isNotEmpty() }
- .map { if (it.isClosed(PathPoint.LOC_EQ)) it else it + it.first() }
- .map { trimRing(it, PathPoint.LOC_EQ) }
- .filter { it.size >= 3 } // 3 points is fine - will draw a line
+ .map { normalizeRing(it, PathPoint.LOC_EQ) }
 
  if (processedRings.isEmpty()) {
  return null
@@ -406,9 +401,13 @@ class PolygonData private constructor(
 
  init {
  require(rings.isNotEmpty()) { "PolygonData should contain at least one ring" }
- require(rings.all { it.size >= 3 }) { "PolygonData ring should contain at least 3 points" }
- require(rings.all { it.first().coord == it.last().coord }) { "PolygonData ring should be closed" }
- require(rings.all { isRingTrimmed(it, PathPoint.LOC_EQ) }) { "PolygonData ring should be trimmed" }
+ require(rings.all { it.isClosed(PathPoint.LOC_EQ) }) { "PolygonData rings should be closed" }
+ require(rings.all {
+ isRingNormalized(
+ it,
+ PathPoint.LOC_EQ
+ )
+ }) { "PolygonData rings should be normalized" }
  }
 
  val aes: DataPointAesthetics by lazy( rings.first().first()::aes ) // decoration aes (only for color, fill, size, stroke)

plot-stem/src/commonMain/kotlin/org/jetbrains/letsPlot/core/spec/config/GeoConfig.kt

@@ -8,6 +8,7 @@ package org.jetbrains.letsPlot.core.spec.config
 import org.jetbrains.letsPlot.commons.intern.json.JsonSupport
 import org.jetbrains.letsPlot.commons.intern.spatial.*
 import org.jetbrains.letsPlot.commons.intern.typedGeometry.*
+import org.jetbrains.letsPlot.commons.intern.typedGeometry.algorithms.normalizeRing
 import org.jetbrains.letsPlot.core.plot.base.Aes
 import org.jetbrains.letsPlot.core.plot.base.DataFrame
 import org.jetbrains.letsPlot.core.plot.base.DataFrame.Variable
@@ -332,7 +333,13 @@ internal abstract class CoordinatesCollector(
  override val supportedFeatures = listOf("Polygon, MultiPolygon")
  override val geoJsonConsumer: SimpleFeature.Consumer<LonLat> = defaultConsumer {
  onPolygon = { it.asSequence().flatten().forEach { p -> coordinates.append(p) } }
- onMultiPolygon = { it.asSequence().flatten().flatten().forEach { p -> coordinates.append(p) } }
+ onMultiPolygon = {
+ it.asSequence()
+ .flatten()
+ .map { ring -> normalizeRing(ring, Vec<LonLat>::equals) }
+ .flatten()
+ .forEach { p -> coordinates.append(p) }
+ }
  }
  }