Skip to content

Migrate needsCustomLayoutForChildren check to the new architecture #34254

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.

Sign up for GitHub

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

Closed
wants to merge 7 commits into from
Closed

Migrate needsCustomLayoutForChildren check to the new architecture #34254

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
282a170
Tried to implement needsCustomLayoutForChildren
grahammendick Jul 23, 2022
707411c
Passed parent shadow view into update mutation
grahammendick Jul 23, 2022
ff81372
Fixed typo
grahammendick Jul 23, 2022
fb9245d
Changed to view manager wrapper
grahammendick Jul 23, 2022
6060591
Added parent shadow view for animations
grahammendick Jul 25, 2022
bd3cb7c
Added another guess for parent shadow view
grahammendick Jul 25, 2022
01b8619
Checked view manager because roots don't have one
grahammendick Jul 27, 2022
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
6 changes: 4 additions & 2 deletions ReactAndroid/src/main/java/com/facebook/react/fabric/jni/FabricMountItem.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,10 @@ CppMountItem CppMountItem::UpdatePropsMountItem(
CppMountItem CppMountItem::UpdateStateMountItem(ShadowView const &shadowView) {
return {CppMountItem::Type::UpdateState, {}, {}, shadowView, -1};
}
CppMountItem CppMountItem::UpdateLayoutMountItem(ShadowView const &shadowView) {
return {CppMountItem::Type::UpdateLayout, {}, {}, shadowView, -1};
CppMountItem CppMountItem::UpdateLayoutMountItem(
ShadowView const &shadowView,
ShadowView const &parentView) {
return {CppMountItem::Type::UpdateLayout, parentView, {}, shadowView, -1};
}
CppMountItem CppMountItem::UpdateEventEmitterMountItem(
ShadowView const &shadowView) {
Expand Down
4 changes: 3 additions & 1 deletion ReactAndroid/src/main/java/com/facebook/react/fabric/jni/FabricMountItem.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,9 @@ struct CppMountItem final {

static CppMountItem UpdateStateMountItem(ShadowView const &shadowView);

static CppMountItem UpdateLayoutMountItem(ShadowView const &shadowView);
static CppMountItem UpdateLayoutMountItem(
ShadowView const &shadowView,
ShadowView const &parentView);

static CppMountItem UpdateEventEmitterMountItem(ShadowView const &shadowView);

Expand Down
24 changes: 13 additions & 11 deletions ReactAndroid/src/main/java/com/facebook/react/fabric/jni/FabricMountingManager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ static inline int getIntBufferSizeForType(CppMountItem::Type mountItemType) {
case CppMountItem::Type::UpdatePadding:
return 5; // tag, top, left, bottom, right
case CppMountItem::Type::UpdateLayout:
return 6; // tag, x, y, w, h, DisplayType
return 7; // tag, parentTag, x, y, w, h, DisplayType
case CppMountItem::Type::UpdateOverflowInset:
return 5; // tag, left, top, right, bottom
case CppMountItem::Undefined:
Expand Down Expand Up @@ -381,7 +381,7 @@ void FabricMountingManager::executeMount(
newChildShadowView.layoutMetrics) {
cppUpdateLayoutMountItems.push_back(
CppMountItem::UpdateLayoutMountItem(
mutation.newChildShadowView));
mutation.newChildShadowView, parentShadowView));
}

// OverflowInset: This is the values indicating boundaries including
Expand Down Expand Up @@ -441,7 +441,8 @@ void FabricMountingManager::executeMount(

// Layout
cppUpdateLayoutMountItems.push_back(
CppMountItem::UpdateLayoutMountItem(newChildShadowView));
CppMountItem::UpdateLayoutMountItem(
newChildShadowView, parentShadowView));

// OverflowInset: This is the values indicating boundaries including
// children of the current view. The layout of current view may not
Expand Down Expand Up @@ -548,7 +549,7 @@ void FabricMountingManager::executeMount(
int intBufferPosition = 0;
int objBufferPosition = 0;
int prevMountItemType = -1;
jint temp[6];
jint temp[7];
for (int i = 0; i < cppCommonMountItems.size(); i++) {
const auto &mountItem = cppCommonMountItems[i];
const auto &mountItemType = mountItem.type;
Expand Down Expand Up @@ -723,13 +724,14 @@ void FabricMountingManager::executeMount(
toInt(mountItem.newChildShadowView.layoutMetrics.displayType);

temp[0] = mountItem.newChildShadowView.tag;
temp[1] = x;
temp[2] = y;
temp[3] = w;
temp[4] = h;
temp[5] = displayType;
env->SetIntArrayRegion(intBufferArray, intBufferPosition, 6, temp);
intBufferPosition += 6;
temp[1] = mountItem.parentShadowView.tag;
temp[2] = x;
temp[3] = y;
temp[4] = w;
temp[5] = h;
temp[6] = displayType;
env->SetIntArrayRegion(intBufferArray, intBufferPosition, 7, temp);
intBufferPosition += 7;
}
}
if (!cppUpdateOverflowInsetMountItems.empty()) {
Expand Down
21 changes: 17 additions & 4 deletions ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/SurfaceMountingManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import com.facebook.react.fabric.mounting.mountitems.MountItem;
import com.facebook.react.modules.core.ReactChoreographer;
import com.facebook.react.touch.JSResponderHandler;
import com.facebook.react.uimanager.IViewManagerWithChildren;
import com.facebook.react.uimanager.IllegalViewOperationException;
import com.facebook.react.uimanager.ReactOverflowViewWithInset;
import com.facebook.react.uimanager.ReactRoot;
Expand Down Expand Up @@ -967,7 +968,7 @@ public void sendAccessibilityEvent(int reactTag, int eventType) {
}

@UiThread
public void updateLayout(int reactTag, int x, int y, int width, int height, int displayType) {
public void updateLayout(int reactTag, int parentTag, int x, int y, int width, int height, int displayType) {
if (isStopped()) {
return;
}
Expand All @@ -992,9 +993,21 @@ public void updateLayout(int reactTag, int x, int y, int width, int height, int
parent.requestLayout();
}

// TODO: T31905686 Check if the parent of the view has to layout the view, or the child has
// to lay itself out. see NativeViewHierarchyManager.updateLayout
viewToUpdate.layout(x, y, x + width, y + height);
ViewState parentViewState = getViewState(parentTag);
ReactViewManagerWrapper parentViewManager = parentViewState.mViewManager;
IViewManagerWithChildren parentViewManagerWithChildren;
if (parentViewManager instanceof IViewManagerWithChildren) {
parentViewManagerWithChildren = (IViewManagerWithChildren) parentViewManager;
} else {
throw new IllegalViewOperationException(
"Trying to use view with tag "
+ parentTag
+ " as a parent, but its Manager doesn't implement IViewManagerWithChildren");
}
if (parentViewManagerWithChildren != null
&& !parentViewManagerWithChildren.needsCustomLayoutForChildren()) {
viewToUpdate.layout(x, y, x + width, y + height);
}

// displayType: 0 represents display: 'none'
int visibility = displayType == 0 ? View.INVISIBLE : View.VISIBLE;
Expand Down
3 changes: 2 additions & 1 deletion .../src/main/java/com/facebook/react/fabric/mounting/mountitems/IntBufferBatchMountItem.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,13 +149,14 @@ public void execute(@NonNull MountingManager mountingManager) {
surfaceMountingManager.updateState(mIntBuffer[i++], castToState(mObjBuffer[j++]));
} else if (type == INSTRUCTION_UPDATE_LAYOUT) {
int reactTag = mIntBuffer[i++];
int parentTag = mIntBuffer[i++];
int x = mIntBuffer[i++];
int y = mIntBuffer[i++];
int width = mIntBuffer[i++];
int height = mIntBuffer[i++];
int displayType = mIntBuffer[i++];

surfaceMountingManager.updateLayout(reactTag, x, y, width, height, displayType);
surfaceMountingManager.updateLayout(reactTag, parentTag, x, y, width, height, displayType);

} else if (type == INSTRUCTION_UPDATE_PADDING) {
surfaceMountingManager.updatePadding(
Expand Down
2 changes: 1 addition & 1 deletion ReactCommon/react/renderer/animations/LayoutAnimationDriver.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ void LayoutAnimationDriver::animationMutationsForFrame(

// Create the mutation instruction
mutationsList.emplace_back(ShadowViewMutation::UpdateMutation(
keyframe.viewPrev, mutatedShadowView));
keyframe.viewPrev, mutatedShadowView, {}));

PrintMutationInstruction("Animation Progress:", updateMutation);

Expand Down
8 changes: 4 additions & 4 deletions ReactCommon/react/renderer/animations/LayoutAnimationKeyFrameManager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1241,7 +1241,7 @@ void LayoutAnimationKeyFrameManager::queueFinalMutationsForCompletedKeyFrame(
break;
case ShadowViewMutation::Type::Update:
mutationsList.push_back(ShadowViewMutation::UpdateMutation(
prev, finalMutation.newChildShadowView));
prev, finalMutation.newChildShadowView, {}));
break;
}
if (finalMutation.newChildShadowView.tag > 0) {
Expand All @@ -1266,7 +1266,7 @@ void LayoutAnimationKeyFrameManager::queueFinalMutationsForCompletedKeyFrame(
auto mutatedShadowView =
createInterpolatedShadowView(1, keyframe.viewStart, keyframe.viewEnd);
auto generatedPenultimateMutation = ShadowViewMutation::UpdateMutation(
keyframe.viewPrev, mutatedShadowView);
keyframe.viewPrev, mutatedShadowView, {});
react_native_assert(
generatedPenultimateMutation.oldChildShadowView.tag > 0);
react_native_assert(
Expand All @@ -1277,7 +1277,7 @@ void LayoutAnimationKeyFrameManager::queueFinalMutationsForCompletedKeyFrame(
mutationsList.push_back(generatedPenultimateMutation);

auto generatedMutation = ShadowViewMutation::UpdateMutation(
mutatedShadowView, keyframe.viewEnd);
mutatedShadowView, keyframe.viewEnd, {});
react_native_assert(generatedMutation.oldChildShadowView.tag > 0);
react_native_assert(generatedMutation.newChildShadowView.tag > 0);
PrintMutationInstruction(
Expand All @@ -1286,7 +1286,7 @@ void LayoutAnimationKeyFrameManager::queueFinalMutationsForCompletedKeyFrame(
mutationsList.push_back(generatedMutation);
} else {
auto mutation = ShadowViewMutation::UpdateMutation(
keyframe.viewPrev, keyframe.viewEnd);
keyframe.viewPrev, keyframe.viewEnd, {});
PrintMutationInstruction(
logPrefix +
"Animation Complete: Queuing up Final Synthetic Mutation:",
Expand Down
8 changes: 4 additions & 4 deletions ReactCommon/react/renderer/mounting/Differentiator.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -574,7 +574,7 @@ static void updateMatchedPair(
if (oldPair.shadowView != newPair.shadowView) {
mutationContainer.updateMutations.push_back(
ShadowViewMutation::UpdateMutation(
oldPair.shadowView, newPair.shadowView));
oldPair.shadowView, newPair.shadowView, parentShadowView));
}
}
}
Expand Down Expand Up @@ -835,7 +835,7 @@ static void calculateShadowViewMutationsFlattener(
newTreeNodePair.isConcreteView && oldTreeNodePair.isConcreteView) {
mutationContainer.updateMutations.push_back(
ShadowViewMutation::UpdateMutation(
oldTreeNodePair.shadowView, newTreeNodePair.shadowView));
oldTreeNodePair.shadowView, newTreeNodePair.shadowView, node.shadowView));
}

// Update children if appropriate.
Expand Down Expand Up @@ -1161,7 +1161,7 @@ static void calculateShadowViewMutationsV2(
oldChildPair.shadowView != newChildPair.shadowView) {
mutationContainer.updateMutations.push_back(
ShadowViewMutation::UpdateMutation(
oldChildPair.shadowView, newChildPair.shadowView));
oldChildPair.shadowView, newChildPair.shadowView, parentShadowView));
}

// Recursively update tree if ShadowNode pointers are not equal
Expand Down Expand Up @@ -1699,7 +1699,7 @@ ShadowViewMutation::List calculateShadowViewMutations(

if (oldRootShadowView != newRootShadowView) {
mutations.push_back(ShadowViewMutation::UpdateMutation(
oldRootShadowView, newRootShadowView));
oldRootShadowView, newRootShadowView, {}));
}

calculateShadowViewMutationsV2(
Expand Down
5 changes: 3 additions & 2 deletions ReactCommon/react/renderer/mounting/ShadowViewMutation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,11 @@ ShadowViewMutation ShadowViewMutation::RemoveDeleteTreeMutation(

ShadowViewMutation ShadowViewMutation::UpdateMutation(
ShadowView oldChildShadowView,
ShadowView newChildShadowView) {
ShadowView newChildShadowView,
ShadowView parentShadowView) {
return {
/* .type = */ Update,
/* .parentShadowView = */ {},
/* .parentShadowView = */ std::move(parentShadowView),
/* .oldChildShadowView = */ std::move(oldChildShadowView),
/* .newChildShadowView = */ std::move(newChildShadowView),
/* .index = */ -1,
Expand Down
3 changes: 2 additions & 1 deletion ReactCommon/react/renderer/mounting/ShadowViewMutation.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@ struct ShadowViewMutation final {
*/
static ShadowViewMutation UpdateMutation(
ShadowView oldChildShadowView,
ShadowView newChildShadowView);
ShadowView newChildShadowView,
ShadowView parentShadowView);

#pragma mark - Type

Expand Down