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.

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

Jump to bottom

Fallback to Shadow Tree measurement if native measure fails #49

Open
wants to merge 11 commits into
base: 0.72.3-discord-1
Choose a base branch
from
+181 −83
Open

Fallback to Shadow Tree measurement if native measure fails #49

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
4e7fe95
WIP fallback to shadow tree measurement if node tag not found
Dec 8, 2023
60fda14
fix type issues, c++ paradigms
Dec 12, 2023
d1b44dc
fix type error with dereference
Dec 12, 2023
7513657
no type issues, untested
Dec 13, 2023
7fe78d0
app still crashes when you tap anything but it starts
Dec 14, 2023
de30705
two separate callbacks, pass successfunction back
Dec 22, 2023
cbd01c0
formatting and readability
Dec 22, 2023
3bbd06d
unneeded #includes
Dec 22, 2023
0275515
revert overzealous formatting
Dec 22, 2023
6a7d884
no need for checking boolean to return, just return
Jan 9, 2024
9759572
more verbose comment
Jan 9, 2024
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
5 changes: 3 additions & 2 deletions Libraries/Animated/NativeFabricMeasurerTurboModule.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,11 @@ type MeasureInWindowOnSuccessCallback = (
) => void;

export interface Spec extends TurboModule {
+measureNatively: (viewTag: number, callback: MeasureOnSuccessCallback) => void,
+measureNatively: (viewTag: number, successCallback: MeasureOnSuccessCallback, failCallback: (successCallback: MeasureOnSuccessCallback) => void) => void,
+measureInWindowNatively: (
viewTag: number,
callback: MeasureInWindowOnSuccessCallback,
successCallback: MeasureInWindowOnSuccessCallback,
failCallback: (successCallback: MeasureInWindowOnSuccessCallback) => void
) => void,
}

Expand Down
40 changes: 26 additions & 14 deletions ReactAndroid/src/main/java/com/facebook/react/animated/NativeFabricMeasurerModule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import com.facebook.react.bridge.UiThreadUtil;
import com.facebook.react.bridge.WritableNativeMap;
import com.facebook.react.module.annotations.ReactModule;
import com.facebook.react.uimanager.IllegalViewOperationException;
import com.facebook.react.uimanager.NativeViewMeasurer;
import com.facebook.react.uimanager.PixelUtil;
import com.facebook.react.uimanager.UIManagerHelper;
Expand All @@ -24,26 +25,37 @@ public NativeFabricMeasurerModule(ReactApplicationContext reactContext) {
}

