Improve Layout and Constraint Implementation (so that the control works well in Interface Builder.)
Closed this issue · 6 comments
Summary
Right now CKCalendarView on the 5.0.0 branch works pretty decently with auto layout when instantiated with code. The problem is that the way layout is defined, we do all of the work in updateConstraints. Views are installed, constraints are created, and constraints are updated. This is far from ideal, for a number of reasons. (See the sources at the bottom for more on this.)
Expected Behavior
The control should work correctly when instantiated from code and from Interface Builder.
Actual Behavior
The control works almost correctly in code (except for initial layout) and completely incorrectly in Interface Builder.
Notes
I noticed that when I moved my subview installation logic from layoutSubviews to updateConstraints, constraint based cell animations became much smoother. However, when I tried to add Interface Builder support, I noticed that things weren't going right.
Instances of the calendar view that were created in code initially were a little too short. Calendars created in Interface Builder had too much space between the cells until the user forced a layout pass.
Interface Builder itself showed a skewed version of the calendar, with a similar problem. Here, though, the bottom border rendered fine, but the calendar itself occupies the top left corner of the control, instead of the full size. The same cell spacing problem occurs as when the code is run.
Digging in a little more deeply, I realized that my layout setup was probably incorrect. So it turns out, that even though it made sense to me to have a single method for installing each view, (_installTable, _installHeader, etc) this design ignores the structure of UIView layout. I need to rethink this.
The fix here is probably to go through the view lifecycle and properly set up all of the parts at the appropriate times.
Sources:
- Auto Layout Best Practices for Minimum Pain (Omar Abdelhafith, Medium)
- Demistifying iOS Layout (Ami Kumar, GC Tech Blog)
- How to Use
updateConstraints(Ole Begemann) - Advanced Autolayout Toolbox (objc.io)
- Render and Inspect Your Custom Views (Apple)
- Modifying Constraints (Apple)
layoutSubviews()documentation (Apple)updateConstraints()documentation (Apple)- Layout Guide: Understanding Auto Layout (Apple)
This article offered a red herring but ultimately led me closer to my solution:
What Went Wrong
tl;dr By using auto layout for the everything but the positions of calendar cells, and using manual layout for the cells themselves, I was creating complexity that I couldn't solve for. When I tried to fix it by naively adding constraints, the visuals looked great, but performance suffered.
So it turns out™ that the cells were incorrectly spaced because the layout engine was doing cell arithmetic instead of properly pinning the views to each other. In a pre-autolayout world, this was accurate and fast. Once autolayout came into play, I made changes to account for updateConstraints and the intrinsic content size of the calendar view.
This increasingly complex layout environment meant that somewhere, the dimensions of each cell, which almost all of the old layout code was based on, ended up wrong in one of the layout passes.
Rather than track down exactly what was causing this, I looked for another layout approach. (I still don't know why the calendar view had the wrong spacing, but the right size for the cells.)
Trying on Autolayout Constraints for Size
My first solution was to pin the cells' trailing and leading edges using constraints. With this approach, I'm still using manually recycled UIView subclasses for calendar cells, and managing the recycling and positioning myself.
Using constraints fixed the layout inconsistencies by leaning on auto layout to guarantee correct pinning, instead of basing it on the width of the calendar view. It also had the added benefit of correct rendering in localized right-to-left environments.
The tradeoff here was an unacceptable performance hit. Running in the simulator on a quad core machine, the demo app performed fine, but an iPod touch running iOS 9 took well over a minute to switch between months. (Instruments verified this.)
Layout Constraints, Memory Constraints, Performance Constraints
I tried iterating on this by caching constraints on the cells, using NSMapTable properties to store vertical and horizontal pinning constraints, by the new cell to pin against. The idea here was that each cell has its constraints deactivated when it animates off-screen. If each cell kept copies of its layout constraints around that could be referenced by keying into the cache with the other item in the constraint, we could activate them based on the paired view, perhaps alleviating the bottleneck.
This wasn't how it worked out. Caching constraints reduces the time spent creating new constraints on subsequent layout passes (by allocating memory for them only once) but it doesn't solve for the first time a cell is pinned to another cell. So if Cell A is pinned to Cell B the first time around, and then is pinned to Cell C, the constraint is created a second time.
This is even worse from a memory standpoint, because even though we have a cache with pretty good access, memory usage is factorial - we have one constraint on each cell for every other cell. A grid requires pinning in both the horizontal and the vertical direction, so, not that it makes a huge difference, but the performance becomes O(2 * n!). Ouch!
More importantly, creating the constraints isn't the expensive part of the layout operation - activating the constraints is. "Activating" a constraint essentially causes the auto layout engine to solve the layout again.
Calling [someView addConstraints:@[constraintX, constraintY]] is the same as calling [NSLayoutConstraint activateConstraints:@[constraintX, constraintY] which is in turn the same as contraintX.isActive = YES; constraintY.isActive = YES. (The addConstraints: call requires you to know which view to call the method on, while the others figure this out for you.)
Doing this about 60 times per transition is what was causing the delay. 🤦♂️
I also want to call out that the documentation for NSLayoutConstraint activateConstraints:` says that this method is "usually more efficient" than manually activating each constraint, but profiling has shown that it didn't seem to make a difference in this case.
Collecting the Pieces with Collection View
Finally, I began to try laying out the cells with UICollectionView. It turns out that implementing a grid representing a calendar month is absurdly easy with UICollectionView. The develop branch is in an incomplete state right now, but as things progress, this approach seems to offer everything I was looking for: Autolayout support, proper layout in right-to-left environments, and most importantly, buttery smooth performance.
This brings me to the point of this comment, which is that this issue is now blocked by #109 Upgrade to UICollectionView as well. It's a work in progress, but so far I'm happy with it.
Final Thoughts
How UIStackView Stacks Up
I have no idea. I didn't try, and don't plan to if UICollectionView pans out.
Interface Builder Support, Built In
Another added benefit of this journey through cleaning up and optimizing MBCalendarKit's layout is that the IBDesignable rendering of the calendar control began to work once I simplified some of the intrinsic content size and layout calculations of CKCalendarView. 🎉
Improve Your iOS Foundations by Reading This
I found this issue of objc.io on characteristics of foundation classes and it looks absolutely amazing.
Collection View is a Go 👍
I got this to work nicely with UICollectionView and a custom subclass of UICollectionViewFlowLayout. Animation was really easy, by implementing initialLayoutAttributesForAppearingItemAtIndexPath: and finalLayoutAttributesForDisappearingItemAtIndexPath:, I was able to re-implement cell animation and not have to worry about any of the positioning. UICollectionViewFlowLayout did all of the hard work.
Benefits
This actually enabled a large number of enhancements to the framework by itself, including proper right-to-left support, and week view animation.
What Actually Went Wrong! 🤦♂️
While I was working on this, though, I came across something really interesting. The reason the why the cell spacing was wrong initially was because views that are instantiated in a storyboard have a default bounds size of 600 pixels by 600 pixels.
The size doesn't change until the view moves to the window, by which time the cells have already been laid out. The simplest fix would have been to reload the cells in didMoveToWindow.
Known Bugs
There is one slightly noticeable issue here, which is that when transitioning between months that have a different number of weeks visible, there are gaps in the animation. This is less notificable if the background color and the cell color is the same. (which it is by default, so I'm not worrying about it for 5.0.0)
Known Bugs Redux
The issue with the cells being slightly off in transitions from month to month with different week lengths is because we're using the contentSize of the collection view before transitioning, and not the height for the new set of rows to determine offset.
Going to open an issue for that transition bug, and then close this.
Opened as #125