API: `sharp.overlayWith` (alpha compositing) #207

gasi · 2015-04-30T06:38:06Z

Adds sharp.overlaysWith for compositing an image on top of another image while handling alpha channels correctly.

I left all commits intact for better understanding but feel free to merge as one squash commit, @lovell.

This is now ready for review.

Tests and usage examples: test/unit/overlay.js

TODO

~~Test with non-PNGs.~~ Non-4-channel images are not supported.
~~Add support for specifying top and left position as in sharp.overlaysWith(filename, top, left). This might be useful for watermarking.~~ Punted.
Benchmark.
Investigate (slight) discrepancy to ImageMagick convert -composite output. Not visible to human eye (at least not mine 😉) but detectable by compare ….

References

See: #97.

gasi · 2015-04-30T06:39:10Z

CONTRIBUTING.md

@@ -30,6 +30,8 @@ Please select the `master` branch as the destination for your Pull Request so yo

 Please squash your changes into a single commit using a command like `git rebase -i upstream/master`.

+To test C++ changes, you can compile the module using `npm run compile`.


This was useful to me and hope it will be to others 😄

This will compile but not test the changes. I usually use npm i to do both.

Oh, you’re right, I didn’t know npm install runs node-gyp on the package itself. I always tried npm rebuild but that only recompiles dependencies. However, you said it ‘will […] test the changes’. This doesn’t happen for me (see below). Am I missing something?

npm i > [email protected] install /Users/dani/workspace/sharp > node-gyp rebuild CXX(target) Release/obj.target/sharp/src/common.o CC(target) Release/obj.target/sharp/src/composite.o CXX(target) Release/obj.target/sharp/src/utilities.o CXX(target) Release/obj.target/sharp/src/metadata.o CXX(target) Release/obj.target/sharp/src/resize.o CXX(target) Release/obj.target/sharp/src/sharp.o SOLINK_MODULE(target) Release/sharp.node SOLINK_MODULE(target) Release/sharp.node: Finished

Sorry, my mistake. Judging by the command line history npm i && npm t will do both.

No worries, I documented npm install and npm test 👍

lovell · 2015-04-30T09:07:48Z

Thanks again Daniel for all your work on this very useful and much-requested feature.

I'm not sure I want to go down the route of using *magick for image comparisons. My experience of managing CI environments warns me otherwise (I'm looking at you, AppVeyor) and, more importantly, I believe libvips can do this with minimum effort.

I'll spend some time this afternoon to see what I can do using libvips to get #122 off the ground.

lovell · 2015-04-30T16:12:15Z

Commit c159948 on the knife branch adds a fixtures.assertSimilar method you can use instead of *magick's "compare".

By way of example, all of the tests for the extract method now employ it.

gasi · 2015-05-01T06:29:05Z

Commit c159948 on the knife branch adds a fixtures.assertSimilar method you can use instead of *magick's "compare".

Awesome! Thanks for providing this and I agree it’s more elegant without the GraphicsMagick dependency. However, I have two observations:

I found a case where the similarity test should fail: c9301c1
Isn’t it valuable for some operations like alpha compositing to do a comparison using no tolerance? If you agree, I could add that as parameter to assertSimilar.

gasi · 2015-05-01T06:29:42Z

.jshintrc

