Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Added SKImage field to WriteableBitmapImpl for improved drawing performance #17717

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

AngryCarrot789
Copy link

What does the pull request do?

It adds two fields, SKBitmap and SKPixmap, which are created in each constructor. The SKImage is used in the Draw() method for improved performance, since in the original implementation, DrawBitmap() allocates a new SKImage each time (which requires copying the entire bitmap according to the C++ implementation when the bitmap is mutable, which it always is in this class).

I also added optimisation to use DrawImage with coordinates instead of Rect, since sk_canvas_draw_image_rect has some sort of overhead: #2849 (comment)

What is the updated/expected behavior with this PR?

The performance of drawing a WriteableBitmap should be improved, especially for large images (4K+). I've noticed a subtle performance increase when drawing a 1920x1080 bitmap.

…egate to SKBitmap for improved performance, since DrawBitmap in the original Draw() implementation allocates a new SKImage each time (which as far as I know, requires copying the entire bitmap). Also added optimisation to skip DrawImage with Rect, since sk_canvas_draw_image_rect has some sort of overhead
@avaloniaui-bot
Copy link

You can test this PR using the following package version. 11.3.999-cibuild0053761-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@AngryCarrot789
Copy link
Author

This doesn't seem to work when starting the app with Win32CompositionMode.DirectComposition in the Win32PlatformOptions

@timunie
Copy link
Contributor

timunie commented Dec 9, 2024

We can't rely on Skia by default. So the idea isn't bad at all but we need a way to figure out if chaching is okay or not.

@kekekeks
Copy link
Member

kekekeks commented Dec 10, 2024

Context: there previously was SKCanvas::drawBitmap, but it apparently got removed around 2021 and SkiaSharp have silently added a shim. We've previously relied on Skia caching gpu bitmap data and NotifyPixelChanged call for invalidation.


The way you are trying to use SKImage with SKPixmap is strictly invalid. The docs explicitly specify that the underlying pixmap should be unchanged until SKImage is destroyed.

For it to work properly we need to manually re-create an SKImage from SKBitmap (so it would make a snapshot) when drawing if pixel data was changed since the last time.

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants