Error: bitblt writing outside image

[matzetronic]

Hi,

when I use the blockout option, I see the following error:

Error: bitblt writing outside image
    at Function.PNG.bitblt (.../test/testcafe/node_modules/testcafe-blink-diff/node_modules/pngjs/lib/png.js:147:11)
    at exports.PNG.PNG.bitblt (.../test/testcafe/node_modules/testcafe-blink-diff/node_modules/pngjs/lib/png.js:171:7)
    at fillPngRect (.../test/testcafe/node_modules/testcafe-blink-diff/lib/image-diff.js:42:13)
    at .../test/testcafe/node_modules/testcafe-blink-diff/lib/image-diff.js:73:11
    at Array.forEach (<anonymous>)
    at ImageDiff.run (.../test/testcafe/node_modules/testcafe-blink-diff/lib/image-diff.js:67:42)
    at runMicrotasks (<anonymous>)
    at processTicksAndRejections (internal/process/task_queues.js:95:5)

I tried to do a root cause analysis but I could not fully understand it. Here is what I know: The error comes from https://github.com/lukeapage/pngjs/blob/master/lib/png.js#L147 as the following applies in my case:

  if (
    deltaX > dst.width ||
    deltaY > dst.height ||
    deltaX + width > dst.width ||
    deltaY + height > dst.height // 87 + 1962 = 2049 > 2048 => error is thrown
  ) {
    throw new Error("bitblt writing outside image");
  }

y (87) is computed here

const y = (area.top * dpi) - 1; // 44 * 2 -1 = 87

and h (1962) here

const h = (area.height * dpi) + 2; // 980 * 2 +2 = 1962

dpi is 2 in my case. What I don't understand though is why you remove 1 pixel for x/y and add 2 for w/h 🤷🏻‍♂️ When I remove the -1/+2 it works without an error: 88 + 1960 = 2048 which is not >2048. So I wonder if the can be removed. If not something is wrong in your logic I think.

This is my complete blockOut.json:

[
    { "left": 0, "right": 43, "top": 44, "bottom": 1024, "width": 43, "height": 980 },
    { "left": 44, "right": 1280, "top": 0, "bottom": 44, "width": 1236, "height": 44 }
]

Let me know if you need more information, thanks!

[pateketrueke]

What I don't understand though is why you remove 1 pixel for x/y and add 2 for w/h 🤷🏻‍♂️

I moved the pixels a bit to fully remove the rectangle, otherwise you'll have some kind of glitches around it.

I think in your case you won't notice because you're reaching the image edges, but a blockOut area that can be in the middle of the image can present such glitches, like a visible border.

What we can do, is just use Math.min/max() to avoid reaching the image limits, what do you think?

[matzetronic]

What we can do, is just use Math.min/max() to avoid reaching the image limits, what do you think?

In my case this would not fix it:

const y = Math.max(0, (area.top * dpi) - 1); // 44 * 2 - 1 = 87
const h = Math.min(dstImage.height, (area.height * dpi) + 2); // min(2048, (980 * 2) +2) = 1962

So 1962 + 87 is still 2049 which is > 2048 => error 🤷🏻‍♂️

Since I'm new in the field of image processing I might need to think a little longer about it 😉

[matzetronic]

But does this make sense in any way?

const y = Math.max(0, (area.top * dpi) - 1);

🤔

[pateketrueke]

Yeah, I got your point more clearly, let me think about it... it is just arithmetic, but please forgive me as I dropped school when I was a child! 🤕

[pateketrueke]

But does this make sense in any way?

Definitely, if area.top = 0, dpi = 2 then (0 * 2) - 1 will result in -1 and that's wrong.

BTW, I double-checked the math and of course, the argument behind: without clamping the values you could end with an image like this:

The area above the glitch is the blockOut area, and while that area is transparent, below is blank: and the glitch is there, in between.

So the math used will only grow the given blockOut area with a pixel around it, to avoid these kind of glitches.

[matzetronic]

Yeah, I got your point more clearly, let me think about it... it is just arithmetic, but please forgive me as I dropped school when I was a child! 🤕

Then you deserve even more respect for creating such a great open source library 👍🏻 👏🏻

But does this make sense in any way?

Definitely, if area.top = 0, dpi = 2 then (0 * 2) - 1 will result in -1 and that's wrong.

