Skip to content
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

Threshold module, not working yet #123

Closed
wants to merge 1 commit into from

Conversation

jywarren
Copy link
Member

Hi, @ccpandhare -- just going through the steps here to add a module again -- re-adding the thresholding module -- and ran into some trouble. I see the error:

Uncaught TypeError: Cannot read property 'src' of undefined
    at drawStep (image-sequencer.js:39450)
    at image-sequencer.js:39459
    at Image.onLoad (image-sequencer.js:40267)

Any idea what I'm missing here? This is also a good "shakedown test" possibly related to #122 before we announce this more widely and completely standardize the module format. Thanks!

@jywarren
Copy link
Member Author

Ooh, yikes. This is all messed up; i was pretty sure i was working from master but it looks like i was working from gh-pages... i'll fix this.

@jywarren
Copy link
Member Author

There, rebased and cleaned it up.

@@ -25,7 +31,13 @@ module.exports = function ImageThreshold(options) {
}).then(function (imageData) {
image = new Image();
image.onload = function onLoad() {
if (options.output) options.output(image);
this.output = {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is being used in the wrong context; The output needs to be set for the "step" object. Like this:

function draw (inputImage) {
  var step = this;
  ...
  ...
  ...
      image = new Image();
      image.onload = function onLoad() {
        step.output = {
                                  format: input.type, 
                                  src: image.src
                               }
       options.step.output = step.output.src;
       callback();
       UI.onComplete(options.step);
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, let's refactor existing modules to save this into var step to avoid this ambiguity.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, i see that existing modules are already this way! Great, it must just be that the Threshold module wasn't.

canvas.width = inputImage.naturalWidth;
canvas.height = inputImage.naturalHeight;
canvas.width = image.naturalWidth;
canvas.height = image.naturalHeight;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will work only on browsers, right? I think it's pretty easy to write a threshold module - we could as well do it without using the native Image and Canvas classes.

Copy link
Collaborator

@ccpandhare ccpandhare left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this can be done without the native Image and Canvas classes to ensure Node.js support.

The issue was that the output of the step was not being set as a property of "step"; A problem of scope. I think this brings more focus on #122. It sure is really confusing to create a module. I shall look into it.

@jywarren
Copy link
Member Author

OK -- i mean, we can achieve this using PixelManipulation, for example. Shall I refactor for that?

Also - we should have a section in Contributing Modules that talks about Node/Browser compatibility with this in mind, so it's more clear to people, and we can also link to that section when people submit modules like this. We can have some possible solutions in that section too. Adding this suggestion to #122.

@ccpandhare
Copy link
Collaborator

ccpandhare commented Sep 27, 2017 via email

@ccpandhare
Copy link
Collaborator

Will it be okay for me to do this?

@jywarren
Copy link
Member Author

jywarren commented Oct 13, 2017 via email

@ccpandhare
Copy link
Collaborator

Introduction

Have been doing some research on this and I came across the following :

I wanted to have a look at the algorithm so as to make a node-and-browser compatible version.

I think there is much more to thresholding than we had currently implemented.

There's Full RGB Quantization, Plane-by-plane RGB Quantization, etc.

Notes

Single Channel Thresholding

When there is only one channel, per pixel the channel value is made into max or min based on whether it is above a certain threshold value.

Multiple Channel (Color Input, Color Output) Thresholding

When there are multiple channels,

  • The above can be done per channel. (Plane by Plane RGB Quantization)
  • OTSU's Method can be used. (Full RGB Quantization)

Multiple Channels (Color Input, Black/White Output) Thresholding

  • Method 1 : Separate thresholds for R,G,B,A and then AND them
  • Method 2 : Convert to HSLA and then do the above

Method 1 above segments the image as per how the camera stores it, but not how a human would view it. Method 2 is how a human brain would analyze the image.

Further Steps

So what do you think we should do, @jywarren ? All of these, some of these?

@jywarren
Copy link
Member Author

jywarren commented Oct 18, 2017 via email

@ccpandhare
Copy link
Collaborator

ccpandhare commented Oct 18, 2017 via email

@grvsachdeva
Copy link
Member

@jywarren @tech4GT

@aashna27 aashna27 mentioned this pull request Jan 27, 2019
4 tasks
@harshkhandeparkar
Copy link
Member

harshkhandeparkar commented Mar 3, 2019

Threshold module added already in #722.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants