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

fix: Add WinUI native helper for GetByteBuffer #3039

Merged
merged 1 commit into from
Oct 23, 2024

Conversation

jeromelaban
Copy link
Contributor

Description of Change

Backports changes from #2920 to the 2x branch.

Bugs Fixed

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

@jeromelaban
Copy link
Contributor Author

/azp run

Copy link

Commenter does not have sufficient privileges for PR 3039 in repo mono/SkiaSharp

@jeromelaban
Copy link
Contributor Author

jeromelaban commented Oct 15, 2024

Hi @mattleibow, @Redth! Could you update the azure devops branch triggers for PRs on the release/2x? Thanks!

@mattleibow
Copy link
Contributor

/azp run

Copy link

Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

@jeromelaban
Copy link
Contributor Author

/azp run

Copy link

Commenter does not have sufficient privileges for PR 3039 in repo mono/SkiaSharp

@mattleibow
Copy link
Contributor

/azp run

Copy link

Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

@mattleibow
Copy link
Contributor

/azp run

Copy link

Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

@mattleibow
Copy link
Contributor

/azp run

Copy link

Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

@mattleibow
Copy link
Contributor

/azp run

Copy link

Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

@mattleibow
Copy link
Contributor

/rebase

@jeromelaban jeromelaban force-pushed the dev/jela/issue2999-2x branch from d410c88 to 3144478 Compare October 19, 2024 13:39
@mattleibow
Copy link
Contributor

/azp run

Copy link

Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

@mattleibow
Copy link
Contributor

/azp run

Copy link

Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

@mattleibow
Copy link
Contributor

Some more of my changes to get CI: unoplatform#33

Still not green yet, but closer i think?

@mattleibow
Copy link
Contributor

/azp run

Copy link

Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

@mattleibow
Copy link
Contributor

Closer!

"D:\a\1\s\source\SkiaSharpSource.Windows-netfx.slnf" (Build target) (1:2) ->
       "D:\a\1\s\source\SkiaSharp.Views.Forms\SkiaSharp.Views.Forms.UWP\SkiaSharp.Views.Forms.UWP.csproj" (default target) (2:6) ->
       "D:\a\1\s\source\SkiaSharp.Views\SkiaSharp.Views.UWP\SkiaSharp.Views.UWP.csproj" (default target) (6:8) ->
       (CoreCompile target) -> 
         D:\a\1\s\source\SkiaSharp.Views\SkiaSharp.Views.UWP\UWPExtensions.cs(6,23): error CS0234: The type or namespace name 'WinUI' does not exist in the namespace 'SkiaSharp.Views' (are you missing an assembly reference?) [D:\a\1\s\source\SkiaSharp.Views\SkiaSharp.Views.UWP\SkiaSharp.Views.UWP.csproj]


       "D:\a\1\s\source\SkiaSharpSource.Windows-netfx.slnf" (Build target) (1:2) ->
       "D:\a\1\s\source\SkiaSharp.Views.WinUI\SkiaSharp.Views.WinUI\SkiaSharp.Views.WinUI.csproj" (default target) (22:14) ->
       "D:\a\1\s\source\SkiaSharp.Views.WinUI\SkiaSharp.Views.WinUI\SkiaSharp.Views.WinUI.csproj" (Build target) (22:16) ->
         D:\a\1\s\source\SkiaSharp.Views\SkiaSharp.Views.UWP\UWPExtensions.cs(177,23): error CS0121: The call is ambiguous between the following methods or properties: 'Utils.GetByteBuffer(IBuffer)' and 'WindowsExtensions.GetByteBuffer(IBuffer)' [D:\a\1\s\source\SkiaSharp.Views.WinUI\SkiaSharp.Views.WinUI\SkiaSharp.Views.WinUI.csproj::TargetFramework=net6.0-windows10.0.19041.0]


       "D:\a\1\s\source\SkiaSharpSource.Windows-netfx.slnf" (Build target) (1:2) ->
       "D:\a\1\s\source\SkiaSharp.Views.WinUI\SkiaSharp.Views.WinUI\SkiaSharp.Views.WinUI.csproj" (default target) (22:14) ->
       "D:\a\1\s\source\SkiaSharp.Views.WinUI\SkiaSharp.Views.WinUI\SkiaSharp.Views.WinUI.csproj" (Build target) (22:15) ->
         D:\a\1\s\source\SkiaSharp.Views\SkiaSharp.Views.UWP\UWPExtensions.cs(177,23): error CS0121: The call is ambiguous between the following methods or properties: 'Utils.GetByteBuffer(IBuffer)' and 'WindowsExtensions.GetByteBuffer(IBuffer)' [D:\a\1\s\source\SkiaSharp.Views.WinUI\SkiaSharp.Views.WinUI\SkiaSharp.Views.WinUI.csproj::TargetFramework=net6.0-windows10.0.18362.0]

