Skiasharp: Missing SkDrawable

Created on 12 Dec 2018 · 39Comments · Source: mono/SkiaSharp

Description

I'm missing the complete SkDrawable concept (object, SkPictureRecorder.EndRecordingAsDrawable, SkCanvas.DrawDrawable). Are there any efforts on this?

VS bug #751309

area-SkiaSharp area-libSkiaSharp.native type-enhancement type-feature-request

Source

charlenni

All 39 comments

I look into this, but it seems to be complicated. All other classes in SkiaSharp are made as wrapper classes and the native Skia objects used as standard c function calls. This doesn't work for SkDrawable, because SkDrawable is more like an interface. Childs have to implement some funktions (like OnDraw), that would be called from the native side.

Does anyone have any idea, how this could be solved?

charlenni on 13 Dec 2018

This looks like something that can be done with the way we did the managed stream wrappers:

The C++ class:
https://github.com/mono/skia/blob/xamarin-mobile-bindings/include/xamarin/SkManagedStream.h

The C API:
https://github.com/mono/skia/blob/xamarin-mobile-bindings/include/xamarin/sk_managedstream.h

The managed code:
https://github.com/mono/SkiaSharp/blob/master/binding/Binding/SKAbstractManagedStream.cs
https://github.com/mono/SkiaSharp/blob/master/binding/Binding/SKManagedStream.cs

mattleibow on 13 Dec 2018

Äh, I thought, that this class couldn’t the first one.

Thank you for pointing me in the right direction. I will see, what I can do.

charlenni on 13 Dec 2018

I am going to leave this one open for now so we can actually do this at some point - or if you are interested, then have a place to talk.

mattleibow on 13 Dec 2018

I now created a first version (see Skia and SkiaSharp). It exposes the 3 functions OnDraw, OnGetBounds and OnNewPictureSnapshot from C/C++ and adds the missing parts in SkiaSharp.

But how to compile to test it? Are there any scripts? Could I do this with VS 2017?

charlenni on 14 Dec 2018

👍1

Wow! This looks cool! I will take a closer look.

For most cases, you can actually be very specific and just build the platform and then run the unit tests. _(I need to update - or rather, create - the "contributing" docs)_

Build Native Code

macOS: ./bootstrapper.sh -t externals-macos
Windows: .\bootstrapper.ps1 -t externals-windows
Linux: ./bootstrapper.sh -t externals-linux

Run Tests in the IDE
Just open the tests/SkiaSharp.Desktop.Tests/SkiaSharp.Desktop.Tests.sln solution and run the tests there.

Run Tests in the Console _(partially broken)_

macOS & Linux: ./bootstrapper.sh -t tests-only
Windows: .\bootstrapper.ps1 -t tests-only

_NOTE: The NetCore tests are a bit different and require the NuGet to be built - which will cause the test to fail. I need to update the tests-only target_

mattleibow on 14 Dec 2018

@charlenni Maybe open a PR to both repos if you want to start a discussion... Then we can talk on each line and stuff. If you are ready, that is.

mattleibow on 14 Dec 2018

Thank you for your help @mattleibow.

The above commands are for building SkiaSharp, but not for Skia. Does it build Skia too?

I open a PR when I sure, that I can compile the code.

charlenni on 14 Dec 2018

Yes, the externals-xxx target downloads dependencies and builds skia.

mattleibow on 14 Dec 2018

But I have my own Skia, because I changed files there too. So I assume, that I have to copy the Skia files to another place, correct?

charlenni on 14 Dec 2018

I usually just work in the skia submodule and change that to target whatever I want.

For example:

Fork skia and SkiaSharp
Create a new skia branch (which you have done already)
Clone SkiaSharp and update submodules
Checkout your new SkiaSharp branch
Add your fork to the skia submodule remotes and checkout your new skia branch

Now, you should be able to work in both skia and SkiaSharp. You can commit all the changes for now, and when I merge, I will first merge skia and then update your branch back to xamarin-mobile-bindings.

mattleibow on 15 Dec 2018

