Report doesn't find color difference

[brendonbribeiro]

I tried using misMatchThreshold 0 and 0.1. There's a big color difference between this two images.

Reference	Test
#fbfcfd	#F5F5F5

[garris]

Oh, sorry. There is an unintuitive quirk with zero for that parameter. We are currently re-factoring this section and will fix this in a future release. In the meantime you can just set it to .001 or even smaller. Hope that works!

[brendonbribeiro]

The issue

After some time of debugging, this seems to be the real issue:

In compare-resemble.js, the data.misMatchPercentage returned is 0.00. Then I read this useful links:

• https://github.com/garris/BackstopJS#modifying-output-settings-of-image-diffs
• https://github.com/HuddleEng/Resemble.js/blob/master/README.md

I though that this could be the problem:

By default, the comparison algorithm skips pixels when the image width or height is larger than 1200 pixels. This is there to mitigate performance issues

Setting largeImageThreshold to 0 didn't solve the problem as well

Then I start to look at node-resemble-js package, and find some interesting code...This package uses some default tolerance:

var tolerance = { // between 0 and 255
   red: 16,
   green: 16,
   blue: 16,
   alpha: 16,
   minBrightness: 16,
   maxBrightness: 240
};

If I change this tolerance, then the tests start to fail (as expected). I didn't find a way to set this configuration with backstopjs.

Conclusion

I would like to help by fixing this problem, and I found two possible solutions:

• Implement some configuration for this tolerance
• Remove node-resemble-js dependency and use https://github.com/HuddleEng/Resemble.js directly. This repository is 'LOOKING FOR MAINTAINER' and have not been updated recently.

[garris]

Very much appreciate your interest in helping. I have been looking at this library... https://github.com/mapbox/pixelmatch

What do you think?

[brendonbribeiro]

Pixelmatch looks awesome.

This weekend, I'll be working on this.

If we decide to use pixelmatch, resembleOutputOptions will not be available anymore.

• There's an issue for setting color output
  • https://github.com/mapbox/pixelmatch/issues/36

@garris Do you think that this is an issue?

[garris]

Great -- It would be very cool to integrate this! I personally don't care about resembleOutputOptions -- but we could see if the pixelmatch guys would accept a PR (looks trivial). If we do still keep this functionality, it should be remapped to diffOptions.

Also, I have developed an experimental image diffing algorithm which detects and handles layout movement changes. See below -- Note the change was to add a line of text in the second card. Pixelmatch (second from the right) behaves as you expect. The new diff (last on the right) aims to only flag the changed pixels...

If you have some interest to also add this after working on pixelmatch integration it would be great. But it's totally optional.

Cheers!

[brendonbribeiro]

This algorithm seems really good!

I was comparing Pixelmatch and Resemble, and I think that the second is a better choice for us. There's why:

• Resemble has a lot of cool diff settings:
  • See the demo page: http://huddleeng.github.io/Resemble.js/
• We could change resembleOutputOptions to something like diffOptions or resembleOptions, and this setting would allow the user to change not only the output, but the diff algorithm as well. See below:

const options = {
        output: {
            errorColor: {
                red: 255,
                green: 0,
                blue: 255
            },
            errorType: "movement",
            transparency: 0.3,
            largeImageThreshold: 1200,
            useCrossOrigin: false,
            outputDiff: true
        },
        scaleToSameSize: true,
        ignore: "antialiasing"
    };

• In our case, by default, we should use ignore: 'nothing'
• Don't need pngjs dependency
• Resemble looks more stable

What do you think?

[brendonbribeiro]

I Open a PR, there still a lot of work to do. With Resemble, the comparison part looks cleaner.

[zigotica]

Hi, I'm very interested in the status of this PR, any ETA? Thank you

[zigotica]

I think the problem might have a different root. Please have a look at the following image:

As you can see, in this case is not just a grey on white kind of subtle difference, but more importantly, the script returns a diff of 0.4 and still do not detect the change while misMatchThreshold: 0.01 (or even smaller). If we're capable of assigning diff a value of 0.4 we shuld be able to match that against misMatchThreshold to accept/reject the test.

More weird to that, when you click the reference image, which on the preview has a grey tick, it shows the pink tick as we have on the test image!

Please let me know if you need further info or test images.

Thank you @brendonbarreto for #852 PR (hey it has some conflicts) and @garris for creating backstop!

[brendonbribeiro]

More weird to that, when you click the reference image, which on the preview has a grey tick, it shows the pink tick as we have on the test image!

I noticed this when I created this issue. If you inspect the image, both the test and the reference points to the test folder.

@zigotica If you send me both images, I can test this in my branch, and try to understand better what's going on.

@garris Do you intend to merge #852 in a future major release? I thought that diverged would somehow replace the resemble and so my PR would not be needed anymore.

[zigotica]

More weird to that, when you click the reference image, which on the preview has a grey tick, it shows the pink tick as we have on the test image!

I noticed this when I created this issue. If you inspect the image, both the test and the reference points to the test folder.

Good, then that's just another issue, unrelated to the diff we're talking about. Sorry I hadn't looked at the page source.

@zigotica If you send me both images, I can test this in my branch, and try to understand better what's going on.

Great! Let me prepare them. Thank you so much!

[zigotica]

Hey @brendonbarreto here the images:

I compared all of them to the one without the tick, with these results: grey diff%: 0.02 blue diff%: 0.04 black diff%: 0.04 pink diff%: 0.04 green diff%: 0.04

