Image-sequencer: Correct default view for brightness module

[aashna27]

We are preparing to participate in Google Code-in, and have reserved this issue for participants in GCI - but we'd love to have your help with another one! Please check out https://code.publiclab.org to see more.

Problem

The brightness module when selected displays a white screen in the image area instead of the image from the previous step or the selected image.

However the default view should be something like this

Instance Count = 1

Thank you!

Your help makes Public Lab better! We deeply appreciate your helping refine and improve this site.

To learn how to write really great issues, which increases the chances they'll be resolved, see:

https://publiclab.org/wiki/developers#Contributing+for+non-coders

[gitmate[bot]]

GitMate.io thinks the contributors most likely able to help are @jywarren, and @SidharthBansal.

Possibly related issues are https://github.com/publiclab/image-sequencer/issues/113 (Add a Brightness module), https://github.com/publiclab/image-sequencer/issues/472 (refactor/speed up Brightness module (and other PixelManipulation modules)), https://github.com/publiclab/image-sequencer/issues/434 (Rotate module), https://github.com/publiclab/image-sequencer/issues/108 (Math Module), and https://github.com/publiclab/image-sequencer/issues/112 (Scale module).

[gitmate[bot]]

GitMate.io thinks the contributors most likely able to help are @jywarren, and @SidharthBansal.

Possibly related issues are https://github.com/publiclab/image-sequencer/issues/113 (Add a Brightness module), https://github.com/publiclab/image-sequencer/issues/472 (refactor/speed up Brightness module (and other PixelManipulation modules)), https://github.com/publiclab/image-sequencer/issues/434 (Rotate module), https://github.com/publiclab/image-sequencer/issues/108 (Math Module), and https://github.com/publiclab/image-sequencer/issues/112 (Scale module).

[oorjitchowdhary]

I'd like to work on this.. Can I please get help regarding the code links for the main image sequencer page and the brightness module .. Thanks

[oorjitchowdhary]

Our task here is to show the image in the brightness box right?

[aashna27]

http://sequencer.publiclab.org/examples/#steps=

http://sequencer.publiclab.org/examples/#steps=brightness{brightness:100}
Go ahead!! 😄

[aashna27]

yes basically the image coming from the previous step should be visible when brightness module is added.

[oorjitchowdhary]

yes basically the image coming from the previous step should be visible when brightness module is added.

Ok

[oorjitchowdhary]

http://sequencer.publiclab.org/examples/#steps=

http://sequencer.publiclab.org/examples/#steps=brightness{brightness:100}
Go ahead!!

Sorry if I wasn't clear.. I meant to ask for the code files which would be required here... I'm not so familiar with this repo...

[aashna27]

http://sequencer.publiclab.org/examples/#steps=
http://sequencer.publiclab.org/examples/#steps=brightness{brightness:100}
Go ahead!!

Sorry if I wasn't clear.. I meant to ask for the code files which would be required here... I'm not so familiar with this repo...

https://github.com/publiclab/image-sequencer/tree/main/src/modules/Brightness
I hope this works for you. This is basically the folder of this module.

[oorjitchowdhary]

@aashna27 Yes, Thanks

[oorjitchowdhary]

One more thing, I can see two instances of this issue on the GCI dash... Pls remove one as it leads to a lot of confusion..
@jywarren @sidharthbansal

[CodeSarthak]

Yes, @oorjitchowdhary , there are two instances of this on GCI. Should be removed asap to avoid confusion.

[JonathanXu1]

Yep there are still the two tasks available on the dashboard. @oorjitchowdhary are you still working on it?

[oorjitchowdhary]

@jonathanxu1 yes I'm working on it

[JonathanXu1]

@oorjitchowdhary you also seem to be working on issue #487 . Keep in mind that according to GCI, you can only work on one issue at a time

[oorjitchowdhary]

@JonathanXu1 Yeah I know.. my #487 got approved so I'll do this now...

[oorjitchowdhary]

@aashna27 I'm stuck here.. Through my changes in the code, it takes the load-image as the image in brightness but somehow doesn't show the file uphead... It is only visible after a change in the brightness .. Check out the gif which explains this..

[aashna27]

