code.pyret.org icon indicating copy to clipboard operation
code.pyret.org copied to clipboard

Why aren't these images equal?

Open jpolitz opened this issue 3 years ago • 6 comments
trafficstars

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.

jpolitz avatar May 23 '22 20:05 jpolitz

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.)

blerner avatar May 23 '22 23:05 blerner

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.

schanzer avatar May 24 '22 11:05 schanzer

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.

blerner avatar May 24 '22 11:05 blerner

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...

schanzer avatar May 24 '22 11:05 schanzer

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.

blerner avatar May 24 '22 12:05 blerner

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.

schanzer avatar May 24 '22 12:05 schanzer