Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove incorrect const constraint from UIManager commit hooks #37588

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,7 @@ - (BOOL)touchesShouldCancelInContentView:(__unused UIView *)view

- (void)scrollViewDidScroll:(UIScrollView *)scrollView
{
if (!_isUserTriggeredScrolling) {
if (!_isUserTriggeredScrolling || CoreFeatures::enableGranularScrollViewStateUpdatesIOS) {
[self _updateStateWithContentOffset];
}

Expand Down
2 changes: 2 additions & 0 deletions packages/react-native/React/Fabric/RCTScheduler.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ NS_ASSUME_NONNULL_BEGIN

- (void)animationTick;

- (void)reportMount:(facebook::react::SurfaceId)surfaceId;

- (void)addEventListener:(std::shared_ptr<facebook::react::EventListener> const &)listener;

- (void)removeEventListener:(std::shared_ptr<facebook::react::EventListener> const &)listener;
Expand Down
5 changes: 5 additions & 0 deletions packages/react-native/React/Fabric/RCTScheduler.mm
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,11 @@ - (void)animationTick
_scheduler->animationTick();
}

- (void)reportMount:(facebook::react::SurfaceId)surfaceId
{
_scheduler->reportMount(surfaceId);
}

- (void)dealloc
{
if (_animationDriver) {
Expand Down
17 changes: 17 additions & 0 deletions packages/react-native/React/Fabric/RCTSurfacePresenter.mm
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,14 @@ - (RCTScheduler *)_createScheduler
CoreFeatures::disableTransactionCommit = true;
}

if (reactNativeConfig && reactNativeConfig->getBool("react_fabric:enable_granular_scroll_view_state_updates_ios")) {
CoreFeatures::enableGranularScrollViewStateUpdatesIOS = true;
}

if (reactNativeConfig && reactNativeConfig->getBool("react_fabric:enable_mount_hooks_ios")) {
CoreFeatures::enableMountHooks = true;
}

auto componentRegistryFactory =
[factory = wrapManagedObject(_mountingManager.componentViewRegistry.componentViewFactory)](
EventDispatcher::Weak const &eventDispatcher, ContextContainer::Shared const &contextContainer) {
Expand Down Expand Up @@ -438,6 +446,15 @@ - (void)mountingManager:(RCTMountingManager *)mountingManager didMountComponents
[observer didMountComponentsWithRootTag:rootTag];
}
}

RCTScheduler *scheduler = [self scheduler];
if (scheduler) {
// Notify mount when the effects are visible and prevent mount hooks to
// delay paint.
dispatch_async(dispatch_get_main_queue(), ^{
[scheduler reportMount:rootTag];
});
}
}

