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

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

B2F
Copy link

@B2F B2F commented Apr 22, 2015

@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:

  • options.lightness: increase diff visibility, default to +25 so there is no more swallowed black pixels)

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

  • options.rgb: set the diff area color balance with a rgb array (ex: [0,0,255])
  • options.stack: stack common pixels on the image instead of a black area

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 !

…dds options.ligthness, options.rgb and options.stack to customize the diff image.

Updates Jasmine ImageDiffSpec with new diff options
@B2F
Copy link
Author

B2F commented Apr 23, 2015

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

@SyntaxStacks
Copy link

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

@B2F
Copy link
Author

B2F commented May 11, 2015

@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
Copy link

Those sound fine, send it on up 👍

@B2F
Copy link
Author

B2F commented May 14, 2015

PR updated, features list:

  • options.lightboost: increases differences visibility with a light boost.
  • options.diffColor: a rgb array used to specify differences color instead of light gap.
  • options.stack: stacks differences on top of the original image, preserving common pixels.

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

@@ -313,13 +313,28 @@ describe('ImageUtils', function() {

it('should calculate difference', function () {
a = imagediff.createImageData(1, 1),
a.data[1] = 200,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

semicolon instead of a comma at the end of the line? I haven't come across this syntax before.

@SyntaxStacks
Copy link

👍

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

Successfully merging this pull request may close these issues.

None yet

2 participants