Update VectorGroup to be fully immutable
Previously VectorGroup instances were
created within VectorAssetBuilder through
an internal method to add child VectorNodes
to the group. While the public API
surface was immutable, the actual VectorGroup
implementation was mutable from within
VectorAssetBuilder. Instead refactor
VectorAssetBuilder to have a separate
in progress data structure to construct
groups and create immutable VectorGroup
objects when the group is fully created
Change-Id: Ie4ffbd76d448c2c9e440292cfc307b4c2d379888
Fixes: 163828220
Test: Added test to VectorAssetBuilderTest
diff --git a/compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/graphics/vector/VectorAsset.kt b/compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/graphics/vector/VectorAsset.kt
index 8666f44..a1a9d9a 100644
--- a/compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/graphics/vector/VectorAsset.kt
+++ b/compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/graphics/vector/VectorAsset.kt
@@ -116,16 +116,15 @@
/**
* Path information used to clip the content within the group
*/
- val clipPathData: List<PathNode> = EmptyPath
+ val clipPathData: List<PathNode> = EmptyPath,
+ /**
+ * Child Vector nodes that are part of this group, this can contain
+ * paths or other groups
+ */
+ private val children: List<VectorNode> = emptyList()
) : VectorNode(), Iterable<VectorNode> {
- private val children = ArrayList<VectorNode>()
-
- internal fun addNode(node: VectorNode) {
- children.add(node)
- }
-
val size: Int
get() = children.size
diff --git a/compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/graphics/vector/VectorAssetBuilder.kt b/compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/graphics/vector/VectorAssetBuilder.kt
index 7b87e2a..f19f6c0 100644
--- a/compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/graphics/vector/VectorAssetBuilder.kt
+++ b/compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/graphics/vector/VectorAssetBuilder.kt
@@ -70,12 +70,12 @@
*/
val viewportHeight: Float
) {
- private val nodes = Stack<VectorGroup>()
+ private val nodes = Stack<GroupParams>()
- private var root = VectorGroup()
+ private var root = GroupParams()
private var isConsumed = false
- private val currentGroup: VectorGroup
+ private val currentGroup: GroupParams
get() = nodes.peek()
init {
@@ -109,7 +109,7 @@
clipPathData: List<PathNode> = EmptyPath
): VectorAssetBuilder {
ensureNotConsumed()
- val group = VectorGroup(
+ val group = GroupParams(
name,
rotate,
pivotX,
@@ -120,7 +120,6 @@
translationY,
clipPathData
)
- currentGroup.addNode(group)
nodes.push(group)
return this
}
@@ -132,7 +131,8 @@
*/
fun popGroup(): VectorAssetBuilder {
ensureNotConsumed()
- nodes.pop()
+ val popped = nodes.pop()
+ currentGroup.children.add(popped.asVectorGroup())
return this
}
@@ -177,7 +177,7 @@
trimPathOffset: Float = DefaultTrimPathOffset
): VectorAssetBuilder {
ensureNotConsumed()
- currentGroup.addNode(
+ currentGroup.children.add(
VectorPath(
name,
pathData,
@@ -204,13 +204,18 @@
*/
fun build(): VectorAsset {
ensureNotConsumed()
+ // pop all groups except for the root
+ while (nodes.size > 1) {
+ popGroup()
+ }
+
val vectorImage = VectorAsset(
name,
defaultWidth,
defaultHeight,
viewportWidth,
viewportHeight,
- root
+ root.asVectorGroup()
)
isConsumed = true
@@ -226,6 +231,42 @@
"VectorAssetBuilder is single use, create a new instance to create a new VectorAsset"
}
}
+
+ /**
+ * Helper method to create an immutable VectorGroup object
+ * from an set of GroupParams which represent a group
+ * that is in the middle of being constructed
+ */
+ private fun GroupParams.asVectorGroup(): VectorGroup =
+ VectorGroup(
+ name,
+ rotate,
+ pivotX,
+ pivotY,
+ scaleX,
+ scaleY,
+ translationX,
+ translationY,
+ clipPathData,
+ children
+ )
+
+ /**
+ * Internal helper class to help assist with in progress creation of
+ * a vector group before creating the immutable result
+ */
+ private class GroupParams (
+ var name: String = DefaultGroupName,
+ var rotate: Float = DefaultRotation,
+ var pivotX: Float = DefaultPivotX,
+ var pivotY: Float = DefaultPivotY,
+ var scaleX: Float = DefaultScaleX,
+ var scaleY: Float = DefaultScaleY,
+ var translationX: Float = DefaultTranslationX,
+ var translationY: Float = DefaultTranslationY,
+ var clipPathData: List<PathNode> = EmptyPath,
+ var children: MutableList<VectorNode> = mutableListOf()
+ )
}
/**
diff --git a/compose/ui/ui/src/test/kotlin/androidx/compose/ui/graphics/vector/VectorAssetBuilderTest.kt b/compose/ui/ui/src/test/kotlin/androidx/compose/ui/graphics/vector/VectorAssetBuilderTest.kt
index 1b8c6b6..d1f0c5e 100644
--- a/compose/ui/ui/src/test/kotlin/androidx/compose/ui/graphics/vector/VectorAssetBuilderTest.kt
+++ b/compose/ui/ui/src/test/kotlin/androidx/compose/ui/graphics/vector/VectorAssetBuilderTest.kt
@@ -58,6 +58,60 @@
Truth.assertThat(dslFunctionVector).isEqualTo(builderFunctionVector)
}
+
+ @Test
+ fun testAddGroup() {
+ val vectorAsset = builder().apply {
+ pushGroup("group1")
+ addPath(name = "path1", pathData = emptyList())
+ pushGroup("group2")
+ addPath(name = "path2", pathData = emptyList())
+ popGroup()
+ pushGroup("group3")
+ addPath(name = "path3", pathData = emptyList())
+ addPath(name = "path4", pathData = emptyList())
+ popGroup()
+ popGroup()
+ pushGroup(name = "group4")
+ addPath(name = "path5", pathData = emptyList())
+ // intentionally avoid popping group as build will pop all groups to the root
+ }.build()
+
+ val root = vectorAsset.root
+ Truth.assertThat(root.size).isEqualTo(2)
+
+ val group1 = root[0] as VectorGroup
+ Truth.assertThat(group1.name).isEqualTo("group1")
+
+ Truth.assertThat(group1.size).isEqualTo(3)
+
+ val path1 = group1[0] as VectorPath
+ Truth.assertThat(path1.name).isEqualTo("path1")
+
+ val group2 = group1[1] as VectorGroup
+ Truth.assertThat(group2.name).isEqualTo("group2")
+ Truth.assertThat(group2.size).isEqualTo(1)
+
+ val path2 = group2[0] as VectorPath
+ Truth.assertThat(path2.name).isEqualTo("path2")
+
+ val group3 = group1[2] as VectorGroup
+ Truth.assertThat(group3.name).isEqualTo("group3")
+ Truth.assertThat(group3.size).isEqualTo(2)
+
+ val path3 = group3[0] as VectorPath
+ Truth.assertThat(path3.name).isEqualTo("path3")
+
+ val path4 = group3[1] as VectorPath
+ Truth.assertThat(path4.name).isEqualTo("path4")
+
+ val group4 = root[1] as VectorGroup
+ Truth.assertThat(group4.name).isEqualTo("group4")
+ Truth.assertThat(group4.size).isEqualTo(1)
+
+ val path5 = group4[0] as VectorPath
+ Truth.assertThat(path5.name).isEqualTo("path5")
+ }
}
private fun builder() = VectorAssetBuilder(