- (NSArray<id<RCTSurfacePresenterObserver>> *)_getObservers
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,4 +156,7 @@ public class ReactFeatureFlags {
* HostObject pattern
*/
public static boolean useNativeState = false;

/** Report mount operations from the host platform to notify mount hooks. */
public static boolean enableMountHooks = false;
}
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ public void setConstraints(

public void driveCxxAnimations();

public void reportMount(int surfaceId);

public ReadableNativeMap getInspectorDataForInstance(EventEmitterWrapper eventEmitterWrapper);

public void register(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,8 @@ public native void setConstraints(

public native void driveCxxAnimations();

public native void reportMount(int surfaceId);

public native ReadableNativeMap getInspectorDataForInstance(
EventEmitterWrapper eventEmitterWrapper);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
import android.annotation.SuppressLint;
import android.content.Context;
import android.graphics.Point;
import android.os.Handler;
import android.os.Looper;
import android.os.SystemClock;
import android.view.View;
import android.view.accessibility.AccessibilityEvent;
Expand Down Expand Up @@ -81,10 +83,13 @@
import com.facebook.react.uimanager.events.RCTEventEmitter;
import com.facebook.react.views.text.TextLayoutManager;
import com.facebook.react.views.text.TextLayoutManagerMapBuffer;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Queue;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.atomic.AtomicBoolean;

/**
* We instruct ProGuard not to strip out any fields or methods, because many of these methods are
Expand Down Expand Up @@ -166,6 +171,9 @@ public class FabricUIManager implements UIManager, LifecycleEventListener {
@NonNull
private final CopyOnWriteArrayList<UIManagerListener> mListeners = new CopyOnWriteArrayList<>();

@NonNull private final AtomicBoolean mMountNotificationScheduled = new AtomicBoolean(false);
@NonNull private final Handler mMainThreadHandler = new Handler(Looper.getMainLooper());

@ThreadConfined(UI)
@NonNull
private final DispatchUIFrameCallback mDispatchUIFrameCallback;
Expand Down Expand Up @@ -1179,17 +1187,50 @@ public Map<String, Long> getPerformanceCounters() {

private class MountItemDispatchListener implements MountItemDispatcher.ItemDispatchListener {
@Override
public void willMountItems() {
public void willMountItems(@Nullable List<MountItem> mountItems) {
for (UIManagerListener listener : mListeners) {
listener.willMountItems(FabricUIManager.this);
}
}

@Override
public void didMountItems() {
public void didMountItems(@Nullable List<MountItem> mountItems) {
for (UIManagerListener listener : mListeners) {
listener.didMountItems(FabricUIManager.this);
}

if (!ReactFeatureFlags.enableMountHooks) {
return;
}

boolean mountNotificationScheduled = mMountNotificationScheduled.getAndSet(true);
if (!mountNotificationScheduled) {
// Notify mount when the effects are visible and prevent mount hooks to
// delay paint.
mMainThreadHandler.postAtFrontOfQueue(
new Runnable() {
@Override
public void run() {
mMountNotificationScheduled.set(false);

if (mountItems == null) {
return;
}

// Collect surface IDs for all the mount items
List<Integer> surfaceIds = new ArrayList();
for (MountItem mountItem : mountItems) {
if (!surfaceIds.contains(mountItem.getSurfaceId())) {
surfaceIds.add(mountItem.getSurfaceId());
}
}

for (int surfaceId : surfaceIds) {
mBinding.reportMount(surfaceId);
}
}
});
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ private boolean dispatchMountItems() {
return false;
}

mItemDispatchListener.willMountItems();
mItemDispatchListener.willMountItems(mountItemsToDispatch);

// As an optimization, execute all ViewCommands first
// This should be:
Expand Down Expand Up @@ -301,7 +301,7 @@ private boolean dispatchMountItems() {
mBatchedExecutionTime += SystemClock.uptimeMillis() - batchedExecutionStartTime;
}

mItemDispatchListener.didMountItems();
mItemDispatchListener.didMountItems(mountItemsToDispatch);

Systrace.endSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE);

Expand Down Expand Up @@ -419,9 +419,9 @@ private static void printMountItem(MountItem mountItem, String prefix) {
}

public interface ItemDispatchListener {
void willMountItems();
void willMountItems(List<MountItem> mountItems);

void didMountItems();
void didMountItems(List<MountItem> mountItems);

void didDispatchMountItems();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,15 @@ void Binding::driveCxxAnimations() {
scheduler_->animationTick();
}

void Binding::reportMount(SurfaceId surfaceId) {
const auto &scheduler = getScheduler();
if (!scheduler) {
LOG(ERROR) << "Binding::reportMount: scheduler disappeared";
return;
}
scheduler->reportMount(surfaceId);
}

#pragma mark - Surface management

void Binding::startSurface(
Expand Down Expand Up @@ -570,6 +579,7 @@ void Binding::registerNatives() {
makeNativeMethod("setConstraints", Binding::setConstraints),
makeNativeMethod("setPixelDensity", Binding::setPixelDensity),
makeNativeMethod("driveCxxAnimations", Binding::driveCxxAnimations),
makeNativeMethod("reportMount", Binding::reportMount),
makeNativeMethod(
"uninstallFabricUIManager", Binding::uninstallFabricUIManager),
makeNativeMethod("registerSurface", Binding::registerSurface),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ class Binding : public jni::HybridClass<Binding>,
void setPixelDensity(float pointScaleFactor);

void driveCxxAnimations();
void reportMount(SurfaceId surfaceId);

void uninstallFabricUIManager();

Expand Down
1 change: 1 addition & 0 deletions packages/react-native/ReactCommon/React-Fabric.podspec
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ Pod::Spec.new do |s|
s.dependency "React-debug"
s.dependency "React-utils"
s.dependency "React-runtimescheduler"
s.dependency "React-cxxreact"

if ENV["USE_HERMES"] == nil || ENV["USE_HERMES"] == "1"
s.dependency "hermes-engine"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,7 @@ bool CoreFeatures::useNativeState = false;
bool CoreFeatures::cacheLastTextMeasurement = false;
bool CoreFeatures::cancelImageDownloadsOnRecycle = false;
bool CoreFeatures::disableTransactionCommit = false;
bool CoreFeatures::enableGranularScrollViewStateUpdatesIOS = false;
bool CoreFeatures::enableMountHooks = false;

} // namespace facebook::react
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,13 @@ class CoreFeatures {
// [CATransaction end] This feature flag disables it to measure its impact in
// production.
static bool disableTransactionCommit;

// When enabled, RCTScrollViewComponentView will trigger ShadowTree state
// updates for all changes in scroll position.
static bool enableGranularScrollViewStateUpdatesIOS;

// Report mount operations from the host platform to notify mount hooks.
static bool enableMountHooks;
};

} // namespace facebook::react
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,23 @@ namespace facebook::react {
* Describes results of layout process for particular shadow node.
*/
struct LayoutMetrics {
// Origin: relative to its parent content frame (unless using a method that
// computes it relative to other parent or the viewport)
// Origin: relative to the outer border of its parent.
// Size: includes border, padding and content.
Rect frame;
// Width of the border + padding in all directions.
// Width of the border + padding in each direction.
EdgeInsets contentInsets{0};
// Width of the border in all directions.
// Width of the border in each direction.
EdgeInsets borderWidth{0};
// See `DisplayType` for all possible options.
DisplayType displayType{DisplayType::Flex};
// See `LayoutDirection` for all possible options.
LayoutDirection layoutDirection{LayoutDirection::Undefined};
// Pixel density. Number of device pixels per density-independent pixel.
Float pointScaleFactor{1.0};
// How much the children of the node actually overflow in each direction.
// Positive values indicate that children are overflowing outside of the node.
// Negative values indicate that children are clipped inside the node
// (like when using `overflow: clip` on Web).
EdgeInsets overflowInset{};

// Origin: the outer border of the node.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,14 +52,14 @@ static LayoutableSmallVector<Rect> calculateTransformedFrames(
if (i != size) {
auto parentShadowNode =
traitCast<LayoutableShadowNode const *>(shadowNodeList.at(i));
auto contentOritinOffset = parentShadowNode->getContentOriginOffset();
auto contentOriginOffset = parentShadowNode->getContentOriginOffset();
if (Transform::isVerticalInversion(transformation)) {
contentOritinOffset.y = -contentOritinOffset.y;
contentOriginOffset.y = -contentOriginOffset.y;
}
if (Transform::isHorizontalInversion(transformation)) {
contentOritinOffset.x = -contentOritinOffset.x;
contentOriginOffset.x = -contentOriginOffset.x;
}
currentFrame.origin += contentOritinOffset;
currentFrame.origin += contentOriginOffset;
}

transformation = transformation * currentShadowNode->getTransform();
Expand Down Expand Up @@ -105,7 +105,12 @@ LayoutMetrics LayoutableShadowNode::computeRelativeLayoutMetrics(
}

auto ancestors = descendantNodeFamily.getAncestors(ancestorNode);
return computeRelativeLayoutMetrics(ancestors, policy);
}

LayoutMetrics LayoutableShadowNode::computeRelativeLayoutMetrics(
AncestorList const &ancestors,
LayoutInspectingPolicy policy) {
if (ancestors.empty()) {
// Specified nodes do not form an ancestor-descender relationship
// in the same tree. Aborting.
Expand Down Expand Up @@ -174,7 +179,7 @@ LayoutMetrics LayoutableShadowNode::computeRelativeLayoutMetrics(
resultFrame.origin = {0, 0};

// Step 3.
// Iterating on a list of nodes computing compound offset.
// Iterating on a list of nodes computing compound offset and size.
auto size = shadowNodeList.size();
for (size_t i = 0; i < size; i++) {
auto currentShadowNode =
Expand All @@ -200,19 +205,35 @@ LayoutMetrics LayoutableShadowNode::computeRelativeLayoutMetrics(

auto isRootNode = currentShadowNode->getTraits().check(
ShadowNodeTraits::Trait::RootNodeKind);

auto shouldApplyTransformation = (policy.includeTransform && !isRootNode) ||
(policy.includeViewportOffset && isRootNode);

// Move frame to the coordinate space of the current node.
resultFrame.origin += currentFrame.origin;

if (shouldApplyTransformation) {
resultFrame.size = resultFrame.size * currentShadowNode->getTransform();
currentFrame = currentFrame * currentShadowNode->getTransform();
// If a node has a transform, we need to use the center of that node as
// the origin of the transform when transforming its children (which
// affects the result of transforms like `scale` and `rotate`).
resultFrame = currentShadowNode->getTransform().applyWithCenter(
resultFrame, currentFrame.getCenter());
}

resultFrame.origin += currentFrame.origin;
if (!shouldCalculateTransformedFrames && i != 0 &&
policy.includeTransform) {
resultFrame.origin += currentShadowNode->getContentOriginOffset();
}

if (policy.enableOverflowClipping) {
auto overflowInset = currentShadowNode->getLayoutMetrics().overflowInset;
auto overflowRect = insetBy(
currentFrame * currentShadowNode->getTransform(), overflowInset);
resultFrame = Rect::intersect(resultFrame, overflowRect);
if (resultFrame.size.width == 0 && resultFrame.size.height == 0) {
return EmptyLayoutMetrics;
}
}
}

// ------------------------------
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ class LayoutableShadowNode : public ShadowNode {
struct LayoutInspectingPolicy {
bool includeTransform{true};
bool includeViewportOffset{false};
bool enableOverflowClipping{false};
};

using UnsharedList = butter::
Expand All @@ -62,6 +63,13 @@ class LayoutableShadowNode : public ShadowNode {
LayoutableShadowNode const &ancestorNode,
LayoutInspectingPolicy policy);

/*
* Computes the layout metrics of a node relative to its specified ancestors.
*/
static LayoutMetrics computeRelativeLayoutMetrics(
AncestorList const &ancestors,
LayoutInspectingPolicy policy);

/*
* Performs layout of the tree starting from this node. Usually is being
* called on the root node.
Expand Down
Loading