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

Feature/mac arm64 #38

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

syaiful6
Copy link

@syaiful6 syaiful6 commented Aug 8, 2021

hi all, this PR allow us to build SKIA in Mac M1 without merging upstream repository, this is minimal changes required. Gn tool set default cpu to x86, so we need to set it to arm64 if it in Darwin.

This PR will also make current Revery compiled natively in M1. For Onivim2 there is small changes needed, but it just a path of homebrew prefix in M1 which is /opt/homebrew.

# ranging from false positive (only copying a prefix of SkRRect from serialized form)
# to possibly useful (memcpy() with non-trivial types). Annoyingly you can't really
# break them up any finer.
# TODO(mtklein): suppress / fix each site as appropriate?
Copy link
Member

Choose a reason for hiding this comment

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

Who is @mtklein ? Does this code from upstream ?

Copy link

Choose a reason for hiding this comment

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

I bet that's me!

This comment and suppressed warning was in upstream Skia for a couple weeks, but removed in December 2019. You can see the timeline at https://bugs.chromium.org/p/skia/issues/detail?id=9674. I think we fixed whatever was bothering -Wclass-memaccess at the time.

Copy link
Author

Choose a reason for hiding this comment

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

yes, this code from upstream, I just cherry pick minimal changes needed to compile in M1.

Copy link
Member

Choose a reason for hiding this comment

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

Ah ok Github showed you were author. So you Co-authored the first commit and fully the second one ?

Does xcrun esy works ? and does our upstream mono/skia (We are fork of fork) compile fine ?

Copy link
Author

@syaiful6 syaiful6 Aug 8, 2021

Choose a reason for hiding this comment

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

@Et7f3 yes, that also works and it produces an arm64 library, previously it produces an x86 library.

The first attempt to compile our fork is actually merge mono SKIA to our repository, but that is too big to manage, and it also can't compile, not sure why. So, I try to build Google SKIA, and it compiled successfully. Based on that, I figure out how they do it. The changes on the first commit are just generated by applying a patch I created with their changes in gn/BUILD.gn file, the work should be credited to the Google team. The second commit is just putting back our changes to that file.

@Et7f3
Copy link
Member

Et7f3 commented Aug 8, 2021

LGTM but I let owner of M1 approve and merge.

@bryphe
Copy link
Member

bryphe commented Aug 22, 2021

Wow what a hero! Thank you @syaiful6 for bringing over this set of changes to unblock the M1 build 😍

@taxilian
Copy link

Hoping to see this get merged soon -- This was something like step 5 (and the last step) on a chain of pull requests to get various projects working on apple silicon devices, the other each wait on the next up until this one =]

@fiddler
Copy link

fiddler commented Feb 17, 2022

Wow, hoping to see this get merged as well. Is there anything I can do to help this move along - test on M1 or something?

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.

6 participants