@mattleibow
Copy link
Contributor

building now and will see unoplatform#35

@jeromelaban
Copy link
Contributor Author

Indeed, I used it incorrectly, it should be the source path, not the target

@mattleibow
Copy link
Contributor

I think my magic copies the src into the target. Sort of my way to type less. You can either sepcify the full path in the src, or try add the target attribute.

But the full path is better as the pack will fail if we forget a file or if there is a missing file. Sort of a verification check.

@mattleibow
Copy link
Contributor

mattleibow commented Oct 21, 2024

This is how I had it in the 3.x series:

image

I think the only difference is this would now be net6.0?

I did duplicate the projection dll in each because it was super easy vs some complicated logic or something.

@mattleibow
Copy link
Contributor

/azp run

Copy link

Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

@mattleibow
Copy link
Contributor

/azp run

Copy link

Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

@mattleibow
Copy link
Contributor

i think this fixes everything: unoplatform#36

but now the agent runs out of storage. I saw it happen a few times, now it is failing on random extractions. I am going to check and then try extract in a separate step. I have to extract all the nugets so the security/secret/compliance/other scanners can read the dlls.

@mattleibow
Copy link
Contributor

mattleibow commented Oct 22, 2024

this is the money i think unoplatform#37

Green on my internal pipelines

@mattleibow
Copy link
Contributor

/azp run

Copy link

Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

@charlesroddie
Copy link

Interesting. This PR could make SkiaSharp 2.88.9 work with UWP + dotnet9/NativeAOT? Should be able to test that once there is a nuget published.

@jeromelaban
Copy link
Contributor Author

Interesting. This PR could make SkiaSharp 2.88.9 work with UWP + dotnet9/NativeAOT? Should be able to test that once there is a nuget published.

This is not the original intent of the change. I'm not too sure either if the CI will happily accept this kind of net9 inclusions either, as most of the CI is running net6 or 7 and it does not play well with the installed versions of VS.

@jeromelaban
Copy link
Contributor Author

jeromelaban commented Oct 23, 2024

Ok, so I can confirm the 2.x binaries are working with 8.0.402/9.0.100 and WinAppSDK 1.6!

Two small updates left to adjust the RIDs and to make the right nuget dependencies. Getting there!

@mattleibow
Copy link
Contributor

/azp run

Copy link

Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

@mattleibow
Copy link
Contributor

Ok, so I can confirm the 2.x binaries are working with 8.0.402/9.0.100 and WinAppSDK 1.6!

What about older wasdk?

@mattleibow
Copy link
Contributor

/azp run

Copy link

Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

@jeromelaban jeromelaban force-pushed the dev/jela/issue2999-2x branch from 9a99ef6 to a72ff59 Compare October 23, 2024 01:58
@mattleibow
Copy link
Contributor

/azp run

Copy link

Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

@jeromelaban jeromelaban force-pushed the dev/jela/issue2999-2x branch from a72ff59 to 0a52a96 Compare October 23, 2024 02:04
@mattleibow
Copy link
Contributor

/azp run

Copy link

Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

@mattleibow mattleibow marked this pull request as ready for review October 23, 2024 04:51
@mattleibow mattleibow merged commit 94a2734 into mono:release/2.x Oct 23, 2024
84 checks passed
@jeromelaban jeromelaban deleted the dev/jela/issue2999-2x branch October 23, 2024 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants