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

Why aren't these images equal? #437

Open
jpolitz opened this issue May 23, 2022 · 6 comments
Open

Why aren't these images equal? #437

jpolitz opened this issue May 23, 2022 · 6 comments

Comments

@jpolitz
Copy link
Member

jpolitz commented May 23, 2022

Consider this program:

fun triple(i :: Image) -> Image:
  beside(i, beside(i, i))
end

c = circle(50, "solid", "red")
bg = square(100, "outline", "black")

i1 = overlay(triple(c), triple(bg))
i2 = triple(overlay(c, bg))

check:
  overlay(triple(c), triple(bg)) is triple(overlay(c, bg))
end

It fails, and reports the images as not equal. Indeed:

image

However, images-difference, meant to calculate a quantitative difference based on differing pixels, cannot find a difference:

image

Is something structural happening in equality when they should be compared by pixel?

Need to investigate.

@blerner
Copy link
Member

blerner commented May 23, 2022

In chrome, the two images are different during the image testing process:
image
vs
image
(look super close at the black lines separating the tangency points between the circles)
But this seems only to be true during image equality (at https://github.com/brownplt/code.pyret.org/blob/horizon/src/web/js/trove/image-lib.js#L538, for I'm-not-sure-which slices) I don't know why we use that code, as compared to the code for images-difference (at https://github.com/brownplt/code.pyret.org/blob/horizon/src/web/js/trove/image-lib.js#L481-L493). @schanzer do you recall? I think this bit of the image library was ported straight from WeScheme...

(Note: I couldn't debug-test this in Firefox, since trying to put a breakpoint on the relevant lines of code caused Firefox to hang...but the program's output is the same in Chrome and Firefox, so I assume Chrome is giving me reasonable information here.)

@schanzer
Copy link
Contributor

images-difference was never part of the WeScheme image library, so I can't speak to that. But this doesn't seem like an image equality issue to me. I mean, as @blerner points out those images really do look different. This seems like a small bug in either overlay or beside for not being properly commutative, and possibly in image-difference for not noticing the bug.

@blerner
Copy link
Member

blerner commented May 24, 2022

Nono, not quite. Try Joe's code in the repl, and the images are identical. The two images I show there are the ones that are rendered by BaseImage.prototype.equals, which is different code than when we render images to show on screen, and also different code than images-difference. So I was asking about the code in the .equals method, which was part of the WeScheme library.

@schanzer
Copy link
Contributor

I was hoping to try it, but I can't access the REPL right now. :)

OK, if that's the case then it's absolutely a bug in .equals. I'll look through the code and report back...

@blerner
Copy link
Member

blerner commented May 24, 2022

After digging into this with @schanzer , we think the problem has to do with https://github.com/brownplt/code.pyret.org/blob/horizon/src/web/js/trove/image-lib.js#L516 and its use at https://github.com/brownplt/code.pyret.org/blob/horizon/src/web/js/trove/image-lib.js#L371, to deal with anti-aliasing. This is why images-difference renders differently than .equals does. Which one is right? Dunno... but that's the difference.

@schanzer
Copy link
Contributor

The antialiasing "fix" I installed was to work around inconsistencies in Chrome and certain versions of Firefox....in 2013. For all I know, those are long gone by now. I'd propose the following heuristic for determining which one is right:

  1. We should keep or nuke the antialiasing hack on line 371 based on which results in more false negatives
  2. Whichever one we pick should be duplicated in images-difference.

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

No branches or pull requests

3 participants