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

Math Module #108

Closed
11 tasks
ccpandhare opened this issue Sep 3, 2017 · 20 comments
Closed
11 tasks

Math Module #108

ccpandhare opened this issue Sep 3, 2017 · 20 comments

Comments

@ccpandhare
Copy link
Collaborator

Hi, this is a first-timers-only issue. This means this has been worked to make it more legible to folks who either haven't contributed to our codebase before, or even folks who haven't contributed to open source before.

If you have contributed before, consider leaving this one for someone new, and looking through our general help wanted issues. Thanks!

The Feature

Add a new Image Sequencer Module which accepts a Math expression

This module will iterate over the pixels and for each pixel, manipulate it based on a given Math equation. For example

R = (R + G + B) / (3)
G = (R - G + B)
B = (R + G - B)

This is inspired from Infragram.org. Have a look at a similar functionality in action on Infragram here.

Towards the solution

Have a look at how to contribute Modules on ./CONTRIBUTING.md

You can also have a look at existing modules here : ./src/modules.

There already are modules which do pixel-by-pixel manipulation. For example: Invert, NdviRed, GreenChannel.

The code for any pixel-by-pixel manipulating module should be almost the same. Each of these have a changePixel method. For example, in the Invert Module :

    function changePixel(r, g, b, a) {
        // r,g,b,a are the channels of the input image.
        // What is being returned is an array of the Red, Green, Blue, Alpha channels of the final image.
        return [255-r, 255-g, 255-b, a];
    }

An Invert module is supposed to invert the R,G,B values of a given image pixel-by-pixel (i.e. r -> 255-r, and so on). The above changePixel does the same.

A (function) called pixelManipulation is made for facilitating the easy making of pixel-by-pixel manipulation modules. All the above examples use this.

The pixelManipulation function accepts an image, a changePixel function, an output function, image format, etc.

The above modules are sufficient to demonstrate its usage.

The Solution

  • Make a new directory in ./src/modules by the name of "Math"
  • In this directory, make the files "info.json" and "Module.js" as prescribed in the CONTRIBUTING.md.
  • Refer to the existing pixel-by-pixel manipulation modules as shown in the section above.
  • Design a changePixel function which takes in equations (For R,G,B) from the user and evaluates the expressions and implement it.
  • List your module in ./src/Modules.js (You won't be able to use it till then)
  • Update the build file by running $ grunt build
  • Make sure you didn't break anything by running $ npm test

Steps to Fix

  • Claim this issue with a comment here, and ask for any issues/clarifications.
  • Clone this repository locally.
  • Try to fix the issue as described in the "The Solution" section above, but even before you have completed doing so, you can:
  • Commit your work and initiate a Pull Request (Mark it as work-needed if you haven't completed the work on the issue)
@ccpandhare
Copy link
Collaborator Author

@jywarren Is this a bit too much for an FTO? I have tried to document it well though.
Maybe I should just keep it as Help Wanted. What do you say?

@ghost
Copy link

ghost commented Sep 3, 2017

Can I give this a shot?

@jywarren
Copy link
Member

jywarren commented Sep 3, 2017 via email

@ghost
Copy link

ghost commented Sep 3, 2017

Sure, thanks ! I will post my questions here if I am stuck.

@ghost
Copy link

ghost commented Sep 4, 2017

I have a question. The module takes arithmetic expressions as inputs and evaluates the expressions. Can I just use eval function for this or are there any other suggested libraries for this purpose?

@ccpandhare
Copy link
Collaborator Author

ccpandhare commented Sep 5, 2017 via email

@ghost
Copy link

ghost commented Sep 5, 2017

Can there be any mathematical operators in the equations or are the operators limited? And about nested parenthesis? So I need to write a complete mathematical expression parser here?

@shyamjesal
Copy link

i want to try too.

@ccpandhare
Copy link
Collaborator Author

ccpandhare commented Sep 5, 2017

Can there be any mathematical operators in the equations or are the operators limited? And about nested parenthesis? So I need to write a complete mathematical expression parser here?

@debeshadhikari Very complex math isn't required, Basic operations like + , - , * , / would do, including variables R, G, B as shown in the issue description

@shyamjesal Sure, why not! You can also have a look at other first-timer-only labelled issues (I don't think there are any more open issues; Will add more!)

@jywarren
Copy link
Member

jywarren commented Sep 5, 2017

@debeshadhikari @ccpandhare -- i think eval may be the easier route here and since it's just in the sandboxed client-side, there shouldn't be a problem. If people want to specify a math parser other than eval later, we can provide that as an overridable option, but I think eval is appropriate in the initial implementation, thanks!

@jywarren
Copy link
Member

jywarren commented Sep 5, 2017

As a follow-up issue, we might ask people to look through the npmjs.org listings for a math parser though!

@ghost
Copy link

ghost commented Sep 6, 2017

How do I handle if the user enters some wrong-syntax expressions? The expression will be evaluated as we are using eval and an error will be thrown stopping the script, should I put try/catch blocks around?

Are there any other easier ways to handle this situation??

Also, what should be the default value if no input is provided? I was thinking of keeping the initial RGB values or should I make it 0?

@ccpandhare
Copy link
Collaborator Author

ccpandhare commented Sep 6, 2017 via email

@jywarren
Copy link
Member

jywarren commented Sep 9, 2017

Excited to see this happening. Thanks and tell us if you get stuck!

@jywarren
Copy link
Member

jywarren commented Sep 9, 2017

Also, see a similar implementation you can try out at https://infragram.org/sandbox

@jywarren
Copy link
Member

Hi, @debeshadhikari - how are things going? any way we can help, or any code you could share in a pull request? Thanks!! 🎈

@ghost
Copy link

ghost commented Sep 25, 2017

Hi, I am extremely sorry for the very late response. I have been really busy and been travelling around for work. I have completed the task, and will send a pull request as soon as I settle back, which is in a couple of days. Again, I am really sorry for the delay.

@jywarren
Copy link
Member

No problem, glad to hear it's solved, and safe travels!

@ccpandhare
Copy link
Collaborator Author

ccpandhare commented Sep 27, 2017 via email

@jywarren
Copy link
Member

jywarren commented Oct 5, 2017

Hi, @debeshadhikari - hope your travels are going well. If you have a moment to open a PR that'd be great, and we'd be happy to help you debug or refine if any of that is needed. Thanks! We can even help rebase if that helps too.

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

No branches or pull requests

3 participants