From 6ab3f9118ea3e4bb3f941a11bdc37a153fc7aa0a Mon Sep 17 00:00:00 2001 From: Huy Nguyen Date: Tue, 16 Jan 2018 19:03:58 +0000 Subject: [PATCH 1/5] Don't force a layout pass on a visible node that enters preload state --- CHANGELOG.md | 2 +- Source/ASDisplayNode.mm | 27 +++++++++++++++++---------- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 87165dc7e..52c3592ee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,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 the subnodes can preload too. [Huy Nguyen](https://github.com/nguyenhuy) [#706](https://github.com/TextureGroup/Texture/pull/706) +- [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) diff --git a/Source/ASDisplayNode.mm b/Source/ASDisplayNode.mm index 0f2bf7719..4b2dae301 100644 --- a/Source/ASDisplayNode.mm +++ b/Source/ASDisplayNode.mm @@ -3079,16 +3079,23 @@ - (void)didEnterPreloadState ASDisplayNodeAssertMainThread(); ASDisplayNodeAssertLockUnownedByCurrentThread(__instanceLock__); - if (self.automaticallyManagesSubnodes) { - // Tell the node to apply its applicable pending layout, if any, so that its subnodes are inserted/deleted - // and start preloading right away. - // - // If this node has an up-to-date layout (and subnodes), calling layoutIfNeeded will be fast. - // - // If this node 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 should be uncommon, and running a measurement pass here is a fine trade-off because preloading - // any time after this point would be late. + // Force a layout pass on the node to apply its applicable pending layout, if any, so that its subnodes are inserted/deleted + // and start preloading right away. + // + // - If this node has an up-to-date layout (and subnodes), calling layoutIfNeeded will be fast. + // + // - If this node 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 should be 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 if the node is already visible. CoreAnimation will soon trigger a (coaloesced) layout pass + // on the backing store, which is more efficient. + BOOL shouldForceLayoutPass = NO; + { + ASDN::MutexLocker l(__instanceLock__); + shouldForceLayoutPass = _automaticallyManagesSubnodes && !ASInterfaceStateIncludesVisible(_interfaceState); + } + if (shouldForceLayoutPass) { [self layoutIfNeeded]; } From 4cce654cdfbf21cdcd169857440e867fecec9000 Mon Sep 17 00:00:00 2001 From: Huy Nguyen Date: Tue, 16 Jan 2018 19:08:25 +0000 Subject: [PATCH 2/5] Update CHANGELOG --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 52c3592ee..1f6ea8c3a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,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) +- [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) - [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) From db9d5a21d08bd8855fe127809364d9c5333bb93b Mon Sep 17 00:00:00 2001 From: Huy Nguyen Date: Tue, 16 Jan 2018 19:10:19 +0000 Subject: [PATCH 3/5] Minor change --- Source/ASDisplayNode.mm | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Source/ASDisplayNode.mm b/Source/ASDisplayNode.mm index 4b2dae301..6a1f7df0a 100644 --- a/Source/ASDisplayNode.mm +++ b/Source/ASDisplayNode.mm @@ -3079,12 +3079,12 @@ - (void)didEnterPreloadState ASDisplayNodeAssertMainThread(); ASDisplayNodeAssertLockUnownedByCurrentThread(__instanceLock__); - // Force a layout pass on the node to apply its applicable pending layout, if any, so that its subnodes are inserted/deleted + // Force a layout pass on this node to apply its applicable pending layout, if any, so that its subnodes are inserted/deleted // and start preloading right away. // - // - If this node has an up-to-date layout (and subnodes), calling layoutIfNeeded will be fast. + // - If the node has an up-to-date layout (and subnodes), calling layoutIfNeeded will be fast. // - // - If this node doesn't have a calculated or pending layout that fits its current bounds, a measurement pass will occur + // - If the node 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 should be uncommon, and running a measurement // pass here is a fine trade-off because preloading any time after this point would be late. // From 55bacac5c046511d04bfc0655c678b8535ce6fed Mon Sep 17 00:00:00 2001 From: Huy Nguyen Date: Tue, 16 Jan 2018 19:16:45 +0000 Subject: [PATCH 4/5] Improve comment --- Source/ASDisplayNode.mm | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/Source/ASDisplayNode.mm b/Source/ASDisplayNode.mm index 6a1f7df0a..01361cc5a 100644 --- a/Source/ASDisplayNode.mm +++ b/Source/ASDisplayNode.mm @@ -3079,17 +3079,17 @@ - (void)didEnterPreloadState ASDisplayNodeAssertMainThread(); ASDisplayNodeAssertLockUnownedByCurrentThread(__instanceLock__); - // Force a layout pass on this node to apply its applicable pending layout, if any, so that its subnodes are inserted/deleted - // and start preloading right away. + // If this node has ASM enabled and is not yet visible, force a layout pass to apply its applicable pending layout, if any, + // so that its subnodes are inserted/deleted and start preloading right away. // - // - If the node has an up-to-date layout (and subnodes), calling layoutIfNeeded will be fast. + // - If it has an up-to-date layout (and subnodes), calling -layoutIfNeeded will be fast. // - // - If the node 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 should be uncommon, and running a measurement - // pass here is a fine trade-off because preloading any time after this point would be late. + // - 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 if the node is already visible. CoreAnimation will soon trigger a (coaloesced) layout pass - // on the backing store, which is more efficient. + // Don't force a layout pass if the node is already visible. Soon CoreAnimation will trigger + // a (coaloesced, thus more efficient) pass on the backing store, so rely on it instead. BOOL shouldForceLayoutPass = NO; { ASDN::MutexLocker l(__instanceLock__); From 72d9033e401798c6a2d97b545da4948aa107fe24 Mon Sep 17 00:00:00 2001 From: Huy Nguyen Date: Tue, 16 Jan 2018 19:20:46 +0000 Subject: [PATCH 5/5] Last change to the comment --- Source/ASDisplayNode.mm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/ASDisplayNode.mm b/Source/ASDisplayNode.mm index 01361cc5a..899331ea4 100644 --- a/Source/ASDisplayNode.mm +++ b/Source/ASDisplayNode.mm @@ -3089,7 +3089,7 @@ - (void)didEnterPreloadState // 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 (coaloesced, thus more efficient) pass on the backing store, so rely on it instead. + // a (coalesced, thus more efficient) pass on the backing store. Rely on it instead. BOOL shouldForceLayoutPass = NO; { ASDN::MutexLocker l(__instanceLock__);