Skip to content

Commit

Permalink
Merge pull request #180 from wayfair/performance
Browse files Browse the repository at this point in the history 
Fixed performance of height recalculation
  • Loading branch information
@jay18001
jay18001 authored Aug 25, 2017
2 parents 208035f + e13f475 commit 17bfb7d
Show file tree
Hide file tree
Showing 21 changed files with 326 additions and 90 deletions.
20 changes: 16 additions & 4 deletions BrickKit.xcodeproj/project.pbxproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,10 @@
9333538F1E36A37F003CEC85 /* GenericBrickTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9333538D1E36A37F003CEC85 /* GenericBrickTests.swift */; };
933FF95E1DC1207900E0B80E /* Swizzle.swift in Sources */ = {isa = PBXBuildFile; fileRef = 933FF95D1DC1207900E0B80E /* Swizzle.swift */; };
933FF95F1DC1207900E0B80E /* Swizzle.swift in Sources */ = {isa = PBXBuildFile; fileRef = 933FF95D1DC1207900E0B80E /* Swizzle.swift */; };
9340B6B21F4DBC2A005C28F0 /* BrickLogger.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9340B6B11F4DBC2A005C28F0 /* BrickLogger.swift */; };
9340B6B31F4DBC2A005C28F0 /* BrickLogger.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9340B6B11F4DBC2A005C28F0 /* BrickLogger.swift */; };
9340B6B51F4DE8F4005C28F0 /* BrickLoggerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9340B6B41F4DE8F4005C28F0 /* BrickLoggerTests.swift */; };
9340B6B61F4DE8F4005C28F0 /* BrickLoggerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9340B6B41F4DE8F4005C28F0 /* BrickLoggerTests.swift */; };
934ADAA31E53B4B200C36587 /* ButtonBrick.xib in Resources */ = {isa = PBXBuildFile; fileRef = 934ADA971E53B48200C36587 /* ButtonBrick.xib */; };
934ADAA41E53B4B200C36587 /* ImageBrick.xib in Resources */ = {isa = PBXBuildFile; fileRef = 934ADA9C1E53B49300C36587 /* ImageBrick.xib */; };
934ADAA51E53B4B200C36587 /* LabelBrick.xib in Resources */ = {isa = PBXBuildFile; fileRef = 934ADA991E53B48C00C36587 /* LabelBrick.xib */; };
Expand Down Expand Up @@ -311,6 +315,8 @@
933353891E368903003CEC85 /* GenericBrick.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; name = GenericBrick.swift; path = Generic/GenericBrick.swift; sourceTree = "<group>"; };
9333538D1E36A37F003CEC85 /* GenericBrickTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = GenericBrickTests.swift; sourceTree = "<group>"; };
933FF95D1DC1207900E0B80E /* Swizzle.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = Swizzle.swift; sourceTree = "<group>"; };
9340B6B11F4DBC2A005C28F0 /* BrickLogger.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = BrickLogger.swift; sourceTree = "<group>"; };
9340B6B41F4DE8F4005C28F0 /* BrickLoggerTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; lineEnding = 0; path = BrickLoggerTests.swift; sourceTree = "<group>"; xcLanguageSpecificationIdentifier = xcode.lang.swift; };
934ADA971E53B48200C36587 /* ButtonBrick.xib */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = file.xib; path = ButtonBrick.xib; sourceTree = "<group>"; };
934ADA991E53B48C00C36587 /* LabelBrick.xib */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = file.xib; path = LabelBrick.xib; sourceTree = "<group>"; };
934ADA9C1E53B49300C36587 /* ImageBrick.xib */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = file.xib; path = ImageBrick.xib; sourceTree = "<group>"; };
Expand Down Expand Up @@ -354,10 +360,10 @@
93D9EBCF1DA4057000D8C87A /* BrickSectionCell.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = BrickSectionCell.swift; sourceTree = "<group>"; };
93D9EBD21DA4057000D8C87A /* ImageDownloader.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ImageDownloader.swift; sourceTree = "<group>"; };
93D9EBD31DA4057000D8C87A /* Info.plist */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.plist.xml; path = Info.plist; sourceTree = "<group>"; };
93D9EBD51DA4057000D8C87A /* BrickFlowLayout.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = BrickFlowLayout.swift; sourceTree = "<group>"; };
93D9EBD51DA4057000D8C87A /* BrickFlowLayout.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; lineEnding = 0; path = BrickFlowLayout.swift; sourceTree = "<group>"; xcLanguageSpecificationIdentifier = xcode.lang.swift; };
93D9EBD61DA4057000D8C87A /* BrickLayout.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = BrickLayout.swift; sourceTree = "<group>"; };
93D9EBD71DA4057000D8C87A /* BrickLayoutInvalidationContext.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = BrickLayoutInvalidationContext.swift; sourceTree = "<group>"; };
93D9EBD81DA4057000D8C87A /* BrickLayoutSection.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = BrickLayoutSection.swift; sourceTree = "<group>"; };
93D9EBD81DA4057000D8C87A /* BrickLayoutSection.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; lineEnding = 0; path = BrickLayoutSection.swift; sourceTree = "<group>"; xcLanguageSpecificationIdentifier = xcode.lang.swift; };
93D9EBDB1DA4057000D8C87A /* BrickCollectionViewDataSource.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = BrickCollectionViewDataSource.swift; sourceTree = "<group>"; };
93D9EBDC1DA4057000D8C87A /* BrickDimension.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = BrickDimension.swift; sourceTree = "<group>"; };
93D9EBDD1DA4057000D8C87A /* Brick.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = Brick.swift; sourceTree = "<group>"; };
Expand All @@ -367,7 +373,7 @@
93D9EBE21DA4057000D8C87A /* BrickUtils.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = BrickUtils.swift; sourceTree = "<group>"; };
93D9EBE31DA4057000D8C87A /* FatalError.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = FatalError.swift; sourceTree = "<group>"; };
93D9EBE51DA4057000D8C87A /* BrickCollectionView+BrickLayoutDataSource.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = "BrickCollectionView+BrickLayoutDataSource.swift"; sourceTree = "<group>"; };
93D9EBE61DA4057000D8C87A /* BrickCollectionView.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = BrickCollectionView.swift; sourceTree = "<group>"; };
93D9EBE61DA4057000D8C87A /* BrickCollectionView.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; lineEnding = 0; path = BrickCollectionView.swift; sourceTree = "<group>"; xcLanguageSpecificationIdentifier = xcode.lang.swift; };
93D9EBE71DA4057000D8C87A /* BrickViewController.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = BrickViewController.swift; sourceTree = "<group>"; };
93D9EC1A1DA4057900D8C87A /* CardLayoutBehaviorTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = CardLayoutBehaviorTests.swift; sourceTree = "<group>"; };
93D9EC1C1DA4057900D8C87A /* HideLayoutBehaviorTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = HideLayoutBehaviorTests.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -407,7 +413,7 @@
93D9EC4B1DA4057900D8C87A /* BrickUtilsTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = BrickUtilsTests.swift; sourceTree = "<group>"; };
93D9EC4C1DA4057900D8C87A /* CollectionViewDataSource.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = CollectionViewDataSource.swift; sourceTree = "<group>"; };
93D9EC4D1DA4057900D8C87A /* DataSources.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = DataSources.swift; sourceTree = "<group>"; };
93D9EC4E1DA4057900D8C87A /* XCTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = XCTests.swift; sourceTree = "<group>"; };
93D9EC4E1DA4057900D8C87A /* XCTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; lineEnding = 0; path = XCTests.swift; sourceTree = "<group>"; xcLanguageSpecificationIdentifier = xcode.lang.swift; };
93D9EC501DA4057900D8C87A /* BrickCollectionViewTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = BrickCollectionViewTests.swift; sourceTree = "<group>"; };
93D9EC511DA4057900D8C87A /* BrickViewControllerTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = BrickViewControllerTests.swift; sourceTree = "<group>"; };
93D9EC521DA4057900D8C87A /* TestBrickViewController.storyboard */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = file.storyboard; path = TestBrickViewController.storyboard; sourceTree = "<group>"; };
Expand Down Expand Up @@ -768,6 +774,7 @@
93D9EBE11DA4057000D8C87A /* BrickExtensions.swift */,
93D9EBE21DA4057000D8C87A /* BrickUtils.swift */,
93D9EBE31DA4057000D8C87A /* FatalError.swift */,
9340B6B11F4DBC2A005C28F0 /* BrickLogger.swift */,
);
path = Utils;
sourceTree = "<group>";
Expand Down Expand Up @@ -901,6 +908,7 @@
9372C5061DA40735007D7EA1 /* FatalErrorTests.swift */,
93D9EC4E1DA4057900D8C87A /* XCTests.swift */,
933FF95D1DC1207900E0B80E /* Swizzle.swift */,
9340B6B41F4DE8F4005C28F0 /* BrickLoggerTests.swift */,
);
path = Utils;
sourceTree = "<group>";
Expand Down Expand Up @@ -1157,6 +1165,7 @@
4E9A253B1DABEB9300D7EA99 /* BrickExtensions.swift in Sources */,
4E387C561DAEAB350087820E /* BrickAppearBehavior.swift in Sources */,
4E9A25331DABEB8800D7EA99 /* BrickLayoutSection.swift in Sources */,
9340B6B31F4DBC2A005C28F0 /* BrickLogger.swift in Sources */,
4E9A25311DABEB8800D7EA99 /* BrickLayout.swift in Sources */,
4E9A25211DABEB6000D7EA99 /* MinimumStickyLayoutBehavior.swift in Sources */,
4E9A25301DABEB8800D7EA99 /* BrickFlowLayout.swift in Sources */,
Expand Down Expand Up @@ -1236,6 +1245,7 @@
4E9A254C1DABECDF00D7EA99 /* StickyFooterLayoutBehaviorTests.swift in Sources */,
4E9A256B1DABED0600D7EA99 /* TestBrickViewController.swift in Sources */,
4E9A255C1DABECF500D7EA99 /* BrickSelfsizingFlowLayoutTests.swift in Sources */,
9340B6B61F4DE8F4005C28F0 /* BrickLoggerTests.swift in Sources */,
4E9A25501DABECE300D7EA99 /* ImageBrickTests.swift in Sources */,
932365731DF4FE1F00BE5183 /* BrickAlignmentTests.swift in Sources */,
4E387C571DAEAB380087820E /* BrickAppearBehaviorTests.swift in Sources */,
Expand Down Expand Up @@ -1272,6 +1282,7 @@
93D9EC0A1DA4057000D8C87A /* BrickLayoutInvalidationContext.swift in Sources */,
93F9B4D21DAD686E00927BE6 /* BrickAppearBehavior.swift in Sources */,
93D9EBF01DA4057000D8C87A /* OnScrollDownStickyLayoutBehavior.swift in Sources */,
9340B6B21F4DBC2A005C28F0 /* BrickLogger.swift in Sources */,
93D9EC151DA4057000D8C87A /* BrickCollectionView+BrickLayoutDataSource.swift in Sources */,
93D9EC121DA4057000D8C87A /* BrickExtensions.swift in Sources */,
93D9EBE91DA4057000D8C87A /* CardLayoutBehavior.swift in Sources */,
Expand Down Expand Up @@ -1318,6 +1329,7 @@
93D9EC821DA4057900D8C87A /* DataSources.swift in Sources */,
93D9EC7D1DA4057900D8C87A /* BrickSectionDataSourceTests.swift in Sources */,
93D9EC5D1DA4057900D8C87A /* SpotlightLayoutBehaviorTests.swift in Sources */,
9340B6B51F4DE8F4005C28F0 /* BrickLoggerTests.swift in Sources */,
9361309B1DB83CB7008FFFEF /* CustomBrickCollectionView.swift in Sources */,
93D9EC691DA4057900D8C87A /* DummyBrickWithoutNib.swift in Sources */,
93D9EC791DA4057900D8C87A /* BrickSelfsizingFlowLayoutTests.swift in Sources */,
Expand Down
1 change: 1 addition & 0 deletions Example/Source/AppDelegate.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ class AppDelegate: UIResponder, UIApplicationDelegate {
func application(_ application: UIApplication,
didFinishLaunchingWithOptions launchOptions: [UIApplicationLaunchOptionsKey : Any]? = nil) -> Bool {
Theme.applyTheme()
BrickLogger.logger = BrickConsoleLogger(logVerbose: true)

return true
}
Expand Down
22 changes: 15 additions & 7 deletions Source/Cells/BrickCell.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ open class BaseBrickCell: UICollectionViewCell {
}

// This value stores the expected width, so we can identify when this is met
private var requestedWidth: CGFloat = 0
private var requestedSize: CGSize = .zero

open func setContent(_ brick: Brick, index: Int, collectionIndex: Int, collectionIdentifier: String?) {
self._brick = brick
Expand Down Expand Up @@ -100,23 +100,19 @@ open class BaseBrickCell: UICollectionViewCell {
open override func apply(_ layoutAttributes: UICollectionViewLayoutAttributes) {
super.apply(layoutAttributes)

self.requestedWidth = layoutAttributes.frame.width
self.requestedSize = layoutAttributes.frame.size

// Setting zPosition instead of relaying on
// UICollectionView zIndex management 'fixes' the issue
// http://stackoverflow.com/questions/12659301/uicollectionview-setlayoutanimated-not-preserving-zindex
self.layer.zPosition = CGFloat(layoutAttributes.zIndex)

if self is AsynchronousResizableCell {
self.layoutIfNeeded()
}
}

open override func layoutSubviews() {
super.layoutSubviews()
brickBackgroundView?.frame = self.bounds

if _brick != nil && frame.width == requestedWidth {
if _brick != nil && frame.width == requestedSize.width {
self.layoutIfNeeded() // This layoutIfNeeded is added to make sure that the subviews are laid out correctly
self.framesDidLayout()
}
Expand Down Expand Up @@ -181,12 +177,17 @@ open class BrickCell: BaseBrickCell {
return UIEdgeInsetsMake(defaultTopConstraintConstant, defaultLeftConstraintConstant, defaultBottomConstraintConstant, defaultRightConstraintConstant)
}

private var didUpdateEdgeInsets: Bool = false
@objc open dynamic var edgeInsets: UIEdgeInsets = UIEdgeInsets.zero {
didSet {
if edgeInsets == oldValue {
return
}
self.topSpaceConstraint?.constant = edgeInsets.top
self.bottomSpaceConstraint?.constant = edgeInsets.bottom
self.leftSpaceConstraint?.constant = edgeInsets.left
self.rightSpaceConstraint?.constant = edgeInsets.right
didUpdateEdgeInsets = true
}
}

Expand Down Expand Up @@ -227,6 +228,13 @@ open class BrickCell: BaseBrickCell {
return layoutAttributes
}

if !didUpdateEdgeInsets {
guard let brickAttributes = layoutAttributes as? BrickLayoutAttributes, brickAttributes.isEstimateSize else {
return layoutAttributes
}
}
didUpdateEdgeInsets = false

let preferred = layoutAttributes

// We're inverting the frame because the given frame is already transformed
Expand Down
55 changes: 53 additions & 2 deletions Source/Layout/BrickFlowLayout.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,20 @@ fileprivate func >= <T : Comparable>(lhs: T?, rhs: T?) -> Bool {
/// BrickFlowLayoutiis a UICollectionViewLayout that can handle behaviors
open class BrickFlowLayout: UICollectionViewLayout, BrickLayout {

var isDirty: Bool {
get {
return !dirtyMap.isEmpty
}
set {
if !newValue {
dirtyMap.removeAll()
dirtyIndexPaths.removeAll()
}
}
}
var dirtyMap: [Int: Int] = [:]
var dirtyIndexPaths: [IndexPath] = []

// Mark: - Public members

open override var description: String {
Expand Down Expand Up @@ -284,7 +298,30 @@ extension BrickFlowLayout {
return contentSize
}

func updateDirtyBricks(updatedAttributes: @escaping OnAttributesUpdatedHandler) {
for (section, item) in dirtyMap {
// This can be unwrapped safely
// because the sections that are in the dirtyMap should be there
let layoutSection = sections![section]!
layoutSection.createOrUpdateCells(from: item, invalidate: false, updatedAttributes: updatedAttributes)
if let sectionAttributes = layoutSection.sectionAttributes {
invalidateHeight(for: sectionAttributes.indexPath, updatedAttributes: updatedAttributes)
}
}

recalculateContentSize()

let cachedDirtyIndexPaths = self.dirtyIndexPaths

isDirty = false

cachedDirtyIndexPaths.forEach { indexPath in
delegate?.brickLayout(self, didUpdateHeightForItemAtIndexPath: indexPath)
}
}

open override func invalidateLayout(with context: UICollectionViewLayoutInvalidationContext) {
BrickLogger.logVerbose("Invalidate layout with context \(context)")
guard sections != nil else { // No need to invalidate if there are no sections
super.invalidateLayout(with: context)
return
Expand Down Expand Up @@ -352,7 +389,20 @@ extension BrickFlowLayout {
}
let shouldInvalidate = preferredAttributes.frame.height != brickAttribute.originalFrame.height
brickAttribute.isEstimateSize = false
return shouldInvalidate

if shouldInvalidate {
let indexPath = preferredAttributes.indexPath
sections![indexPath.section]?.update(height: preferredAttributes.frame.height, at: indexPath.item, continueCalculation: false, updatedAttributes: nil)
if let current = dirtyMap[indexPath.section] {
if indexPath.item < current {
dirtyMap[indexPath.section] = indexPath.item
}
} else {
dirtyMap[indexPath.section] = indexPath.item
}
dirtyIndexPaths.append(indexPath)
}
return false
}

open override func invalidationContext(forPreferredLayoutAttributes preferredAttributes: UICollectionViewLayoutAttributes, withOriginalAttributes originalAttributes: UICollectionViewLayoutAttributes) -> UICollectionViewLayoutInvalidationContext {
Expand Down Expand Up @@ -487,7 +537,7 @@ extension BrickFlowLayout: BrickLayoutInvalidationProvider {
let contentOffsetAdjustment = shouldAdjustContentOffset ? height - firstAttributes.frame.height : 0

updateSection(section, updatedAttributes: updatedAttributes) {
section.update(height: height, at: indexPath.item, updatedAttributes: { attributes, oldFrame in
section.update(height: height, at: indexPath.item, continueCalculation: true, updatedAttributes: { attributes, oldFrame in
updatedAttributes(attributes, oldFrame)
self.attributesWereUpdated(attributes, oldFrame: oldFrame, fromBehaviors: false, updatedAttributes: updatedAttributes)
})
Expand Down Expand Up @@ -544,6 +594,7 @@ extension BrickFlowLayout: BrickLayoutInvalidationProvider {
self.contentSize = contentSize
}

@discardableResult
func recalculateContentSize() -> CGSize {
let oldContentSize = self.contentSize
contentSize = sections?[0]?.frame.size ?? CGSize.zero
Expand Down
Loading

0 comments on commit 17bfb7d

Please sign in to comment.