@aashna27 I'm stuck here.. Through my changes in the code, it takes the load-image as the image in brightness but somehow doesn't show the file uphead... It is only visible after a change in the brightness .. Check out the gif which explains this..

I am busy with a submission today so if anyone from @publiclab/mentors could have a look till then.

[oorjitchowdhary]

Okay sure

[oorjitchowdhary]

@aashna27 Could you help me..

[aashna27]

@aashna27 Could you help me..

Yeah sure. I ll look for the bug.

[oorjitchowdhary]

https://github.com/publiclab/image-sequencer/blob/main/src/modules/Convolution/Module.js
https://github.com/publiclab/image-sequencer/blob/main/src/modules/Brightness/Module.js
There is no difference between the modules of brightness and convolution apart from the different functions.. But still we can see the picture by using convolution whereas it doesn't show up in brightness

[oorjitchowdhary]

I'm unable to understand this..

[oorjitchowdhary]

@aashna27 @Paarmita

[oorjitchowdhary]

Anyone pls help.. I have my deadline today

[tech4GT]

What’s the problem, can you please elaborate? 😄

[oorjitchowdhary]

@tech4GT with no changes in the respective code modules.. The brightness module doesn't show the picture whereas the contrast, convolution.. All show the picture from the previous step...

[tech4GT]

I guess that’s because the default value is set to 0 no?

[tech4GT]

Maybe make it 50?

[oorjitchowdhary]

@tech4GT I'm unable to change it through the brightness module

[oorjitchowdhary]

The default .. where do I change it?

[jywarren]

It should be in the info.json file inside /src/modules/Brightness/, I
believe!

On Thu, Nov 29, 2018 at 11:19 AM Oorjit Chowdhary [e-mail]
wrote:

The default .. where do I change it?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/publiclab/image-sequencer/issues/491#issuecomment-442894895,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AABfJ0oMOQcHuiT4Hj5lMA3JZxliqitvks5u0AjogaJpZM4Yv36a
.

[oorjitchowdhary]

It should be in the info.json file inside /src/modules/Brightness/, I believe!

If I edit it there, nothing happens.. It still shows brightness=100 in the default

[jywarren]

Did you run grunt build after making the change? That will compile
everything in /src/ into the final /dist/ directory.

On Thu, Nov 29, 2018 at 11:35 AM Oorjit Chowdhary [e-mail]
wrote:

It should be in the info.json file inside /src/modules/Brightness/, I
believe!

If I edit it there, nothing happens.. It still shows brightness=100 in the
default

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/publiclab/image-sequencer/issues/491#issuecomment-442901612,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AABfJx_4PEMyHYrszoHIarqpeX8zkemGks5u0AyfgaJpZM4Yv36a
.

[oorjitchowdhary]

Oh.. lemme check that out

[jywarren]

ah, have you installed all modules with npm install? It looks like it's
missing a dependency. Great work sticking with this, you'll get it soon,
i'm sure!

On Thu, Nov 29, 2018 at 11:52 AM Oorjit Chowdhary [e-mail]
wrote:

[image: screenshot 39]
https://user-images.githubusercontent.com/39333058/49237825-164e5600-f425-11e8-9fd4-e9cc7c47ffc9.png
Pls help me with this...

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/publiclab/image-sequencer/issues/491#issuecomment-442908247,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AABfJ9CrLh-mFiJSU47CJ99dSSdaQAAbks5u0BCRgaJpZM4Yv36a
.

[tech4GT]

You’ll have to change it in Module.js file of the brightness module

[tech4GT]

https://github.com/publiclab/image-sequencer/blob/ddf4bd0dae5bb40e8144b2a10979e59dc12752ed/src/modules/Channel/Module.js#L6
This is how you can add that default value.
Something like

Options.brightness = options.brightness || 0

[oorjitchowdhary]

This is how you can add that default value.
Something like

Options.brightness = options.brightness || 0

Yes I figured that out.. That doesn't work out too..

[oorjitchowdhary]

@tech4GT @jywarren @aashna27 As I see it, when the brightness module is added it doesn't take the input from the previous output until you change the brightness and click the save button... If you add the brightness module and then click on the image it simply downloads the blank white screen as the image.. But after you change the brightness, it takes the previous output as the input and then you could download the image showing the edited output..