Of course 🤦🏻‍♂️ for cases where the result is negative it makes sense, your are right 👍🏻

So the math used will only grow the given blockOut area with a pixel around it, to avoid these kind of glitches.

I like the fact that you move the computation of the blockout areas to a separate function 👍🏻 Perhaps you could give it a more meaningful name like getBlockOutArea.

Speaking of the method I think what you area currently missing is removing the extra pixels when computing width and height. So my take for the method would be:

function getBlockOutArea(image, area, dpi) {
  // if left|top were 0, then we would write outside the image!
  const x = Math.max(0, (area.left * dpi) - 1);
  const y = Math.max(0, (area.top * dpi) - 1);

  // if x+w|y+h > width|height, we would write outside the image!
  const w = Math.min(x + (area.width * dpi) + 2, image.width - x);
  const h = Math.min(y + (area.height * dpi) + 2, image.height - y);

  return {
    x, y, w, h,
  };
}

With this my error is gone. When I check my out.png it looks like the blockOut areas are now at the correct place and they are transparent. But the screenshot in out.png does not cover the complete page. Is this correct?!

[pateketrueke]

You've nailed it, thank you! I'll apply the feedback you gave me.

Update: oh noes, there's a flaw in your changes, because you're summing up the x/y to the final width/height and that's not good, because the blockOut area now is even larger than the original.

[pateketrueke]

But the screenshot in out.png does not cover the complete page. Is this correct?!

Yeah, this is fine, the generated image with the differences (if any) would not grow to the original size, but rather it will keep the size where all the differences are inside.

[matzetronic]

You've nailed it, thank you! I'll apply the feedback you gave me.

Update: oh noes, there's a flaw in your changes, because you're summing up the x/y to the final width/height and that's not good, because the blockOut area now is even larger than the original.

If it is larger than the original (image.width) it would use image.width minus x which is needed because otherwise that blockout area width could run out of the screen. For example:

x = 100; // this is where the blockOut area begins
area.width = 900; // a big area to block out
image.width = 1000;
// then
const w = Math.min(x + (area.width * dpi) + 2, image.width - x);
             //min(100 + (900 * 1) + 2 = 1002, 1000 - 100 = 900) => 900

Now in your fillPngRect method it would fill an area with a width of 900 starting at x = 100. Which is still inside image.width.

On the other hand if you have a smaller area that completely fits inside the screen:

x = 100; // this is where the blockOut area begins
area.width = 897; // a smaller area to block out
image.width = 1000;
// then
const w = Math.min(x + (area.width * dpi) + 2, image.width - x);
             //min(100 + (897 * 1) + 2 = 999, 1000 - 100 = 900) => 999 also inside image.width

But of course there is also a chance that I did not understand the following logic. I assume that you want to compute the x/y coordinates of the to be blocked out area and then insert that area in an image at that coordinates and the given width and height.

The same applies for h.

[matzetronic]

Ah now that I posted it I see an error:

//min(100 + (897 * 1) + 2 = 999, 1000 - 100 = 900) => 999 also inside image.width

the box itself would of course also contribute to the width and then the box would be written from 100 to 1099 wich is outside the image 😭. So x must not be added in the first case. But let me think about it 🤣

[matzetronic]

Perhaps this is it

  // if x+w|y+h > width|height, we would write outside the image!
  const w = Math.min((area.width * dpi) + 2, image.width - x);
  const h = Math.min((area.height * dpi) + 2, image.height - y);

What do you think?

[pateketrueke]

I like it, because it includes the correction as fallback, and it will pick the smaller value which always be good, right?

[matzetronic]

Yes, now the width of the box can never get wider than the image width minus the x position where it will be inserted so it should fit. The same for the height. Perhaps you can test it for yourself by adding a blockout area which is as wide/high as the screen. Then try to insert it anywhere where x/y > 0. It should not crash.

[matzetronic]

Though I find it a bit strange that I'm the only one who is using a block out area at the edges of the screen and that error never appeared before 🤔 🤷🏻‍♂️

[pateketrueke]

Well, I upgraded this library from blink-diff to pixelmatch+pngjs last week, and while working on it I discovered the dpi were needed to be considered. I ran a few tests saving the modified images to confirm and I noticed the glitches, so my solution was actually buggy as you experencied.

Probably more people will ran on this issue over time, so I am glad you found this problem soon! :beers: