Skip to content

Commit

Permalink
Use per-drawable "scratch images" instead of allocating bitmaps for e…
Browse files Browse the repository at this point in the history
…ach XImage.

This fixes major memory leaks with Cairo Xlib surfaces,
and should constitute something of a performance improvement.
  • Loading branch information
waddlesplash committed Feb 3, 2022
1 parent 3357328 commit 957f90f
Show file tree
Hide file tree
Showing 6 changed files with 69 additions and 31 deletions.
11 changes: 8 additions & 3 deletions xlib/Cursor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "Drawing.h"
#include "Drawables.h"
#include "Debug.h"
#include "Image.h"

extern "C" {
#include <X11/Xlib.h>
Expand Down Expand Up @@ -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)
Expand All @@ -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),
Expand All @@ -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);
Expand Down
1 change: 1 addition & 0 deletions xlib/Drawables.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ XDrawable::XDrawable(Display* dpy, BRect rect)
XDrawable::~XDrawable()
{
XFreeGC(_display, default_gc);
delete scratch_bitmap;
Drawables::erase(id());
remove();
}
Expand Down
2 changes: 2 additions & 0 deletions xlib/Drawables.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
29 changes: 22 additions & 7 deletions xlib/Drawing.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include <interface/Bitmap.h>
#include <interface/Polygon.h>

#include "Color.h"
#include "Drawables.h"
#include "Font.h"
#include "GC.h"
Expand Down Expand Up @@ -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;
Expand Down
44 changes: 23 additions & 21 deletions xlib/Image.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "Drawing.h"
#include "Debug.h"
#include "Color.h"
#include "Image.h"

extern "C" {
#include <X11/Xlib.h>
Expand All @@ -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;
}
Expand Down Expand Up @@ -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*
Expand All @@ -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;
}

Expand Down
13 changes: 13 additions & 0 deletions xlib/Image.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/*
* Copyright 2021, Haiku, Inc. All rights reserved.
* Distributed under the terms of the MIT license.
*/
#pragma once

#include <interface/Bitmap.h>

extern "C" {
#include <X11/Xlib.h>
}

BBitmap* _bbitmap_for_ximage(XImage* image, uint32 flags = 0);

0 comments on commit 957f90f

Please sign in to comment.