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

Monochrome "fallback" on Dynamic module not working #146

Closed
jywarren opened this issue Oct 18, 2017 · 5 comments
Closed

Monochrome "fallback" on Dynamic module not working #146

jywarren opened this issue Oct 18, 2017 · 5 comments

Comments

@jywarren
Copy link
Member

jywarren commented Oct 18, 2017

On the new Dynamic module, you put in an expression and it runs it on every pixel for each color. But if you haven't specified anything for a channel, it runs the "monochrome" expression on it. This logic is a bit odd and seems to be causing some errors.

Input (r-b)/(r+b)*128 + 128 or even just r into the Dynamic module here: https://publiclab.github.io/image-sequencer/examples/#steps=dynamic

And you get a black image. That code is here, and perhaps this line needs to be multiplied by 255?

https://github.com/publiclab/image-sequencer/blob/master/src/modules/Dynamic/Module.js#L18

Or perhaps it's something more subtle, like the R, G, and B channels evaluating to 0?

@jywarren
Copy link
Member Author

The contributors most likely able to help are:

@ccpandhare
@jywarren

@jywarren jywarren changed the title Monochrome fallback on Dynamic module not working Monochrome "fallback" on Dynamic module not working Oct 18, 2017
@ccpandhare
Copy link
Collaborator

Lines 31-35 ./src/modules/Dynamic/Module.js

    channels.forEach(function(channel) {
      if (options.hasOwnProperty(channel))
            options[channel + '_function'] = generator(options[channel]);
      ...
    });

Shouldn't that be

    channels.forEach(function(channel) {
      if (options.hasOwnProperty(channel) && options[channel] != "")
            options[channel + '_function'] = generator(options[channel]);
      ...
    });

Fixed it for me!

@jywarren
Copy link
Member Author

Ahh! great. Hey, if you'd like to make that change and push it to a branch with a name beginning with first-timer-, I think our new first-timers-bot will pick it up and make a FTO issue out of it! :-)))

Give it a try! New outreach technology :-)

@ccpandhare
Copy link
Collaborator

ccpandhare commented Oct 27, 2017 via email

@rishabhshuklax
Copy link
Member

fixed in #1451

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