search

publiclab / image-sequencer

A pure JavaScript sequential image processing system, inspired by storyboards
https://sequencer.publiclab.org
GNU General Public License v3.0
110 stars 210 forks source link

Histogram Module test passes even though it is not working #1536

Open root00198 opened 4 years ago

root00198 commented 4 years ago

Please describe the problem (or idea)

The test written in test/core/modules/histogram.js passes even though the benchmark and result image produced are different.

According to mee the issue is looks-same module. Somehow it is not returning the correct value. Another question is why are we using the looks-same module when we can directly compare the dataURI?

Runnig only the histogram module test. output

What happened just before the problem occurred? Or what problem could this idea solve?

What did you expect to see that you didn't? The image for benchmark and result should have been same.

Please show us where to look

https://beta.sequencer.publiclab.org

What's your PublicLab.org username?

This can help us diagnose the issue:

Browser, version, and operating system

Many bugs are related to these -- please help us track it down and reproduce what you're seeing!

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

root00198 commented 4 years ago

@publiclab/is-reviewers @HarshKhandeparkar @jywarren @blurry-x-face....

rishabhshuklax commented 4 years ago

What happened when you compared the base64 string of two images instead of comparing it with looks-same? Were the tests still passing?

root00198 commented 4 years ago

When I directly compared the base64, the test failed as it should.

rishabhshuklax commented 4 years ago

Didn't you removed looks-same in #1530 or #1497 for the same reason?

root00198 commented 4 years ago

I did removed the looks-same module in #1497, but then #1490 came and there were a lot of conflicts, so I recreated a new PR, keeping the looks-same module and working only on the GIF's rather combining couple of problems together.

rishabhshuklax commented 4 years ago

Have you thought of any alternative to looks-same?

rishabhshuklax commented 4 years ago

How about pixelmatch, this looks promising! @HarshKhandeparkar?

root00198 commented 4 years ago

According to me, this problem can be solved if we directly compare the base64 of the images... But before making changes we need to answer 2 questions... Can same image have two different base64? Can two different images have same base64?

jywarren commented 3 years ago

Can we re-try this now that we're on GitHub Actions and not Travis? The whole testing setup was tweaked. Thanks!