@@ -4,6 +4,7 @@
  "maxparams": 4,
  "maxcomplexity": 13,
  "globals": {
+    "before": true,


Although I don’t use this anymore, I figure it might be valuable for the future 😄

gasi · 2015-05-01T06:33:49Z

src/composite.h

+/*
+  Composite images `src` and `dst`.
+*/
+int Composite(VipsObject *context, VipsImage *src, VipsImage *dst, VipsImage **out);


Getting used to your 140 line length limit. I usually operate at 80 out of habit 😉

People who use an 80 limit do so to leave room at the side of their monitor for Twitter.

Haha, good one 😜 On a more serious note: I do it because I like to do three-way merges on my laptop 👍

Then there is science 😉 http://baymard.com/blog/line-length-readability

gasi · 2015-05-02T04:20:13Z

@lovell I made another pass on this today and it should be ready to merge given the restrictions we talked about (only RGBA images can be composited). I might be able to add more functionality at some point but for now, I think this might be already valuable for some people 😄

lovell · 2015-05-02T10:55:03Z

src/composite.c

+  vips_object_local(context, joined);
+
+  // Return a reference to the output image:
+  g_object_ref(joined);


I think this g_object_ref can be removed as vips_object_local will reference joined.

lovell · 2015-05-02T10:58:30Z

Thanks again for all your work on this Daniel. Other than the possible reference leak, this is definitely ready to merge into the knife WIP branch. Are you able to squash to a single commit?

Composites an overlay image with alpha channel into the input image (which must have alpha channel) using ‘over’ alpha compositing blend mode. This API requires both images to have the same dimensions. References: - http://en.wikipedia.org/wiki/Alpha_compositing#Alpha_blending - libvips/ruby-vips#28 (comment) See lovell#97.

gasi · 2015-05-04T03:39:40Z

Other than the possible reference leak, this is definitely ready to merge into the knife WIP branch. Are you able to squash to a single commit?

Thanks, I simplified the out reference as you suggested and squashed everything into a single commit 😄 It’s been a pleasure collaborating with you and I hope I’ll be able to contribute more in the future.

coveralls · 2015-05-04T03:44:44Z

Coverage decreased (-0.45%) to 92.56% when pulling 6c682e8 on gasi:overlays into de67ab6 on lovell:knife.

gasi · 2015-05-04T18:57:35Z

I forgot to add overlayWith documentation to README. Will do that later today.

Initial version of overlayWith method for alpha compositing

lovell · 2015-05-05T08:48:11Z

No worries on the docs. I intend to add the ability to overlay onto non-alpha images too and will update the README after that. Thanks again @gasi for all your work on this new feature!

gasi · 2015-05-05T18:26:15Z

Thanks, @lovell, I appreciate it 👍

gasi reviewed Apr 30, 2015
View reviewed changes

gasi mentioned this pull request Apr 30, 2015

Feature request: overlay images with transparency (was: Resize, combine multiple images) #97

Closed

gasi force-pushed the overlays branch from d9b795b to 8e05ea2 Compare April 30, 2015 07:20

gasi force-pushed the overlays branch from f3ff154 to c9301c1 Compare May 1, 2015 06:29

gasi reviewed May 1, 2015
View reviewed changes

gasi force-pushed the overlays branch from c9301c1 to 8f6d31d Compare May 1, 2015 06:33

gasi reviewed May 1, 2015
View reviewed changes

lovell reviewed May 2, 2015
View reviewed changes

gasi force-pushed the overlays branch from 057acca to e3c3ffd Compare May 4, 2015 03:35

gasi changed the title ~~API: sharp.overlaysWith (alpha compositing)~~ API: sharp.overlayWith (alpha compositing) May 4, 2015

gasi force-pushed the overlays branch from e3c3ffd to 6c682e8 Compare May 4, 2015 03:38

lovell added a commit that referenced this pull request May 5, 2015

Merge pull request #207 from gasi/overlays

0f8bb4e

Initial version of overlayWith method for alpha compositing

lovell merged commit 0f8bb4e into lovell:knife May 5, 2015

gasi deleted the overlays branch May 7, 2015 05:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

API: `sharp.overlayWith` (alpha compositing) #207

API: `sharp.overlayWith` (alpha compositing) #207

gasi commented Apr 30, 2015

gasi Apr 30, 2015

lovell Apr 30, 2015

gasi Apr 30, 2015

lovell Apr 30, 2015

gasi Apr 30, 2015

lovell commented Apr 30, 2015

lovell commented Apr 30, 2015

gasi commented May 1, 2015

gasi May 1, 2015

gasi May 1, 2015

lovell May 1, 2015

gasi May 1, 2015

gasi May 1, 2015

gasi commented May 2, 2015

lovell May 2, 2015

lovell commented May 2, 2015

gasi commented May 4, 2015

coveralls commented May 4, 2015

gasi commented May 4, 2015

lovell commented May 5, 2015

gasi commented May 5, 2015

		@@ -30,6 +30,8 @@ Please select the `master` branch as the destination for your Pull Request so yo

		Please squash your changes into a single commit using a command like `git rebase -i upstream/master`.

		To test C++ changes, you can compile the module using `npm run compile`.

API: sharp.overlayWith (alpha compositing) #207

API: sharp.overlayWith (alpha compositing) #207

Conversation

gasi commented Apr 30, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lovell commented Apr 30, 2015

lovell commented Apr 30, 2015

gasi commented May 1, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gasi commented May 2, 2015

Choose a reason for hiding this comment

lovell commented May 2, 2015

gasi commented May 4, 2015

coveralls commented May 4, 2015

gasi commented May 4, 2015

lovell commented May 5, 2015

gasi commented May 5, 2015

API: `sharp.overlayWith` (alpha compositing) #207

API: `sharp.overlayWith` (alpha compositing) #207