-
Notifications
You must be signed in to change notification settings - Fork 209
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added Edge Detection module #168
Conversation
Signed-off-by: tech4GT <varun.gupta1798@gmail.com>
Oh, this is cool! Could you try running it on the image here and writing back in that thread? I'd like to get some input from these folks: This is so cool! @ccpandhare what do you think? |
|
@jywarren right now this is basic edge detection we can use algorithms like hysteresis to improve the quality of edge detection😁 |
@jywarren since jpg does not support transparency , can i convert an input jpg file to png and convert the output back to jpeg, will that be an acceptable solution? |
This is so cool!!!
@jywarren One more off the bucket list. :-)
…On Sat, Feb 10, 2018, 03:17 Varun Gupta ***@***.***> wrote:
@jywarren <https://github.com/jywarren> since jpg does not support
transparency , can i convert an input jpg file to png and convert the
output back to jpeg, will that be an acceptable solution?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#168 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AT0xnL91o0muRqlQ17SS7I9lCIO_JG8sks5tTLzWgaJpZM4SAXrx>
.
|
Why isn't this working with JPEG?
On Sat, Feb 10, 2018, 08:30 Chinmay Pandhare <pandharechinmay882@gmail.com>
wrote:
… This is so cool!!!
@jywarren One more off the bucket list. :-)
On Sat, Feb 10, 2018, 03:17 Varun Gupta ***@***.***> wrote:
> @jywarren <https://github.com/jywarren> since jpg does not support
> transparency , can i convert an input jpg file to png and convert the
> output back to jpeg, will that be an acceptable solution?
>
> —
> You are receiving this because you were mentioned.
>
>
> Reply to this email directly, view it on GitHub
> <#168 (comment)>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AT0xnL91o0muRqlQ17SS7I9lCIO_JG8sks5tTLzWgaJpZM4SAXrx>
> .
>
|
Oh this is cool! I echo chinmay's question!
Please post to the plastics question, they'll be interested to see this!!!
On Feb 9, 2018 10:03 PM, "Chinmay Pandhare" <notifications@github.com>
wrote:
… Why isn't this working with JPEG?
On Sat, Feb 10, 2018, 08:30 Chinmay Pandhare ***@***.***
>
wrote:
> This is so cool!!!
>
> @jywarren One more off the bucket list. :-)
>
> On Sat, Feb 10, 2018, 03:17 Varun Gupta ***@***.***>
wrote:
>
>> @jywarren <https://github.com/jywarren> since jpg does not support
>> transparency , can i convert an input jpg file to png and convert the
>> output back to jpeg, will that be an acceptable solution?
>>
>> —
>> You are receiving this because you were mentioned.
>>
>>
>> Reply to this email directly, view it on GitHub
>> <#168#
issuecomment-364578404>,
>> or mute the thread
>> <https://github.com/notifications/unsubscribe-auth/
AT0xnL91o0muRqlQ17SS7I9lCIO_JG8sks5tTLzWgaJpZM4SAXrx>
>> .
>>
>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#168 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABfJ1my4MdkjnQ2-qpKdVU3fZd64lP8ks5tTQbegaJpZM4SAXrx>
.
|
@jywarren @ccpandhare actually jpegs do not support transparency values and edge detection requires that |
@jywarren should i post the complete code on the plastics question? |
Cool! I think you could just post the image of edges and link back here.
…On Feb 10, 2018 7:14 AM, "Varun Gupta" ***@***.***> wrote:
@jywarren <https://github.com/jywarren> should i post the complete code
on the plastics question?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#168 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABfJ2rQxtYsg43hFkUvrJY4Vro5VT1wks5tTYgQgaJpZM4SAXrx>
.
|
@jywarren done😁 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, maybe we need to specify exactly what the output of the module is. It's a greyscale image?I guess I'm more accustomed to seeing edges as black pixels surrounding white regions but this is cool.
@jywarren right now i have just implemented Gaussian blur with sobel filter on the image after conversion to greyscale hence basic edge detection, i will implement remaining steps for canny edge detection in future which will finally output image with black edges surrounding white regions |
@jywarren right now this outputs a greyscale image you are right😁 |
That's awesome. Im thinking some of this explanation could go into the
module description to help people understand what it does and what the
output means. What do you think?
…On Feb 10, 2018 10:21 AM, "Varun Gupta" ***@***.***> wrote:
@jywarren <https://github.com/jywarren> right now this outputs a
greyscale image you are right😁
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#168 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABfJ2GcjToe4cgA83HIMeao3lvqJrQWks5tTbO3gaJpZM4SAXrx>
.
|
@jywarren sure no problem but right now I don't have access to a computer I'll do it a bit later if that's fine?😅 |
Oh of course. Me neither, I'm on my phone :-)
…On Feb 10, 2018 10:40 AM, "Varun Gupta" ***@***.***> wrote:
@jywarren <https://github.com/jywarren> sure no problem but right now I
don't have access to a computer I'll do it a bit later if that's fine?😅
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#168 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABfJ66tN4KQejjLa-46ybR15fyxSfgxks5tTbhQgaJpZM4SAXrx>
.
|
Signed-off-by: tech4GT <varun.gupta1798@gmail.com>
@jywarren please have a look if anything else needs to be added, rest assured this is ready to be merged and i will surely open issues for enhancing this module and adding tests, thanks a lot😁 |
@jywarren is there anything else i can do in this😅 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apart from this small thing, this looks fine to me!
What do you say, @jywarren?
src/modules/EdgeDetect/info.json
Outdated
@@ -0,0 +1,11 @@ | |||
{ | |||
"name": "Detect Edges", | |||
"description": "Detects the edges in an image and outputs a a greyscale image with the canny method", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please fix the double "a" in the description field's value
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, I apologize for being that picky!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ccpandhare no apologies needed in fact thank you for being so observant😁
Also, how would the edges thing work? Would it be like thresholding the greyscale output image of this module or would it have an entirely different approach? |
Signed-off-by: tech4GT <varun.gupta1798@gmail.com>
@ccpandhare i was thinking once i implement complete canny edge detection we can get the edge pixels and output the positions, what say? |
Hi, this is looking really good; i wasn't previously familiar with how edge detection works, so I had to do some research to understand fully. With that in mind, I think we might add this to the description just for clarity:
Do we also allow setting the threshold? https://en.wikipedia.org/wiki/Canny_edge_detector#Parameters Finally, the Wikipedia page shows a black background instead of alpha. Would it be possible to do this too? Then it'd work in JPG too. This is really impressive work! Potentially we could base a Blur module off of components of this too! |
@jywarren thanks a lot and yes we already have everything for the Gaussian blur so we can create a module for that. |
Signed-off-by: tech4GT <varun.gupta1798@gmail.com>
@jywarren i have also taken care of the changes that @ccpandhare mentioned😁 |
Signed-off-by: tech4GT <varun.gupta1798@gmail.com>
Signed-off-by: tech4GT <varun.gupta1798@gmail.com>
@jywarren finally done with this, this was a tough one😅. Full canny edge detection🎉 |
@jywarren now this also takes low and high threshold and we also have a edge pixel positions array which can be exported as json as well |
Signed-off-by: tech4GT <varun.gupta1798@gmail.com>
Hi, this sounds great and looks great. If you can make a follow-up issue to make the background black, and for any other follow-up issues, then this sounds good and I'm merging it in! Thanks! |
I just wanted to say that this was a very impressive project! 🎉 🚀 👍 |
@jywarren thanks a lot😁 |
@jywarren please have a look , right now exporting the edge detected image, will create a separate issue for adding support for json output, what say?