From 957f90f29d2d9b31feabbc5229c8af181a8769e2 Mon Sep 17 00:00:00 2001 From: Augustin Cavalier Date: Wed, 2 Feb 2022 19:29:49 -0500 Subject: [PATCH] Use per-drawable "scratch images" instead of allocating bitmaps for each XImage. This fixes major memory leaks with Cairo Xlib surfaces, and should constitute something of a performance improvement. --- xlib/Cursor.cpp | 11 ++++++++--- xlib/Drawables.cpp | 1 + xlib/Drawables.h | 2 ++ xlib/Drawing.cpp | 29 ++++++++++++++++++++++------- xlib/Image.cpp | 44 +++++++++++++++++++++++--------------------- xlib/Image.h | 13 +++++++++++++ 6 files changed, 69 insertions(+), 31 deletions(-) create mode 100644 xlib/Image.h diff --git a/xlib/Cursor.cpp b/xlib/Cursor.cpp index 9cba68e..6bc1788 100644 --- a/xlib/Cursor.cpp +++ b/xlib/Cursor.cpp @@ -10,6 +10,7 @@ #include "Drawing.h" #include "Drawables.h" #include "Debug.h" +#include "Image.h" extern "C" { #include @@ -111,7 +112,8 @@ XCreatePixmapCursor(Display* display, Pixmap source, Pixmap mask, XImage* srcImg = XGetImage(display, source, 0, 0, rect.width, rect.height, -1, ZPixmap), *maskImg = XGetImage(display, mask, 0, 0, rect.width, rect.height, -1, ZPixmap), *resultImg = XCreateImage(display, NULL, 32, ZPixmap, 0, NULL, rect.width, rect.height, 32, 0); - if (!srcImg || !maskImg || !resultImg) { + BBitmap* resultBitmap = _bbitmap_for_ximage(resultImg); + if (!srcImg || !maskImg || !resultImg || !resultBitmap) { if (srcImg) XDestroyImage(srcImg); if (maskImg) @@ -120,7 +122,7 @@ XCreatePixmapCursor(Display* display, Pixmap source, Pixmap mask, XDestroyImage(resultImg); return None; } - resultImg->data = (char*)((BBitmap*)resultImg->obdata)->Bits(); + resultImg->data = (char*)resultBitmap->Bits(); unsigned long fg = foreground_color->pixel | (0xFF << 24), bg = background_color->pixel | (0xFF << 24), @@ -134,7 +136,10 @@ XCreatePixmapCursor(Display* display, Pixmap source, Pixmap mask, } } - BCursor* cursor = new BCursor((BBitmap*)resultImg->obdata, BPoint(x, y)); + BCursor* cursor = new BCursor(resultBitmap, BPoint(x, y)); + + delete resultBitmap; + resultImg->data = NULL; XDestroyImage(srcImg); XDestroyImage(maskImg); XDestroyImage(resultImg); diff --git a/xlib/Drawables.cpp b/xlib/Drawables.cpp index 1812687..e9e3214 100644 --- a/xlib/Drawables.cpp +++ b/xlib/Drawables.cpp @@ -109,6 +109,7 @@ XDrawable::XDrawable(Display* dpy, BRect rect) XDrawable::~XDrawable() { XFreeGC(_display, default_gc); + delete scratch_bitmap; Drawables::erase(id()); remove(); } diff --git a/xlib/Drawables.h b/xlib/Drawables.h index a9bf50d..b03e158 100644 --- a/xlib/Drawables.h +++ b/xlib/Drawables.h @@ -65,6 +65,8 @@ class XDrawable : XLIBE_DRAWABLES_PROTECTED BView { GC gc = NULL; GC default_gc = NULL; + BBitmap* scratch_bitmap = NULL; + public: XDrawable(Display* dpy, BRect rect); virtual ~XDrawable() override; diff --git a/xlib/Drawing.cpp b/xlib/Drawing.cpp index 12fca44..85caf35 100644 --- a/xlib/Drawing.cpp +++ b/xlib/Drawing.cpp @@ -9,6 +9,7 @@ #include #include +#include "Color.h" #include "Drawables.h" #include "Font.h" #include "GC.h" @@ -357,18 +358,32 @@ XPutImage(Display *display, Drawable d, GC gc, XImage* image, if (!drawable) return BadDrawable; - BBitmap* bbitmap = (BBitmap*)image->obdata; - if (image->data != bbitmap->Bits()) { - // We must import the bits before drawing. - // TODO: Optimization: Import only the bits we are about to draw! - bbitmap->ImportBits(image->data, image->height * image->bytes_per_line, - image->bytes_per_line, image->xoffset, bbitmap->ColorSpace()); + const BRect srcRect = brect_from_xrect(make_xrect(src_x, src_y, width, height)); + const BRect scratchBounds = drawable->scratch_bitmap + ? drawable->scratch_bitmap->Bounds() : BRect(); + if (drawable->scratch_bitmap == NULL + || scratchBounds.Width() < srcRect.Width() + || scratchBounds.Height() < srcRect.Height()) { + // We need a bigger scratch bitmap. + BSize size = srcRect.Size(); + if (size.width < scratchBounds.Width()) + size.width = scratchBounds.Width(); + if (size.height < scratchBounds.Height()) + size.height = scratchBounds.Height(); + + delete drawable->scratch_bitmap; + drawable->scratch_bitmap = new BBitmap(BRect(BPoint(0, 0), size), 0, B_RGB32); + // TODO: or FIXME: get the actual color space? } + // TODO: Optimization: Import only the bits we are about to draw! + drawable->scratch_bitmap->ImportBits(image->data, image->height * image->bytes_per_line, + image->bytes_per_line, image->xoffset, _x_color_space(NULL, image->bits_per_pixel)); + BView* view = drawable->view(); view->LockLooper(); bex_check_gc(drawable, gc); - view->DrawBitmap(bbitmap, brect_from_xrect(make_xrect(src_x, src_y, width, height)), + view->DrawBitmap(drawable->scratch_bitmap, srcRect, brect_from_xrect(make_xrect(dest_x, dest_y, width, height))); view->UnlockLooper(); return Success; diff --git a/xlib/Image.cpp b/xlib/Image.cpp index c043a36..82d756d 100644 --- a/xlib/Image.cpp +++ b/xlib/Image.cpp @@ -10,6 +10,7 @@ #include "Drawing.h" #include "Debug.h" #include "Color.h" +#include "Image.h" extern "C" { #include @@ -26,10 +27,7 @@ _XInitImageFuncPtrs(XImage *image) static int DestroyImage(XImage* image) { - BBitmap* bbitmap = (BBitmap*)image->obdata; - if (image->data != bbitmap->Bits()) - free(image->data); - delete bbitmap; + free(image->data); delete image; return Success; } @@ -168,16 +166,20 @@ XInitImage(XImage* image) image->f.get_pixel = ImageGetPixel; image->f.put_pixel = ImagePutPixel; - // Create the auxiliary bitmap. - BBitmap* bbitmap = new BBitmap(brect_from_xrect(make_xrect(0, 0, image->width, image->height)), 0, - _x_color_space(NULL, image->bits_per_pixel), image->bytes_per_line); - if (!bbitmap || bbitmap->InitCheck() != B_OK) { + return 1; +} + +BBitmap* +_bbitmap_for_ximage(XImage *image, uint32 flags) +{ + BBitmap* bitmap = new BBitmap(brect_from_xrect(make_xrect(0, 0, image->width, image->height)), + flags, _x_color_space(NULL, image->bits_per_pixel), image->bytes_per_line); + if (!bitmap || bitmap->InitCheck() != B_OK) { fprintf(stderr, "libX11: Failed to create bitmap for XImage!\n"); - return 0; + debugger("X"); + return NULL; } - image->obdata = (char*)bbitmap; - - return 1; + return bitmap; } extern "C" XImage* @@ -196,26 +198,26 @@ XGetSubImage(Display* display, Drawable d, // TODO: plane_mask? - BBitmap* bbitmap = (BBitmap*)dest_image->obdata; if (!dest_image->data) - dest_image->data = (char*)bbitmap->Bits(); + dest_image->data = (char*)malloc(dest_image->bytes_per_line * dest_image->height); + + BBitmap* import = _bbitmap_for_ximage(dest_image, B_BITMAP_NO_SERVER_LINK); + if (!import) + return NULL; const BRect dest_rect = brect_from_xrect(make_xrect(dest_x, dest_y, width, height)); #if B_HAIKU_VERSION >= B_HAIKU_VERSION_1_PRE_BETA_4 - bbitmap->ImportBits(pixmap->offscreen(), BPoint(x, y), dest_rect.LeftTop(), + import->ImportBits(pixmap->offscreen(), BPoint(x, y), dest_rect.LeftTop(), dest_rect.Size()); #else // NOTE: Unlike most other Be API functions, ImportBits() takes pixel count, not span! // BSize variants are being added that make much more sense. - bbitmap->ImportBits(pixmap->offscreen(), BPoint(x, y), dest_rect.LeftTop(), + import->ImportBits(pixmap->offscreen(), BPoint(x, y), dest_rect.LeftTop(), dest_rect.IntegerWidth() + 1, dest_rect.IntegerHeight() + 1); #endif - if (dest_image->data != bbitmap->Bits()) { - memcpy(dest_image->data, bbitmap->Bits(), - dest_image->height * dest_image->bytes_per_line); - } - + memcpy(dest_image->data, import->Bits(), dest_image->height * dest_image->bytes_per_line); + delete import; return dest_image; } diff --git a/xlib/Image.h b/xlib/Image.h new file mode 100644 index 0000000..29a5635 --- /dev/null +++ b/xlib/Image.h @@ -0,0 +1,13 @@ +/* + * Copyright 2021, Haiku, Inc. All rights reserved. + * Distributed under the terms of the MIT license. + */ +#pragma once + +#include + +extern "C" { +#include +} + +BBitmap* _bbitmap_for_ximage(XImage* image, uint32 flags = 0);