HumbleSoftware / js-imagediff

JavaScript / Canvas based image diff utility with Jasmine matchers for testing canvas.
http://humblesoftware.github.com/js-imagediff/
MIT License
788 stars 99 forks source link

Fixes #34 (Diff swallows black color in added areas): adds options.ligth... #52

Open B2F opened 9 years ago

B2F commented 9 years ago

@see PR features update (changes default values and options names but below screenshots still relevant).

The fix for #34 is the same but with improved factoring and commented code:

I also improved #51 with two more or less useful functionalities found in resemble.js.

As examples, I'll give you some tests I've made from images found at http://humblesoftware.github.io/js-imagediff/.

1) .example.normal, default behavior without options, the +25 boost is not much altering the original rendering in this case: normal-a--testcase--1366x768

2) .example.transparency, +25 default boost is giving a slightly white transparent background: normal-a--testcase--1366x768

3) .example.size, still default behavior, I just set option.align manually to top: normal-a--testcase--1366x768

4) .example.normal, now setting options.lightness to 255 (all differences are clearly visible): normal-a--testcase--1366x768

5) .example.transparency, using options.rgb with [0,0,255] to give a blue tint in area zone: normal-a--testcase--1366x768

6) .example.normal, setting options.stack and options.rgb to [185,0,185] giving a purple rendering on common pixels like with resemble.js: normal-a--testcase--1366x768

7) .example.size, option.align center and options.stack and yellow balance (options.rgb=[255,255,0]): normal-a--testcase--1366x768

The previous images are generated by Succss, ask me if you want the configuration file to reproduce locally. Thanks !

B2F commented 9 years ago

I updated Jasmine specs to take the new options into account, adding three new steps. All pass locally.

SyntaxStacks commented 9 years ago

This is awesome, I'm currently having an issue with this as well. Any idea on when this will be merged?

B2F commented 9 years ago

@SyntaxStacks thanks for your feedback on this. I think it could speed up the process if I make the Jasmine tests pass without altering existing specs, Carl would like to increase the version number before merging the pull request because of this.

Since the pull request is still available for review, I was thinking of replacing the new options: First I propose to rename option.lightness to option.lightboost because the option is increasing the light value not resetting it and I'll set its default value to 0 thus not altering the specs. Also, in its current state option.rgb is a kind of color balance but it could be relavant to change it to pure diff coloring (above 6 and 7 screenshots can be interpreted like this already), option.lightboost will be applicable to it (unlike currently) and option.rgb could be renamed to option.diffColor also more relevant.

I already have a local update accordingly and I would like to update the PR with it, are you ok with these changes ?

SyntaxStacks commented 9 years ago

Those sound fine, send it on up :+1:

B2F commented 9 years ago

PR updated, features list:

Existing specs are unaltered, it should be good for next release. We need Carl's review though.

SyntaxStacks commented 9 years ago

:+1: