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

Support arm64 native assets for Tizen #2324

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

rookiejava
Copy link
Contributor

Description of Change

This PR is for arm64 support in SkiaSharp.NativeAssets.Tizen and HarfBuzzSahrp.NativeAssets.Tizen.

Bugs Fixed

None.

API Changes

None.

Behavioral Changes

None.

Required skia PR

None.

PR Checklist

  • Has tests (if omitted, state reason in description)
  • Rebased on top of main at time of PR
  • Merged related skia PRs
  • Changes adhere to coding standard
  • Updated documentation

@rookiejava
Copy link
Contributor Author

I think arm64 arch build should be supported on externals-tizen targets. @mattleibow Is there anything I missed?

An error occurred when executing task 'libSkiaSharp'.
Error: One or more errors occurred. (Process 'D:/a/1/s/externals/depot_tools/ninja.exe' failed with error: 1)
	Process 'D:/a/1/s/externals/depot_tools/ninja.exe' failed with error: 1
An error occurred when executing task 'externals-tizen'.

https://dev.azure.com/xamarin/public/_build/results?buildId=74492&view=logs&j=a6d0c0ff-0249-521d-3982-e70dd54072d8&t=3efcc29f-56bd-59b3-ca9d-9ae8e056213b&l=131

@mattleibow
Copy link
Contributor

Let me have a look at CI and see what is happening...

<None Include="..\..\output\native\tizen\armel\libHarfBuzzSharp.*" Link="nuget\runtimes\tizen-armel\%(Filename)%(Extension)" />
<None Include="..\..\output\native\tizen\arm64\libHarfBuzzSharp.*" Link="nuget\runtimes\tizen-arm64\%(Filename)%(Extension)" />
<None Include="..\..\output\native\tizen\i386\libHarfBuzzSharp.*" Link="nuget\runtimes\tizen-x86\%(Filename)%(Extension)" />
<None Include="..\..\output\native\tizen\i386\libHarfBuzzSharp.*" Link="nuget\runtimes\linux-x86\%(Filename)%(Extension)" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we still need the linux-x86 runtime? I am not sure we can do this as it will replace the linux binary - and will also cause the tizen binary to run on a linux (desktop) machine.

What we can do is actually just keep the tizen-x86 version, and pack it twice in the tizen nuget.

@@ -39,11 +41,14 @@ Please visit https://go.microsoft.com/fwlink/?linkid=868517 to view the release
<file src="build/tizen40/HarfBuzzSharp.targets" target="buildTransitive/tizen40/HarfBuzzSharp.NativeAssets.Tizen.targets" />

<!-- libHarfBuzzSharp.dll and other native files -->
<file src="build/tizen40/arm/libHarfBuzzSharp.so" />
<file src="build/tizen40/x86/libHarfBuzzSharp.so" />
<file src="runtimes/linux-x86/native/libHarfBuzzSharp.so" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<file src="runtimes/linux-x86/native/libHarfBuzzSharp.so" />
<file src="runtimes/tizen-x86/native/libHarfBuzzSharp.so" target="runtimes/linux-x86/native/libHarfBuzzSharp.so" />

<None Include="..\..\output\native\tizen\armel\libSkiaSharp.*" Link="nuget\runtimes\tizen-armel\libSkiaSharp.so" />
<None Include="..\..\output\native\tizen\arm64\libSkiaSharp.*" Link="nuget\runtimes\tizen-arm64\libSkiaSharp.so" />
<None Include="..\..\output\native\tizen\i386\libSkiaSharp.*" Link="nuget\runtimes\tizen-x86\libSkiaSharp.so" />
<None Include="..\..\output\native\tizen\i386\libSkiaSharp.*" Link="nuget\runtimes\linux-x86\libSkiaSharp.so" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above, don't copy here, just pack twice.

@@ -40,11 +42,14 @@ Please visit https://go.microsoft.com/fwlink/?linkid=868517 to view the release
<file src="build/tizen40/SkiaSharp.targets" target="buildTransitive/tizen40/SkiaSharp.NativeAssets.Tizen.targets" />

<!-- libSkiaSharp.dll and other native files -->
<file src="build/tizen40/arm/libSkiaSharp.so" />
<file src="build/tizen40/x86/libSkiaSharp.so" />
<file src="runtimes/linux-x86/native/libSkiaSharp.so" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<file src="runtimes/linux-x86/native/libSkiaSharp.so" />
<file src="runtimes/tizen-x86/native/libSkiaSharp.so" target="runtimes/linux-x86/native/libSkiaSharp.so" />

@mattleibow mattleibow changed the title Support arm64 native asserts for Tizen Support arm64 native assets for Tizen Jun 14, 2023
@mattleibow
Copy link
Contributor

Hopefully my review makes sense... I think I forgot the picture I have in my head is not really visible outside my mind...

@mattleibow
Copy link
Contributor

I tried using the latest Tizen Studio on the new main, and it appears the be missing headers. I forget now, but it was like missing math headers or something. I was able to see the arm/x86 had more headers than the arm64/x64.

@mattleibow mattleibow added area/libSkiaSharp.native area/libHarfBuzzSharp.native os/Tizen partner/tizen Issues and PRs that are/should being looked at or worked on by the Samsung Tizen partners. labels Aug 20, 2023
@mattleibow mattleibow marked this pull request as draft February 12, 2024 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/libHarfBuzzSharp.native area/libSkiaSharp.native os/Tizen partner/tizen Issues and PRs that are/should being looked at or worked on by the Samsung Tizen partners.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants