From 7de207e18f5030430b012c013acc0966fb97b5d7 Mon Sep 17 00:00:00 2001 From: Artem Smirnov Date: Wed, 8 May 2024 18:48:29 +0200 Subject: [PATCH 1/5] Improve linetype experience (LPK-220). --- future_changes.md | 1 + .../core/plot/base/render/linetype/NamedLineType.kt | 10 +++++----- .../letsPlot/core/plot/livemap/PathConverterTest.kt | 4 ++-- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/future_changes.md b/future_changes.md index f0f62dc710c..4bd34598442 100644 --- a/future_changes.md +++ b/future_changes.md @@ -8,3 +8,4 @@ - Livemap: improve "tiles" documentation [[#1093](https://github.com/JetBrains/lets-plot/issues/1093)]. - Undesired vertical scroller when displaying `gggrid` in Jupyter notebook. +- Improve border line type experience [[LPK-220](https://github.com/JetBrains/lets-plot-kotlin/issues/220)]. \ No newline at end of file diff --git a/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/render/linetype/NamedLineType.kt b/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/render/linetype/NamedLineType.kt index 322d3cbf6f2..d2e842da827 100644 --- a/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/render/linetype/NamedLineType.kt +++ b/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/render/linetype/NamedLineType.kt @@ -16,14 +16,14 @@ enum class NamedLineType(val code: Int, private val myDashArray: List?) override val isSolid: Boolean get() = true }, - DASHED(2, listOf(4.3, 4.3)), - DOTTED(3, listOf(1.0, 3.2)), + DASHED(2, listOf(4.0, 4.0)), + DOTTED(3, listOf(1.0, 3.0)), @Suppress("SpellCheckingInspection") - DOTDASH(4, listOf(1.0, 3.2, 4.3, 3.2)), + DOTDASH(4, listOf(1.0, 3.0, 4.0, 3.0)), @Suppress("SpellCheckingInspection") - LONGDASH(5, listOf(7.4, 3.2)), + LONGDASH(5, listOf(7.0, 3.0)), @Suppress("SpellCheckingInspection") - TWODASH(6, listOf(2.4, 2.4, 6.4, 2.4)); + TWODASH(6, listOf(2.0, 2.0, 6.0, 2.0)); override val isSolid: Boolean get() = false diff --git a/plot-livemap/src/commonTest/kotlin/org/jetbrains/letsPlot/core/plot/livemap/PathConverterTest.kt b/plot-livemap/src/commonTest/kotlin/org/jetbrains/letsPlot/core/plot/livemap/PathConverterTest.kt index c70249f32dd..7b238ee3b82 100644 --- a/plot-livemap/src/commonTest/kotlin/org/jetbrains/letsPlot/core/plot/livemap/PathConverterTest.kt +++ b/plot-livemap/src/commonTest/kotlin/org/jetbrains/letsPlot/core/plot/livemap/PathConverterTest.kt @@ -48,7 +48,7 @@ class PathConverterTest { matcher!! .strokeWidth(eq(1.1)) - .lineDash(vectorEq(listOf(4.73, 4.73))) + .lineDash(vectorEq(listOf(4.4, 4.4))) assertMapObject() } @@ -61,7 +61,7 @@ class PathConverterTest { matcher!! .strokeWidth(eq(4.4)) - .lineDash(vectorEq(listOf(18.92, 18.92))) + .lineDash(vectorEq(listOf(17.6, 17.6))) assertMapObject() } From 9931a32319d28c9398732b19bc62fe58d8ec4857 Mon Sep 17 00:00:00 2001 From: Artem Smirnov Date: Mon, 20 May 2024 13:03:37 +0200 Subject: [PATCH 2/5] LPK-220: Change linewidth calculation for the midline of the boxplot. --- .../jetbrains/letsPlot/core/plot/base/geom/util/BoxHelper.kt | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/geom/util/BoxHelper.kt b/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/geom/util/BoxHelper.kt index c86cbea2f60..927f948444f 100644 --- a/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/geom/util/BoxHelper.kt +++ b/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/geom/util/BoxHelper.kt @@ -51,13 +51,11 @@ object BoxHelper { DoubleVector(x - width / 2, middle), DoubleVector(x + width / 2, middle), p - ) ?: continue + ) { AesScaling.strokeWidth(it) * fatten } ?: continue // TODO: use strokeScale in createLine() function // adjust thickness require(line is SvgShape) - val thickness = line.strokeWidth().get()!! - line.strokeWidth().set(thickness * fatten) root.add(line) } From fcc886555b58f04e5654884be813765538b01af9 Mon Sep 17 00:00:00 2001 From: Artem Smirnov Date: Mon, 20 May 2024 16:04:15 +0200 Subject: [PATCH 3/5] LPK-220: Refactor code and fix linewidth calculation for the crossbars. --- .../core/plot/base/geom/BoxplotGeom.kt | 12 ++++- .../core/plot/base/geom/CrossBarGeom.kt | 50 ++++--------------- .../core/plot/base/geom/util/BoxHelper.kt | 18 ++++--- 3 files changed, 32 insertions(+), 48 deletions(-) diff --git a/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/geom/BoxplotGeom.kt b/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/geom/BoxplotGeom.kt index 4c33b35460a..bac9722e2a7 100644 --- a/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/geom/BoxplotGeom.kt +++ b/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/geom/BoxplotGeom.kt @@ -53,7 +53,17 @@ class BoxplotGeom : GeomBase() { ctx: GeomContext, geomHelper: GeomHelper ) { - BoxHelper.buildMidlines(root, aesthetics, middleAesthetic = Aes.MIDDLE, ctx, geomHelper, fatten = fattenMidline) + BoxHelper.buildMidlines( + root, + aesthetics, + xAes = Aes.X, + middleAes = Aes.MIDDLE, + sizeAes = Aes.WIDTH, + ctx, + geomHelper, + linesRotation = { it }, + fatten = fattenMidline + ) val elementHelper = geomHelper.createSvgElementHelper() for (p in aesthetics.dataPoints()) { diff --git a/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/geom/CrossBarGeom.kt b/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/geom/CrossBarGeom.kt index d80d2ffbf0f..0c0943509f8 100644 --- a/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/geom/CrossBarGeom.kt +++ b/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/geom/CrossBarGeom.kt @@ -15,7 +15,6 @@ import org.jetbrains.letsPlot.core.plot.base.geom.util.GeomHelper import org.jetbrains.letsPlot.core.plot.base.geom.util.HintColorUtil import org.jetbrains.letsPlot.core.plot.base.render.LegendKeyElementFactory import org.jetbrains.letsPlot.core.plot.base.render.SvgRoot -import org.jetbrains.letsPlot.datamodel.svg.dom.SvgShape class CrossBarGeom( private val isVertical: Boolean @@ -48,10 +47,6 @@ class CrossBarGeom( return flipHelper.flip(rectangle) } - private fun afterRotation(vector: DoubleVector): DoubleVector { - return flipHelper.flip(vector) - } - override fun buildIntern( root: SvgRoot, aesthetics: Aesthetics, @@ -64,7 +59,17 @@ class CrossBarGeom( root, aesthetics, pos, coord, ctx, rectFactory = clientRectByDataPoint(ctx, geomHelper, isHintRect = false) ) - buildMidlines(root, aesthetics, ctx, geomHelper, fatten = fattenMidline) + BoxHelper.buildMidlines( + root, + aesthetics, + xAes = afterRotation(Aes.X), + middleAes = afterRotation(Aes.Y), + sizeAes = Aes.WIDTH, // do not flip as height is not defined for CrossBarGeom + ctx, + geomHelper, + linesRotation = flipHelper::flip, + fatten = fattenMidline + ) // tooltip flipHelper.buildHints( listOf(Aes.YMIN, Aes.YMAX).map(::afterRotation), @@ -114,39 +119,6 @@ class CrossBarGeom( } } - private fun buildMidlines( - root: SvgRoot, - aesthetics: Aesthetics, - ctx: GeomContext, - geomHelper: GeomHelper, - fatten: Double - ) { - val elementHelper = geomHelper.createSvgElementHelper() - val xAes = afterRotation(Aes.X) - val yAes = afterRotation(Aes.Y) - val sizeAes = Aes.WIDTH // do not flip as height is not defined for CrossBarGeom - for (p in aesthetics.dataPoints()) { - val x = p.finiteOrNull(xAes) ?: continue - val middle = p.finiteOrNull(yAes) ?: continue - val w = p.finiteOrNull(sizeAes) ?: continue - - val width = w * ctx.getResolution(xAes) - val (line) = elementHelper.createLine( - afterRotation(DoubleVector(x - width / 2, middle)), - afterRotation(DoubleVector(x + width / 2, middle)), - p - ) ?: continue - - // TODO: use strokeScale in createLine() function - // adjust thickness - require(line is SvgShape) - val thickness = line.strokeWidth().get()!! - line.strokeWidth().set(thickness * fatten) - - root.add(line) - } - } - companion object { const val HANDLES_GROUPS = false private val LEGEND_FACTORY = BoxHelper.legendFactory(false) diff --git a/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/geom/util/BoxHelper.kt b/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/geom/util/BoxHelper.kt index 927f948444f..e5b41415ac4 100644 --- a/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/geom/util/BoxHelper.kt +++ b/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/geom/util/BoxHelper.kt @@ -34,27 +34,29 @@ object BoxHelper { fun buildMidlines( root: SvgRoot, aesthetics: Aesthetics, - middleAesthetic: Aes, + xAes: Aes, + middleAes: Aes, + sizeAes: Aes, ctx: GeomContext, geomHelper: GeomHelper, + linesRotation: (DoubleVector) -> DoubleVector, fatten: Double ) { val elementHelper = geomHelper.createSvgElementHelper() for (p in aesthetics.dataPoints()) { - val x = p.finiteOrNull(Aes.X) ?: continue - val middle = p.finiteOrNull(middleAesthetic) ?: continue - val w = p.finiteOrNull(Aes.WIDTH) ?: continue + val x = p.finiteOrNull(xAes) ?: continue + val middle = p.finiteOrNull(middleAes) ?: continue + val w = p.finiteOrNull(sizeAes) ?: continue - val width = w * ctx.getResolution(Aes.X) + val width = w * ctx.getResolution(xAes) val (line) = elementHelper.createLine( - DoubleVector(x - width / 2, middle), - DoubleVector(x + width / 2, middle), + linesRotation(DoubleVector(x - width / 2, middle)), + linesRotation(DoubleVector(x + width / 2, middle)), p ) { AesScaling.strokeWidth(it) * fatten } ?: continue // TODO: use strokeScale in createLine() function - // adjust thickness require(line is SvgShape) root.add(line) From 788b5281c1cbbf1a74422f9b9350ba7f89ee94a0 Mon Sep 17 00:00:00 2001 From: Artem Smirnov Date: Fri, 24 May 2024 11:35:42 +0200 Subject: [PATCH 4/5] LPK-220: Refactor code. --- .../jetbrains/letsPlot/core/plot/base/geom/BoxplotGeom.kt | 1 - .../letsPlot/core/plot/base/geom/CrossBarGeom.kt | 4 ++-- .../letsPlot/core/plot/base/geom/util/BoxHelper.kt | 8 ++++---- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/geom/BoxplotGeom.kt b/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/geom/BoxplotGeom.kt index bac9722e2a7..2b035d31b29 100644 --- a/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/geom/BoxplotGeom.kt +++ b/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/geom/BoxplotGeom.kt @@ -61,7 +61,6 @@ class BoxplotGeom : GeomBase() { sizeAes = Aes.WIDTH, ctx, geomHelper, - linesRotation = { it }, fatten = fattenMidline ) diff --git a/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/geom/CrossBarGeom.kt b/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/geom/CrossBarGeom.kt index 0c0943509f8..e99a1818e3e 100644 --- a/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/geom/CrossBarGeom.kt +++ b/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/geom/CrossBarGeom.kt @@ -67,8 +67,8 @@ class CrossBarGeom( sizeAes = Aes.WIDTH, // do not flip as height is not defined for CrossBarGeom ctx, geomHelper, - linesRotation = flipHelper::flip, - fatten = fattenMidline + fatten = fattenMidline, + flip = !isVertical ) // tooltip flipHelper.buildHints( diff --git a/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/geom/util/BoxHelper.kt b/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/geom/util/BoxHelper.kt index e5b41415ac4..dbed9f473bd 100644 --- a/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/geom/util/BoxHelper.kt +++ b/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/geom/util/BoxHelper.kt @@ -39,8 +39,8 @@ object BoxHelper { sizeAes: Aes, ctx: GeomContext, geomHelper: GeomHelper, - linesRotation: (DoubleVector) -> DoubleVector, - fatten: Double + fatten: Double, + flip: Boolean = false ) { val elementHelper = geomHelper.createSvgElementHelper() for (p in aesthetics.dataPoints()) { @@ -51,8 +51,8 @@ object BoxHelper { val width = w * ctx.getResolution(xAes) val (line) = elementHelper.createLine( - linesRotation(DoubleVector(x - width / 2, middle)), - linesRotation(DoubleVector(x + width / 2, middle)), + DoubleVector(x - width / 2, middle).flipIf(flip), + DoubleVector(x + width / 2, middle).flipIf(flip), p ) { AesScaling.strokeWidth(it) * fatten } ?: continue From 9d712923886786ed6504a3e9604f70519f97cf83 Mon Sep 17 00:00:00 2001 From: Artem Smirnov Date: Fri, 24 May 2024 12:14:22 +0200 Subject: [PATCH 5/5] LPK-220: Another small refactor in BoxHelper. --- .../org/jetbrains/letsPlot/core/plot/base/geom/BoxplotGeom.kt | 3 ++- .../jetbrains/letsPlot/core/plot/base/geom/util/BoxHelper.kt | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/geom/BoxplotGeom.kt b/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/geom/BoxplotGeom.kt index 2b035d31b29..616733b684a 100644 --- a/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/geom/BoxplotGeom.kt +++ b/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/geom/BoxplotGeom.kt @@ -61,7 +61,8 @@ class BoxplotGeom : GeomBase() { sizeAes = Aes.WIDTH, ctx, geomHelper, - fatten = fattenMidline + fatten = fattenMidline, + flip = false ) val elementHelper = geomHelper.createSvgElementHelper() diff --git a/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/geom/util/BoxHelper.kt b/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/geom/util/BoxHelper.kt index dbed9f473bd..678aa229e6d 100644 --- a/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/geom/util/BoxHelper.kt +++ b/plot-base/src/commonMain/kotlin/org/jetbrains/letsPlot/core/plot/base/geom/util/BoxHelper.kt @@ -40,7 +40,7 @@ object BoxHelper { ctx: GeomContext, geomHelper: GeomHelper, fatten: Double, - flip: Boolean = false + flip: Boolean ) { val elementHelper = geomHelper.createSvgElementHelper() for (p in aesthetics.dataPoints()) {