Erode / dilate preliminary implementation. #1744
Conversation
|
Thank you very much for this PR. I expect to be able to take a closer look at it next week. |
|
@lovell Did you manage to have a look already? |
|
@WRidder you missed a comma in package.json. |
|
@rustyguts Thanks! |
|
Any update on this @lovell? If you want help with a review, I'm happy to help! Would love to have this merged in. |
There was a problem hiding this comment.
Thanks for bearing with me whilst I got around to looking at this PR in more detail. It's always a pleasure to see lots of test cases! I like the API you've proposed with its simple, automagic creation of a matrix as this doesn't prevent a future possible enhancement to provide a custom matrix as an array of values e.g. cross-shaped for a more rounded result. I've added a few minor comments inline, with the only one that I think we should really address being the addition of an upper bound on the width.
| if (!is.defined(width)) { | ||
| // No arguments: default to 1px dilation (3x3 mask) | ||
| this.options.dilateWidth = 1; | ||
| } else if (is.integer(width) && width > 0) { |
There was a problem hiding this comment.
We should place an upper bound on this. Any thoughts on a suitable value?
| // Numeric argument: specific width | ||
| this.options.dilateWidth = width; | ||
| } else { | ||
| throw new Error('Invalid dilation width (positive integer) ' + width); |
There was a problem hiding this comment.
Could use is.invalidParameterError here.
| @@ -1271,6 +1281,8 @@ NAN_METHOD(pipeline) { | |||
| baton->gammaOut = AttrTo<double>(options, "gammaOut"); | |||
| baton->linearA = AttrTo<double>(options, "linearA"); | |||
| baton->linearB = AttrTo<double>(options, "linearB"); | |||
| baton->dilateWidth = AttrTo<int32_t>(options, "dilateWidth"); | |||
| * Dilate an image | ||
| */ | ||
| VImage Dilate(VImage image, int const width) { | ||
| int maskWidth = 2*width + 1; |
| @@ -170,6 +170,44 @@ function blur (sigma) { | |||
| return this; | |||
| } | |||
|
|
|||
| /** | |||
| * Dilate the image. | |||
There was a problem hiding this comment.
It might be nice to expand a little about what this means and mention things like "morphology" and the use of binary/two-tone/greyscale images being more suitable.
|
Thanks for you comments! I'll have a look at your suggestions asap :) |
|
If you hadn't noticed, there's been an internal migration to N-API hence the conflicts. As always, happy to keep PRs open for as long as required. |
|
This PR has drifted a bit and there haven't been any updates in 18 months so I'm going to close it for now. Please feel free to re-open with updates or, for anyone else reading this, submit a new PR with this feature. Thanks again Wilbert for taking the time to work on this PR and improving sharp. |
Fixes #1719.
Initial attempt to add erode/dilate functionality. It works, but for some reason the morph dilate/erode functionality of libvips (https://jcupitt.github.io/libvips/API/current/libvips-morphology.html#vips-morph) seems to invert the image. While there is a reference to that behaviour in the documentation, I'd like someone to shed some light on that.
Open to any suggestions of course.