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] Expose existing RTree optimization option on SkPictureRecorder.BeginRecording() #2372

Closed
kcoop opened this issue Feb 3, 2023 · 5 comments · Fixed by #2889
Closed

Comments

@kcoop
Copy link

kcoop commented Feb 3, 2023

We're looking to pan and zoom some SkiaSharp's SkPictures that consist of a reasonably large number of simple operations (~25K, text, lines, rect fills). We're finding a zoomed and translated DrawPicture is rendering on the order of 5 fps on recent model iOS devices, which is clearly unacceptable.

But yay! It turns out Skia has had built in support for RTree-based operations culling for a long time, and it looks very promising:

https://groups.google.com/g/skia-discuss/c/EPOuhhloIF4/m/rZyakh1fBwAJ
https://bugs.chromium.org/p/chromium/issues/detail?id=485597#c22

But oh no! SkiaSharp doesn't expose this functionality, either in C# or the C wrappers in the Mono fork of Skia.

But yay! The fix looks very straightforward - just exposing the constructor of an RTreeFactory instance, and passing it as an extra parameter to BeginRecording().

I've created forks of both SkiaSharp and mono/skia, and implemented a swag at a simple C# api, C wrappers, and some smoke tests. You can find the (fairly small, simple, but unvetted) changes here:

main...kcoop:SkiaSharp:kcoop-rtree
mono/skia@xamarin-mobile-bindings...kcoop:mono-skia:kcoop-rtree

This seems like a valuable, big bang for the buck PR. Very small change, and who wouldn't want significant rendering improvements for large-ish documents by simply adding a bool? I'm happy to do what's needed to make this happen.

[Edited to remove noise - I've now discovered this repo uses submodules to build mono/skia]

@TwinkyDaniel
Copy link

I would be interested in that feature as well. I want to record draw calls and then replay them with different zoom and translation. Right now this is very slow when only rendering a tiny part of the actual recorded calls (i.e. when zooming in). Any chance to get this feature merged?

@mattleibow
Copy link
Contributor

I have added this to the list of things for v3. I'll have a look and merge some things.

Thanks!

@TwinkyDaniel
Copy link

When do you plan 3.0? Any chance to get this merged earlier? Can we do something to help?

@wieslawsoltes
Copy link

This would definitely help with drawing apps, even UI apps, and rendering SVG's with zoom as I use SKPicture extensively there :)

@mattleibow
Copy link
Contributor

When do you plan 3.0? Any chance to get this merged earlier? Can we do something to help?

If there is a PR, we can easily backport a smaller change like this. SkiaSharp is less scary than all these repos and files.

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 a pull request may close this issue.

4 participants