Incorrect behavior of overlayWith()

[Nateowami]

The situation is that, under certain reproducible circumstances, overlayWith() overlays at the wrong location. This is best demonstrated with an example:

// colors.png is 100x100, blue.png is 50x50
sharp('colors.png').overlayWith('blue.png', {
  top: 49,
  left: 40
}).toFile('out.png');

This works, but because top is set to 49, there is a 1px gap at the bottom. Changing top to 50 causes it to move all the way to the top of the image (probably gravity gets set to north for some reason).

I am guessing this is caused by an off-by-one-error (though that doesn't explain why it is overlayed at the top).

In any case, if the overlay cannot fit inside the image, it seems it should either throw an error, or it should simply clip so it fits (this would even allow negative coordinates). That is how JIMP works, and also the desired behavior in my case (I have improvised by dynamically cropping the overlay to only that area that should fit inside the image).

Of course, I can see why you may not want to do that, but I don't think suddenly jumping the image to the top is the intended behavior.

I'm attaching a minimal, complete, verifiable example so you can reproduce this yourself. Download, extract, and npm install. Then run node index.js and check the output in out.png.

[lovell]

The top and left parameters are zero-indexed, so a 50px high image overlaid at top: 50 will overflow into the 101st row of pixels.

Clip-to-fit would be my expected outcome here too, so I'll need to investigate why that's not occurring. Thanks for the report.

EDIT: I'm completely wrong here, sorry, a 50px high image overlaid at top: 50 will cover up to and including the last row of pixels, and will not overflow.

[Nateowami]

Perhaps I'm missing something here, but if I do top: 1 there will be a 1px space above the overlay. If I do top: 49 there is a 49px space above the overlay, a 50px overlay, and 1px space below it. Setting top to 50 would then create a 50px space above the overlay, followed by 50px of overlay, with 0px space beneath. No overflow into the 101st row.

Actually, in the context of a coordinate space like this, I'm not thinking in terms of zero-indexing or one-indexing, but in terms of distance from the edge. Not pixels, but points between pixels. Here's a diagram of a 2x2 image with the coordinates as I understand them:

Given that top: 49 leaves 1px of space at the bottom, I think top: 50 should use the last row but not overflow to the 101st row.

If clip-top-fit is added, then it will no longer be necessary to make sure the overlay is smaller than the target image.

[lovell]

It looks like there's a typo at common.cc#L394 - the following diff seems to fix it locally, so this is a confirmed bug.

-    } else if(x >= (inHeight - outHeight)) {
+    } else if(y >= (inHeight - outHeight)) {

Thanks for the test case - I'll be adding something very similar to the functional tests to guard against any regression.

[lovell]

Commit 28ce33f adds a test that would have caught this problem along with the fix. Thanks again for reporting this!

[Nateowami]

Thank you! That was pretty quick. I'll try to test it out soon. I think you can close this issue.

[lovell]

This fix is in v0.16.1, now available via npm.

[Nateowami]

@lovell Thanks, and fix confirmed on my end. I tried literally a few hours ago to install via the GitHub repo, but failed (Some build requirement failed to download. Probably just the firewall here in China). Just tried it now via npm and it works perfectly. 🎉