Avoid merging paths beyond string length limits

changelog.md

@@ -4,13 +4,14 @@
 
 ### Added
 - `com.jzbrooks.vgo.core.util.math.Surveyor`, which computes the bounding box of an arbitrary list of commands 
-- Bezier curve interpolation for all variants and elliptical arc bounding box functions
+- Bézier curve interpolation for all variants and elliptical arc bounding box functions
 
 ### Changed
 
 - `vgo-plugin` (`com.jzbrooks.vgo.plugin`) no longer requires a particular version of Android Gradle Plugin.
   Note: `:vgo` is an abstract implementation of the tool which does not assume either a cli or plugin context. CLI related logic has been relocated into `:vgo-cli`.
 - **Breaking:** `CubicCurve<*>.interpolate` has been split into `CubicBezierCurve.interpolate` and `SmoothCubicBezierCurve.interpolate`
+- `com.jzbrooks.vgo.core.optimization.MergePaths` constructor accepts constraints. See `com.jzbrooks.vgo.core.optimization.MergePaths.Constraints`.
 - Paths with an even odd fill rule can be merged
 
 ### Deprecated
@@ -22,7 +23,8 @@
 - Overlapping paths are no longer merged, which avoids some image warping issues (#88, #101)
 - Conversions without a specified output file will write a file the file extension corresponding to the format.
 - Decimal separators are locale-invariant.
-- Crash when using the cli to convert an svg containing a clip path to vector drawable.
+- 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)
 
 ### Security
 

vgo-core/api/vgo-core.api

