From 34bc659623851daff31cddbc4de4cc82c726ff1b Mon Sep 17 00:00:00 2001 From: Justin Brooks Date: Fri, 29 Nov 2024 21:32:50 -0600 Subject: [PATCH] Make initial command absolute before merging paths (#113) * Make initial command absolute before merging paths * Consider modified commands for merge path constraints * Update baseline --- changelog.md | 1 + .../vgo/core/optimization/MergePaths.kt | 97 ++++++++++++++++++- .../vgo/core/optimization/MergePathsTests.kt | 64 ++++++++++++ .../baseline/regression_101_optimized.xml | 2 +- 4 files changed, 160 insertions(+), 4 deletions(-) diff --git a/changelog.md b/changelog.md index 6fe254f6..65d3e54d 100644 --- a/changelog.md +++ b/changelog.md @@ -25,6 +25,7 @@ - Decimal separators are locale-invariant. - Crash when using the CLI to convert an SVG containing a clip path to vector drawable. - (Vector Drawable) Path merging avoids merging a single path data string beyond the framework string length limit (#82) +- Paths with an initial relative command are modified to make that command absolute when merged (#111) ### Security diff --git a/vgo-core/src/main/kotlin/com/jzbrooks/vgo/core/optimization/MergePaths.kt b/vgo-core/src/main/kotlin/com/jzbrooks/vgo/core/optimization/MergePaths.kt index 2d2b1bdf..d32649da 100644 --- a/vgo-core/src/main/kotlin/com/jzbrooks/vgo/core/optimization/MergePaths.kt +++ b/vgo-core/src/main/kotlin/com/jzbrooks/vgo/core/optimization/MergePaths.kt @@ -7,7 +7,20 @@ import com.jzbrooks.vgo.core.graphic.Extra import com.jzbrooks.vgo.core.graphic.Graphic import com.jzbrooks.vgo.core.graphic.Group import com.jzbrooks.vgo.core.graphic.Path +import com.jzbrooks.vgo.core.graphic.command.Command import com.jzbrooks.vgo.core.graphic.command.CommandPrinter +import com.jzbrooks.vgo.core.graphic.command.CommandVariant +import com.jzbrooks.vgo.core.graphic.command.CubicBezierCurve +import com.jzbrooks.vgo.core.graphic.command.EllipticalArcCurve +import com.jzbrooks.vgo.core.graphic.command.HorizontalLineTo +import com.jzbrooks.vgo.core.graphic.command.LineTo +import com.jzbrooks.vgo.core.graphic.command.MoveTo +import com.jzbrooks.vgo.core.graphic.command.ParameterizedCommand +import com.jzbrooks.vgo.core.graphic.command.QuadraticBezierCurve +import com.jzbrooks.vgo.core.graphic.command.SmoothCubicBezierCurve +import com.jzbrooks.vgo.core.graphic.command.SmoothQuadraticBezierCurve +import com.jzbrooks.vgo.core.graphic.command.VerticalLineTo +import com.jzbrooks.vgo.core.util.math.Point import com.jzbrooks.vgo.core.util.math.Surveyor import com.jzbrooks.vgo.core.util.math.intersects @@ -73,7 +86,7 @@ class MergePaths( if (unableToMerge(previous, current)) { mergedPaths.add(current) } else { - previous.commands += current.commands + previous.commands += makeFirstCommandAbsolute(current.commands) } } @@ -99,14 +112,15 @@ class MergePaths( for (current in paths.drop(1)) { val previous = mergedPaths.last() - val currentLength = current.commands.joinToString("", transform = constraints.commandPrinter::print).length + val mergeableCommands = makeFirstCommandAbsolute(current.commands) + val currentLength = mergeableCommands.joinToString("", transform = constraints.commandPrinter::print).length val accumulatedLength = pathLength + currentLength if (accumulatedLength > constraints.maxLength || unableToMerge(previous, current)) { mergedPaths.add(current) pathLength = currentLength } else { - previous.commands += current.commands + previous.commands += mergeableCommands pathLength = accumulatedLength } } @@ -138,6 +152,83 @@ class MergePaths( first.strokeLineJoin == second.strokeLineJoin && first.strokeMiterLimit == second.strokeMiterLimit + private fun makeFirstCommandAbsolute(commands: List): List { + val firstCommand = commands.firstOrNull() as? ParameterizedCommand<*> ?: return commands + + if (firstCommand.variant == CommandVariant.RELATIVE) { + var currentPoint = Point.ZERO + + when (firstCommand) { + is MoveTo, is LineTo, is SmoothQuadraticBezierCurve -> { + firstCommand.parameters = + firstCommand.parameters.map { point -> + (point + currentPoint).also { point -> currentPoint = point } + } + } + is HorizontalLineTo -> { + firstCommand.parameters = + firstCommand.parameters.map { x -> + (x + currentPoint.x).also { x -> currentPoint = currentPoint.copy(x = x) } + } + } + is VerticalLineTo -> { + firstCommand.parameters = + firstCommand.parameters.map { x -> + (x + currentPoint.x).also { x -> currentPoint = currentPoint.copy(x = x) } + } + } + is CubicBezierCurve -> { + firstCommand.parameters = + firstCommand.parameters.map { parameter -> + val newEnd = parameter.end + currentPoint + parameter + .copy( + startControl = parameter.startControl + currentPoint, + endControl = parameter.endControl + currentPoint, + end = newEnd, + ).also { currentPoint = newEnd } + } + } + is SmoothCubicBezierCurve -> { + firstCommand.parameters = + firstCommand.parameters.map { parameter -> + val newEnd = parameter.end + currentPoint + parameter + .copy( + endControl = parameter.endControl + currentPoint, + end = newEnd, + ).also { currentPoint = newEnd } + } + } + is QuadraticBezierCurve -> { + firstCommand.parameters = + firstCommand.parameters.map { parameter -> + val newEnd = parameter.end + currentPoint + parameter + .copy( + control = parameter.control + currentPoint, + end = newEnd, + ).also { currentPoint = newEnd } + } + } + is EllipticalArcCurve -> { + firstCommand.parameters = + firstCommand.parameters.map { parameter -> + val newEnd = parameter.end + currentPoint + parameter + .copy( + end = newEnd, + ).also { currentPoint = newEnd } + } + } + } + + firstCommand.variant = CommandVariant.ABSOLUTE + } + + return commands + } + sealed interface Constraints { /** Constraints the optimization by preventing merging paths beyond a given maximum length */ data class PathLength( diff --git a/vgo-core/src/test/kotlin/com/jzbrooks/vgo/core/optimization/MergePathsTests.kt b/vgo-core/src/test/kotlin/com/jzbrooks/vgo/core/optimization/MergePathsTests.kt index 43207a53..97325787 100644 --- a/vgo-core/src/test/kotlin/com/jzbrooks/vgo/core/optimization/MergePathsTests.kt +++ b/vgo-core/src/test/kotlin/com/jzbrooks/vgo/core/optimization/MergePathsTests.kt @@ -1,18 +1,24 @@ package com.jzbrooks.vgo.core.optimization +import assertk.all import assertk.assertThat import assertk.assertions.containsExactly +import assertk.assertions.first import assertk.assertions.hasSize import assertk.assertions.index import assertk.assertions.isEqualTo +import assertk.assertions.isInstanceOf +import assertk.assertions.prop import com.jzbrooks.vgo.core.Color import com.jzbrooks.vgo.core.graphic.Group +import com.jzbrooks.vgo.core.graphic.Path import com.jzbrooks.vgo.core.graphic.command.Command import com.jzbrooks.vgo.core.graphic.command.CommandVariant import com.jzbrooks.vgo.core.graphic.command.EllipticalArcCurve import com.jzbrooks.vgo.core.graphic.command.FakeCommandPrinter import com.jzbrooks.vgo.core.graphic.command.LineTo import com.jzbrooks.vgo.core.graphic.command.MoveTo +import com.jzbrooks.vgo.core.graphic.command.ParameterizedCommand import com.jzbrooks.vgo.core.graphic.command.QuadraticBezierCurve import com.jzbrooks.vgo.core.graphic.command.SmoothCubicBezierCurve import com.jzbrooks.vgo.core.util.element.createGraphic @@ -455,4 +461,62 @@ class MergePathsTests { ), ) } + + @Test + fun mergedPathsInitialCommandIsMadeAbsolute() { + val paths = + listOf( + createPath( + listOf(MoveTo(CommandVariant.ABSOLUTE, listOf(Point(0f, 0f)))), + ), + createPath( + listOf(MoveTo(CommandVariant.RELATIVE, listOf(Point(10f, 10f), Point(10f, 10f)))), + ), + ) + + val graphic = createGraphic(paths) + val optimization = MergePaths(MergePaths.Constraints.None) + + traverseBottomUp(graphic) { it.accept(optimization) } + + assertThat(graphic::elements) + .first() + .isInstanceOf() + .prop(Path::commands) + .index(1) + .isInstanceOf>() + .all { + prop(ParameterizedCommand<*>::variant).isEqualTo(CommandVariant.ABSOLUTE) + prop(ParameterizedCommand<*>::variant.name) { it.parameters } + .isEqualTo(listOf(Point(10f, 10f), Point(20f, 20f))) + } + } + + @Test + fun mergedPathsInitialCommandIsMadeAbsoluteBeforeConstraints() { + // This would be merged if directly considered by constraints (merged length is 15) + // M0,0 + // m10,10 1,1 -> M0,0 m10,10 1, 1 + + // When the relative command is made absolute for merging, the merged path would + // be longer (17 chars) than the constraint. + // M0,0 + // M10,10 11,11 -> M0,0 M10,10 11,11 + val paths = + listOf( + createPath( + listOf(MoveTo(CommandVariant.ABSOLUTE, listOf(Point(0f, 0f)))), + ), + createPath( + listOf(MoveTo(CommandVariant.RELATIVE, listOf(Point(10f, 10f), Point(1f, 1f), Point(1f, 1f)))), + ), + ) + + val graphic = createGraphic(paths) + val optimization = MergePaths(MergePaths.Constraints.PathLength(FakeCommandPrinter(), 16)) + + traverseBottomUp(graphic) { it.accept(optimization) } + + assertThat(graphic::elements).hasSize(2) + } } diff --git a/vgo/src/test/resources/baseline/regression_101_optimized.xml b/vgo/src/test/resources/baseline/regression_101_optimized.xml index 8f76c1bf..0e5da6fc 100644 --- a/vgo/src/test/resources/baseline/regression_101_optimized.xml +++ b/vgo/src/test/resources/baseline/regression_101_optimized.xml @@ -1,4 +1,4 @@ - +