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

Provide a global tolerance when comparing for equality #36

Open
wants to merge 3 commits into
base: v1.1
Choose a base branch
from

Conversation

cburgmer
Copy link
Contributor

@cburgmer cburgmer commented Jun 9, 2014

This PR tries to solve #18.

As proposed it implements an optional parameter set along the following example:

{
  tolerance : 0.05,
  totalTolerance : true
}

Following those changes imagediff.equal(a, b, 128) would be expressed through imagediff.equal(a, b, {tolerance: 0.5}) in the future (however the old API remains intact).
A global tolerance can be given through adding totalTolerance: true to the option object.

@cburgmer
Copy link
Contributor Author

cburgmer commented Jun 9, 2014

My thoughts: I did want to follow your suggestion and see how things pan out. However, I am not happy with the API. There are a few things:

  • Providing a by-pixel-tolerance through a rate from 0 to 1 seems pretty pointless and counter-intuitive. And on top of that, do we divide by 255 or by 256? Technically only values up to 255 exist, but 256 feels natural and would give "exact" floats (as far as I understand floating point arithmetic with powers of two) making comparison easy.
  • Providing globalTolerance: true feels cumbersome.
  • It is unclear/not well communicated how the globalTolerance: false implementation works.
  • Providing new ways of providing tolerance values would in the future collide with the rather fixed API above.

Will try to come up with an alternative idea.

@cburgmer
Copy link
Contributor Author

Here's my current proposal:

imagediff.equal(a, b, {toleranceValue: 5, toleranceMethod: 'relativePerPixel'});

and

imagediff.equal(a, b, {toleranceValue: 0.25, toleranceMethod: 'totalRatio'});

@cesutherland
Copy link
Member

Can you give me an example of relativePerPixel? Would that be a total difference of 5 across the rgba values for a single pixel?

@cburgmer
Copy link
Contributor Author

Sorry for the late response.

relativePerPixel is the current implementation, where any pixel maybe different as long as it each absolute difference stays below the threshold.

totalRation is what I'd like to add, where an accumulated difference is used.

I'm not sure about the proposed API anymore though. I think it would be easier if we'd provide a method on it's own for the different tolerances?

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.

2 participants