composite: top/left values can be erroneously clipped

[SHG42]

What are you trying to achieve?
Extract image data from a 900x700px KonvaJS stage with layered canvases, send to server and composite the layer data buffers with another image data buffer for a sprite that was created on a separate, single-canvas 500x500px Konva stage. On the backend, Sharp receives the most recent buffers from both stages and "sandwiches" the buffer from the smaller stage in between the two buffers from the larger stage. I am attempting to use the absolute position of a sprite on the larger canvas to set the top/left parameters for the composite function.

Sharp returns a buffer which is then passed to an upload method for Cloudinary, which creates the final png file upon upload.

Extra details: the smaller stage ("creator stage") allows for the creation of a game sprite, and the larger stage ("equip stage") allows the user to equip items in front of and behind the sprite to create a scene. On the equip stage, the base sprite is loaded by itself onto a middle canvas layer, and the user can drag it around to position it relative to its items (for instance, standing on top of a fallen log or next to a lake). On form-submit from the equip page, the absolute position of the base sprite on the equip stage is retained in the sprite's MongoDB document to be used as the top/left parameter in the composite function.

The problem: In cases where the base sprite's bounds went past the equip canvas's bottom bound or right-side bound, the sprite's positioning in the composited png is incorrect. To my eye, it looks like the base sprite in the composite is being pushed up and/or left by whatever amount its bounding box has gone past the canvas's bottom or right bounds.

Have you searched for similar questions?
Yes, here and on StackOverflow. The problem is that I can't tell where the issue is coming from. It could be coming from Sharp, KonvaJS, the Cloudinary upload processing, or my code. There are no errors thrown at any point. Console logs at every step along the way haven't been conclusive enough to narrow down what part of the process is at fault. I've searched for solutions relating to each step and none have helped.

Are you able to provide a minimal, standalone code sample that demonstrates this question?
This CodeSandbox replicates the functionality of the main project as closely as possible but without database functionality. If the database could be a factor in the problem, a link can be provided upon request to view the full source code.

Are you able to provide a sample image that helps explain the question?
Here is a Drive folder containing screenshots for all cases.

[lovell]

There's a lot going on here. Please can you provide minimal, standalone code where sharp is the only dependency as well as sample input images that fail in this manner.

(It also looks like you've added Cloudinary secrets in the public codesandbox so you may need to revoke those.)

[SHG42]

Hi, thanks for your response. Thank you for the tipoff about hiding the Cloudinary secrets. This issue is part of a much larger project, so I felt it prudent to include some level of context in my question, so it would be clear what the intended functionality is. I tried to keep the explanation as concise as possible, it seems perhaps I should have pared it down even more. To that same end, I felt it prudent that the previously included CodeSandbox should retain as much pared-down functionality as possible, so that the problem could be seen in its proper context.

Here is an additional Sandbox that's been pared down even more.. It was necessary to retain one canvas, along with Multer, because that is where the input buffer for Sharp comes from. One of the two input buffers has been stored as a file; the other must come from the canvas. Storing both input buffers as files proved unfeasible due to file size limitations. Please let me know if this is sufficient, and thanks for your patience. Here are two images demonstrating the problem. There are additional screenshots for other cases in the previously linked Drive folder.

[lovell]

Thank you for the simplified example, that's much clearer and I can see it's hitting this logic in sharp that clips the value of top.

sharp/src/common.cc

Lines 700 to 704 in 2020839

	if (y < (inHeight - outHeight)) {
	top = y;
	} else if (y >= (inHeight - outHeight)) {
	top = inHeight - outHeight;
	}

This clipping logic isn't required for most composite tasks so I think the following diff should fix this:

--- a/src/pipeline.cc
+++ b/src/pipeline.cc
@@ -594,8 +594,13 @@ class PipelineWorker : public Napi::AsyncWorker {
           int top;
           if (composite->hasOffset) {
             // Composite image at given offsets
-            std::tie(left, top) = sharp::CalculateCrop(image.width(), image.height(),
-              compositeImage.width(), compositeImage.height(), composite->left, composite->top);
+            if (composite->tile) {
+              std::tie(left, top) = sharp::CalculateCrop(image.width(), image.height(),
+                compositeImage.width(), compositeImage.height(), composite->left, composite->top);
+            } else {
+              left = composite->left;
+              top = composite->top;
+            }

Happy to accept a PR with this change and a unit test, if you're able.

[SHG42]

Thanks for looking into it! I'm new to working with PRs but I'll give it a shot and see what the result is, and I'll get back to you!

[SHG42]

Hi, thanks for waiting, I've been busy with my day job and didn't have an opportunity to dive into this till today. As I mentioned, I'm new to working with PRs so I wanted to get clarification on a couple things.

1. Could you explain the bug in a little more detail? You said my code is running afoul of Sharp's clipping logic, what exactly is happening when that occurs?
2. The diff is the only code I'm changing, correct? I'm removing the red-highlighted code and replacing it with the green-highlighted code in src/pipeline.cc and making no other changes?
3. To test the fix in my project, I set up the sandbox example code locally and went to node_modules/sharp/src/pipeline.cc and swapped in the diff, and I still get the same bugged output. Did I need to do something differently to test the proposed fix in my code?

[lovell]

It looks like https://github.com/SHG42/sharp/commit/b78b95b7d1d746e99524aa034e538dc115e682e9 has copied the + symbols from the diff, which will prevent compilation. To recompile the C++ locally you'll need to run npm install --build-from-source.

[lovell]

I've just added a note about the need to use the --build-from-source flag to the docs via 0c1075c as I realised this rather important info was missing.

Thanks for attempting to tackle this - please do let me know if you need any more help.

[SHG42]

Oh, interesting! I'll take another shot at it, thanks!

[SHG42]

Well, this was my first encounter with C++, but I was able to successfully make the changes and run npm test! However, when I tried to integrate the revised code into my demo, I still get the same incorrect output. No difference. I downloaded the source code from my Sandbox and set it up locally, then I replaced pipeline.cc in demo/node_modules/sharp/src with the revised file. Do I need to do something different to test the fix in my demo?

[lovell]

Perhaps you could copy your modified, compiled node_modules/sharp/build/Release/sharp.node file into your demo?

[SHG42]

It worked! That solves the bug! What is the next step for the PR?

[lovell]

v0.28.0 now available, thank you very much for both reporting and fixing this.