Summary:
Rename `_header` to `header_` to align with the C++ naming scheme we use.
Rename `readKey` to `readUnsignedShort` as purpose of the method have changed.
Changelog: [Internal]
Reviewed By: javache
Differential Revision: D33637127
fbshipit-source-id: a82f4d6c1b753b21e0567fbe919af98e4c78105d
Summary:
In the case of T110060790, there is a JavaScript crash that causes RN teardown to race with a ScrollView event being emitted, which ends up being reported as the "real" cause of the crash.
This is not correct, and it's better to just ignore this case. If there's no EventDispatcher, it means something has gone wrong (usually teardown) before, RN is in an inconsistent state, and we should
not crash here or report here.
Changelog: [Android][Fixed] Fix crash from ScrollView that occurs while reporting an error from JS
Reviewed By: mdvacca
Differential Revision: D33644692
fbshipit-source-id: 41c3defde795b804239cc8401c8aff71d017d59d
Summary:
Minor optimization, but spotted this while reviewing the implementation for D33622997
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D33622996
fbshipit-source-id: 8712753803fc46e6a046d50f77454a813e4a641a
Summary:
`onPointerMove` events get dispatched with an `offset: [x, y]` attribute (API is not yet available in OSS, so subject to change), but `EventAnimationDriver` in RN Android is not able to extract values with such keys (even though the equivalent JS implementation does allow it).
(TODO: verify iOS behaviour)
Changelog:
[General][Added] - Animated.event can be used to extract values with numeric keys from native events
Reviewed By: mdvacca
Differential Revision: D32531117
fbshipit-source-id: 918a5443c5d8f5f8200d86bb67f84e8bc175c1d3
Summary:
MapBuffer uses unsigned short in C++, but Java doesn't really have a type that represents that. That means that MapBuffer would be limited to max 32768 values instead of 65536, which doesn't make much sense as a limitation.
Considering weird (and usually not performant) handling of short values in Java in general, this change replaces them with ints, converting keys from short when needed with `key & 0xFFFF`.
Changelog: [Internal]
Reviewed By: javache
Differential Revision: D33595308
fbshipit-source-id: a7adde8a207bb4aa1d81d367ab5d7b41ace2e291
Summary:
Adds a check to verify that we are not trying to insert a new view id into a non-existent set.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D33621031
fbshipit-source-id: 8468af69bea250a70d656789ea819c39b55a9de6
Summary:
Cleans up `ReadableMapBuffer` APIs and migrates to use `std::vector` instead of raw pointer array.
Also uses `fbjni` utility to allocate the bytes instead of making manual JNI calls.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D33513027
fbshipit-source-id: afe7d320d12830503de4c9994117d849f0b25245
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/32848
If we leverage the side-by-side configuration of the NDK
(see https://developer.android.com/studio/projects/configure-agp-ndk#agp_version_41)
we will not have to specify the NDK Path or Version at all.
We will automatically pick the best NDK version selected by AGP.
Changelog:
[Android] [Changed] - Use side-by-side NDK for Android
Reviewed By: ShikaSD
Differential Revision: D33475818
fbshipit-source-id: 16aa4acfc44b94e2f92df89d71e104bf46d7f162
Summary:
`WritableNativeMap` has two constructors:
```c++
WritableNativeMap();
WritableNativeMap(folly::dynamic &&val);
```
but `WritableNativeMap` has only one constructor:
```c++
WritableNativeMap();
```
Without a second constructor, I am not able to create `WritableNativeMap` from the existing array. I added this second constructor because I need it for `react-native-reanimated`.
## Changelog
[Android] [Added] - Added missing constructor to WritableNativeArray
Pull Request resolved: https://github.com/facebook/react-native/pull/32796
Test Plan:
```c++
std::shared_ptr<jsi::Runtime> rt = facebook::hermes::makeHermesRuntime();
jsi::Value value;
WritableNativeMap::newObjectCxxArgs(jsi::dynamicFromValue(*rt.get(), value));
```
Reviewed By: cortinico
Differential Revision: D33285316
Pulled By: lunaleaps
fbshipit-source-id: 1bbd816892f34ae6fef747066893fe3b803c088d
Summary:
Adds an experimental replacement for preallocation revision checks on Android mounting layer. Instead of checking revisions of props, we remember tags of preallocated/previously allocated views and use this as a filter to avoid issuing extra create and preallocate commands on copy.
This method is a bit more expensive on memory, but is more precise than relying on revisions.
Changelog: [Internal]
Reviewed By: sammy-SC, mdvacca
Differential Revision: D33038768
fbshipit-source-id: fc98c2a7912e162a3845ca80488635bffaf8cc7f
Summary:
Quick refactor of ReactRootView where I extract 'Fabric' check into a method
changelog: [internal] internal
Reviewed By: fkgozali
Differential Revision: D33269025
fbshipit-source-id: 2647d5870f3b5fcc5a940a5924a39ca43b5bebac
Summary:
This diff gates the useage of overflowinset to hit test algorithm to fabric renderer only.
Changelog:
[Internal][Android] - Use overflowInset for fabric only
Reviewed By: JoshuaGross, mdvacca
Differential Revision: D33237281
fbshipit-source-id: e5cd78ee97f62f100d42d016241d1544fb0953ad
Summary:
Renaming the `better` utilities to `butter`:
- to prevent claims that this library is superior to others - it really depends on use cases
- to indicate ease of use throughout the codebase, easily spread like butter
Changelog: [C++][Changed] Renaming C++ better util to butter, used by Fabric internals
Reviewed By: JoshuaGross
Differential Revision: D33242764
fbshipit-source-id: 26dc95d9597c61ce8e66708e44ed545e0fc5cff5
Summary:
The previous diff shows how `overflowInset` could improve hit test algorithm performance. This diff adds experiment to it in order to:
1. Understand the perf improvement in production
2. Provide quick way to rollback in production -- the changes are used by FB4A as well as VR
3. The changes will pass more instructions over JNI, which may have impact on perf.
To share the MC param values in both Java and C++ side, the value is hosted by `ReactFeatureFlags` and fetched in `FbReactInstanceHolder`.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D33179310
fbshipit-source-id: 327100d41f7b5a668ff0d2afabcdd1fc16cb5a18
Summary:
This diff adds `overflowInset` values in RN Android. These values are used to give an enlarged boundary of a view group that also contains all its children layout. Here is [the post](https://fb.workplace.com/groups/yogalayout/permalink/2264980363573947/) that discuss more on why this is useful. I steal the pic in that post here as TLDR:
{F687030994}
In the above case, we will get overflowInset for ViewGroup A as something like `top: 0, right: -20, bottom: -20, left: 0`.
This has been added in the [Fabric core](https://fburl.com/code/f8c5tg7b) and [in IOS](https://fburl.com/code/vkh0hpt6). In Android, since we used to ignore all event coordinates outside of a ViewGroup boundary, this is not an issue. However, that caused unregistered touch area problem and got fixed in D30104853 (https://github.com/facebook/react-native/commit/e35a963bfb93bbbdd92f4dd74d14e2ad6df5e14a), which dropped the boundary check and made the hit test algorithm in [TouchTargetHelper.java](https://fburl.com/code/dj8jiz22) worse as we now need to explore all the child node under ReactRootNode.
This perf issue is getting obvious when a view loads too many items, which matches our experience with "Hover getting slow after scrolling", "Hover getting slow after going back from PDP view", and "The saved list view (in Explore) is very fast (because it has very few components)"
To fix this issue, I added the support to `overflowInset` to RN Android by
1. Sending the `overflowInset` values from Binding.cpp in RN Android as a separate mount instruction
2. Update `IntBufferBatchMountItem.java` to read the int buffer sent over JNI, and pass the `overflowInset` values to `SurfaceMountingManager.java`
3. Creating new interface `ReactOverflowViewWithInset.java` and extending the existing `ReactOverflowView.java` usages
4. Adding implementation of getter and setter for `overflowInset` in various views
5. Update `TouchTargetHelper.java` to read the values and check boundaries before exploring ViewGroup's children
Note that in #3 I didn't change `ReactOverflowView.java` interface directly. I am concerned about backward compatibility issues in case this interface is being used in OSS. I suggest we deprecate it as we are not using it anymore in our code.
Changelog:
[Internal][Android]
Reviewed By: JoshuaGross
Differential Revision: D33133977
fbshipit-source-id: 64e3e837fe7ca6e6dbdbc836ab0615182e10f28c
Summary:
Binding.cpp shares many responsibilities, including surface management and mount. This change refactors mount (and mount items) into separate files, while retaining the same functionality and interface.
Changelog:
[Internal][Android] - Refactor mount into FabricMountingManager
Reviewed By: mdvacca
Differential Revision: D32977902
fbshipit-source-id: 07cdf5a19033fccaa81901e5b9a4d44192ec7853
Summary: Changelog: [Internal] - Update the source of the changes in generated files, no longer bump-oss-version but set-rn-version
Reviewed By: sota000
Differential Revision: D33110408
fbshipit-source-id: 8cd5004f5d40dde82fe4d6271d5b8598cd27ca31
Summary:
Inits RootView to display size for the early surface start experiment.
Before that experiment, we always had the view measured before the surface was initialized, but here layout can sometimes happen before measure. Surface defaults use [0; Inf) for size constraints, potentially causing a crash in Yoga. This change avoids that by making a guess on default layout size with `displayMetrics`
Changelog: [Internal]
Reviewed By: feedthejim
Differential Revision: D33190397
fbshipit-source-id: 3b1b84135a4980ef2fde4024ec84a448199e00b8
Summary:
This diff adds extra logging to track how long does it take to exeucte different stages of the fabric commit
changelog: [internal] internal
Reviewed By: philIip
Differential Revision: D33183070
fbshipit-source-id: 69e31bef69c9d9ec57dc7e00e4c36e278eb30cb6
Summary:
Just released a new version that supports Android App Bundle correctly
https://github.com/facebook/SoLoader/releases/tag/v0.10.3
Reviewed By: passy
Differential Revision: D33167010
fbshipit-source-id: 69c54b1674b06b05a01a468d9f324475e5c232cf
Summary:
Similar to the previous diff, we should not allow view group that has clipChildren set to true to respond events that are out of bounds.
Changelog:
[Internal][Android]
Reviewed By: ShikaSD
Differential Revision: D33102331
fbshipit-source-id: de3a5ffdd5293ada1d2c211659e79edc697b5d15
Summary:
In D30104853 (https://github.com/facebook/react-native/commit/e35a963bfb93bbbdd92f4dd74d14e2ad6df5e14a), we added fix to the issue where touches on the child view that is outside of its parent view's boundary are not registered. The only exception to that is if the parent view has overflow style `overflow: hidden`. In that case, touches happen outside of the parent view should be ignored.
{F686521911}
That fix works, but it increases the complexity for the DFS algorithm used to find the touch target in two ways:
1. Before we only traverse views that contain the touch event point. If the touch event point is outside of the view, we won't step in and traverse that part of the tree. This is actually what caused the initial problem. The fix removed that boundary check (where `isTransformedTouchPointInView` used to do) and push it later. This increases the number of tree traversal a lot.
2. The check for `overflow: hidden` is happened after we find a potential target view, which means we've spent time to find the target view before we decide if the target view is even a candidate.
This diff aims to update for the #2 item above. Since we are checking the style of the parent view, not the target view, it's not necessary to check that after we find the target view. We could check the parent view and if it has `overflow: hidden` on it, any pointer event that are not in the boundary can be ignored already.
Changelog:
[Internal][Android]
Reviewed By: mdvacca, ShikaSD
Differential Revision: D33079157
fbshipit-source-id: c79c2b38b8affb9ea0fd25b5e880b22466ab7ed9
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/32770
Fest is EOL and is replaced by assertj, originally a fork of Fest. This change replaces the usages in internal tests and removes the dependency.
Changelog:
[Internal][Android] Replace fest with assertj
Reviewed By: genkikondo
Differential Revision: D33143454
fbshipit-source-id: c1cbe5b064d007018f73858b2913806c11aa08af
Summary:
Scheduler and JavaUIManager mutexes share the lifetime (between install-uninstall), so we can use singular shared lock to ensure that both sections are locked exclusively for install/uninstall and in shared mode for access.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D33130686
fbshipit-source-id: bf283fd5410e386d2635645e17680a9f4cb7f288
Summary:
Adds a return value to `MessageQueueThread#runOnQueue`, it's implementors and one caller.
It is currently possible for a callback to not be called (because the HandlerThread has been shutdown). If the callback is mission-critical (frees resources, releases a lock, etc) the callee has no indication that it needs to do something.
This surfaces that information to the callee.
Changelog:
[Android][Changed] - Made `MessageQueueThread#runOnQueue` return a boolean. Made `MessageQueueThreadImpl#runOnQueue` return false when the runnable is not submitted.
Reviewed By: RSNara
Differential Revision: D33075516
fbshipit-source-id: bd214a39ae066c0e7d413f2eaaaa05bd072ead2a
Summary:
Fabric uses a cache where it stores the result of the text measurement in C++ (to avoid unnecessary text measurement that are very costly). This cache has a "max size" of 256 and this size is not enough to store all the texts we have in the screen
In my tests, the amount of texts being measured are ~290 and after scrolling many times they increase to 611.
This diff increases the size of the TextMeasure to 1024 for users in the experiment. As a result this improves performance of HoverEvents by +5x times (see test plan)
changelog: [internal] internal
Reviewed By: JoshuaGross
Differential Revision: D33112788
fbshipit-source-id: e15feecf0f54da62b252892d37a64fb4ead29e22
Summary:
With the updates to touch processing rolled out, we can remove the feature flag and clean up the old code. The old path is now used exclusively by legacy renderer and Fabric uses new `EventEmitter#receiveTouches` method to process touches.
Changelog:
[Changed][Android] - Update touch processing internals
Reviewed By: mdvacca
Differential Revision: D32953664
fbshipit-source-id: 517a4ce6ce9bc15528c2db94d7d11bdff8b78743
Summary:
Applies suggestions from clang-tidy. The automatic linter seems to be broken atm, so I used default config from Android Studio, filtering out meaningless suggestions.
Changelog: [Internal]
Reviewed By: cortinico, sammy-SC
Differential Revision: D32994138
fbshipit-source-id: e8edf53f0cb8b0eda4ff8bb8bf8f5196827efd68
Summary:
Follow up to https://github.com/facebook/react-native/issues/32683 to also link hermes-inspector statically.
## Changelog
[Android] [Fix] - Static link for hermes-inspector
Pull Request resolved: https://github.com/facebook/react-native/pull/32694
Test Plan: Tested a clean build and made sure hermes-inspector.so doesn't exist anymore.
Reviewed By: cortinico
Differential Revision: D32791208
Pulled By: ShikaSD
fbshipit-source-id: 6076b263469b9e2c6176d488d13292d4f1731dcc
Summary:
The IDE warning suggests that passing folly::dynamic by value will create a copy on each call.
Changelog: [Internal]
Reviewed By: JoshuaGross
Differential Revision: D32978154
fbshipit-source-id: a47a60c332a9d299eb2110d3537dfab0bc2398b6
Summary:
Originally introduced in D2022018
Tried to make the processor optional when no rounding is required, but found even that was not strictly necessary.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D32675492
fbshipit-source-id: 8dfdbf0e4347155045f77b1fba00a59086fe7a33
Summary:
This diff add custom prediction for fling distance support. This is needed for customize fling animator to calculate predicted fling distance, instead of using the overscroller that may not be used by the animator.
More context on this -- when fling happens, our code will first predict the final fling position `p`, apply the snapping logic to decide the expected snapping position `pSnapping` given `p`, scroll velocity and children layout, then trigger the overscroller (existing) or custom fling animator to finish the fling.
Currently, the prediction logic is done with overscroller, and custom fling animator has no control over how the predicted fling distance should be. Changes in this diff allow the animator to override `getExtrapolatedDistance` method and provide that information.
Changelog:
[Android][Added] - Add new API for custom fling animator to provide predicted travel distance for its fling animation.
Reviewed By: mdvacca
Differential Revision: D32571734
fbshipit-source-id: d34b969206f8b6cb5c68d2f50a18749bfebbc97e
Summary:
This diff refactors method `predictFinalScrollPosition` in `ReactScrollView` and `ReactHorizontalScrollView` to the helper class. This will make future changes to the prediction logic easier.
Changelog:
[Internal]
Reviewed By: javache
Differential Revision: D32571735
fbshipit-source-id: 7e7e21ac51f929a017cd43de094ed39478fe4032
Summary:
This diff fixes an edge case where scroll to the end of the list may trigger a bounce back effect.
This issue is a regression from D32487846 (https://github.com/facebook/react-native/commit/f70018b37532622f08f20b2c51cdbfca55d730ea) (See [this comment](https://www.internalfb.com/diff/D32487846 (https://github.com/facebook/react-native/commit/f70018b37532622f08f20b2c51cdbfca55d730ea)?dst_version_fbid=263960175698224&transaction_fbid=566201141113715)) that zero velocity fling at the end of the scroll view makes the next fling animator use previous post animation position. This is due to cached `postAnimationValue` is applied mistakenly.
- Pass velocity instead of velocity sign to the helper class
- Update helper class logic to decide if we need to use post animated value from last fling animation
Changelog:
[Internal]
Reviewed By: javache
Differential Revision: D32566010
fbshipit-source-id: 1c61659030151f8f2c7648ca901b8b4158835538
Summary:
This diff fixes an edge case where fling and snap failed to find the correct target position when children views not fill up all the scrollable space. In this case, the target position would be calculated as the end of the scrollable space, which case the snap logic to go to the end of the scrollable area, instead of stop at the expected snapping position.
Changelog:
[Android][Fixed] - Fix fling and snap with recycler viewgroup where fling to the end of scrollable distance when it goes over current rendered children views.
Reviewed By: mdvacca
Differential Revision: D32565459
fbshipit-source-id: 319ef6e2d4e1c4deb9e45ed02c1bff7d807575c3
Summary:
changelog: [internal]
Background executor has been shipped on both platforms for a long time.
I've kept the flag around because I wanted to run tests and compare Concurrent Mode vs Background Executor. The intention was to see if we can get rid of Background Executor to simplify the threading model.
Since then, React team has moved away from Concurrent Mode towards more gradual rollout of concurrent rendering and it no longer makes sense to do this comparison. Right now, we don't have a concern with concurrent rendering and Background Executor. If we ever want to run the an experiment, this gating will need to be added again.
Reviewed By: javache
Differential Revision: D32674798
fbshipit-source-id: a1e51c9c5b8e48efa4cb0f25379d58e7eb80ccd9
Summary:
I've noticed that the `performance.now()` implementations differ on iOS and Android.
iOS:
```objc
PerformanceNow iosPerformanceNowBinder = []() {
// CACurrentMediaTime() returns the current absolute time, in seconds
return CACurrentMediaTime() * 1000;
};
```
Android:
```c++
double reactAndroidNativePerformanceNowHook() {
auto time = std::chrono::steady_clock::now();
auto duration = std::chrono::duration_cast<std::chrono::nanoseconds>(
time.time_since_epoch())
.count();
constexpr double NANOSECONDS_IN_MILLISECOND = 1000000.0;
return duration / NANOSECONDS_IN_MILLISECOND;
}
```
For consistency, I thought why not just use the same implementation on both iOS and Android.
It also seems more logical to use Chrono on iOS, since it has nanosecond precision and we just multiply it to milliseconds, whereas `CACurrentMediaTime` multiplies to seconds, and we divide it down to milliseconds again.
## Changelog
(internal change only)
Pull Request resolved: https://github.com/facebook/react-native/pull/32695
Test Plan:
Run on iOS and Android:
```ts
const now = global.performance.now()
console.log(`${Platform.OS}: ${now}`)
```
Reviewed By: feedthejim
Differential Revision: D32793838
Pulled By: ShikaSD
fbshipit-source-id: e7967780be95956a75a3a3757311af0077976d23
Summary:
The helper class for ScrollView should not need to detect the scroll direction. If it has to do so, that means the corresponding logic should live in the `ReactScrollView` or `ReactHorizontalScrollView` respectively.
This diff is to remove the scroll direction detection logic from the scroll view helper. Long term we should keep it this way as the shared code got moved to the helper class from the scroll view classes.
Changelog:
[Internal]
Reviewed By: javache
Differential Revision: D32514429
fbshipit-source-id: 2165f2eba90cc25d14834c39148fe8ce8805bea6
Andrei Shikov
Joshua Gross
Pieter De Baets
Ramanpreet Nara
Nicola Corti
grgr-dkrk
Lulu Wu
Mengke Ding
Krzysztof Piaskowy
Andres Suarez
David Vacca
Xin Chen
Kevin Gozali
Luna Wei
Sim Sun
Marshall Mann-Wood
Janic Duplessis
Genki Kondo
zpd106
Samuel Susla
Marc Rousavy