* Fix: Transfer leadingScreensForBatching from pending state on node load
and add tests
* Address PR comments
* try to fix build
* Adding whitespace to see if builds run
* Revert prev commit to check if builds are working
---------
Co-authored-by: Gianmarco <gfolchi@pinterest.com>
* Add default image provider
## Summary
Right now image nodes hardcode logic to use either PINRemoteImage or the basic image downloader. Additionally, Texture is missing annotations for nullability and sendable for several of the image blocks and types.
Introduce ASDefaultImageDownloader as a singleton. It takes blocks to return the default image downloader and image cache. Update both image nodes to use this. By default ASDefaultImageDownloader uses the same logic the image nodes used to, so change in behavior.
## Test plan
Run the Kittens example and verify it works.
* Appears we need Xcode 16.4
* And need at least 18.5
When compiling in Xcode 26 we get the warning `variable length array folded to constant array as an extension` for the arrays in this struct:
```
typedef struct ASLayoutElementStyleExtensions {
// Values to store extensions
BOOL boolExtensions[kMaxLayoutElementBoolExtensions];
NSInteger integerExtensions[kMaxLayoutElementStateIntegerExtensions];
UIEdgeInsets edgeInsetsExtensions[kMaxLayoutElementStateEdgeInsetExtensions];
} ASLayoutElementStyleExtensions;
```
To make it happy we could make these constants `#define` or stick them in an `enum`. I chose the latter.
## Summary
For collection views, we do not currently check for batch fetching on scroll. This appears to be a bug, as `scrollViewDidScroll:` does call `_checkForBatchFetching`, and [the commit that added it states it's trying to check on each scroll](https://github.com/TextureGroup/Texture/commit/df497b82c286771658a0ef0826945c716baaa783). Unfortunately, `_checkForBatchFetching` checks for `isTracking` and `isDragging` first and returns if either are `YES`. Since we're in a scroll, they are `YES`. So the call is effectively a no-op.
This bug has been around for 9 years and I'm unsure of the performance implications of turning the batch fetching check on for each scroll tick. Therefore, put the fix behind an experiment feature flag _and_ only call it on the first scroll tick for the scroll session, instead of every scroll tick. Finally, I moved the check after the delegate call, in case the delegate has logic in it to turn on or off the batch fetching.
## Test plan
Ran the app and manually tested batching getting called or not. Also ran `build.sh tests`.
* Add experimental feature of hierarchyDisplayDidFinish being recursive
## Summary
`hierarchyDisplayDidFinish` is currently called when a node's immediate subnodes implement custom CALayer rendering. For example, ASNetworkImageNode and ASTextNode perform this sort of rendering. However, it can be useful -- and less fragile -- if `hierarchyDisplayDidFinish` is propagated further up the node hierarchy. That way, if a feature is refactored such that an extra node appears between the node implementing `hierarchyDisplayDidFinish` and the nodes doing CALayer rendering, we don't lose the callback.
## Test plan
Manually tested using a modified version of the Kittens example. Also ran `build.sh tests`.
* Revert SDK change since it's still Xcode 15
* [ASDisplayNode] Fix a crash in insertSubnode
If a node is already a subnode to a supernode, Inserting it again can lead to a crash.
Here is a simple repro of the crash:
```
ASDisplayNode *subnode = [[ASDisplayNode alloc] init];
ASDisplayNode *supernode = [[ASDisplayNode alloc] init];
[supernode addSubnode:subnode];
// Crash on next line
[supernode insertSubnode:subnode atIndex:1];
```
The issue is that all the checks around subnode array boundaries are done BEFORE `subnode` is removed from its `supernode`. If it happens that the `supernode` is self, then removing the `subnode` causes all our index checks to no longer be valid.
Here is the relevant code:
```
__instanceLock__.lock();
NSUInteger subnodesCount = _subnodes.count;
__instanceLock__.unlock();
////// Here we check our indexes
if (subnodeIndex > subnodesCount || subnodeIndex < 0) {
ASDisplayNodeFailAssert(@"Cannot insert a subnode at index %ld. Count is %ld", (long)subnodeIndex, (long)subnodesCount);
return;
}
…
///////// Here our indexes could invalidate if self subnode’s supernode
[subnode removeFromSupernode];
[oldSubnode removeFromSupernode];
__instanceLock__.lock();
if (_subnodes == nil) {
_subnodes = [[NSMutableArray alloc] init];
}
////// Here would can crash if our index is too big
[_subnodes insertObject:subnode atIndex:subnodeIndex];
_cachedSubnodes = nil;
__instanceLock__.unlock();
```
* add a separate check for this special case
There is built in protection around setting invalid bounds for `_ASDisplayLayer`. Let’s extend this to also include protecting against setting an invalid position and an invalid transform.
* Update to Xcode 15
## Summary
We want to make Texture build with recent tools like Xcode 15. Part of this is bumping the minimum supported OS to iOS 14.
I only removed the dead code (i.e. code gated on versions older than iOS 14). There are still some warnings about outdated APIs in use, but they are non-trivial changes to fix.
## Test plan
Run all the examples. Run all the unit tests and make them pass.
* Use Xcode 15.3 so we have 17.4 simulators
* Fixing an asset catalog compiler error when building against iPhone SE
* Fix a couple of file extensions for assets
A couple of PNGs were marked as JPGs
We have `ASCellNodeVisibilityEvent` events that roughly correlate to the scrollViewDid… delegate methods in UIScrollView. With the current events we get a callback when a user stops dragging a cell, but if the cell decelerates we do not get an event when it comes to a rest. I’ve added `ASCellNodeVisibilityEventDidStopScrolling` to have both `ASTableView` and `ASCollectionView` send this event to the cells in `_cellsForVisibilityUpdates` in `- (void)scrollViewDidEndDecelerating:(UIScrollView *)scrollView`.
I created unit tests to ensure that the proper events are being called for the proper scroll delegate methods.
While NSCache is thread safe, that alone does not make `rendererForAttributes` thread safe. This experiment will add a lock to `rendererForAttributes` to see how that affects performance/stability. As second experiment will forego the cache altogether to validate that we are getting some value out of having a renderer cache.
In PR 2032 we added alloc/dealloc texture initialization methods for the case where texture is automatically initializing. However, in doing so we have removed the `ASInitializeFrameworkMainThread` method that clients may already be using.
This PR brings back the `ASInitializeFrameworkMainThread` to live side by side with `ASInitializeFrameworkMainThreadOnConstructor` and `ASInitializeFrameworkMainThreadOnDestructor`. This should keep the old functionality in place for any client using `ASInitializeFrameworkMainThread` directly.
* replace destructor for after main execute
* separate ASLoadFrameworkInitializer between constructor and destructor
* AS_EXTERN -> ASDK_EXTERN
* initialSetNeedsDisplayCount should be called once on iOS 15
* Loaded node of contentsScale must be 2.0 on layer backed
* initialSetNeedsDisplayCount must called once
As of iOS15 the AuthenticationServices framework has a class named `ASNavigationController`. We need to rename our `ASNavigationController` to protect against undefined behavior.
Note: This change was based on this PR https://github.com/TextureGroup/Texture/pull/2014. We were slow in merging it and the author has not replied, so I'm making a new one to get this landed.
`flipsHorizontallyInOppositeLayoutDirection` is available in iOS11 and greater. Texture still supports iOS9 so we need to make sure not to call this it in those cases.
* [RTL/Batching] Make ASDisplayShouldFetchBatchForScrollView aware of flipped CV layouts
UICollectionViewLayout has a property called `flipsHorizontallyInOppositeLayoutDirection`. If this is set to `YES` then a RTL collectionView’s contentOffset behaves like it does in LTR. In other words, the first item is at contentOffset 0. In this case, the existing logic for `ASDisplayShouldFetchBatchForScrollView` works in RTL.
If you don’t override `flipsHorizontallyInOppositeLayoutDirection` to be `YES`, then it means that in RTL languages the first item in your collectionView will actually be at x offset `collectionView.contentSize.width - collectionView.frame.size.width`. As you scroll to the right, the content offset will decrease until you reach the end of the data at a content offset of 0,0. In this case, `ASDisplayShouldFetchBatchForScrollView` needs to know that you are in RTL and the layout is not flipped. It can then use the contentOffset as the `remainingDistance` to determine when to fetch.
* fix indentation
* assert that we are on main when accessing CV layout
* [Layout] Add RTL support to LayoutSpecs
This is largely a slight update for https://github.com/TextureGroup/Texture/pull/1805. If RTL is enabled, `calculateLayoutLayoutSpec:` will flip the origin of all sublayouts.
The new part of the diff is that ASBatchFetching now supports proper fetching on RTL horizontal scrollViews.
* Fix build and add RTL batch fetching tests
Having a semi-colon between a method parameters list and a method
body is not not correct and is usually caused by a copy and paste
error while creating the method definition from its declaration.
Fixes the following compilation warnings when building with
-Wsemicolon-before-method-body (which is part of -Wextra):
ASPINRemoteImageDownloader.mm:230:85: error: semicolon before method body is ignored [-Werror,-Wsemicolon-before-method-body]
- (id <ASImageContainerProtocol>)synchronouslyFetchedCachedImageWithURL:(NSURL *)URL;
^
ASPINRemoteImageDownloader.mm:275:76: error: semicolon before method body is ignored [-Werror,-Wsemicolon-before-method-body]
completion:(ASImageDownloaderCompletion)completion;
^
2 errors generated.
Fixes applied to both code, examples and samples in documentation.
A previous commit
(https://github.com/TextureGroup/Texture/commit/8f7444e0ece61d6ab12ecceb590be26d8d7cc99d)
aimed to fix a preloading bug for ASCollectionView. This commit expands this
fix to ASTableView as the bug occurs there too.
Previous commit message for context:
This experiment makes sure a ASCollectionView's `rangeController` updates when
a changeset WITH updates is applied. Currently it is possible for nodes
inserted into the preload range to not get preloaded when performing a batch
update.
For example, suppose a collection node has:
- Tuning parameters with a preload range of 1 screenful for the given range
mode.
- Nodes A and B where A is visible and B is off screen.
Currently if node B is deleted and a new node C is inserted in its place, node
C will not get preloaded until the collection node is scrolled. This is because
the preloading mechanism relies on a `setNeedsUpdate` call on the range
controller as part of the `-collectionView:willDisplayCell:forItemAtIndexPath:`
delegate method when the batch update is submitted. However, in the example
outlined above, this sometimes doesn't happen automtically, causing the range
update to be delayed until the next the view scrolls.
This experiment makes sure a ASCollectionView's `rangeController` updates when
a changeset WITH updates is applied. Currently it is possible for nodes
inserted into the preload range to not get preloaded when performing a batch
update.
For example, suppose a collection node has:
- Tuning parameters with a preload range of 1 screenful for the given range
mode.
- Nodes A and B where A is visible and B is off screen.
Currently if node B is deleted and a new node C is inserted in its place, node
C will not get preloaded until the collection node is scrolled. This is because
the preloading mechanism relies on a `setNeedsUpdate` call on the range
controller as part of the `-collectionView:willDisplayCell:forItemAtIndexPath:`
delegate method when the batch update is submitted. However, in the example
outlined above, this sometimes doesn't happen automtically, causing the range
update to be delayed until the next the view scrolls.
* Exposes a new option in ASImageDownloaderProtocol to retry image downloads
At the moment the ASBasicImageDownloader does not automatically retry image downloads if the remote
host is unreachable. On the contrary the ASPINRemoteImageDownloader automatically retries. Retrying is
something that ultimately clients need to be able to control, for example to fail fast to an alternative image
rather than keep retrying for more than one minute while not displaying any image. This change exposes
a new option in the ASImageDownloaderProtocol to retry image downloads. It also uses this new option
in both ASNetworkImageNode and also ASMultiplexImageNode, setting it to YES to preserve the current
behaviour.
* Fixes a failing test in ASMultiplexImageNodeTests
* Fixes ScreenNode.m
ScreenNode.m is implementing ASImageDownloaderProtocol and needs to
be fixed to reflect changes in the latter.
* Return nil instead of empty array when no accessibility elements are found. Fixes#1954.
* Use nullability annotations to fix static analyzer warnings.
* Add UI test target.
* Add UI test to make sure web view stays accessible.
* Revert "Add UI test to make sure web view stays accessible."
This reverts commit 00253f49a0af329602b0d9709b58bc92dcd90147.
* Revert "Add UI test target."
This reverts commit 288b5e0f564ef3ba3fb5568baa832bc03124cdc9.
* Add unit test to make sure accessibility elements are correct when a WKWebView is wrapped in an ASDisplayNode.
* Trying to make ASTextLinePositionModifier public
* d’oh
* be a little more restrictive on the files we pull into the pod
* Never mind, I guess we need all of these
* update the project file as well
* try this again
* I think this will work this time.
* Account for possible inverted transform during hit test
When ASCollectionNode has the `inverted` flag set, a transform gets
set on the cell node. We need to make sure that we account for that
when dealing in the view coordinates.
* Store self.node and self.node.view in local variables for better readability.
* Add a test for hit testing in an inverted ASCollectionNode
Follow up on #1911: it's not enough to execute step 3 on the main thread because -_allocateNodesFromElements: uses ASDispatchApply to offload the work to other threads. So this diff adds a flag to tell that method to do everything serially on the calling thread.
* Add an experiment that makes ASDataController to do everything on main thread
Under this experiment, ASDataController will allocate and layout all nodes on the main thread. This helps to avoid deadlocks that would otherwise occur if some of the node allocations or layouts caused ASDataController's background queue to block on main thread. As a bonus, this experiment also helps to measure how much performance wins we get from doing the work off main.
* Remove ASSERT_ON_EDITING_QUEUE
* Do not expose tgmath.h to all clients of Texture
- tgmath.h #undef the `log` macro for mathematical reasons. Code that may also use a log name (such as CocoaLumberjack) will get confused by this when they try to use `NS_SWIFT_NAME` with `log` as part of the name.
- `ABS` from NSObjCRuntime.h is what is typically used for abs on `CGFloat`.
- Note: removing tgmath.h from the Texture umbrella header may expose clients that implicitly depended upon it being imported. Sources may have to be updated after this to `#import <tgmath.h>` explicitly.
* Remove background deallocation helper code
Last use removed in Texture with #1840, now PINS no longer uses it either. Less OOMs is so nice.
* remove methods from docs
We did not notice any effect on performance of the Pinterest app by not caching `accessibilityElements` in `_ASDisplayView`. By not caching the elements, we can be sure that the elements will be correct even when nodes change visibility state. There will be a performance impact when voice over is enabled, but providing the correct elements for the current state of a view is more important than performance in this case.
https://github.com/TextureGroup/Texture/issues/1853
* Renames AS_EXTERN and ASViewController
To ASDK_EXTERN and ASDKViewController.
This is to avoid conflicting with AuthenticationServices in
Xcode 12
* Fix up examples and docs
* Add bit about updating ASViewController rename
Most of this code comes from an old PR that @fruitcoder put up https://github.com/TextureGroup/Texture/pull/795 2 years ago.
When creating our array of accessibilityElements, we need to respect the value of `accessibilityElementsHidden`. If the value of this property changes, we need to invalidate the cached accessibility elements (unless we are in the experiment that doesn’t cache `accessibilityElements`).
I created a simple test app and made sure this matched UIKit’s implementation. I also added a test case that changes the value of `accessibilityElementsHidden` and makes sure the proper accessibilityElements are returned.
* [ASDisplayNode] Implement accessibilityViewIsModal
A PR to add support for `accessibilityViewIsModal` in `CollectAccessibilityElements`.
If in a list of subnodes more than 1 subnode has `accessibilityViewIsModal` marked as `YES`, then the node with the highest index in `subnodes` will be the one that is considered modal. This behavior matches UIKit.
If the value of `accessibilityViewIsModal` changes, we need to clear all the cached `accessibilityElements` from that view up. I added this in ASDisplayNode’s `setAccessibilityViewIsModal` method. Note that if we ship `ASExperimentalDoNotCacheAccessibilityElements` then we can remove the invalidation step.
Finally, I changed all the tests to ask the view for accessibilityElements, not the node. This is a better representation of what will really happen when UIKit asks a node’s view for its accessibility elements. It also allowed me to test that clearing the accessibilityElements was working.
* add some experiment checks
* fix tests and address jon’s comment
* Fix tests
* remove debug code
- Followup to #1742
- At Pinterest this shipped with D516974 in late 02/2020
- As discussed in #858 this is iOS10 or later, so the runtime `gMutex_unfair` check is still necessary for Texture.
GianmarcoandGianmarco
Andy Finnell
ricky
Gabriel Liévano
Carlos Compean
Koichiro Ueki
Garrett Moon
Mussa Charles
David Ha
Sylvain Defresne
rqueue
rqueue
Christos Gkekas
Zev Eisenberg
Zev Eisenberg
Douglas Poveda
Huy Nguyen
Ben Dolman
Greg Bolsinga
ricky
_ivanC
_ivanC