[ASDisplayNode+Layout] Add check for orphaned nodes after layout transition to clean up. #336
Conversation
There was a problem hiding this comment.
@appleguy Dang this is an annoying issue.
Since this is a reasonably hot code path, let's use [NSHashTable hashTableWithOptions:NSHashTableObjectPointerPersonality] to avoid needless -hash and -isEqual: and then let's land.
There was a problem hiding this comment.
I think this is a better place to remove orphaned nodes. However, I'm willing to accept this approach.
In the near future, we should aim to address the root cause, which I believe is due to the fact that ASLayoutTransition accepts it previous layout in the initializer, instead of at commit time (i.e -commitTransition). It means that the transition assumes that if it's allowed to proceed on main, the previous layout remains valid which is not true if there was layout pass triggered on main via -setNeedsLayout.
appleguy
force-pushed
the
SubnodeRemoval
branch
2 times, most recently
from
0875e09
to
4fc0dc0
Compare
Source/ASDisplayNode+Layout.mm
Outdated
| NSArray<ASDisplayNode *> *subnodes = [self subnodes]; | ||
| NSHashTable<ASDisplayNode *> *currentSubnodes = | ||
| [[NSHashTable alloc] initWithOptions:NSHashTableObjectPointerPersonality | ||
| capacity:subnodes.count]; |
There was a problem hiding this comment.
Totally optional, you can do auto currentSubnodes = [[NSHashTable<ASDisplayNode *> alloc]…] to save some characters.
…sition to clean up. It is rare that this code has any effect, but I've discovered a case in which it occurs. This task tracks moving this code to a DEBUG-only assertion: #335
| auto currentSubnodes = [[NSHashTable alloc] initWithOptions:NSHashTableObjectPointerPersonality | ||
| capacity:subnodes.count]; | ||
| auto layoutSubnodes = [[NSHashTable alloc] initWithOptions:NSHashTableObjectPointerPersonality | ||
| capacity:sublayouts.count];; |
There was a problem hiding this comment.
Except this double-semicolon =)
…sition to clean up. (TextureGroup#336) It is rare that this code has any effect, but I've discovered a case in which it occurs. This task tracks moving this code to a DEBUG-only assertion: TextureGroup#335
It is rare that this code has any effect, but I've discovered a case in which it occurs.
This task tracks moving this code to a DEBUG-only assertion: #335