Fix issue where Repeater would be ignored if not at top level (airbnb#2221)

Author: calda

Sources/Private/CoreAnimation/Layers/ShapeLayer.swift

@@ -36,27 +36,8 @@ final class ShapeLayer: BaseCompositionLayer {
   private let shapeLayer: ShapeLayerModel
 
   private func setUpGroups(context: LayerContext) throws {
-    // If the layer has a `Repeater`, the `Group`s are duplicated and offset
-    // based on the copy count of the repeater.
-    if let repeater = shapeLayer.items.first(where: { $0 is Repeater }) as? Repeater {
-      try setUpRepeater(repeater, context: context)
-    } else {
-      let shapeItems = shapeLayer.items.map { ShapeItemLayer.Item(item: $0, groupPath: []) }
-      try setupGroups(from: shapeItems, parentGroup: nil, parentGroupPath: [], context: context)
-    }
-  }
-
-  private func setUpRepeater(_ repeater: Repeater, context: LayerContext) throws {
-    let items = shapeLayer.items.filter { !($0 is Repeater) }
-    let copyCount = Int(try repeater.copies.exactlyOneKeyframe(context: context, description: "repeater copies").value)
-
-    for index in 0..<copyCount {
-      let shapeItems = items.map { ShapeItemLayer.Item(item: $0, groupPath: []) }
-      for groupLayer in try makeGroupLayers(from: shapeItems, parentGroup: nil, parentGroupPath: [], context: context) {
-        let repeatedLayer = RepeaterLayer(repeater: repeater, childLayer: groupLayer, index: index)
-        addSublayer(repeatedLayer)
-      }
-    }
+    let shapeItems = shapeLayer.items.map { ShapeItemLayer.Item(item: $0, groupPath: []) }
+    try setupGroups(from: shapeItems, parentGroup: nil, parentGroupPath: [], context: context)
   }
 
 }
@@ -181,6 +162,9 @@ final class GroupLayer: BaseAnimationLayer {
 }
 
 extension CALayer {
+
+  // MARK: Fileprivate
+
   /// Sets up `GroupLayer`s for each `Group` in the given list of `ShapeItem`s
   ///  - Each `Group` item becomes its own `GroupLayer` sublayer.
   ///  - Other `ShapeItem` are applied to all sublayers
@@ -191,21 +175,79 @@ extension CALayer {
     context: LayerContext)
     throws
   {
-    let groupLayers = try makeGroupLayers(
-      from: items,
-      parentGroup: parentGroup,
-      parentGroupPath: parentGroupPath,
-      context: context)
-
-    for groupLayer in groupLayers {
-      addSublayer(groupLayer)
+    // If the layer has any `Repeater`s, set up each repeater
+    // and then handle any remaining groups like normal.
+    if items.contains(where: { $0.item is Repeater }) {
+      let repeaterGroupings = items.split(whereSeparator: { $0.item is Repeater })
+
+      // Iterate through the groupings backwards to preserve the expected rendering order
+      for repeaterGrouping in repeaterGroupings.reversed() {
+        // Each repeater applies to the previous items in the list
+        if let repeater = repeaterGrouping.trailingSeparator?.item as? Repeater {
+          try setUpRepeater(
+            repeater,
+            items: repeaterGrouping.grouping,
+            parentGroup: parentGroup,
+            parentGroupPath: parentGroupPath,
+            context: context)
+        }
+
+        // Any remaining items after the last repeater are handled like normal
+        else {
+          try setupGroups(
+            from: repeaterGrouping.grouping,
+            parentGroup: parentGroup,
+            parentGroupPath: parentGroupPath,
+            context: context)
+        }
+      }
+    }
+
+    else {
+      let groupLayers = try makeGroupLayers(
+        from: items,
+        parentGroup: parentGroup,
+        parentGroupPath: parentGroupPath,
+        context: context)
+
+      for groupLayer in groupLayers {
+        addSublayer(groupLayer)
+      }
+    }
+  }
+
+  // MARK: Private
+
+  /// Sets up this layer using the given `Repeater`
+  private func setUpRepeater(
+    _ repeater: Repeater,
+    items allItems: [ShapeItemLayer.Item],
+    parentGroup: Group?,
+    parentGroupPath: [String],
+    context: LayerContext)
+    throws
+  {
+    let items = allItems.filter { !($0.item is Repeater) }
+    let copyCount = Int(try repeater.copies.exactlyOneKeyframe(context: context, description: "repeater copies").value)
+
+    for index in 0..<copyCount {
+      let groupLayers = try makeGroupLayers(
+        from: items,
+        parentGroup: parentGroup,
+        parentGroupPath: parentGroupPath,
+        context: context)
+
+      for groupLayer in groupLayers {
+        let repeatedLayer = RepeaterLayer(repeater: repeater, childLayer: groupLayer, index: index)
+        addSublayer(repeatedLayer)
+      }
     }
   }
 
   /// Creates a `GroupLayer` for each `Group` in the given list of `ShapeItem`s
   ///  - Each `Group` item becomes its own `GroupLayer` sublayer.
   ///  - Other `ShapeItem` are applied to all sublayers
-  fileprivate func makeGroupLayers(
+  private func makeGroupLayers(
     from items: [ShapeItemLayer.Item],
     parentGroup: Group?,
     parentGroupPath: [String],
@@ -346,6 +388,32 @@ extension Collection {
 
     return (trueElements, falseElements)
   }
+
+  /// Splits this collection into an array of grouping separated by the given separator.
+  /// For example, `[A, B, C]` split by `B` returns an array with two elements:
+  ///  1. `(grouping: [A], trailingSeparator: B)`
+  ///  2. `(grouping: [C], trailingSeparator: nil)`
+  func split(whereSeparator separatorPredicate: (Element) -> Bool)
+    -> [(grouping: [Element], trailingSeparator: Element?)]
+  {
+    guard !isEmpty else { return [] }
+
+    var groupings: [(grouping: [Element], trailingSeparator: Element?)] = []
+
+    for element in self {
+      if groupings.isEmpty || groupings.last?.trailingSeparator != nil {
+        groupings.append((grouping: [], trailingSeparator: nil))
+      }
+
+      if separatorPredicate(element) {
+        groupings[groupings.indices.last!].trailingSeparator = element
+      } else {
+        groupings[groupings.indices.last!].grouping.append(element)
+      }
+    }
+
+    return groupings
+  }
 }
 
 // MARK: - ShapeRenderGroup

Tests/Samples/Issues/issue_2220.json

@@ -53,12 +53,20 @@ extension SnapshotConfiguration {
     "Nonanimating/FirstText": .precision(0.99),
     "Nonanimating/verifyLineHeight": .precision(0.99),
     "Nonanimating/blend_mode_test": .precision(0.99),
+    "Nonanimating/base64Test": .precision(0.9),
     "Issues/issue_2066": .precision(0.9),
     "LottieFiles/dog_car_ride": .precision(0.95),
     "Issues/issue_1800": .precision(0.95),
     "Issues/issue_1882": .precision(0.95),
     "Issues/issue_1717": .precision(0.95),
+    "Issues/issue_1887": .precision(0.95),
+    "Issues/issue_1683": .precision(0.93),
+    "Issues/pr_1763": .precision(0.95),
     "Issues/pr_1964": .precision(0.95),
+    "Issues/pr_1930_rx": .precision(0.93),
+    "Issues/pr_1930_ry": .precision(0.93),
+    "Issues/pr_1930_all_axis": .precision(0.93),
+    "Issues/issue_1169_four_shadows": .precision(0.93),
     "DotLottie/animation_external_image": .precision(0.95),
     "DotLottie/animation_inline_image": .precision(0.95),
     "LottieFiles/gradient_shapes": .precision(0.95),
@@ -95,7 +103,7 @@ extension SnapshotConfiguration {
 
     "Issues/issue_1664": .customValueProviders([
       AnimationKeypath(keypath: "**.base_color.**.Color"): ColorValueProvider(.black),
-    ]),
+    ]).precision(0.95),
 
     "Issues/issue_1854": .customValueProviders([
       AnimationKeypath(keypath: "**.Colors"): GradientValueProvider(

Tests/SnapshotConfiguration.swift

@@ -143,7 +143,7 @@ class SnapshotTests: XCTestCase {
           matching: animationView,
           as: .imageOfPresentationLayer(
             precision: SnapshotConfiguration.forSample(named: sampleAnimationName).precision,
-            perceptualPrecision: 0.98),
+            perceptualPrecision: 0.97),
           named: "\(sampleAnimationName) (\(Int(percent * 100))%)",
           testName: testName)
       }

Tests/SnapshotTests.swift

@@ -0,0 +1 @@
+Supports Core Animation engine