@@ -545,13 +545,38 @@ public final class com/jzbrooks/vgo/core/optimization/ConvertCurvesToArcs : com/
 
 public final class com/jzbrooks/vgo/core/optimization/MergePaths : com/jzbrooks/vgo/core/optimization/BottomUpOptimization {
 	public fun <init> ()V
+	public fun <init> (Lcom/jzbrooks/vgo/core/optimization/MergePaths$Constraints;)V
+	public synthetic fun <init> (Lcom/jzbrooks/vgo/core/optimization/MergePaths$Constraints;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
 	public fun visit (Lcom/jzbrooks/vgo/core/graphic/ClipPath;)V
 	public fun visit (Lcom/jzbrooks/vgo/core/graphic/Extra;)V
 	public fun visit (Lcom/jzbrooks/vgo/core/graphic/Graphic;)V
 	public fun visit (Lcom/jzbrooks/vgo/core/graphic/Group;)V
 	public fun visit (Lcom/jzbrooks/vgo/core/graphic/Path;)V
 }
 
+public abstract interface class com/jzbrooks/vgo/core/optimization/MergePaths$Constraints {
+}
+
+public final class com/jzbrooks/vgo/core/optimization/MergePaths$Constraints$None : com/jzbrooks/vgo/core/optimization/MergePaths$Constraints {
+	public static final field INSTANCE Lcom/jzbrooks/vgo/core/optimization/MergePaths$Constraints$None;
+	public fun equals (Ljava/lang/Object;)Z
+	public fun hashCode ()I
+	public fun toString ()Ljava/lang/String;
+}
+
+public final class com/jzbrooks/vgo/core/optimization/MergePaths$Constraints$PathLength : com/jzbrooks/vgo/core/optimization/MergePaths$Constraints {
+	public fun <init> (Lcom/jzbrooks/vgo/core/graphic/command/CommandPrinter;I)V
+	public final fun component1 ()Lcom/jzbrooks/vgo/core/graphic/command/CommandPrinter;
+	public final fun component2 ()I
+	public final fun copy (Lcom/jzbrooks/vgo/core/graphic/command/CommandPrinter;I)Lcom/jzbrooks/vgo/core/optimization/MergePaths$Constraints$PathLength;
+	public static synthetic fun copy$default (Lcom/jzbrooks/vgo/core/optimization/MergePaths$Constraints$PathLength;Lcom/jzbrooks/vgo/core/graphic/command/CommandPrinter;IILjava/lang/Object;)Lcom/jzbrooks/vgo/core/optimization/MergePaths$Constraints$PathLength;
+	public fun equals (Ljava/lang/Object;)Z
+	public final fun getCommandPrinter ()Lcom/jzbrooks/vgo/core/graphic/command/CommandPrinter;
+	public final fun getMaxLength ()I
+	public fun hashCode ()I
+	public fun toString ()Ljava/lang/String;
+}
+
 public abstract interface class com/jzbrooks/vgo/core/optimization/Optimization {
 	public abstract fun optimize (Lcom/jzbrooks/vgo/core/graphic/Element;)V
 }

vgo-core/src/main/kotlin/com/jzbrooks/vgo/core/optimization/MergePaths.kt

@@ -7,13 +7,16 @@ 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.CommandPrinter
 import com.jzbrooks.vgo.core.util.math.Surveyor
 import com.jzbrooks.vgo.core.util.math.intersects
 
 /**
  * Merges multiple paths into a single path where possible
  */
-class MergePaths : BottomUpOptimization {
+class MergePaths(
+    private val constraints: Constraints = Constraints.None,
+) : BottomUpOptimization {
     private val surveyor = Surveyor()
 
     override fun visit(graphic: Graphic) = merge(graphic)
@@ -52,28 +55,75 @@ class MergePaths : BottomUpOptimization {
         element.elements = elements
     }
 
-    private fun merge(paths: List<Path>): List<Path> {
+    private fun merge(paths: List<Path>): List<Path> =
+        when (constraints) {
+            is Constraints.PathLength -> mergeConstrained(paths, constraints)
+            Constraints.None -> mergeUnconstrained(paths)
+        }
+
+    private fun mergeUnconstrained(paths: List<Path>): List<Path> {
+        if (paths.isEmpty()) return emptyList()
+
+        val mergedPaths = ArrayList<Path>(paths.size)
+        mergedPaths.add(paths.first())
+
+        for (current in paths.drop(1)) {
+            val previous = mergedPaths.last()
+
+            if (unableToMerge(previous, current)) {
+                mergedPaths.add(current)
+            } else {
+                previous.commands += current.commands
+            }
+        }
+
+        return mergedPaths
+    }
+
+    private fun mergeConstrained(
+        paths: List<Path>,
+        constraints: Constraints.PathLength,
+    ): List<Path> {
         if (paths.isEmpty()) return emptyList()
 
         val mergedPaths = ArrayList<Path>(paths.size)
         mergedPaths.add(paths.first())
 
+        var pathLength =
+            paths
+                .first()
+                .commands
+                .joinToString("", transform = constraints.commandPrinter::print)
+                .length
+
         for (current in paths.drop(1)) {
             val previous = mergedPaths.last()
 
-            // Intersecting paths can cause problems with path fill rules and with transparency.
-            if (!haveSameAttributes(current, previous) ||
-                surveyor.findBoundingBox(previous.commands) intersects surveyor.findBoundingBox(current.commands)
-            ) {
+            val currentLength = current.commands.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
+                pathLength = accumulatedLength
             }
         }
 
         return mergedPaths
     }
 
+    // Paths must have the same visual parameters to be merged
+    // Intersecting paths can cause problems with path fill rules and with transparency
+    // If constraints exist on a path, they must be updated
+    private fun unableToMerge(
+        previous: Path,
+        current: Path,
+    ): Boolean =
+        !haveSameAttributes(current, previous) ||
+            surveyor.findBoundingBox(previous.commands) intersects surveyor.findBoundingBox(current.commands)
+
     private fun haveSameAttributes(
         first: Path,
         second: Path,
@@ -87,4 +137,15 @@ class MergePaths : BottomUpOptimization {
             first.strokeLineCap == second.strokeLineCap &&
             first.strokeLineJoin == second.strokeLineJoin &&
             first.strokeMiterLimit == second.strokeMiterLimit
+
+    sealed interface Constraints {
+        /** Constraints the optimization by preventing merging paths beyond a given maximum length */
+        data class PathLength(
+            val commandPrinter: CommandPrinter,
+            /** The maximum length of a single path */
+            val maxLength: Int,
+        ) : Constraints
+
+        data object None : Constraints
+    }
 }

vgo-core/src/test/kotlin/com/jzbrooks/vgo/core/optimization/MergePathsTests.kt

@@ -10,6 +10,7 @@ import com.jzbrooks.vgo.core.graphic.Group
 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.QuadraticBezierCurve
@@ -40,7 +41,7 @@ class MergePathsTests {
             )
 
         val graphic = createGraphic(paths)
-        val optimization = MergePaths()
+        val optimization = MergePaths(MergePaths.Constraints.None)
 
         traverseBottomUp(graphic) { it.accept(optimization) }
 
@@ -90,7 +91,7 @@ class MergePathsTests {
 
         val group = Group(paths)
         val graphic = createGraphic(listOf(group))
-        val optimization = MergePaths()
+        val optimization = MergePaths(MergePaths.Constraints.None)
 
         traverseBottomUp(graphic) { it.accept(optimization) }
 
@@ -161,7 +162,7 @@ class MergePathsTests {
             )
 
         val graphic = createGraphic(paths)
-        val optimization = MergePaths()
+        val optimization = MergePaths(MergePaths.Constraints.None)
 
         traverseBottomUp(graphic) { it.accept(optimization) }
 
@@ -229,7 +230,7 @@ class MergePathsTests {
             )
 
         val graphic = createGraphic(paths)
-        val optimization = MergePaths()
+        val optimization = MergePaths(MergePaths.Constraints.None)
 
         traverseBottomUp(graphic) { it.accept(optimization) }
 
@@ -291,7 +292,7 @@ class MergePathsTests {
             )
 
         val graphic = createGraphic(paths)
-        val optimization = MergePaths()
+        val optimization = MergePaths(MergePaths.Constraints.None)
 
         traverseBottomUp(graphic) { it.accept(optimization) }
 
@@ -411,10 +412,47 @@ class MergePathsTests {
             )
 
         val graphic = createGraphic(listOf(firstHeart, offsetHeart))
-        val optimization = MergePaths()
+        val optimization = MergePaths(MergePaths.Constraints.None)
 
         traverseBottomUp(graphic) { it.accept(optimization) }
 
         assertThat(graphic::elements).hasSize(2)
     }
+
+    @Test
+    fun pathLengthConstraints() {
+        val paths =
+            listOf(
+                createPath(
+                    listOf(MoveTo(CommandVariant.ABSOLUTE, listOf(Point(0f, 0f)))),
+                ),
+                createPath(
+                    listOf(MoveTo(CommandVariant.ABSOLUTE, listOf(Point(10f, 10f)))),
+                ),
+                createPath(
+                    listOf(MoveTo(CommandVariant.ABSOLUTE, listOf(Point(40f, 40f)))),
+                ),
+                createPath(
+                    listOf(MoveTo(CommandVariant.ABSOLUTE, listOf(Point(50f, 50f), Point(10f, 10f), Point(20f, 30f), Point(40f, 0f)))),
+                ),
+            )
+
+        val graphic = createGraphic(paths)
+        val optimization = MergePaths(MergePaths.Constraints.PathLength(FakeCommandPrinter(), 16))
+
+        traverseBottomUp(graphic) { it.accept(optimization) }
+
+        assertThat(graphic::elements).containsExactly(
+            createPath(
+                listOf(
+                    MoveTo(CommandVariant.ABSOLUTE, listOf(Point(0f, 0f))),
+                    MoveTo(CommandVariant.ABSOLUTE, listOf(Point(10f, 10f))),
+                    MoveTo(CommandVariant.ABSOLUTE, listOf(Point(40f, 40f))),
+                ),
+            ),
+            createPath(
+                listOf(MoveTo(CommandVariant.ABSOLUTE, listOf(Point(50f, 50f), Point(10f, 10f), Point(20f, 30f), Point(40f, 0f)))),
+            ),
+        )
+    }
 }

vgo/src/main/kotlin/com/jzbrooks/vgo/svg/SvgOptimizationRegistry.kt

@@ -21,7 +21,7 @@ class SvgOptimizationRegistry :
                 BakeTransformations(),
                 CollapseGroups(),
                 RemoveEmptyGroups(),
-                MergePaths(),
+                MergePaths(MergePaths.Constraints.None),
             ),
         topDownOptimizations =
             listOf(

vgo/src/main/kotlin/com/jzbrooks/vgo/vd/VectorDrawableOptimizationRegistry.kt

@@ -21,7 +21,8 @@ class VectorDrawableOptimizationRegistry :
                 BakeTransformations(),
                 CollapseGroups(),
                 RemoveEmptyGroups(),
-                MergePaths(),
+                // https://cs.android.com/android/platform/superproject/main/+/2e48e15a8097916063eacc023044bc90bb93c73e:frameworks/base/libs/androidfw/StringPool.cpp;l=328
+                MergePaths(MergePaths.Constraints.PathLength(VectorDrawableCommandPrinter(3), (1 shl 15) - 1)),
             ),
         topDownOptimizations =
             listOf(