Make initial command absolute before merging paths (#113)

* Make initial command absolute before merging paths * Consider modified commands for merge path constraints * Update baseline
jzbrooks · Nov 30, 2024 · 34bc659 · 34bc659
1 parent 49eac60
commit 34bc659
Show file tree

Hide file tree

Showing 4 changed files with 160 additions and 4 deletions.
diff --git 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
@@ -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<Command>): List<Command> {
+        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
@@ -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<Path>()
+            .prop(Path::commands)
+            .index(1)
+            .isInstanceOf<ParameterizedCommand<*>>()
+            .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
@@ -1,4 +1,4 @@
 <vector xmlns:android="http://schemas.android.com/apk/res/android" android:height="62dp" android:viewportHeight="62" android:viewportWidth="36" android:width="36dp">
   <path android:fillColor="#880099" android:pathData="m30.57-0H5A5,5,0,0,0,0,5v51.08a5,5,0,0,0,5,5h25.6a5,5,0,0,0,5-5V5a5,5,0,0,0,-5.03-5Zm2,56.08a2,2,0,0,1,-2,2H5a2,2,0,0,1,-2-2V5a2,2,0,0,1,2-2h25.6a2,2,0,0,1,2,2Z"/>
-  <path android:fillColor="#880099" android:pathData="m28.67,9H6.87a1.5,1.5,0,0,0,0,3h21.8a1.5,1.5,0,0,0,0-3"/>
+  <path android:fillColor="#880099" android:pathData="M28.67,9H6.87a1.5,1.5,0,0,0,0,3h21.8a1.5,1.5,0,0,0,0-3"/>
 </vector>