Ok, I now integrated Skia into SkiaSharp, downloaded my branch and tried to compile it. But it seems, that I'm missing something. The output is
````
Preparing to run build script...
Running build script...
Analyzing build script...
Processing build script...
Installing tools...
Installing addins...
The assembly 'Cake.Xamarin, Version=3.0.0.0, Culture=neutral, PublicKeyToken=null'
is referencing an older version of Cake.Core (0.26.1).
For best compatibility it should target Cake.Core version 0.28.0.
The assembly 'Cake.XCode, Version=4.0.0.0, Culture=neutral, PublicKeyToken=null'
is referencing an older version of Cake.Core (0.26.1).
For best compatibility it should target Cake.Core version 0.28.0.
The assembly 'Cake.FileHelpers, Version=3.0.0.0, Culture=neutral, PublicKeyToken=null'
is referencing an older version of Cake.Core (0.26.1).
For best compatibility it should target Cake.Core version 0.28.0.
Compiling build script...

Tool Paths:
Cake.exe: C:/Entwicklung/SkiaSharp/tools/Cake/Cake.exe
mdoc: C:/Entwicklung/SkiaSharp/tools/mdoc.5.7.3.1/tools/mdoc.exe
msbuild: [NULL]
nuget.exe: C:/Entwicklung/SkiaSharp/tools/nuget.exe
python: python

Build Paths:
~: C:/Users/weltz
NuGet Cache: C:/Users/weltz/.nuget/packages
root: C:/Entwicklung/SkiaSharp
docs: C:/Entwicklung/SkiaSharp/docs/xml
package_cache: C:/Entwicklung/SkiaSharp/externals/package_cache
ANGLE: C:/Entwicklung/SkiaSharp/externals/angle
depot_tools: C:/Entwicklung/SkiaSharp/externals/depot_tools
harfbuzz: C:/Entwicklung/SkiaSharp/externals/harfbuzz
skia: C:/Entwicklung/SkiaSharp/externals/skia

SDK Paths:
Android SDK: /Library/Developer/Xamarin/android-sdk-macosx
Android NDK: /Library/Developer/Xamarin/android-ndk
Tizen Studio: /tizen-studio

Environment Variables (whitelisted):
NUMBER_OF_PROCESSORS: 8
OS: Windows_NT
Path: C:\VulkanSDK\1.1.85.0\Bin
C:\Windows\system32
C:\Windows
C:\Windows\System32\Wbem
C:\Windows\System32\WindowsPowerShell\v1.0\
C:\Windows\System32\OpenSSH\
C:\Program Files\dotnet\
C:\Program Files (x86)\Common Files\Acronis\VirtualFile\
C:\Program Files (x86)\Common Files\Acronis\VirtualFile64\
C:\Program Files (x86)\Common Files\Acronis\FileProtector\
C:\Program Files (x86)\Common Files\Acronis\FileProtector64\
C:\Program Files (x86)\Common Files\Acronis\SnapAPI\
C:\Program Files\Git\cmd
C:\Users\weltz\AppData\Local\Microsoft\WindowsApps

PROCESSOR_ARCHITECTURE: AMD64
PROCESSOR_IDENTIFIER: Intel64 Family 6 Model 58 Stepping 9, GenuineIntel
PSModulePath: C:\Users\weltz\Documents\WindowsPowerShell\Modules
C:\Program Files\WindowsPowerShell\Modules
C:\Windows\system32\WindowsPowerShell\v1.0\Modules

========================================

externals-angle-uwp

Skipping task: externals-angle-uwp

========================================

externals-harfbuzz

Skipping task: externals-harfbuzz

========================================

externals-init

Executing task: externals-init
An error occurred when executing task 'externals-init'.
Error: Mindestens ein Fehler ist aufgetreten.
Das System kann die angegebene Datei nicht finden
````
At the end I get a "File not found" error.

I assume, that I'm missing a package, that is used while compiling. I added Python to VS 2017, because I havn't installed it up to now, but now succes.

If someone has a good idea...

charlenni on 19 Dec 2018

It probably is python. Make sure it is python 2 and it is in the PATH.

mattleibow on 19 Dec 2018

