* 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.
* Try to get CI to work
* update to xcode that exists
* Use a more recent sim; update podfiles not to use github as source
* fix clang 10 error about 10 being extern
* don’t override deprecated method
* Fix tests and snapshot tests
* Fix references of ASViewController (non-existent) to ASDKViewController
* More references changes to old ASViewController
* Also update ASNavigationController -> ASDKNavigationController
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
It looks like github updated so that `macos-latest` is now macos-11. It can't find Xcode_11_5. Let's try to set the runs-on version explicitly and see if some magic happens.
also allow warnings for podlint because that started failing too
- The framework isn't available on tvOS. This causes CocoaPods linting to fail which prevented me from pushing the new release out.
- One way to fix this is to have a different `default_subspecs` for tvOS that doesn't have AssetsLibrary subspec, but per-platform `default_subspecs` doesn't seem to be supported by CocoaPods. So I updated the subspec itself to only depend on the framework for iOS. This means the subspec is empty/useless for tvOS (and other platforms FWIW).
- Tested with `pod spec lint Texture.podspec`.
- Fixes#1992 and part of #1549. Also unblocks 3.1.0 release.
- For the long term, we can remove the subspec entirely when iOS 9 is deprecated (#1828).
With the breaking change of renaming ASNavigationController to ASDKNavigationController, we have released a new version of Texture. Please see `ThreeMigrationGuide.md` for how to handle the breaking changes in 3.1.0.
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
GianmarcoandGianmarco
Andy Finnell
ricky
Gabriel Liévano
Carlos Compean
Koichiro Ueki
Joe Ferrucci
Garrett Moon
Mussa Charles
David Ha
Huy Nguyen
Sylvain Defresne
rqueue
Andrew Yates
rqueue
Christos Gkekas
Ted Janeczko
Zev Eisenberg
Joseph Price
Zev Eisenberg
Douglas Poveda
Ben Dolman