@Override
public void measureNatively(double viewTag, Callback callback) {
public void measureNatively(double viewTag, Callback onSuccess, Callback onFail) {
getReactApplicationContext().runOnUiQueueThread(() -> {
int[] output = measurer.measure((int) viewTag);
float x = PixelUtil.toDIPFromPixel(output[0]);
float y = PixelUtil.toDIPFromPixel(output[1]);
float width = PixelUtil.toDIPFromPixel(output[2]);
float height = PixelUtil.toDIPFromPixel(output[3]);
callback.invoke(0, 0, width, height, x, y);
try {
int[] output = measurer.measure((int) viewTag);
float x = PixelUtil.toDIPFromPixel(output[0]);
float y = PixelUtil.toDIPFromPixel(output[1]);
float width = PixelUtil.toDIPFromPixel(output[2]);
float height = PixelUtil.toDIPFromPixel(output[3]);
onSuccess.invoke(0, 0, width, height, x, y);
}
catch(IllegalViewOperationException e) {
// To avoid a scoping bug in UIManagerBinding.cpp, we need to pass the successCallback back here.
onFail.invoke(onSuccess);
}
});
}

@Override
public void measureInWindowNatively(double viewTag, Callback callback) {
public void measureInWindowNatively(double viewTag, Callback onSuccess, Callback onFail) {
getReactApplicationContext().runOnUiQueueThread(() -> {
int[] output = measurer.measureInWindow((int) viewTag);
float x = PixelUtil.toDIPFromPixel(output[0]);
float y = PixelUtil.toDIPFromPixel(output[1]);
float width = PixelUtil.toDIPFromPixel(output[2]);
float height = PixelUtil.toDIPFromPixel(output[3]);
callback.invoke(x, y, width, height);
try {
int[] output = measurer.measureInWindow((int) viewTag);
float x = PixelUtil.toDIPFromPixel(output[0]);
float y = PixelUtil.toDIPFromPixel(output[1]);
float width = PixelUtil.toDIPFromPixel(output[2]);
float height = PixelUtil.toDIPFromPixel(output[3]);
onSuccess.invoke(x, y, width, height);
}
catch (IllegalViewOperationException e) {
onFail.invoke(onSuccess);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If keeping this approach, I'd put the above comment about scoping here too.

}
});
}

Expand Down
203 changes: 142 additions & 61 deletions ReactCommon/react/renderer/uimanager/UIManagerBinding.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,63 @@ void UIManagerBinding::invalidate() const {
uiManager_->setDelegate(nullptr);
}

void measureFromShadowTree(
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to break this work out into a method. It's my understanding c++ doesn't allow you to introduce functions inside functions, so I had to do it here. Is that right? Is there not a way for this code to show up closer to where it's used?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems correct. You could maybe move it closer by making a lambda, or a macro, but those introduce their own set of tradeoffs.

You probably want to make this a proper private method of the UIManagerBinding class though.

UIManager *uiManager,
ShadowNode::Shared shadowNode,
jsi::Function &onSuccessFunction,
jsi::Runtime &runtime) {
auto layoutMetrics = uiManager->getRelativeLayoutMetrics(
*shadowNode, nullptr, {/* .includeTransform = */ true});

if (layoutMetrics == EmptyLayoutMetrics) {
onSuccessFunction.call(runtime, {0, 0, 0, 0, 0, 0});
return;
}
auto newestCloneOfShadowNode =
uiManager->getNewestCloneOfShadowNode(*shadowNode);

auto layoutableShadowNode =
traitCast<LayoutableShadowNode const *>(newestCloneOfShadowNode.get());
Point originRelativeToParent = layoutableShadowNode != nullptr
? layoutableShadowNode->getLayoutMetrics().frame.origin
: Point();

auto frame = layoutMetrics.frame;
onSuccessFunction.call(
runtime,
{jsi::Value{runtime, (double)originRelativeToParent.x},
jsi::Value{runtime, (double)originRelativeToParent.y},
jsi::Value{runtime, (double)frame.size.width},
jsi::Value{runtime, (double)frame.size.height},
jsi::Value{runtime, (double)frame.origin.x},
jsi::Value{runtime, (double)frame.origin.y}});
}

void measureInWindowFromShadowTree(
UIManager *uiManager,
ShadowNode::Shared shadowNode,
jsi::Function &onSuccessFunction,
jsi::Runtime &runtime) {
auto layoutMetrics = uiManager->getRelativeLayoutMetrics(
*shadowNode,
nullptr,
{/* .includeTransform = */ true,
/* .includeViewportOffset = */ true});

if (layoutMetrics == EmptyLayoutMetrics) {
onSuccessFunction.call(runtime, {0, 0, 0, 0});
return;
}

auto frame = layoutMetrics.frame;
onSuccessFunction.call(
runtime,
{jsi::Value{runtime, (double)frame.origin.x},
jsi::Value{runtime, (double)frame.origin.y},
jsi::Value{runtime, (double)frame.size.width},
jsi::Value{runtime, (double)frame.size.height}});
}

jsi::Value UIManagerBinding::get(
jsi::Runtime &runtime,
jsi::PropNameID const &name) {
Expand Down Expand Up @@ -577,48 +634,59 @@ jsi::Value UIManagerBinding::get(
size_t /*count*/) noexcept -> jsi::Value {
auto shadowNode = shadowNodeFromValue(runtime, arguments[0]);
bool turboModuleCalled = false;
auto nativeMeasurerValue = runtime.global().getProperty(runtime, "__turboModuleProxy")
.asObject(runtime).asFunction(runtime).call(runtime, "NativeFabricMeasurerTurboModule");
auto nativeMeasurerValue =
runtime.global()
.getProperty(runtime, "__turboModuleProxy")
.asObject(runtime)
.asFunction(runtime)
.call(runtime, "NativeFabricMeasurerTurboModule");
auto onSuccessFunction =
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move this into the if statement

arguments[1].getObject(runtime).getFunction(runtime);

if (nativeMeasurerValue.isObject()) {
// This calls measureNatively if the NativeFabricMeasurerTurboModule is found.
// The return value doesn't matter here because the measure values will be passed through the callback.
jsi::Value returnValue = nativeMeasurerValue.asObject(runtime).getPropertyAsFunction(runtime, "measureNatively")
.call(runtime, shadowNode.get()->getTag(), arguments[1].getObject(runtime).getFunction(runtime));
turboModuleCalled = true;
// This calls measureNatively if the NativeFabricMeasurerTurboModule
// is found. The return value doesn't matter here because the
// measure values will be passed through the callback.
jsi::Value returnValue =
nativeMeasurerValue.asObject(runtime)
.getPropertyAsFunction(runtime, "measureNatively")
.call(
runtime,
shadowNode.get()->getTag(),
// measureNatively takes two callbacks, one for if the
// layout is found and one for if an error occurs so we
// can gracefully fallback to measuring out of the
// shadow tree.
onSuccessFunction,
jsi::Function::createFromHostFunction(
runtime,
jsi::PropNameID::forAscii(
runtime, "onNativeMeasureFail"),
1,
[uiManager, shadowNode](
jsi::Runtime &rt,
const jsi::Value &thisVal,
const jsi::Value *args,
size_t count) {
// We return onSuccessFunction out of the onFail
// instead of using the one defined above to avoid
// a scoping bug.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might want to rewrite this comment to describe a bit better what the scoping bug is. Essentially something about how functions can't be passed into capture lists because they can't be copied so we need to get a new reference to the function. Unsure of how to phrase that 🤔

auto onSuccessFunction =
args[0].getObject(rt).getFunction(rt);
measureFromShadowTree(
uiManager, shadowNode, onSuccessFunction, rt);
return jsi::Value::undefined();
}));
turboModuleCalled = true;
}

if (turboModuleCalled) {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we're setting turboModuleCalled = true two lines above this. why check now if it's set? why not just return Value::undefined() above?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably was a rough edge around the initial implementation of this. Your recommendation sounds like a cleaner way to do it.

return jsi::Value::undefined();
}

auto layoutMetrics = uiManager->getRelativeLayoutMetrics(
*shadowNode, nullptr, {/* .includeTransform = */ true});
auto onSuccessFunction =
arguments[1].getObject(runtime).getFunction(runtime);

if (layoutMetrics == EmptyLayoutMetrics) {
onSuccessFunction.call(runtime, {0, 0, 0, 0, 0, 0});
return jsi::Value::undefined();
}
auto newestCloneOfShadowNode =
uiManager->getNewestCloneOfShadowNode(*shadowNode);

auto layoutableShadowNode = traitCast<LayoutableShadowNode const *>(
newestCloneOfShadowNode.get());
Point originRelativeToParent = layoutableShadowNode != nullptr
? layoutableShadowNode->getLayoutMetrics().frame.origin
: Point();
measureFromShadowTree(
uiManager, shadowNode, onSuccessFunction, runtime);

auto frame = layoutMetrics.frame;
onSuccessFunction.call(
runtime,
{jsi::Value{runtime, (double)originRelativeToParent.x},
jsi::Value{runtime, (double)originRelativeToParent.y},
jsi::Value{runtime, (double)frame.size.width},
jsi::Value{runtime, (double)frame.size.height},
jsi::Value{runtime, (double)frame.origin.x},
jsi::Value{runtime, (double)frame.origin.y}});
return jsi::Value::undefined();
});
}
Expand All @@ -635,42 +703,55 @@ jsi::Value UIManagerBinding::get(
size_t /*count*/) noexcept -> jsi::Value {
auto shadowNode = shadowNodeFromValue(runtime, arguments[0]);
bool turboModuleCalled = false;
auto nativeMeasurerValue = runtime.global().getProperty(runtime, "__turboModuleProxy")
.asObject(runtime).asFunction(runtime).call(runtime, "NativeFabricMeasurerTurboModule");
auto nativeMeasurerValue =
runtime.global()
.getProperty(runtime, "__turboModuleProxy")
.asObject(runtime)
.asFunction(runtime)
.call(runtime, "NativeFabricMeasurerTurboModule");
auto onSuccessFunction =
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as the if comment above

arguments[1].getObject(runtime).getFunction(runtime);

if (nativeMeasurerValue.isObject()) {
// This calls measureNatively if the NativeFabricMeasurerTurboModule is found.
// The return value doesn't matter here because the measure values will be passed through the callback.
jsi::Value returnValue = nativeMeasurerValue.asObject(runtime).getPropertyAsFunction(runtime, "measureInWindowNatively")
.call(runtime, shadowNode.get()->getTag(), arguments[1].getObject(runtime).getFunction(runtime));
turboModuleCalled = true;
// This calls measureNatively if the NativeFabricMeasurerTurboModule
// is found. The return value doesn't matter here because the
// measure values will be passed through the callback.
jsi::Value returnValue =
nativeMeasurerValue.asObject(runtime)
.getPropertyAsFunction(runtime, "measureInWindowNatively")
.call(
runtime,
shadowNode.get()->getTag(),
onSuccessFunction,
jsi::Function::createFromHostFunction(
runtime,
jsi::PropNameID::forAscii(
runtime, "onNativeMeasureFail"),
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

createFromHostFunction is decorating the JS runtime, and I'm not sure what the behavior would be if you had conflicting names. I'd rename this to something like onNativeMeasureInWindowFail

1,
[uiManager, shadowNode](
jsi::Runtime &rt,
const jsi::Value &thisVal,
const jsi::Value *args,
size_t count) {
// We return onSuccessFunction out of the onFail
// instead of using the one defined above to avoid
// a scoping bug.
auto onSuccessFunction =
args[0].getObject(rt).getFunction(rt);
measureInWindowFromShadowTree(
uiManager, shadowNode, onSuccessFunction, rt);
return jsi::Value::undefined();
}));
turboModuleCalled = true;
}

if (turboModuleCalled) {
return jsi::Value::undefined();
}

auto layoutMetrics = uiManager->getRelativeLayoutMetrics(
*shadowNode,
nullptr,
{/* .includeTransform = */ true,
/* .includeViewportOffset = */ true});

auto onSuccessFunction =
arguments[1].getObject(runtime).getFunction(runtime);

if (layoutMetrics == EmptyLayoutMetrics) {
onSuccessFunction.call(runtime, {0, 0, 0, 0});
return jsi::Value::undefined();
}

auto frame = layoutMetrics.frame;
onSuccessFunction.call(
runtime,
{jsi::Value{runtime, (double)frame.origin.x},
jsi::Value{runtime, (double)frame.origin.y},
jsi::Value{runtime, (double)frame.size.width},
jsi::Value{runtime, (double)frame.size.height}});
measureInWindowFromShadowTree(
uiManager, shadowNode, onSuccessFunction, runtime);

return jsi::Value::undefined();
});
}
Expand Down