Re-Enable DrawTransformBitmapExDirect for render backends

Desktop / LibreOffice - Armin Le Grand (Allotropia) [me.com] - 7 December 2021 17:51 UTC

Unfortunately the add/usage of HasFastDrawTransformedBitmap did disable the system-dependent implementations/fast-path for DrawTransformBitmapExDirect and it's implemenations, except for Skia.

This means that the current backends for Windows/Mac/Cairo/headless/Qt5 have to do expensive pixel operations when a Bitmap is 'really' transformed (rotate/shear) since some time.

The nine implementations using ::hasFastDrawTransformedBitmap (grep for it) all return false, except the Skia one.

Since HasFastDrawTransformedBitmap() uses that and itself is used in the very central mehod OutputDevice::DrawTransformedBitmapEx(...) to decide if that fast-path shall/can be used at all, it was *no longer used* - except for Skia - what makes Skia definitely performing better with transformed Bitmaps, or the other way around - the others worse.

HasFastDrawTransformedBitmap() is used in only two places, the second is in the canvas helper to decide if to try to use that fast-path for presentation rendering.

A method at OutputDevice to see if that fast-path is implemented is therefore currently needed, but for the canvas helper only. Since this will/should be converted to primitive usage (hopefully) anyways, nine impementations calling these virtual functions often and the danger to produce a mismatch/ error beween implementations of hasFastDrawTransformedBitmap and drawTransformedBitmap (as happened here, but can also happen when someone adds or removes an implementation) I looked for a way to solve that differenly and more safe.

Since SalGraphics::DrawTransformedBitmap anyways returns a bool to signal it's success I take this as base to implement a buffered test directly at SalGraphics, also directly set a local flag to detect that functionality if DrawTransformedBitmap is used anyways before the test is/would be needed. Combined wih that small test to check only if this was not yet used and thus tested by DrawTransformedBitmap anyways I can offer a reliable non-virtual method at OutputDevice called ImplementsFastDrawTransformedBitmap() that will be used at the single necessary location - in the canvas helper. Since that small test direcly uses one of the nine implementations of hasFastDrawTransformedBitmap it is fundamenally more reliable and probably the copy bitmap/writeBack never really used (I tested that it works) due to an earlier use of DrawTransformedBitmap did the check potentially already.

I also took a look at the cairo version (since I had this one running here) and ensured that the buffering of the system-dependent form of the Bitmap as cairo surface still works. Regarding the newly introduced fAlpha parameter I want to add some remarks:- It should be called fOpacity to make clear that it describes opacity, defining that if 1.0 == fAlpha means *no* transparency. That word is used in other graphic systems and makes more clear what function it has. It is the opposite of transparency, but works the same.- Currently all implementations of ::drawTransformedBitmap - except Skia where it was implemented - do not use it, but return false. It will in most cases not be too complicated to add/implement it, e.g. for cairo anyways a transparency surface will/is created, fAlpha can just be merged in, and the criteria for buffering that may be extended to remember for which value (if at all) of fAlpha that was prepared. I strongly recommend implementing these for our main graphic backends.- The primitive renderer uses another more general way to add an extra alpha channel to paint when needed - it draws the content (any content) that needs to be transparent to a buffer and then that buffer using the intended transparency. This is discussable since may be more expensive, but more general and keeps the interface less complex. We can see here that adding that complexity to the existing interface at OutputDevice makes the implementations more complex what might be the reason his was only implemented for one of nine backends. When adding something like this and extending the complexity I would prefer that at the same time it gets also *implemented* in all or most or at least most used cases. I want to make clear that from my POV in those cases choosing possible runtime speed over complexity is not always preferable.

Change-Id: I5bab59f59fca878a7b11a20094e49e8b50196063 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/126480

7e5af164b7d2 Re-Enable DrawTransformBitmapExDirect for render backends
bin/find-can-be-private-symbols.functions.results | 2 +-
canvas/source/vcl/canvashelper.cxx | 2 +-
include/vcl/outdev.hxx | 8 ++--
vcl/headless/svpgdi.cxx | 5 ---
vcl/inc/headless/svpgdi.hxx | 1 -
vcl/inc/qt5/QtGraphics.hxx | 2 -
vcl/inc/quartz/salgdi.h | 2 -
vcl/inc/salgdi.hxx | 15 +++-----
vcl/inc/salgdiimpl.hxx | 4 --
vcl/inc/skia/gdiimpl.hxx | 2 -
vcl/inc/unx/GenPspGfxBackend.hxx | 2 -
vcl/inc/unx/salgdi.h | 2 -
vcl/inc/win/salgdi.h | 2 -
vcl/qt5/QtGraphics_GDI.cxx | 3 +-
vcl/quartz/AquaGraphicsBackend.cxx | 2 -
vcl/skia/gdiimpl.cxx | 7 ----
vcl/source/gdi/salgdilayout.cxx | 46 +++++++++++++++++++++--
vcl/source/outdev/bitmap.cxx | 12 ------
vcl/source/outdev/bitmapex.cxx | 7 +++-
vcl/unx/generic/gdi/gdiimpl.cxx | 5 ---
vcl/unx/generic/gdi/gdiimpl.hxx | 2 -
vcl/unx/generic/gdi/salgdi2.cxx | 9 ++---
vcl/unx/generic/print/GenPspGfxBackend.cxx | 2 -
vcl/win/gdi/gdiimpl.cxx | 5 ---
vcl/win/gdi/gdiimpl.hxx | 2 -
vcl/win/gdi/salgdi_gdiplus.cxx | 9 ++---
26 files changed, 68 insertions(+), 92 deletions(-)

Upstream: cgit.freedesktop.org


  • Share