You can either add it to your PATH permanently or just append it temporarily as part of the script invocation. Or, you can use the PYTHON_EXE env var.

mattleibow on 19 Dec 2018

@mattleibow Thank you for the fast support.

Yes, I found in the code, that it uses PYTHON_EXE and MSBUILD_EXE from environment. I now set this two variables and get
Tool Paths: Cake.exe: C:/Entwicklung/SkiaSharp/tools/Cake/Cake.exe mdoc: C:/Entwicklung/SkiaSharp/tools/mdoc.5.7.3.1/tools/mdoc.exe msbuild: C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\MSBuild\15.0\Bin\amd64 nuget.exe: C:/Entwicklung/SkiaSharp/tools/nuget.exe python: C:\Program Files (x86)\Microsoft Visual Studio\Shared\Python36_64
With this, the error message changes from a "File not found" to a "Access denied" error. But it doesn't tell, which file or folder should be accessed :)

charlenni on 19 Dec 2018

Did you check out the submodule? There should be an externals/depot_tools

mattleibow on 19 Dec 2018

Yes, I did this. There are many Git Python files and a ninja.exe.

charlenni on 19 Dec 2018

I didn't set the environment variables for Android and Tizen, but I assume, that this isn't the problem, because I build for Windows.

charlenni on 19 Dec 2018

Ah, your python must be to the exe itself. I see you just go to the folder

See this line as it is called directly https://github.com/mono/SkiaSharp/blob/master/cake/BuildExternals.cake#L64

mattleibow on 19 Dec 2018

You are right. So I changed the environment virables PYTHON_EXE and MSBUILD_EXE to the corresponding EXE files. Now I get a step further.

I now get

````

externals-init

Executing task: externals-init
Traceback (most recent call last):
File "C:/Entwicklung/SkiaSharp/externals/skia/tools/git-sync-deps", line 253, in
exit(main(sys.argv[1:]))
File "C:/Entwicklung/SkiaSharp/externals/skia/tools/git-sync-deps", line 245, in main
git_sync_deps(deps_file_path, argv, verbose)
File "C:/Entwicklung/SkiaSharp/externals/skia/tools/git-sync-deps", line 190, in git_sync_deps
deps_file = parse_file_to_dict(deps_file_path)
File "C:/Entwicklung/SkiaSharp/externals/skia/tools/git-sync-deps", line 170, in parse_file_to_dict
execfile(path, dictionary)
NameError: name 'execfile' is not defined
An error occurred when executing task 'externals-init'.
Error: Mindestens ein Fehler ist aufgetreten.
Process 'C:/Program Files (x86)/Microsoft Visual Studio/Shared/Python36_64/python.exe' failed with error: 1
````

charlenni on 19 Dec 2018

I would try just adding the python directory to the PATH ... I am not sure how well that has been tested. The tooling might be making some assumptions.

If it works in the PATH, but not the PYTHON_EXE env var, then I need to fix that.

mattleibow on 19 Dec 2018

Ok, found the problem: Python 3.x doesn't contain execfile() anymore. So I have to install a Python 2.x version ;)

charlenni on 19 Dec 2018

👍1

It seems to work. The first 380 from 1056 Skia files are allready compiled ;)

charlenni on 19 Dec 2018

Ok. The compiling of Skia run trough 2 times. At the end of the second round, I get an error [ERROR:winsdk.bat] Windows SDK 8.1 : 'include' not found and a path not found error. But at the end a libSkiaSharp.dll in folder output is created.

