TextureGroup
Cell size isn't always properly updated by layout system.
Sample Project
Here is a link to the sample project that demonstrates the issue: https://github.com/jeffdav/TextureTest
Summary
I have an ASEditableTextNode subclass that implements calculateSizeThatFits. When I call setNeedsLayout and return a different size from calculateSizeThatFits it is not always respected leading to clipped text. This only occurs in cells that are initially outside the visible range.
Details
Consider this class:
class EditableTextNode: ASEditableTextNode {
override func calculateSizeThatFits(_ constrainedSize: CGSize) -> CGSize {
if SOME_CONDITION {
return SIZE_FINAL
}
return SIZE_INITIAL
}
override func didLoad() {
super.didLoad()
setNeedsLayout() // Update the size.
}
}
For completeness, here is the cell class:
class Cell: ASCellNode {
let textNode: EditableTextNode
init(indexPath: IndexPath) {
self.textNode = EditableTextNode()
super.init()
// ** Initialize textNode with attributed text here. **
automaticallyManagesSubnodes = true
}
override func layoutSpecThatFits(_ constrainedSize: ASSizeRange) -> ASLayoutSpec {
return ASStaticLayoutSpec(sizing: .sizeToFit, children: [textNode])
}
}
It should not matter what SOME_CONDITION is, since, as far as I know, there is no explicit restriction on how and when I can change my size. The layout system should simply respect whatever value I return (within the constraints of the overall layout).
Take a moment to re-read the above statement.
In the sample project the condition I'm checking is, "are we on the main thread?"
override func calculateSizeThatFits(_ constrainedSize: CGSize) -> CGSize {
if Thread.isMainThread {
return SIZE_FINAL
}
return SIZE_INITIAL
}
Why? When we initially load the cell, we can't properly calculate the size of the text because of reasons beyond the scope of this bug (though I'd be happy to have a discussion about that out of band). So we wait for an opportunity to use the backing textView to measure the text, which can only be done from the main thread. But this is irrelevant to the actual bug, because why I decide to change sizes shouldn't matter.
Example of Bug
If you run the sample project, you will see a simple collection view with text cells. It will look fine until you scroll. Then you will start to see clipped cells:
The sample project logs what it returns for each cell (by IndexPath.item) in calculateSizeThatFits. If we filter the log output for cell 31 (the last cell, just to make it easy), we see:
2017-07-15 10:36:12.472 TextureTest[60546:2852928] indexPath: 31, isMainThread: no, size: (414.0, 30.0)
2017-07-15 10:36:12.485 TextureTest[60546:2852724] indexPath: 31, isMainThread: yes, size: (414.0, 80.0)
2017-07-15 10:36:14.931 TextureTest[60546:2852724] indexPath: 31, isMainThread: yes, size: (414.0, 80.0)
2017-07-15 10:37:49.829 TextureTest[60546:2852724] indexPath: 31, isMainThread: yes, size: (414.0, 80.0)
This clearly shows that we initially returned a height of 30 but then changed to 80 and stuck with it. Yet the cell is 30pts high.
Additionally, if we look at the event log for the node and its supernode, we see the node is calculating it's size properly, yet the parent stubbornly refuses to resize:
(lldb) po [0x7ff1eac06de0 eventLog]
Event log for <TextureTest.EditableTextNode: 0x7ff1eac06de0>. Events: (
"<(8440) t= 0.046: init>",
"<(8440) t= 0.083: asyncTraitCollectionDidChange: { userInterfaceIdiom = Phone; containerSize = {414, 736}; horizontalSizeClass = Compact; verticalSizeClass = Regular; forceTouchCapability = Available }>",
"<(8440) t= 0.089: calculatedSize: {414, 30}>",
"<(Main) t= 0.102: calculatedSize: {414, 80}>",
"<(Main) t= 0.149: calculatedSize: {414, 80}>",
"<(Main) t= 0.150: supernodeDidChange: <TextureTest.Cell: 0x7ff1eac00150>, oldValue = (null)>",
"<(Main) t= 0.150: interfaceStateDidChange: { MeasureLayout | Preload | Display }, old: { No state }>",
"<(Main) t= 0.150: setHierarchyState: oldState = { Normal }, newState = { RangeManaged }>",
"<(Main) t= 0.174: didLoad>",
"<(Main) t= 0.175: calculatedSize: {414, 80}>",
"<(Main) t= 1.503: enterHierarchy>",
"<(Main) t= 1.504: interfaceStateDidChange: { MeasureLayout | Preload | Display | Visible }, old: { MeasureLayout | Preload | Display }>"
)
(lldb) po [[0x7ff1eac06de0 supernode] eventLog]
Event log for <TextureTest.Cell: 0x7ff1eac00150>. Events: (
"<(8440) t= 0.046: init>",
"<(8440) t= 0.082: setHierarchyState: oldState = { Normal }, newState = { RangeManaged }>",
"<(8440) t= 0.082: asyncTraitCollectionDidChange: { userInterfaceIdiom = Phone; containerSize = {414, 736}; horizontalSizeClass = Compact; verticalSizeClass = Regular; forceTouchCapability = Available }>",
"<(8440) t= 0.089: computedLayout: <ASLayout: 0x6080000b7760; layoutElement = <TextureTest.Cell: 0x7ff1eac00150>; position = {nan, nan}; size = {414, 30}>>",
"<(Main) t= 0.102: computedLayout: <ASLayout: 0x6000000b8600; layoutElement = <TextureTest.Cell: 0x7ff1eac00150>; position = {nan, nan}; size = {414, 80}>>",
"<(Main) t= 0.149: computedLayout: <ASLayout: 0x6180000bab20; layoutElement = <TextureTest.Cell: 0x7ff1eac00150>; position = {nan, nan}; size = {414, 30}>>",
"<(Main) t= 0.150: insertSubnodes: (\n \"<TextureTest.EditableTextNode: 0x7ff1eac06de0>\"\n)>",
"<(Main) t= 0.150: interfaceStateDidChange: { MeasureLayout | Preload | Display }, old: { No state }>",
"<(Main) t= 0.175: didLoad>",
"<(Main) t= 1.503: enterHierarchy>",
"<(Main) t= 1.504: interfaceStateDidChange: { MeasureLayout | Preload | Display | Visible }, old: { MeasureLayout | Preload | Display }>"
)
The Workaround
Now open EditableTextNode.swift, and change line 12 to enable the workaround:
let ENABLE_WORKAROUND = true
The workaround adds a check to see if the node is loaded before changing size, so the implementation becomes:
override func calculateSizeThatFits(_ constrainedSize: CGSize) -> CGSize {
if Thread.isMainThread && isNodeLoaded {
return SIZE_FINAL
}
...
Now when we run the sample project all the cells are properly sized. But a clue as to the issue can be found in the log output, again filtering for cell 31:
2017-07-15 10:42:52.422 TextureTest[60849:2875306] indexPath: 31, isMainThread: no, size: (414.0, 30.0)
2017-07-15 10:42:52.434 TextureTest[60849:2874838] indexPath: 31, isMainThread: yes, size: (414.0, 30.0)
2017-07-15 10:42:52.532 TextureTest[60849:2874838] indexPath: 31, isMainThread: yes, size: (414.0, 30.0)
2017-07-15 10:42:52.561 TextureTest[60849:2874838] indexPath: 31, isMainThread: yes, size: (414.0, 30.0)
2017-07-15 10:42:52.569 TextureTest[60849:2874838] indexPath: 31, isMainThread: yes, size: (414.0, 30.0)
2017-07-15 10:42:52.576 TextureTest[60849:2874838] indexPath: 31, isMainThread: yes, size: (414.0, 30.0)
2017-07-15 10:42:54.320 TextureTest[60849:2874838] indexPath: 31, isMainThread: yes, size: (414.0, 80.0)
2017-07-15 10:42:54.322 TextureTest[60849:2874838] indexPath: 31, isMainThread: yes, size: (414.0, 80.0)
2017-07-15 10:42:54.323 TextureTest[60849:2874838] indexPath: 31, isMainThread: yes, size: (414.0, 80.0)
2017-07-15 10:42:54.326 TextureTest[60849:2874838] indexPath: 31, isMainThread: yes, size: (414.0, 80.0)
Notice we now return the initial size until the cell is loaded. Notice also calculateSizeThatFits is called a lot more. This is consistent across runs.
More information
In the course of debugging this, I attempted to compare ASDK 2.2.1 (which did NOT have this bug) to Texture 2.3.4 (which does; the bug first occurs in Texture 2.3). I did this by debugging __layout calls for the cell in question side-by-side. Execution diverges here, in _locked_measureNodeWithBoundsIfNecessary::
// Figure out previous and pending layouts for layout transition
std::shared_ptr<ASDisplayNodeLayout> nextLayout = _pendingDisplayNodeLayout;
#define layoutSizeDifferentFromBounds !CGSizeEqualToSize(nextLayout->layout.size, boundsSizeForLayout)
// nextLayout was likely created by a call to layoutThatFits:, check if it is valid and can be applied.
// If our bounds size is different than it, or invalid, recalculate. Use #define to avoid nullptr->
if (nextLayout == nullptr || nextLayout->isDirty() == YES || layoutSizeDifferentFromBounds) {
In versions of Texture that exhibit the issue _pendingDisplayNodeLayout is replaced with new instances after my class calls setNeedsLayout, thus isDirty() is NO because the dirty bit I set has been thrown away. These versions of Texture call - layoutThatFits:parentSize: much more frequently than previous versions, and that is the code that replaces _pendingDisplayNodeLayout. I'm not sure if this is relevant or not, but I'm including it because I think it might help debug the issue.
