-
-
Notifications
You must be signed in to change notification settings - Fork 3.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix issue where shape items could be unexpectedly ignored #2156
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -41,7 +41,8 @@ final class ShapeLayer: BaseCompositionLayer { | |
if let repeater = shapeLayer.items.first(where: { $0 is Repeater }) as? Repeater { | ||
try setUpRepeater(repeater, context: context) | ||
} else { | ||
try setupGroups(from: shapeLayer.items, parentGroup: nil, parentGroupPath: [], context: context) | ||
let shapeItems = shapeLayer.items.map { ShapeItemLayer.Item(item: $0, groupPath: []) } | ||
try setupGroups(from: shapeItems, parentGroup: nil, parentGroupPath: [], context: context) | ||
} | ||
} | ||
|
||
|
@@ -50,7 +51,8 @@ final class ShapeLayer: BaseCompositionLayer { | |
let copyCount = Int(try repeater.copies.exactlyOneKeyframe(context: context, description: "repeater copies").value) | ||
|
||
for index in 0..<copyCount { | ||
for groupLayer in try makeGroupLayers(from: items, parentGroup: nil, parentGroupPath: [], context: context) { | ||
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) | ||
} | ||
|
@@ -127,7 +129,7 @@ final class GroupLayer: BaseAnimationLayer { | |
private func setupLayerHierarchy(context: LayerContext) throws { | ||
// Groups can contain other groups, so we may have to continue | ||
// recursively creating more `GroupLayer`s | ||
try setupGroups(from: group.items, parentGroup: group, parentGroupPath: groupPath, context: context) | ||
try setupGroups(from: items, parentGroup: group, parentGroupPath: groupPath, context: context) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the key change -- There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. All of the other changes in this file are a consequence of changing this param from |
||
|
||
// Create `ShapeItemLayer`s for each subgroup of shapes that should be rendered as a single unit | ||
// - These groups are listed from front-to-back, so we have to add the sublayers in reverse order | ||
|
@@ -183,7 +185,7 @@ extension CALayer { | |
/// - Each `Group` item becomes its own `GroupLayer` sublayer. | ||
/// - Other `ShapeItem` are applied to all sublayers | ||
fileprivate func setupGroups( | ||
from items: [ShapeItem], | ||
from items: [ShapeItemLayer.Item], | ||
parentGroup: Group?, | ||
parentGroupPath: [String], | ||
context: LayerContext) | ||
|
@@ -204,29 +206,27 @@ extension CALayer { | |
/// - Each `Group` item becomes its own `GroupLayer` sublayer. | ||
/// - Other `ShapeItem` are applied to all sublayers | ||
fileprivate func makeGroupLayers( | ||
from items: [ShapeItem], | ||
from items: [ShapeItemLayer.Item], | ||
parentGroup: Group?, | ||
parentGroupPath: [String], | ||
context: LayerContext) | ||
throws -> [GroupLayer] | ||
{ | ||
var (groupItems, otherItems) = items | ||
.filter { !$0.hidden } | ||
.grouped(by: { $0 is Group }) | ||
var groupItems = items.compactMap { $0.item as? Group }.filter { !$0.hidden } | ||
var otherItems = items.filter { !($0.item is Group) && !$0.item.hidden } | ||
|
||
// Handle the top-level `shapeLayer.items` array. This is typically just a single `Group`, | ||
// but in practice can be any combination of items. The implementation expects all path-drawing | ||
// shape items to be managed by a `GroupLayer`, so if there's a top-level path item we | ||
// have to create a placeholder group. | ||
if parentGroup == nil, otherItems.contains(where: { $0.drawsCGPath }) { | ||
groupItems = [Group(items: items, name: "")] | ||
if parentGroup == nil, otherItems.contains(where: { $0.item.drawsCGPath }) { | ||
groupItems = [Group(items: items.map { $0.item }, name: "")] | ||
otherItems = [] | ||
} | ||
|
||
// Any child items that wouldn't be included in a valid shape render group | ||
// need to be applied to child groups (otherwise they'd be silently ignored). | ||
let inheritedItemsForChildGroups = otherItems | ||
.map { ShapeItemLayer.Item(item: $0, groupPath: parentGroupPath) } | ||
.shapeRenderGroups(groupHasChildGroupsToInheritUnusedItems: !groupItems.isEmpty) | ||
.unusedItems | ||
|
||
|
@@ -235,8 +235,6 @@ extension CALayer { | |
let groupsInZAxisOrder = groupItems.reversed() | ||
|
||
return try groupsInZAxisOrder.compactMap { group in | ||
guard let group = group as? Group else { return nil } | ||
|
||
var pathForChildren = parentGroupPath | ||
if !group.name.isEmpty { | ||
pathForChildren.append(group.name) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
{"v":"5.6.6","ip":0,"op":160,"fr":60,"w":70,"h":70,"assets":[],"layers":[{"ind":386,"nm":"surface1542","ao":0,"ip":0,"op":264,"st":0,"ty":4,"ks":{"ty":"tr","o":{"k":100},"r":{"k":0},"p":{"k":[0,0]},"a":{"k":[0,0]},"s":{"k":[133.33,133.33]},"sk":{"k":0},"sa":{"k":0}},"shapes":[{"ty":"gr","hd":false,"nm":"surface1542","it":[{"ty":"gr","hd":false,"it":[{"ty":"sh","ks":{"k":{"i":[[0,0],[-1.24,0],[0,-1.24],[0,0],[0.92,-0.51],[0,0],[0.59,1.05],[-1.05,0.59],[0,0]],"o":[[0,-1.24],[1.24,0],[0,0],[0.32,0.96],[0,0],[-1.05,0.59],[-0.59,-1.04],[0,0],[0,0]],"v":[[24.75,12],[27,9.75],[29.25,12],[29.25,25.76],[28.25,28.32],[15.23,35.62],[12.27,34.78],[13.11,31.83],[24.75,25.3]],"c":true}}},{"ty":"sh","ks":{"k":{"i":[[0,0],[0,0.68],[0.69,0],[0,-0.69],[-0.68,0]],"o":[[0.69,0],[0,-0.69],[-0.68,0],[0,0.68],[0,0]],"v":[[26.84,27.82],[28.08,26.57],[26.84,25.33],[25.6,26.57],[26.84,27.82]],"c":true}}},{"ty":"fl","o":{"a":1,"k":[{"i":{"x":[0.67],"y":[1]},"o":{"x":[0.33],"y":[0]},"t":59,"s":[0]},{"t":109,"s":[100]}],"ix":5},"c":{"k":[0.29,0.75,0.69,1]}},{"ty":"tr","o":{"k":100},"r":{"k":0},"p":{"k":[0,0]},"a":{"k":[0,0]},"s":{"k":[100,100]},"sk":{"k":0},"sa":{"k":0},"hd":false}]},{"ty":"gr","hd":false,"it":[{"ty":"sh","ks":{"k":{"i":[[0,0],[3.11,0],[0,3.11],[-3.11,0],[0,-3.11]],"o":[[0,3.11],[-3.11,0],[0,-3.11],[3.11,0],[0,0]],"v":[[32.25,26.62],[26.62,32.25],[21,26.62],[26.62,21],[32.25,26.62]],"c":false}}},{"ty":"fl","o":{"a":1,"k":[{"i":{"x":[0.67],"y":[1]},"o":{"x":[0.33],"y":[0]},"t":59,"s":[0]},{"t":109,"s":[30]}],"ix":5},"c":{"k":[0.29,0.75,0.69,1]}},{"ty":"tr","o":{"k":100},"r":{"k":0},"p":{"k":[0,0]},"a":{"k":[0,0]},"s":{"k":[100,100]},"sk":{"k":0},"sa":{"k":0},"hd":false}]},{"ty":"gr","hd":false,"it":[{"ty":"sh","ks":{"k":{"i":[[0,0],[18.78,0],[0,18.78],[-18.78,0],[0,-18.78]],"o":[[0,18.78],[-18.78,0],[0,-18.78],[18.78,0],[0,0]],"v":[[69,35],[35,69],[1,35],[35,1],[69,35]],"c":false}}},{"ty":"st","lc":1,"lj":1,"ml":4,"o":{"k":100},"w":{"k":2},"c":{"k":[0.29,0.75,0.69,1]},"hd":false},{"ty":"tr","o":{"k":100},"r":{"k":0},"p":{"k":[0,0]},"a":{"k":[0,0]},"s":{"k":[75,75]},"sk":{"k":0},"sa":{"k":0},"hd":false}]},{"ty":"tr","o":{"k":100},"r":{"k":0},"p":{"k":[0,0]},"a":{"k":[0,0]},"s":{"k":[100,100]},"sk":{"k":0},"sa":{"k":0},"hd":false}]},{"ty":"tm","s":{"a":0,"k":0,"ix":1},"e":{"a":1,"k":[{"i":{"x":[0.27],"y":[1]},"o":{"x":[0.5],"y":[0]},"t":0,"s":[0]},{"t":66,"s":[100]}],"ix":2},"o":{"a":0,"k":0,"ix":3},"m":1,"ix":2,"hd":false}]}],"meta":{"g":"LF SVG to Lottie"}} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
Does not support Core Animation engine. Encountered compatibility issues: | ||
[surface1542.Layer] The Core Animation rendering engine doesn't currently support applying trims to filled shapes (only stroked shapes). | ||
[surface1542.Layer] The Core Animation rendering engine doesn't currently support applying trims to filled shapes (only stroked shapes). | ||
[surface1542.Layer] The Core Animation rendering engine doesn't currently support applying trims to filled shapes (only stroked shapes). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you still want this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, we had this configured in the UIKit example app but until now were missing it in the SwiftUI example app