Despite my settings include misMatchThreshold: 0.001 and diffs are greater than that, none of the tests would fail.

Thank you again for any help.

[garris]

@brendonbaretto

@garris Do you intend to merge #852 in a future major release? I thought that diverged would somehow replace the resemble and so my PR would not be needed anymore

Many apologies! I got very busy at work and didn't see your changes! Please give me some time to rereview-- obviously this is a big changes so we need to make sure it will not disrupt current users. Also, diverged is experimental-- it will not replace conventional diff for some time.

Also, @zigotica has a very good point -- we should investigate this! Perhaps all that is broken is the threshold evaluation?

[zigotica]

Also, @zigotica has a very good point -- we should investigate this! Perhaps all that is broken is the threshold evaluation?

That's my main suspect, yes, but would need to dig into the code. Probably the threshold is doing something like "if diff x and diff y are smaller than threshold, pass", instead of comparing diff% to the threshold.

[brendonbribeiro]

@zigotica @garris Tomorrow I'll investigate this, and fix conflicts as well. Thanks you all!

[zigotica]

no man, thank you both, again

[zigotica]

I don't really know about the details, and as such I might be wrong, but I'd swear the suspect is https://github.com/garris/BackstopJS/blob/f9070c5c388bb3c173ea1370cce7bfa33215de84/core/util/compare/compare-hash.js#L23

[brendonbribeiro]

@zigotica Can you test your scenario in my branch? I made some changes today.

yarn add backstopjs@brendonbarreto/BackstopJS#replacing-node-resemble-js or npm install --save backstopjs@brendonbarreto/BackstopJS#replacing-node-resemble-js

Or give me you configuration file, so I can test myself.

[zigotica]

Sorry @brendonbarreto I cannot run your script, npm dependencies system is a complete mess (not your fault) :)

If I install it in package.json, it would run the global backstopjs. If then I remove the global instance to use the one in my node_modules there's not way the script can run. I'm missing smth, any help appreciated.

[brendonbribeiro]

Can't you install backstopjs locally and run it by using npx backstop?

[zigotica]

No because somehow it breaks my yarn dev (smth webpack related). I will isolate this in a barebones folder, will keep you posted

[zigotica]

Sorry @brendonbarreto a bit embarrassed but there's no way I can run the local backstopjs, only the 3.7.0, any step-by-step help would be appreciated.

[zigotica]

OK got it! took me longer than expected, sorry. @brendonbarreto Same problem with your branch, diff% 0.04, misMatchThreshold: 0.01, and test passes. I used the no-tick image as reference, then the blue tick. Hope that helps.

[brendonbribeiro]

Hey @zigotica, sorry for the delay in answering (got busy at work). I didn't manage to reproduce your issue. Can you somehow share your scenario (config and public url)?

[zigotica]

hi @brendonbarreto sorry for the delay.

you mean you cannot reproduce the "false pass" using my images from https://github.com/garris/BackstopJS/issues/850#issuecomment-433117854 ? Not in your branch and not even in master? Weird, then must be something in my side then, but we're not using anything special and it certainly works except for the issue described above. Here's my config for the record:

const config = {
  id: 'backstop_default',
  viewports,
  onReadyScript: 'onReady.js',
  scenarios: createScenarios(),
  paths: {
    bitmaps_reference: 'backstop_data/bitmaps_reference',
    bitmaps_test: 'backstop_data/bitmaps_test',
    engine_scripts: 'backstop_data/engine_scripts',
    html_report: 'backstop_data/html_report',
    ci_report: 'backstop_data/ci_report',
  },
  report: ['CLI', 'browser'],
  engine: 'puppeteer',
  engineFlags: [],
  asyncCaptureLimit: 1,
  asyncCompareLimit: 50,
  debug: false,
  debugWindow: false,
  misMatchThreshold: 0.01,
  resembleOutputOptions: {
    errorColor: {
      red: 255,
      green: 0,
      blue: 150,
    },
    transparency: 0.3,
  },
};

This is executed from a ES script that creates the scenarios in createScenarios(). This shouldn't affect, since it works for everything except this. Cannot provide a public URL, sorry, but I run the tests to these images alone, nothing else.

Thank you again for all.

[zigotica]

For the record, still pretty sure the problem is the threshold evaluation. Been doing quite a lot of small changes, all using misMatchThreshold: 0.01, and I'm consistently getting wrong PASSED up until diff%: 0.10 diff-x: 0 diff-y: 0. When changes are slightly bigger, from diff%: 0.11 diff-x: 0 diff-y: 0 onwards, they're correctly evaluated as a FAILED.

[zigotica]

OK I found the problem @brendonbarreto

Not sure how it might happened though. Whatever I set for misMatchThreshold as seen above is neglected, it still uses the default misMatchThreshold: 0.1. I added a console.log on compare-resemble.js and is is comparing the data.misMatchPercentage <= misMatchThreshold but misMatchThreshold "is" 0.1 at that point.

[zigotica]

BINGO, sorry all the noise, I was setting the misMatchThreshold on two places, the only valid being inside the scenario. Please excuse my stupidity...

[brendonbribeiro]

Nice @zigotica, happy to hear that you find the issue.

[garris]

Just quickly posting a reference to a resolved issue here as it's a bit related. https://github.com/garris/BackstopJS/issues/310#issuecomment-433967093

Gotta run. Thanks!