Skip to content
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

Don't cache subelements #468

Merged
merged 3 commits into from
Sep 21, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 13 additions & 15 deletions BlueprintUI/Sources/Element/LayoutStorage.swift
Original file line number Diff line number Diff line change
Expand Up @@ -126,21 +126,19 @@ extension LayoutStorage: LegacyContentStorage {
extension LayoutStorage: CaffeinatedContentStorage {

private func subelements(from node: LayoutTreeNode, environment: Environment) -> LayoutSubelements {
node.layoutSubelements {
var identifierFactory = ElementIdentifier.Factory(elementCount: children.count)
return children.map { child in
let identifier = identifierFactory.nextIdentifier(
for: type(of: child.element),
key: child.key
)
return LayoutSubelement(
identifier: identifier,
content: child.content,
environment: environment,
node: node.subnode(key: identifier),
traits: child.traits
)
}
var identifierFactory = ElementIdentifier.Factory(elementCount: children.count)
return children.map { child in
let identifier = identifierFactory.nextIdentifier(
for: type(of: child.element),
key: child.key
)
return LayoutSubelement(
identifier: identifier,
content: child.content,
environment: environment,
node: node.subnode(key: identifier),
traits: child.traits
)
Comment on lines +135 to +141
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Details of the alternatives I attempted:

  • Caching each LayoutSubelement by identifier. This didn't work because child.content may vary and need to be re-evaluated.
  • Making LayoutSubelement capture child.content as a closure and lazily evaluate it once the first time it is needed. Had no impact on performance.

}
}

Expand Down
10 changes: 0 additions & 10 deletions BlueprintUI/Sources/Internal/LayoutTreeNode.swift
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ final class LayoutTreeNode {

// These commonly used properties have dedicated storage. If we need to hang more generalized
// things off this type we may want to store them in a heterogeneous dictionary.
private var _layoutSubelements: [LayoutSubelement]?
private var _associatedCache: Any?

init(path: String, signpostRef: AnyObject, options: LayoutOptions) {
Expand All @@ -48,15 +47,6 @@ final class LayoutTreeNode {
return subnode
}

func layoutSubelements(create: () -> [LayoutSubelement]) -> [LayoutSubelement] {
if let layoutSubelements = _layoutSubelements {
return layoutSubelements
}
let layoutSubelements = create()
_layoutSubelements = layoutSubelements
return layoutSubelements
}

func associatedCache<AssociatedCache>(create: () -> AssociatedCache) -> AssociatedCache {
assert(
_associatedCache is AssociatedCache?,
Expand Down
54 changes: 54 additions & 0 deletions BlueprintUI/Tests/GeometryReaderTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,60 @@ final class GeometryReaderTests: XCTestCase {

wait(for: [layoutExpectation], timeout: 5)
}

func test_dynamicSubelements() {
let threshold: CGFloat = 100
let size = CGSize(width: 120, height: 120)

let element: Element = Row { outerRow in
outerRow.horizontalUnderflow = .growUniformly
outerRow.horizontalOverflow = .condenseUniformly
outerRow.verticalAlignment = .fill

outerRow.addFlexible(
child: GeometryReader { geometry in

Row { innerRow in
innerRow.horizontalUnderflow = .growUniformly
innerRow.horizontalOverflow = .condenseUniformly
innerRow.verticalAlignment = .fill

if let width = geometry.constraint.width.constrainedValue, width < threshold {
// If constrained < 100, 2 children
innerRow.addFixed(child: Spacer(1))
innerRow.addFixed(child: Spacer(1))
} else {
// else 1 child
innerRow.addFixed(child: Spacer(threshold))
}
}
}
)

outerRow.addFlexible(child: Spacer(threshold / 2))
}

// during layout:
// 1. Outer row measures GR with full width
// 2. GR body evaluates as a row with 1 child
// 3. Outer row measures again with reduced width
// 4. GR body evaluates as a row with 2 children
// 5. Subelement count has changed, as well as content of child 1

let frames = element
.layout(frame: CGRect(origin: .zero, size: size))
.queryLayout(for: Spacer.self)
.map { $0.layoutAttributes.frame }

XCTAssertEqual(
frames,
[
CGRect(origin: CGPoint(x: 0, y: 0), size: CGSize(width: 1, height: 120)),
CGRect(origin: CGPoint(x: 1, y: 0), size: CGSize(width: 1, height: 120)),
CGRect(origin: CGPoint(x: 85, y: 0), size: CGSize(width: 35, height: 120)),
]
)
}
}

extension SizeConstraint.Axis {
Expand Down
14 changes: 14 additions & 0 deletions BlueprintUI/Tests/LayoutResultNode+Testing.swift
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,18 @@ extension LayoutResultNode {
}
return nil
}

func queryLayout(for elementType: Element.Type) -> [LayoutResultNode] {
var results: [LayoutResultNode] = []

if type(of: element) == elementType {
results.append(self)
}

for child in children {
results.append(contentsOf: child.node.queryLayout(for: elementType))
}

return results
}
}