Skip to content

Enable ARM64 in Yoga.Universal.#1

Closed
JunielKatarn wants to merge 1 commit into
ReactWindows:masterfrom
jurocha-ms:arm64
Closed

Enable ARM64 in Yoga.Universal.#1
JunielKatarn wants to merge 1 commit into
ReactWindows:masterfrom
jurocha-ms:arm64

Conversation

@JunielKatarn

Copy link
Copy Markdown

No description provided.

@JunielKatarn

Copy link
Copy Markdown
Author

Required to unblock microsoft/react-native-windows#3909.

@JunielKatarn

Copy link
Copy Markdown
Author

Wrong branch.

JunielKatarn pushed a commit to jurocha-ms/yoga that referenced this pull request Feb 21, 2020
Summary:
In D17439957, I noted that YogaLogger#log throws a NoMethodFoundException when called from C++ b/c C++ and Java's signatures of that method don't match. C++ uses YogaNodeJNIBase for the first param, Java uses YogaNode. Both my attempts to fix this failed.

Attempt ReactWindows#1 - Make Java use YogaNodeJNIBase. This doesn't work because the :java-interface target includes YogaLogger but not YogaNodeJNIBase. Moving YogaLogger to the impl target doesn't work either b/c other files in :java-interface reference YogaLogger.

Attempt ReactWindows#2 - Make C++ use YogaNode. This doesn't work b/c we try to call the log method with objects of fbjni type YogaNodeJNIBase. This would be fine in Java since YogaNodeJNIBase extends YogaNode. But fbjni's typing isn't advanced enough to know this, so the Yoga C++ fails to compile.

At this point, I was wondering what the value of having this param in the log function at all was. None of the implementations in our codebase use it today. It might be easier to just remove it all together. This also removes a bug with YGNodePrint where we pass a null layout context that eventually causes a SIG_ABRT when we use it to try to find a YogaNode to pass to this function. (https://fburl.com/diffusion/ssw9h8lv).

Reviewed By: amir-shalem

Differential Revision: D17470379

fbshipit-source-id: 8fc2d95505971a52af2399a9fbb60b63f27f0ec2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant