NativeImage with scale factor is corrupted when passed through remote #276

When a NativeImage whose only representation has a scale factor other than 1 crosses the remote boundary, it arrives with the wrong dimensions and scrambled pixels. In Zulip Desktop this showed up as a striped, tiled tray badge on Linux with display scaling (zulip/zulip-desktop#1224), because the renderer passed a devicePixelRatio-scaled image to tray.setImage on a remote Tray.

Cause

For a single representation, serializeNativeImage in src/common/type-utils.ts sends the raw bitmap with image.getSize(scaleFactor):

const size = image.getSize(scaleFactor);
const buffer = image.toBitmap({ scaleFactor });
representations.push({ scaleFactor, size, buffer });

getSize returns the size in DIPs (ImageSkiaRep::GetWidth() is pixel_width() / scale()), but toBitmap returns physical pixels. deserializeNativeImage passes that size to addRepresentation({ buffer, scaleFactor, width, height }), which reads a raw buffer as width × height pixels. So a 200×200 bitmap at scale factor 2 is read as 100×100 pixels using a 100-pixel row stride, taking the first quarter of the buffer.

Both directions are affected, since valueToMeta and wrapArgs both use serialize(). Images with multiple representations go through the data URL path and are fine, as are images at scale factor 1.

Reproduction

@electron/remote 2.1.3, Electron 44.4.1, Linux:

const {app, BrowserWindow, nativeImage} = require("electron");
const remoteMain = require("@electron/remote/main");

remoteMain.initialize();

// 200x200 pixels at scale factor 2 (100x100 DIPs), each pixel unique.
const makeImage = `(nativeImage) => {
  const buffer = Buffer.alloc(200 * 200 * 4);
  for (let y = 0; y < 200; y++) {
    for (let x = 0; x < 200; x++) {
      buffer.writeUInt32LE((0xff000000 | (x << 8) | y) >>> 0, (y * 200 + x) * 4);
    }
  }
  return nativeImage.createFromBitmap(buffer, {width: 200, height: 200, scaleFactor: 2});
}`;

const expected = eval(makeImage)(nativeImage);

global.check = (image) => ({
  scaleFactors: image.getScaleFactors(),
  dipSize: image.getSize(2),
  pixelSize: image.toBitmap({scaleFactor: 2}).length / 4,
  identical: image.toBitmap({scaleFactor: 2}).equals(expected.toBitmap({scaleFactor: 2})),
});

app.whenReady().then(async () => {
  console.log("in main:   ", global.check(expected));
  const win = new BrowserWindow({
    show: false,
    webPreferences: {nodeIntegration: true, contextIsolation: false},
  });
  remoteMain.enable(win.webContents);
  await win.loadURL("about:blank");
  console.log(
    "via remote:",
    await win.webContents.executeJavaScript(`
      require("@electron/remote").getGlobal("check")(
        (${makeImage})(require("electron").nativeImage),
      );
    `),
  );
  app.quit();
});

Output:

in main:    { scaleFactors: [ 2 ], dipSize: { width: 100, height: 100 }, pixelSize: 40000, identical: true }
via remote: { scaleFactors: [ 2 ], dipSize: { width: 50, height: 50 }, pixelSize: 10000, identical: false }

At scale factor 1.5, the image arrives as 133×133 pixels instead of 200×200.

Possible fix

The public API doesn't expose a representation's pixel dimensions, and multiplying the DIP size by the scale factor is lossy because GetWidth() truncates. Keeping the raw bitmap for scale factor 1 and sending a PNG otherwise avoids that. AddImageSkiaRepFromBuffer tries PNG decoding first and takes the dimensions from the PNG, so deserializeNativeImage doesn't need to change:

     const size = image.getSize(scaleFactor);
-    const buffer = image.toBitmap({ scaleFactor });
+    const buffer = scaleFactor === 1
+      ? image.toBitmap({ scaleFactor })
+      : image.toPNG({ scaleFactor });
     representations.push({ scaleFactor, size, buffer });

With that change, the reproduction above prints identical: true at scale factors 1, 1.25, 1.5, and 2.