Complete Edge Detect Module

[harshkhandeparkar]

Fixes #176

• Hysteresis is implemented(it slows down the browsers very significantly)
• Black background
• General refactoring

It Works!!

Old:

• This output had a lot of transparency(alpha = 0)
• Thick edges
• No hysteresis

New:

• Black background
• No transparency
• Pure white edges
• Thin edges

Note: The default high and low threshold ratio values are not necessarily meant to be the best. They are to be found empirically for different cases.

Note: Higher high threshold takes less time to process but it's lossier (some edges are neglected).

Note: Lower low/high threshold takes very long to process and can render a browser unresponsive.

Note: Low threshold ratio can be greater than the high threshold ratio since it is multiplied to the high threshold value. i.e low_threshold_ratio = low_threshold_value/high_threshold_value and high_threshold_ratio = high_threshold_value/maximum_gradient

Note: Here value is the actual value which is compared to the gradients and ratio is a relative ratio of different values.

It is slow though

It takes very long to iterate through all the edges. This algorithm/module should be mainly used on servers. This can benefit a lot from the REST API project which is under development.

@publiclab/is-reviewers @jywarren

Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!

• tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR -- or run tests locally with npm test
• code is in uniquely-named feature branch and has no merge conflicts
• PR is descriptively titled
• ask @publiclab/reviewers and @publiclab/is-reviewers for help, in a comment below

---

Thanks!

[jywarren]

Ah, did you manage to add the addition so that hysteresis defaults to false, and is an option (with checkbox?) Thank you!!!

[harshkhandeparkar]

I will add everything as soon as I get time so I was asking for all the required suggestions.

[harshkhandeparkar]

@jywarren I have rebased and made all the changes. A demo will be available here

[harshkhandeparkar]

Please try to merge this before the others. Thank you!

[harshkhandeparkar]

Oh yea, please be sure to clear the cache.

[harshkhandeparkar]

An updated link demo will be available here. Sorry for the inconvenience.

[harshkhandeparkar]

@jywarren I think this is ready to merge. I have checked everything. It works. Below is hysteresis v/s non-hysteresis comparison(left image is without hysteresis).

Thank You!

cc @tech4GT @publiclab/is-reviewers

[harshithpabbati]

Hey Harsh the UI looks different in mobile.

[harshkhandeparkar]

Hello harshith, try clearing the cache and do not use the new UI. And css will be fixed by @aashna27

[harshithpabbati]

EDIT: image removed as it covers the whole screen
check this out. I cleared the cache.

[harshkhandeparkar]

I removed your image because it was covering the screen. That issue will be fixed by @aashna27 in #867

[harshithpabbati]

If you want I can make the changes in css.

[harshkhandeparkar]

Atleast not now. As I turned off my pc and won't be able to do anything. You can open a pr in my gh-pages branch if you want

[harshkhandeparkar]

This current PR focuses on edgeDetect so css isn't much important. If you still want to improve the demo, you can. Thank you.

[jywarren]

This looks great; I did find a small typo. Did you want to add a quick test to this using looks-like to protect it against future bugs? I think that's a really good idea now that the project is becoming more complex. Thanks!

[jywarren]

And noting that it passed in almost the same amount of time:

this branch: Module edge-detect ran in: 2672 milliseconds
main: Module edge-detect ran in: 2709 milliseconds

[harshkhandeparkar]

It takes a lot less time without hysteresis. Maybe it ran faster here as there is no different gradient and there are less pixels to be preserved. I.e binary image. I'm not sure though.

[harshkhandeparkar]

For the tests, maybe we can open an issue for creation of looks-same tests for all modules.

[jywarren]

Hi Harsh, i'm OK with merging this without a test, but I think in general we should be transitioning to requiring a test with every module before merging it, rather than doing it in follow-up, as a matter of policy. What do you think?

[harshkhandeparkar]

Yes sure. Thanks. I can add tests to all the remaining modules once I am free and maybe add the new policy to the contributing file.

[jywarren]

Great, thank you! Really nice work here!

[harshkhandeparkar]

Thanks a lot!!! @harshithpabbati this was just merged. Try running tge update script when you get time. Let's see if the changes are visible there.

[harshithpabbati]

Sure @harshkhandeparkar I will run it now.