From 4dbd69960ecd30cb3d4634a0bd874e8640ab5975 Mon Sep 17 00:00:00 2001 From: Huy Nguyen Date: Fri, 22 Mar 2019 06:40:18 -0700 Subject: [PATCH] Fix bug in ASRangeController that causes some cell nodes of a collection view which is about to becomes invisible to load their backing view/layer and render (#1418) --- Schemas/configuration.json | 3 ++- Source/ASExperimentalFeatures.h | 1 + Source/ASExperimentalFeatures.mm | 3 ++- Source/Details/ASRangeController.mm | 28 ++++++++++++++++------------ Tests/ASConfigurationTests.mm | 6 ++++-- 5 files changed, 25 insertions(+), 16 deletions(-) diff --git a/Schemas/configuration.json b/Schemas/configuration.json index 12957a12b..11aeca211 100644 --- a/Schemas/configuration.json +++ b/Schemas/configuration.json @@ -25,7 +25,8 @@ "exp_disable_a11y_cache", "exp_dispatch_apply", "exp_image_downloader_priority", - "exp_text_drawing" + "exp_text_drawing", + "exp_fix_range_controller" ] } } diff --git a/Source/ASExperimentalFeatures.h b/Source/ASExperimentalFeatures.h index c3d56eb7d..287c910bf 100644 --- a/Source/ASExperimentalFeatures.h +++ b/Source/ASExperimentalFeatures.h @@ -31,6 +31,7 @@ typedef NS_OPTIONS(NSUInteger, ASExperimentalFeatures) { ASExperimentalDispatchApply = 1 << 10, // exp_dispatch_apply ASExperimentalImageDownloaderPriority = 1 << 11, // exp_image_downloader_priority ASExperimentalTextDrawing = 1 << 12, // exp_text_drawing + ASExperimentalFixRangeController = 1 << 13, // exp_fix_range_controller ASExperimentalFeatureAll = 0xFFFFFFFF }; diff --git a/Source/ASExperimentalFeatures.mm b/Source/ASExperimentalFeatures.mm index 653db2c37..4dfe5ddde 100644 --- a/Source/ASExperimentalFeatures.mm +++ b/Source/ASExperimentalFeatures.mm @@ -24,7 +24,8 @@ @"exp_disable_a11y_cache", @"exp_dispatch_apply", @"exp_image_downloader_priority", - @"exp_text_drawing"])); + @"exp_text_drawing", + @"exp_fix_range_controller"])); if (flags == ASExperimentalFeatureAll) { return allNames; } diff --git a/Source/Details/ASRangeController.mm b/Source/Details/ASRangeController.mm index d16b281d5..5625fd59e 100644 --- a/Source/Details/ASRangeController.mm +++ b/Source/Details/ASRangeController.mm @@ -349,21 +349,25 @@ - (void)_updateVisibleNodeIndexPaths } } } else { - // If selfInterfaceState isn't visible, then visibleIndexPaths represents what /will/ be immediately visible at the - // instant we come onscreen. So, preload and display all of those things, but don't waste resources preloading yet. - // We handle this as a separate case to minimize set operations for offscreen preloading, including containsObject:. + // If selfInterfaceState isn't visible, then visibleIndexPaths represents either what /will/ be immediately visible at the + // instant we come onscreen, or what /will/ no longer be visible at the instant we come offscreen. + // So, preload and display all of those things, but don't waste resources preloading others. + // We handle this as a separate case to minimize set operations, including -containsObject:. + // + // DO NOT set Visible: even though these elements are in the visible range / "viewport", + // our overall container object is itself not yet, or no longer, visible. + // The moment it becomes visible, we will run the condition above. + + BOOL shouldUpdateInterfaceState = NO; + if (ASActivateExperimentalFeature(ASExperimentalFixRangeController)) { + shouldUpdateInterfaceState = [visibleIndexPaths containsObject:indexPath]; + } else { + shouldUpdateInterfaceState = [allCurrentIndexPaths containsObject:indexPath]; + } - if ([allCurrentIndexPaths containsObject:indexPath]) { - // DO NOT set Visible: even though these elements are in the visible range / "viewport", - // our overall container object is itself not visible yet. The moment it becomes visible, we will run the condition above - - // Set Layout, Preload + if (shouldUpdateInterfaceState) { interfaceState |= ASInterfaceStatePreload; - if (rangeMode != ASLayoutRangeModeLowMemory) { - // Add Display. - // We might be looking at an indexPath that was previously in-range, but now we need to clear it. - // In that case we'll just set it back to MeasureLayout. Only set Display | Preload if in allCurrentIndexPaths. interfaceState |= ASInterfaceStateDisplay; } } diff --git a/Tests/ASConfigurationTests.mm b/Tests/ASConfigurationTests.mm index 695292485..5738edc31 100644 --- a/Tests/ASConfigurationTests.mm +++ b/Tests/ASConfigurationTests.mm @@ -30,7 +30,8 @@ ASExperimentalDisableAccessibilityCache, ASExperimentalDispatchApply, ASExperimentalImageDownloaderPriority, - ASExperimentalTextDrawing + ASExperimentalTextDrawing, + ASExperimentalFixRangeController }; @interface ASConfigurationTests : ASTestCase @@ -55,7 +56,8 @@ + (NSArray *)names { @"exp_disable_a11y_cache", @"exp_dispatch_apply", @"exp_image_downloader_priority", - @"exp_text_drawing" + @"exp_text_drawing", + @"exp_fix_range_controller" ]; }