I may not sound very clear.. I'll make a gif to demonstrate

[oorjitchowdhary]

Brightness module takes input as blank white screen on default...

[oorjitchowdhary]

The output of previous step is only taken as input when you change the brightness through the bar..

[oorjitchowdhary]

The output of previous step is only taken as input when you change the brightness through the bar..

So when you download this one, it downloads the edited picture

[tech4GT]

Hmm I would have too look through this in code then, I’ll see this as soon as I get some time. 🙂

[oorjitchowdhary]

So what should I do for my gci task. I have 12 hrs left..

[tech4GT]

Oh, sorry I did not know that, I’ll look through this in an hour or two then. Don’t worry we’ll get this done. :)

[oorjitchowdhary]

Okay

[tech4GT]

@oorjitchowdhary This looks like a browser issue to me, this works fine in my browser!

[oorjitchowdhary]

@tech4GT so the issue is solved in your browser? That is umm.. Surprising 🤔

[bhavayAnand9]

Confirmed this issue with chrome, safari.

[oorjitchowdhary]

@tech4GT @jywarren @aashna27 If it is a browser issue, then the code is working fine and any changes to it aren't required... Right?
So, I believe, I should submit my task for review 🤔

[bhavayAnand9]

This isn't a browser issue. I figured out what is going wrong
Try console.log the value of val by adding this code console.log(val) after var val = (options.brightness)/100.0 this line in Brightness module.js, you'll find the val variable is NaN and that's why there's a blank image is displaying.
So if you hardcode the value of val like give it 1.5 it'll work just as expected.

[tech4GT]

I wonder why it works on my computer though?

[bhavayAnand9]

If you do a fresh build & run and then add the first step as brightness then only this issue will occur.

[oorjitchowdhary]

@bhavayAnand9 So i have to the rgb values to 1.5? and add that console.log?

[bhavayAnand9]

@oorjitchowdhary No, not rgb values but val variable which is declared in line 23 in module.js. and this isn't a fix, instead just a way to help you figure out what is going wrong and where.

[oorjitchowdhary]

Uhh

[oorjitchowdhary]

So i remove (options.brightness)/100.0 and put it as 1.5

[oorjitchowdhary]

So i remove (options.brightness)/100.0 and put it as 1.5

Not working

[bhavayAnand9]

I think this'll fix it.

Explanation:
val value is dependent on option.brightness which is a parameter to Brightness function. If you console log options.brightness you'll find it undefined, which I don't know why that is.

so adding this line options.brightness = options.brightness || 100; will set the option.brightness value to 100 only if it's undefined.

@oorjitchowdhary Here's what you need to do. Just add these two lines of code in starting of 'changePixel' function.

//If options.brightness is undefined, set it to 100 by default
options.brightness = options.brightness || 100;

GIve it a try and let me know If it worked.

[oorjitchowdhary]

@bhavayAnand9 Thanks a ton... Cheers 😄

[oorjitchowdhary]

This is how you can add that default value.
Something like

Options.brightness = options.brightness || 0

Yes I figured that out.. That doesn't work out too..

Wow.. I had tried that long ago.. but placed it in above the function changePixels 😕 😆

[jywarren]

oh wow, so were you all able to figure this out? That's awesome!

On Fri, Nov 30, 2018 at 10:55 AM Oorjit Chowdhary [e-mail]
wrote:

This is how you can add that default value.
Something like

Options.brightness = options.brightness || 0

Yes I figured that out.. That doesn't work out too..

Wow.. I had tried that long ago.. but placed it in above the function
changePixels 😕 😆

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/publiclab/image-sequencer/issues/491#issuecomment-443247110,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AABfJyFBWl_Bu6tvfRH8amPvBHctVb8Kks5u0VTmgaJpZM4Yv36a
.

[oorjitchowdhary]

@jywarren yes.. TEAMWORK = OPEN SOURCE = 💯🔥

[oorjitchowdhary]

https://codein.withgoogle.com/tasks/6322686255955968/?sp-organization=6519545549291520&sp-is_beginner=False

Another thing.. There is duplicate of this same task.. It should be removed asap

[jywarren]

Great! did this. 🙌 👍 💯