Skip to content

Commit

Permalink
[ASDisplayNode] Force a layout pass on a visible node as soon as it e…
Browse files Browse the repository at this point in the history
…nters preload state (#779)

This reverts commit 2e98588 (#751).

The reason we can't wait for the coming CA's layout pass is that cell node visibility events occur before the pass, at which time the cell's subnodes don't have correct frames for impression tracking.

The root cause of this problem is that, right now, cell node visible states are set by ASRangeController well before the layout pass of the hosting collection/table view. That means we're "jumping the gun". The more I think about this, the more I agree with @Adlai-Holler that we need to treat visible state differently. That is, a node should only be visible (and thus get visibility events) after it's fully loaded, it's view/layer attached to the hierarchy and laid out by a CA transaction. In other words, at the end of the CA layout pass. Such change needs time and effort to be thoroughly reviewed and tested. Until then, let's roll with this fix.
  • Loading branch information
nguyenhuy committed Jan 31, 2018
1 parent d697198 commit ea54727
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 10 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
- [ASScrollNode] Ensure the node respects the given size range while calculating its layout. [#637](https://github.com/TextureGroup/Texture/pull/637) [Huy Nguyen](https://github.com/nguyenhuy)
- [ASScrollNode] Invalidate the node's calculated layout if its scrollable directions changed. Also add unit tests for the class. [#637](https://github.com/TextureGroup/Texture/pull/637) [Huy Nguyen](https://github.com/nguyenhuy)
- Add new unit testing to the layout engine. [Adlai Holler](https://github.com/Adlai-Holler) [#424](https://github.com/TextureGroup/Texture/pull/424)
- [Automatic Subnode Management] Nodes with ASM enabled now insert/delete their subnodes as soon as they enter preload state, so subnodes can start preloading right away. [Huy Nguyen](https://github.com/nguyenhuy) [#706](https://github.com/TextureGroup/Texture/pull/706) [#751](https://github.com/TextureGroup/Texture/pull/751)
- [Automatic Subnode Management] Nodes with ASM enabled now insert/delete their subnodes as soon as they enter preload state, so subnodes can start preloading right away. [Huy Nguyen](https://github.com/nguyenhuy) [#706](https://github.com/TextureGroup/Texture/pull/706)
- [ASCollectionNode] Added support for interactive item movement. [Adlai Holler](https://github.com/Adlai-Holler)
- Added an experimental "no-copy" rendering API. See ASGraphicsContext.h for info. [Adlai Holler](https://github.com/Adlai-Holler)
- Dropped support for iOS 8. [Adlai Holler](https://github.com/Adlai-Holler)
Expand Down
10 changes: 1 addition & 9 deletions Source/ASDisplayNode.mm
Original file line number Diff line number Diff line change
Expand Up @@ -3086,15 +3086,7 @@ - (void)didEnterPreloadState
// - If it doesn't have a calculated or pending layout that fits its current bounds, a measurement pass will occur
// (see -__layout and -_u_measureNodeWithBoundsIfNecessary:). This scenario is uncommon,
// and running a measurement pass here is a fine trade-off because preloading any time after this point would be late.
//
// Don't force a layout pass if the node is already visible. Soon CoreAnimation will trigger
// a (coalesced, thus more efficient) pass on the backing store. Rely on it instead.
BOOL shouldForceLayoutPass = NO;
{
ASDN::MutexLocker l(__instanceLock__);
shouldForceLayoutPass = _automaticallyManagesSubnodes && !ASInterfaceStateIncludesVisible(_interfaceState);
}
if (shouldForceLayoutPass) {
if (self.automaticallyManagesSubnodes) {
[self layoutIfNeeded];
}

Expand Down

0 comments on commit ea54727

Please sign in to comment.