From e0ca00e55be81801827a5f3a8aef80c48a23202d Mon Sep 17 00:00:00 2001 From: Pietro Gagliardi Date: Sun, 5 Aug 2018 18:39:29 -0400 Subject: [PATCH] Resolved confusion about the terminology of strides in uiImageAppend(). Also prevents overallocation on some platforms. Thanks to @mischnic and @msink for spotting this. Update #402. --- darwin/image.m | 25 ++++++++++++------------- uitable.h | 9 ++++----- unix/image.c | 20 +++++++++++--------- unix/table.c | 2 ++ windows/image.cpp | 6 +++--- 5 files changed, 32 insertions(+), 30 deletions(-) diff --git a/darwin/image.m b/darwin/image.m index 28881ca3..9492cd07 100644 --- a/darwin/image.m +++ b/darwin/image.m @@ -30,28 +30,27 @@ void uiFreeImage(uiImage *i) uiprivFree(i); } -void uiImageAppend(uiImage *i, void *pixels, int pixelWidth, int pixelHeight, int pixelStride) +void uiImageAppend(uiImage *i, void *pixels, int pixelWidth, int pixelHeight, int byteStride) { NSBitmapImageRep *repCalibrated, *repsRGB; uint8_t *swizzled, *bp, *sp; - int x, y; + int n; unsigned char *pix[1]; // OS X demands that R and B are in the opposite order from what we expect // we must swizzle :( // LONGTERM test on a big-endian system - swizzled = (uint8_t *) uiprivAlloc((pixelStride * pixelHeight * 4) * sizeof (uint8_t), "uint8_t[]"); + swizzled = (uint8_t *) uiprivAlloc((byteStride * pixelHeight) * sizeof (uint8_t), "uint8_t[]"); bp = (uint8_t *) pixels; sp = swizzled; - for (y = 0; y < pixelHeight * pixelStride; y += pixelStride) - for (x = 0; x < pixelStride; x++) { - sp[0] = bp[2]; - sp[1] = bp[1]; - sp[2] = bp[0]; - sp[3] = bp[3]; - sp += 4; - bp += 4; - } + for (n = 0; n < byteStride * pixelHeight; n += 4) { + sp[0] = bp[2]; + sp[1] = bp[1]; + sp[2] = bp[0]; + sp[3] = bp[3]; + sp += 4; + bp += 4; + } pix[0] = (unsigned char *) swizzled; repCalibrated = [[NSBitmapImageRep alloc] initWithBitmapDataPlanes:pix @@ -63,7 +62,7 @@ void uiImageAppend(uiImage *i, void *pixels, int pixelWidth, int pixelHeight, in isPlanar:NO colorSpaceName:NSCalibratedRGBColorSpace bitmapFormat:0 - bytesPerRow:pixelStride + bytesPerRow:byteStride bitsPerPixel:32]; repsRGB = [repCalibrated bitmapImageRepByRetaggingWithColorSpace:[NSColorSpace sRGBColorSpace]]; diff --git a/uitable.h b/uitable.h index 8d069eb5..15f33ebe 100644 --- a/uitable.h +++ b/uitable.h @@ -36,13 +36,12 @@ _UI_EXTERN void uiFreeImage(uiImage *i); // uiImageAppend adds a representation to the uiImage. // pixels should point to a byte array of non-premultiplied pixels -// stored in [A R G B] order (so ((uint8_t *) pixels)[0] is the A of the +// stored in [R G B A] order (so ((uint8_t *) pixels)[0] is the A of the // first pixel and [3] is the B of the first pixel). pixelWidth and // pixelHeight is the size *in pixels* of the image, and pixelStride is -// the number *of pixels* per row of the pixels array. Therefore, -// pixels itself must be at least 4 * pixelStride * pixelHeight bytes -// long. -_UI_EXTERN void uiImageAppend(uiImage *i, void *pixels, int pixelWidth, int pixelHeight, int pixelStride); +// the number *of bytes* per row of the pixels array. Therefore, +// pixels itself must be at least byteStride * pixelHeight bytes long. +_UI_EXTERN void uiImageAppend(uiImage *i, void *pixels, int pixelWidth, int pixelHeight, int byteStride); typedef struct uiTableValue uiTableValue; diff --git a/unix/image.c b/unix/image.c index 3b5db020..eecdec18 100644 --- a/unix/image.c +++ b/unix/image.c @@ -34,24 +34,26 @@ void uiFreeImage(uiImage *i) uiprivFree(i); } -void uiImageAppend(uiImage *i, void *pixels, int pixelWidth, int pixelHeight, int pixelStride) +void uiImageAppend(uiImage *i, void *pixels, int pixelWidth, int pixelHeight, int byteStride) { cairo_surface_t *cs; unsigned char *buf, *p; uint8_t *src = (uint8_t *) pixels; - int cstride; - int y; + int cByteStride; + int n; - cstride = cairo_format_stride_for_width(CAIRO_FORMAT_ARGB32, pixelWidth); - buf = (unsigned char *) uiprivAlloc((cstride * pixelHeight * 4) * sizeof (unsigned char), "unsigned char[]"); + // unfortunately for optimal performance cairo expects its own stride values and we will have to reconcile them if they differ + cByteStride = cairo_format_stride_for_width(CAIRO_FORMAT_ARGB32, pixelWidth); + buf = (unsigned char *) uiprivAlloc((cByteStride * pixelHeight) * sizeof (unsigned char), "unsigned char[]"); p = buf; - for (y = 0; y < pixelStride * pixelHeight; y += pixelStride) { - memmove(p, src + y, cstride); - p += cstride; + for (n = 0; n < byteStride * pixelHeight; n += byteStride) { + memmove(p, src + n, cByteStride); + p += cByteStride; } + // also note that stride here is also in bytes cs = cairo_image_surface_create_for_data(buf, CAIRO_FORMAT_ARGB32, pixelWidth, pixelHeight, - cstride); + cByteStride); if (cairo_surface_status(cs) != CAIRO_STATUS_SUCCESS) /* TODO */; cairo_surface_flush(cs); diff --git a/unix/table.c b/unix/table.c index 9e6fe01e..d1e6fe60 100644 --- a/unix/table.c +++ b/unix/table.c @@ -2,6 +2,8 @@ #include "uipriv_unix.h" #include "table.h" +// TODO with GDK_SCALE set to 2 the 32x32 images are scaled up to 64x64? + struct uiTable { uiUnixControl c; GtkWidget *widget; diff --git a/windows/image.cpp b/windows/image.cpp index 0058aa13..368c77fa 100644 --- a/windows/image.cpp +++ b/windows/image.cpp @@ -39,14 +39,14 @@ void uiFreeImage(uiImage *i) uiprivFree(i); } -void uiImageAppend(uiImage *i, void *pixels, int pixelWidth, int pixelHeight, int pixelStride) +void uiImageAppend(uiImage *i, void *pixels, int pixelWidth, int pixelHeight, int byteStride) { IWICBitmap *b; HRESULT hr; hr = uiprivWICFactory->CreateBitmapFromMemory(pixelWidth, pixelHeight, - GUID_WICPixelFormat32bppRGBA, pixelStride, - pixelStride * pixelHeight, (BYTE *) pixels, + GUID_WICPixelFormat32bppRGBA, byteStride, + byteStride * pixelHeight, (BYTE *) pixels, &b); if (hr != S_OK) logHRESULT(L"error calling CreateBitmapFromMemory() in uiImageAppend()", hr);