Then the scripts go on and produces more errors. It stops when creating HarfBuzz with the following error
"C:\Entwicklung\SkiaSharp\native-builds\libHarfBuzzSharp_windows\libHarfBuzzSharp.sln" (Build Ziel) (1) -> "C:\Entwicklung\SkiaSharp\native-builds\libHarfBuzzSharp_windows\libHarfBuzzSharp.vcxproj" (Standardziel) (2) -> (PlatformPrepareForBuild Ziel) -> C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\Common7\IDE\VC\VCTargets\Microsoft.Cpp.Platform.targets (67,5): error MSB8020: Die Buildtools für v140 (Plattformtoolset = "v140") wurden nicht gefunden. Installieren Sie zum Erstellen mithilfe der v140-Buildtools die v140-Buildtools. Alternativ können Sie ein Upgrade auf die aktuellen Visual Studio-Tools durchführen, indem Sie das Menü "Projekt" auswählen oder mit der rechten Maustaste auf die Projektmappe kl icken und dann "Projektmappe neu zuweisen" auswählen. [C:\Entwicklung\SkiaSharp\native-builds\libHarfBuzzSharp_windows\ libHarfBuzzSharp.vcxproj]
Build tools not found for version 140. I assume, that both errors, because I don't have VS 2015 installed.

charlenni on 19 Dec 2018

It seems, that I need the VS 2015 15.3 development tools. Could be installed via VS 2017 installer. Only addinitional 3.26 GB of space ;)

charlenni on 19 Dec 2018

Ok, with this, the command .\bootstrapper.ps1 -t externals-windows run through without any errors.

Now I get finally to the real problems. The SkDrawable.OnGetBounds() returns a SkRect. But how to convert this to a IntPtr? Any idea?

charlenni on 19 Dec 2018

Yes, I could compile all and run the tests. Works so far.

How to create a Nuget package, so that I could add it to an own project?

charlenni on 19 Dec 2018

Awesome! The NuGet... That will require all the Windows platforms at least. If you want to test, you can hook things up manually. Just add a reference to the SkiaSharp.dll and make sure the platform libSkiSharp.dll is marked as Content and copied to output.

Also, you could test it via one of the sample apps https://github.com/mono/SkiaSharp/blob/master/samples/Basic/Desktop/SkiaSharpSample/SkiaSharpSample.csproj

mattleibow on 19 Dec 2018

I test it now via the sample. Works good and so I could go for hunting of bugs.

charlenni on 19 Dec 2018

It seems, that the libSkiaSharp doesn't contain any of the new C/C++ files. Should I add the new file names at any place, so that they compiled and added to the lib?

charlenni on 19 Dec 2018

ah, yeah. there is a <skia>/src/xamarin/SkiaKeeper.c just add a line for one symbol in the array. We have a strict linking process and that is what is kept.

mattleibow on 20 Dec 2018

And I had to add the new files to BUILD.gn.

Now I'm at a point where I could compile the library and a sample works. Now I have one more question: should I do anything when dispose the object? Need I a unref or destroy function?

charlenni on 20 Dec 2018

depending on how you created the object. if you did a new in C++, then you need a delete, but if you got it via method that had a sp_ref<SkDrawable> return, then usually you can just SkSafeUnref.

I see you just make a few PRs! NICE! I'll add comments to that so we can talk in code :)

mattleibow on 20 Dec 2018

Ok, than I should delete the SkManagedDrawable created here.

charlenni on 20 Dec 2018

👍1

I had a look at the PRs and they are really good - I felt happy reading them :)
Most of my comments relate to the inheritance as I would like to separate the Xamarin bits and the pure binding bits. This is for two reasons:

to match what is happening in the C++ world as Google is prone to changing things, and I would like to be able to just rewrite the Xamarin bits if they decide to break the world.
we have a dream here... to eventually upstream the C API - or at least parts of it (it is a far off dream, but it is still there, somewhere)

mattleibow on 20 Dec 2018

I wanted to add the sk_drawable.h and sk_drawable.cpp. I added the h-file and a reference into SkiaKeeper.c. But it isn't respected while compile Skia? in BUILD.gn I didn't find anything about the folder include/c or src/c. Are I'm missing something?

charlenni on 20 Dec 2018

I saw you found the gn/core.gni file that did things. That is the correct place to put it.

mattleibow on 21 Dec 2018

Thanks for the PRs, I have merged them into master and CI is taking over... The development branch built fine, so I think all is well.

mattleibow on 27 Dec 2018

@mattleibow I hope, you had a good start into the new year.

Is there a timeline, when the 1.68.1-beta is released?

charlenni on 9 Jan 2019

Was this page helpful?

0 / 5 - 0 ratings