From c241032704f74071eb6dfa7e1378726e60db5409 Mon Sep 17 00:00:00 2001 From: Ivan Kupriyanov Date: Wed, 27 Mar 2024 11:33:34 +0100 Subject: [PATCH] Fix #1037 - fix polygon-based geoms in polar coordinate system --- .../algorithms/AdaptiveResampler.kt | 4 +- .../typedGeometry/algorithms/Geometry.kt | 5 +- .../core/plot/base/coord/CoordinatesMapper.kt | 4 +- .../letsPlot/core/plot/base/geom/PieGeom.kt | 2 +- .../core/plot/base/geom/PolygonGeom.kt | 10 +- .../core/plot/base/geom/util/GeomHelper.kt | 2 +- .../core/plot/base/geom/util/LinesHelper.kt | 152 ++++++++++++------ .../plot/base/geom/util/RectanglesHelper.kt | 4 +- .../base/geom/util/TargetCollectorHelper.kt | 16 +- .../core/plot/builder/PolarAxisUtil.kt | 2 +- 10 files changed, 126 insertions(+), 75 deletions(-) diff --git a/commons/src/commonMain/kotlin/org/jetbrains/letsPlot/commons/intern/typedGeometry/algorithms/AdaptiveResampler.kt b/commons/src/commonMain/kotlin/org/jetbrains/letsPlot/commons/intern/typedGeometry/algorithms/AdaptiveResampler.kt index 15e191224d7..7a8ee35cfe7 100644 --- a/commons/src/commonMain/kotlin/org/jetbrains/letsPlot/commons/intern/typedGeometry/algorithms/AdaptiveResampler.kt +++ b/commons/src/commonMain/kotlin/org/jetbrains/letsPlot/commons/intern/typedGeometry/algorithms/AdaptiveResampler.kt @@ -9,7 +9,7 @@ import org.jetbrains.letsPlot.commons.geometry.DoubleVector import org.jetbrains.letsPlot.commons.intern.math.distance2 import org.jetbrains.letsPlot.commons.intern.math.distance2ToLine - +// Note that resampled points may contain duplicates, i.e. rings detection may fail. class AdaptiveResampler private constructor( private val transform: (T) -> T?, precision: Double, @@ -18,7 +18,7 @@ class AdaptiveResampler private constructor( private val precisionSqr: Double = precision * precision companion object { - const val PIXEL_RESAMPLING_PRECISION = 0.95 + const val PIXEL_PRECISION = 0.95 private const val MAX_DEPTH_LIMIT = 9 // 1_025 points maximum (2^(LIMIT + 1) + 1) private val DOUBLE_VECTOR_ADAPTER = object : DataAdapter { diff --git a/commons/src/commonMain/kotlin/org/jetbrains/letsPlot/commons/intern/typedGeometry/algorithms/Geometry.kt b/commons/src/commonMain/kotlin/org/jetbrains/letsPlot/commons/intern/typedGeometry/algorithms/Geometry.kt index 32800dc5741..2c982e5bfd2 100644 --- a/commons/src/commonMain/kotlin/org/jetbrains/letsPlot/commons/intern/typedGeometry/algorithms/Geometry.kt +++ b/commons/src/commonMain/kotlin/org/jetbrains/letsPlot/commons/intern/typedGeometry/algorithms/Geometry.kt @@ -25,17 +25,18 @@ fun splitRings(points: List, eq: (T, T) -> Boolean): List> { val rings = findRingIntervals(points, eq).map(points::sublist).toMutableList() if (rings.isNotEmpty()) { - if (!rings.last().isClosed()) { + if (!rings.last().isClosed(eq)) { rings[rings.lastIndex] = makeClosed(rings.last()) } } + //require(rings.sumOf { it.size } == points.size) { "Split rings error: ${rings.sumOf { it.size }} != ${points.size}" } return rings } private fun makeClosed(path: List) = path.toMutableList() + path.first() -fun List.isClosed() = first() == last() +fun List.isClosed(eq: (T, T) -> Boolean = { p1, p2 -> p1 == p2 }) = eq(first(), last()) private fun findRingIntervals(path: List, eq: (T, T) -> Boolean): List { val intervals = ArrayList() diff --git a/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/coord/CoordinatesMapper.kt b/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/coord/CoordinatesMapper.kt index 4fe23b5a277..9095a145f00 100644 --- a/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/coord/CoordinatesMapper.kt +++ b/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/coord/CoordinatesMapper.kt @@ -9,7 +9,7 @@ import org.jetbrains.letsPlot.commons.geometry.DoubleRectangle import org.jetbrains.letsPlot.commons.geometry.DoubleRectangles.boundingBox import org.jetbrains.letsPlot.commons.geometry.DoubleVector import org.jetbrains.letsPlot.commons.intern.spatial.projections.Projection -import org.jetbrains.letsPlot.commons.intern.typedGeometry.algorithms.AdaptiveResampler.Companion.PIXEL_RESAMPLING_PRECISION +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.core.plot.base.ScaleMapper import org.jetbrains.letsPlot.core.plot.base.scale.Mappers @@ -140,7 +140,7 @@ fun projectDomain( val hLines = points(domain.top, domain.bottom).map { DoubleVector(domain.left, it) to DoubleVector(domain.right, it) } val vLines = points(domain.left, domain.right).map { DoubleVector(it, domain.top) to DoubleVector(it, domain.bottom) } - val grid = (hLines + vLines).map { (p1, p2) -> resample(p1, p2, PIXEL_RESAMPLING_PRECISION, projection::project) } + val grid = (hLines + vLines).map { (p1, p2) -> resample(p1, p2, PIXEL_PRECISION, projection::project) } val projectedDomain = boundingBox(grid.flatten()) ?: error("Can't calculate bounding box for projected domain") diff --git a/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/geom/PieGeom.kt b/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/geom/PieGeom.kt index 0b39953653d..9e1560dd54c 100644 --- a/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/geom/PieGeom.kt +++ b/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/geom/PieGeom.kt @@ -208,7 +208,7 @@ class PieGeom : GeomBase(), WithWidth, WithHeight { val segmentLength = startPoint.subtract(endPoint).length() - return resample(startPoint, endPoint, AdaptiveResampler.PIXEL_RESAMPLING_PRECISION) { p: DoubleVector -> + return resample(startPoint, endPoint, AdaptiveResampler.PIXEL_PRECISION) { p: DoubleVector -> val ratio = p.subtract(startPoint).length() / segmentLength if (ratio.isFinite()) { arcPoint(sector.startAngle + sector.angle * ratio) diff --git a/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/geom/PolygonGeom.kt b/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/geom/PolygonGeom.kt index 29d1733cb70..f1fa403d557 100644 --- a/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/geom/PolygonGeom.kt +++ b/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/geom/PolygonGeom.kt @@ -26,12 +26,14 @@ open class PolygonGeom : GeomBase() { ) { val dataPoints = dataPoints(aesthetics) val linesHelper = LinesHelper(pos, coord, ctx) + linesHelper.setResamplingEnabled(coord.isPolar) + val targetCollectorHelper = TargetCollectorHelper(GeomKind.POLYGON, ctx) - val pathData = linesHelper.createPathDataByGroup(dataPoints, GeomUtil.TO_LOCATION_X_Y) - targetCollectorHelper.addPolygons(pathData) - val svgPath = linesHelper.renderPaths(pathData.values, filled = true) - root.appendNodes(svgPath) + linesHelper.createPolygon(dataPoints, GeomUtil.TO_LOCATION_X_Y).forEach { (svg, polygonData) -> + targetCollectorHelper.addPolygons(polygonData) + root.add(svg) + } } companion object { diff --git a/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/geom/util/GeomHelper.kt b/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/geom/util/GeomHelper.kt index 1f0b01bbd67..72413ba020b 100644 --- a/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/geom/util/GeomHelper.kt +++ b/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/geom/util/GeomHelper.kt @@ -114,7 +114,7 @@ open class GeomHelper( private var myArrowSpec: ArrowSpec? = null private var myStrokeAlphaEnabled = false private var myResamplingEnabled = false - private var myResamplingPrecision = AdaptiveResampler.PIXEL_RESAMPLING_PRECISION + private var myResamplingPrecision = AdaptiveResampler.PIXEL_PRECISION private var mySpacer: Double = 0.0 private var myDebugRendering = false diff --git a/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/geom/util/LinesHelper.kt b/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/geom/util/LinesHelper.kt index 43d67faeff1..43fa28255ad 100644 --- a/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/geom/util/LinesHelper.kt +++ b/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/geom/util/LinesHelper.kt @@ -7,16 +7,18 @@ 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.AdaptiveResampler +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.splitRings import org.jetbrains.letsPlot.commons.values.Colors.withOpacity -import org.jetbrains.letsPlot.core.commons.geometry.PolylineSimplifier import org.jetbrains.letsPlot.core.commons.geometry.PolylineSimplifier.Companion.DOUGLAS_PEUCKER_PIXEL_THRESHOLD +import org.jetbrains.letsPlot.core.commons.geometry.PolylineSimplifier.Companion.douglasPeucker import org.jetbrains.letsPlot.core.plot.base.* import org.jetbrains.letsPlot.core.plot.base.aes.AesScaling import org.jetbrains.letsPlot.core.plot.base.aes.AestheticsUtil +import org.jetbrains.letsPlot.core.plot.base.geom.util.GeomUtil.createPathGroups import org.jetbrains.letsPlot.core.plot.base.render.svg.LinePath +import org.jetbrains.letsPlot.datamodel.svg.dom.SvgNode open class LinesHelper( pos: PositionAdjustment, @@ -49,12 +51,23 @@ open class LinesHelper( return renderPaths(pathDataByGroup.values, filled = false) } + // TODO: filled parameter is always false fun renderPaths(paths: Map>, filled: Boolean): List { return renderPaths(paths.values.flatten(), filled) } - fun renderPaths(paths: Collection, filled: Boolean): List { - return paths.map { path -> renderPaths(path.aes, path.coordinates, filled) } + // TODO: filled parameter is always false + private fun renderPaths(paths: Collection, filled: Boolean): List { + return paths.map { path -> + val visualPath = douglasPeucker(path.coordinates, DOUGLAS_PEUCKER_PIXEL_THRESHOLD) + val element = when (filled) { + true -> LinePath.polygon(visualPath) + false -> LinePath.line(visualPath) + } + + decorate(element, path.aes, filled) + element + } } fun createPathData( @@ -62,8 +75,44 @@ open class LinesHelper( locationTransform: (DataPointAesthetics) -> DoubleVector? = GeomUtil.TO_LOCATION_X_Y, closePath: Boolean = false, ): Map> { - val domainInterpolatedData = preparePathData(dataPoints, locationTransform, closePath) - return toClient(domainInterpolatedData) + val domainData = preparePathData(dataPoints, locationTransform, closePath) + return toClient(domainData) + } + + fun createPolygon( + dataPoints: Iterable, + locationTransform: (DataPointAesthetics) -> DoubleVector? = GeomUtil.TO_LOCATION_X_Y, + ): List> { + val domainPathData = createPathGroups(dataPoints, locationTransform, sorted = true, closePath = true).values + + // split in domain space! after resampling coordinates may repeat and splitRings will return wrong results + val domainPolygonData = domainPathData + .map { splitRings(it.points) { p1, p2 -> p1.coord == p2.coord } } + .map { PolygonData(it) } + + val clientPolygonData = domainPolygonData.map { polygon -> + polygon.rings + .map { resample(it) + it.last().let { (aes, coord) -> PathPoint(aes, toClient(coord, aes)!!) } } + .let { PolygonData(it) } + } + + val svg = clientPolygonData.map { polygon -> + val element = polygon.coordinates + .map { douglasPeucker(it, DOUGLAS_PEUCKER_PIXEL_THRESHOLD) } + .let(::insertPathSeparators) + .let { LinePath.polygon(it) } + + decorate(element, polygon.aes, filled = true) + element.rootGroup + } + + return svg.zip(clientPolygonData) + } + + private fun resample(linestring: List): List { + return linestring.windowed(size = 2) + .map { (p1, p2) -> p1.aes to resample(p1.coord, p2.coord, PIXEL_PRECISION) { p -> toClient(p, p1.aes) } } + .flatMap { (aes, coords) -> coords.map { PathPoint(aes, it) } } } // TODO: return list of PathData for consistency @@ -71,10 +120,9 @@ open class LinesHelper( dataPoints: Iterable, toLocation: (DataPointAesthetics) -> DoubleVector? ): Map { - return GeomUtil.createPathGroups(dataPoints, toClientLocation(toLocation), sorted = true, closePath = false) + return createPathGroups(dataPoints, toClientLocation(toLocation), sorted = true, closePath = false) } - fun createSteps(paths: Map, horizontalThenVertical: Boolean): List { val linePaths = ArrayList() @@ -145,7 +193,12 @@ open class LinesHelper( val points = pathData.coordinates if (points.isNotEmpty()) { - val path = LinePath.polygon(if (simplifyBorders) simplify(points) else points) + val path = LinePath.polygon( + when { + simplifyBorders -> douglasPeucker(points, DOUGLAS_PEUCKER_PIXEL_THRESHOLD) + else -> points + } + ) decorateFillingPart(path, pathData.aes) path } else { @@ -154,11 +207,6 @@ open class LinesHelper( } } - private fun simplify(points: List): List { - val weightLimit = 0.25 // in px for Douglas–Peucker algorithm - return PolylineSimplifier.douglasPeucker(points).setWeightLimit(weightLimit).points - } - fun decorate( path: LinePath, p: DataPointAesthetics, @@ -191,40 +239,24 @@ open class LinesHelper( path.fill().set(withOpacity(fill, fillAlpha)) } - private fun renderPaths( - aes: DataPointAesthetics, - points: List, - filled: Boolean - ): LinePath { - val element = when (filled) { - true -> LinePath - .polygon( - splitRings(points) - .map { PolylineSimplifier.douglasPeucker(it, DOUGLAS_PEUCKER_PIXEL_THRESHOLD) } - .let(::insertPathSeparators) - ) - false -> LinePath.line(PolylineSimplifier.douglasPeucker(points, DOUGLAS_PEUCKER_PIXEL_THRESHOLD)) - } - decorate(element, aes, filled) - return element - } - private fun preparePathData( dataPoints: Iterable, locationTransform: (DataPointAesthetics) -> DoubleVector?, closePath: Boolean ): Map> { - val domainPathData = GeomUtil.createPathGroups(dataPoints, locationTransform, sorted = true, closePath = closePath) + val domainPathData = createPathGroups(dataPoints, locationTransform, sorted = true, closePath = closePath) return domainPathData.mapValues { (_, pathData) -> listOf(pathData) } } private fun toClient(domainPathData: Map>): Map> { return when (myResamplingEnabled) { true -> { - val domainVariadicPathData = domainPathData.mapValues { (_, groupPath) -> groupPath.flatMap(::splitByStyle) } - val domainInterpolatedPathData = interpolatePathData(domainVariadicPathData) - resamplePathData(domainInterpolatedPathData) + domainPathData + .mapValues { (_, groupPath) -> groupPath.flatMap(::splitByStyle) } + .let { interpolatePathData(it) } + .mapValues { (_, paths) -> paths.map { PathData(resample(it.points)) } } } + false -> { val clientPathData = domainPathData.mapValues { (_, groupPath) -> groupPath.map { segment -> @@ -237,25 +269,13 @@ open class LinesHelper( } } - val clientVariadicPathData = clientPathData.mapValues { (_, pathData) -> pathData.flatMap(::splitByStyle) } + val clientVariadicPathData = + clientPathData.mapValues { (_, pathData) -> pathData.flatMap(::splitByStyle) } interpolatePathData(clientVariadicPathData) } } } - // TODO: refactor - inconsistent and implicit usage of the toClient method in a whole LinesHelper class - private fun resamplePathData(pathData: Map>): Map> { - return pathData.mapValues { (_, path) -> - path.map { segment -> - val smoothed = segment.points - .windowed(size = 2) - .map { (p1, p2) -> p1.aes to resample(p1.coord, p2.coord, AdaptiveResampler.PIXEL_RESAMPLING_PRECISION) { toClient(it, p1.aes) } } - .flatMap { (aes, points) -> points.map { PathPoint(aes, it) } } - PathData(smoothed) - } - } - } - companion object { private fun insertPathSeparators(rings: Iterable>): List { val result = ArrayList() @@ -339,7 +359,37 @@ data class PathData( val aes: DataPointAesthetics by lazy(points.first()::aes) // decoration aes (only for color, fill, size, stroke) val aesthetics by lazy { points.map(PathPoint::aes) } - val coordinates by lazy { points.map(PathPoint::coord) } + val coordinates by lazy { points.map(PathPoint::coord) } // may contain duplicates, don't work well for polygon +} + +data class PolygonData( + val rings: List> +) { + 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" } + } + + val aes: DataPointAesthetics by lazy( rings.first().first()::aes ) // decoration aes (only for color, fill, size, stroke) + val aesthetics by lazy { rings.map { ring -> ring.map { it.aes } } } + val coordinates by lazy { rings.map { ring -> ring.map { it.coord } } } + val flattenCoordinates by lazy { // guaranteed to have no duplicates on ends caused by resampling + val output = mutableListOf() + rings.forEach { ring -> + val firstPoint = ring.first().coord + val lastPoint = ring.last().coord + + // trim duplications at start and end to make the ring detection work + + output.add(firstPoint) + output.addAll(ring.asSequence().dropWhile { it.coord == firstPoint }.map { it.coord }) + while (output.last() == lastPoint) { + output.removeLast() + } + output.add(lastPoint) + } + output + } } data class PathPoint( diff --git a/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/geom/util/RectanglesHelper.kt b/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/geom/util/RectanglesHelper.kt index 66573d2ab5c..6d4247b4075 100644 --- a/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/geom/util/RectanglesHelper.kt +++ b/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/geom/util/RectanglesHelper.kt @@ -31,7 +31,7 @@ class RectanglesHelper( myAesthetics.dataPoints().forEach { p -> geometryFactory(p)?.let { rect -> val polyRect = resample( - precision = AdaptiveResampler.PIXEL_RESAMPLING_PRECISION, + precision = AdaptiveResampler.PIXEL_PRECISION, points = listOf( DoubleVector(rect.left, rect.top), DoubleVector(rect.right, rect.top), @@ -95,7 +95,7 @@ class RectanglesHelper( inner class SvgRectHelper { private var onGeometry: (DataPointAesthetics, DoubleRectangle?, List?) -> Unit = { _, _, _ -> } private var myResamplingEnabled = false - private var myResamplingPrecision = AdaptiveResampler.PIXEL_RESAMPLING_PRECISION + private var myResamplingPrecision = AdaptiveResampler.PIXEL_PRECISION fun setResamplingEnabled(b: Boolean) { myResamplingEnabled = b diff --git a/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/geom/util/TargetCollectorHelper.kt b/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/geom/util/TargetCollectorHelper.kt index abcc3fd96a8..e2e064294e4 100644 --- a/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/geom/util/TargetCollectorHelper.kt +++ b/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/geom/util/TargetCollectorHelper.kt @@ -51,15 +51,13 @@ class TargetCollectorHelper( } } - fun addPolygons(pathDataList: Map) { - pathDataList.values.forEach { pathData -> - targetCollector.addPolygon( - pathData.coordinates, - pathData.aes.index(), - TooltipParams(markerColors = colorMarkerMapper(pathData.aes)), - TipLayoutHint.Kind.CURSOR_TOOLTIP - ) - } + fun addPolygons(polygonData: PolygonData) { + targetCollector.addPolygon( + polygonData.flattenCoordinates, + polygonData.aes.index(), + TooltipParams(markerColors = colorMarkerMapper(polygonData.aes)), + TipLayoutHint.Kind.CURSOR_TOOLTIP + ) } private fun addPath(path: PathData, tooltipParams: TooltipParams) { diff --git a/plot-builder/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/builder/PolarAxisUtil.kt b/plot-builder/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/builder/PolarAxisUtil.kt index 88ac2b171cd..2cdc6f5f870 100644 --- a/plot-builder/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/builder/PolarAxisUtil.kt +++ b/plot-builder/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/builder/PolarAxisUtil.kt @@ -127,7 +127,7 @@ object PolarAxisUtil { DoubleVector(gridDomain.xRange().lowerEnd, it), DoubleVector(gridDomain.xRange().upperEnd, it) ) - }.map { line -> AdaptiveResampler.resample(line, AdaptiveResampler.PIXEL_RESAMPLING_PRECISION) { toClient(it) } } + }.map { line -> AdaptiveResampler.resample(line, AdaptiveResampler.PIXEL_PRECISION, ::toClient) } } private fun buildAxis(): List {