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

@@ -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 (n = 0; n < byteStride * pixelHeight; n += 4) {
-		for (x = 0; x < pixelStride; x++) {
+		sp[0] = bp[2];
-			sp[0] = bp[2];
+		sp[1] = bp[1];
-			sp[1] = bp[1];
+		sp[2] = bp[0];
-			sp[2] = bp[0];
+		sp[3] = bp[3];
-			sp[3] = bp[3];
+		sp += 4;
-			sp += 4;
+		bp += 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]];
 

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,
+// the number *of bytes* per row of the pixels array. Therefore,
-// pixels itself must be at least 4 * pixelStride * pixelHeight bytes
+// pixels itself must be at least byteStride * pixelHeight bytes long.
-// long.
+_UI_EXTERN void uiImageAppend(uiImage *i, void *pixels, int pixelWidth, int pixelHeight, int byteStride);
-_UI_EXTERN void uiImageAppend(uiImage *i, void *pixels, int pixelWidth, int pixelHeight, int pixelStride);
 
 typedef struct uiTableValue uiTableValue;
 

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 cByteStride;
-	int y;
+	int n;
 
-	cstride = cairo_format_stride_for_width(CAIRO_FORMAT_ARGB32, pixelWidth);
+	// unfortunately for optimal performance cairo expects its own stride values and we will have to reconcile them if they differ
-	buf = (unsigned char *) uiprivAlloc((cstride * pixelHeight * 4) * sizeof (unsigned char), "unsigned char[]");
+	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) {
+	for (n = 0; n < byteStride * pixelHeight; n += byteStride) {
-		memmove(p, src + y, cstride);
+		memmove(p, src + n, cByteStride);
-		p += cstride;
+		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);

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;

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,
+		GUID_WICPixelFormat32bppRGBA, byteStride,
-		pixelStride * pixelHeight, (BYTE *) pixels,
+		byteStride * pixelHeight, (BYTE *) pixels,
 		&b);
 	if (hr != S_OK)
 		logHRESULT(L"error calling CreateBitmapFromMemory() in uiImageAppend()", hr);