Skip to content

Commit ef14bb4

Browse files
geoff-powellgeoff-powell
authored
Fix differ comparison between black pixels with 100% alpha and black pixels with 0% alpha (#2078)
1 parent ced88ae commit ef14bb4

File tree

15 files changed

+311
-35
lines changed

15 files changed

+311
-35
lines changed

‎paparazzi/api/paparazzi.api

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -119,11 +119,11 @@ public final class app/cash/paparazzi/Flags {
119119

120120
public final class app/cash/paparazzi/HtmlReportWriter : app/cash/paparazzi/SnapshotHandler {
121121
public static final field $stable I
122-
public fun <init> (DLapp/cash/paparazzi/internal/Differ;)V
123-
public fun <init> (Ljava/lang/String;DLapp/cash/paparazzi/internal/Differ;)V
124-
public fun <init> (Ljava/lang/String;Ljava/io/File;DLapp/cash/paparazzi/internal/Differ;)V
125-
public fun <init> (Ljava/lang/String;Ljava/io/File;DLapp/cash/paparazzi/internal/Differ;Ljava/io/File;)V
126-
public synthetic fun <init> (Ljava/lang/String;Ljava/io/File;DLapp/cash/paparazzi/internal/Differ;Ljava/io/File;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
122+
public fun <init> (DLapp/cash/paparazzi/Differ;)V
123+
public fun <init> (Ljava/lang/String;DLapp/cash/paparazzi/Differ;)V
124+
public fun <init> (Ljava/lang/String;Ljava/io/File;DLapp/cash/paparazzi/Differ;)V
125+
public fun <init> (Ljava/lang/String;Ljava/io/File;DLapp/cash/paparazzi/Differ;Ljava/io/File;)V
126+
public synthetic fun <init> (Ljava/lang/String;Ljava/io/File;DLapp/cash/paparazzi/Differ;Ljava/io/File;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
127127
public fun close ()V
128128
public fun newFrameHandler (Lapp/cash/paparazzi/Snapshot;II)Lapp/cash/paparazzi/SnapshotHandler$FrameHandler;
129129
}

‎paparazzi/src/main/java/app/cash/paparazzi/HtmlReportWriter.kt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
package app.cash.paparazzi
1717

1818
import app.cash.paparazzi.SnapshotHandler.FrameHandler
19-
import app.cash.paparazzi.internal.Differ
2019
import app.cash.paparazzi.internal.ImageUtils
2120
import app.cash.paparazzi.internal.PaparazziJson
2221
import app.cash.paparazzi.internal.apng.ApngWriter

‎paparazzi/src/main/java/app/cash/paparazzi/internal/differs/Mssim.kt

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
package app.cash.paparazzi.internal.differs
22

3+
import androidx.compose.ui.util.fastCoerceAtLeast
34
import app.cash.paparazzi.Differ
45
import app.cash.paparazzi.Differ.DiffResult
56
import java.awt.Color
67
import java.awt.image.BufferedImage
78
import kotlin.math.exp
9+
import kotlin.math.min
810
import kotlin.math.pow
911

1012
internal object Mssim : Differ {
@@ -13,7 +15,7 @@ internal object Mssim : Differ {
1315

1416
val width = expected.width
1517
val height = expected.height
16-
val windowSize = 11
18+
val windowSize = 11.coerceAtMost(min(width, height))
1719
val sigma = 1.5
1820
val gaussianKernel = createGaussianKernel(windowSize, sigma)
1921

@@ -25,8 +27,8 @@ internal object Mssim : Differ {
2527

2628
val deltaImage = BufferedImage(width, height, BufferedImage.TYPE_INT_RGB)
2729

28-
for (y in 0 until height - windowSize) {
29-
for (x in 0 until width - windowSize) {
30+
for (y in 0 until (height - windowSize).coerceAtLeast(1)) {
31+
for (x in 0 until (width - windowSize).coerceAtLeast(1)) {
3032
val window1 = extractWindow(expectedLuma, x, y, windowSize)
3133
val window2 = extractWindow(actualLuma, x, y, windowSize)
3234

@@ -41,7 +43,7 @@ internal object Mssim : Differ {
4143
}
4244
}
4345

44-
val mssim = sumSSIM / numWindows
46+
val mssim = (sumSSIM / numWindows)
4547
val percentDifference = ((1.0 - mssim) * 100).toFloat()
4648

4749
return when {

‎paparazzi/src/main/java/app/cash/paparazzi/internal/differs/OffByTwo.kt

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ internal object OffByTwo : Differ {
5050
continue
5151
}
5252

53+
val deltaA = (actualRgb and -0x1000000).ushr(24) - (expectedRgb and -0x1000000).ushr(24)
5354
val deltaR = (actualRgb and 0xFF0000).ushr(16) - (expectedRgb and 0xFF0000).ushr(16)
5455
val deltaG = (actualRgb and 0x00FF00).ushr(8) - (expectedRgb and 0x00FF00).ushr(8)
5556
val deltaB = (actualRgb and 0x0000FF) - (expectedRgb and 0x0000FF)
@@ -61,7 +62,7 @@ internal object OffByTwo : Differ {
6162
((expectedRgb and -0x1000000).ushr(24) + (actualRgb and -0x1000000).ushr(24)) / 2 shl 24
6263
val newRGB = avgAlpha or (newR shl 16) or (newG shl 8) or newB
6364

64-
if (abs(deltaR) <= 2 && abs(deltaG) <= 2 && abs(deltaB) <= 2) {
65+
if (abs(deltaR) <= 2 && abs(deltaG) <= 2 && abs(deltaB) <= 2 && abs(deltaA) <= 2) {
6566
similarPixels++
6667
deltaImage.setRGB(expectedWidth + x, y, 0xFF0000FF.toInt())
6768
continue
@@ -85,7 +86,13 @@ internal object OffByTwo : Differ {
8586

8687
// 3 different colors, 256 color levels
8788
val total = actualHeight.toLong() * actualWidth.toLong() * 3L * 256L
88-
val percentDifference = (delta * 100 / total.toDouble()).toFloat()
89+
var percentDifference = (delta * 100 / total.toDouble()).toFloat()
90+
91+
// If the delta diff is all black pixels, the computed difference is 0, but there are still
92+
// different pixels. We can fallback to the amount of different pixels to less precise difference to ensure difference is reported.
93+
if (differentPixels > 0 && percentDifference == 0f) {
94+
percentDifference = differentPixels * 100 / (actualWidth * actualHeight.toDouble()).toFloat()
95+
}
8996

9097
return if (differentPixels > 0) {
9198
DiffResult.Different(

‎paparazzi/src/main/java/app/cash/paparazzi/internal/differs/PixelPerfect.kt

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,13 @@ internal object PixelPerfect : Differ {
7979

8080
// 3 different colors, 256 color levels
8181
val total = actualHeight.toLong() * actualWidth.toLong() * 3L * 256L
82-
val percentDifference = (delta * 100 / total.toDouble()).toFloat()
82+
var percentDifference = (delta * 100 / total.toDouble()).toFloat()
83+
84+
// If the delta diff is all black pixels, the computed difference is 0, but there are still
85+
// different pixels. We can fallback to the amount of different pixels to less precise difference to ensure difference is reported.
86+
if (differentPixels > 0 && percentDifference == 0f) {
87+
percentDifference = differentPixels * 100 / (actualWidth * actualHeight.toDouble()).toFloat()
88+
}
8389

8490
return if (percentDifference == 0f) {
8591
DiffResult.Identical(delta = deltaImage)

‎paparazzi/src/main/java/app/cash/paparazzi/internal/differs/Sift.kt

Lines changed: 16 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -8,17 +8,21 @@ import java.awt.image.BufferedImage
88
import kotlin.math.ceil
99
import kotlin.math.exp
1010
import kotlin.math.hypot
11+
import kotlin.math.min
1112
import kotlin.math.pow
1213

1314
internal object Sift : Differ {
1415
override fun compare(expected: BufferedImage, actual: BufferedImage): DiffResult {
1516
require(expected.width == actual.width && expected.height == actual.height)
1617

18+
val width = expected.width
19+
val height = expected.height
20+
1721
val grayscaleExpected = toGrayscale(expected)
1822
val grayscaleActual = toGrayscale(actual)
1923

20-
val octaves = 4
21-
val scalesPerOctave = 3
24+
val octaves = 4.coerceAtMost(min(width, height).coerceAtMost(128))
25+
val scalesPerOctave = 3.coerceAtMost(octaves)
2226

2327
val gExpected = buildGaussianPyramid(grayscaleExpected, octaves, scalesPerOctave)
2428
val gActual = buildGaussianPyramid(grayscaleActual, octaves, scalesPerOctave)
@@ -31,7 +35,7 @@ internal object Sift : Differ {
3135

3236
val matches = matchKeypoints(keypointsExpected, keypointsActual)
3337

34-
val delta = BufferedImage(expected.width, expected.height, BufferedImage.TYPE_INT_RGB)
38+
val delta = BufferedImage(width, height, BufferedImage.TYPE_INT_RGB)
3539
val g = delta.createGraphics()
3640
g.drawImage(expected, 0, 0, null)
3741
drawKeypoints(g, keypointsExpected, Color.RED)
@@ -52,14 +56,12 @@ internal object Sift : Differ {
5256
private fun toGrayscale(image: BufferedImage): Array<DoubleArray> {
5357
val w = image.width
5458
val h = image.height
55-
val gray = Array(h) { DoubleArray(w) }
56-
for (y in 0 until h) {
57-
for (x in 0 until w) {
59+
return Array(h) { y ->
60+
DoubleArray(w) { x ->
5861
val rgb = Color(image.getRGB(x, y))
59-
gray[y][x] = 0.299 * rgb.red + 0.587 * rgb.green + 0.114 * rgb.blue
62+
0.299 * rgb.red + 0.587 * rgb.green + 0.114 * rgb.blue
6063
}
6164
}
62-
return gray
6365
}
6466

6567
private fun gaussianBlur(src: Array<DoubleArray>, sigma: Double): Array<DoubleArray> {
@@ -135,25 +137,21 @@ internal object Sift : Differ {
135137
private fun subtractImages(a: Array<DoubleArray>, b: Array<DoubleArray>): Array<DoubleArray> {
136138
val h = a.size
137139
val w = a[0].size
138-
val result = Array(h) { DoubleArray(w) }
139-
for (y in 0 until h) {
140-
for (x in 0 until w) {
141-
result[y][x] = a[y][x] - b[y][x]
140+
return Array(h) { y ->
141+
DoubleArray(w) { x ->
142+
a[y][x] - b[y][x]
142143
}
143144
}
144-
return result
145145
}
146146

147147
private fun downsample(image: Array<DoubleArray>): Array<DoubleArray> {
148148
val h = image.size / 2
149149
val w = image[0].size / 2
150-
val result = Array(h) { DoubleArray(w) }
151-
for (y in 0 until h) {
152-
for (x in 0 until w) {
153-
result[y][x] = image[y * 2][x * 2]
150+
return Array(h) { y ->
151+
DoubleArray(w) { x ->
152+
image[y * 2][x * 2]
154153
}
155154
}
156-
return result
157155
}
158156

159157
private fun detectDoGExtrema(dogPyramid: List<List<Array<DoubleArray>>>): List<SiftKeypoint> {

‎paparazzi/src/test/java/app/cash/paparazzi/HtmlReportWriterTest.kt

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,7 @@
1616
package app.cash.paparazzi
1717

1818
import app.cash.paparazzi.FileSubject.Companion.assertThat
19-
import app.cash.paparazzi.internal.Differ
20-
import app.cash.paparazzi.internal.OffByTwo
21-
import app.cash.paparazzi.internal.PixelPerfect
19+
import app.cash.paparazzi.internal.differs.PixelPerfect
2220
import com.google.common.truth.Truth.assertThat
2321
import org.junit.Rule
2422
import org.junit.Test

‎paparazzi/src/test/java/app/cash/paparazzi/internal/differs/DeltaE2000Test.kt

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
package app.cash.paparazzi.internal.differs
2+
3+
import app.cash.paparazzi.Differ
4+
import com.google.common.truth.Truth.assertThat
5+
import org.junit.Test
6+
import java.awt.image.BufferedImage
7+
8+
class DeltaE2000Test {
9+
@Test
10+
fun `compare identical images`() {
11+
val expected = createImage(width = 1, height = 1)
12+
val actual = createImage(width = 1, height = 1)
13+
val result = DeltaE2000.compare(expected, actual)
14+
assertThat(result).isInstanceOf(Differ.DiffResult.Identical::class.java)
15+
}
16+
17+
@Test
18+
fun `compare similar images`() {
19+
val expected = createImage(width = 1, height = 1, rgb = 0xFFFFFFFE)
20+
val actual = createImage(width = 1, height = 1)
21+
val result = DeltaE2000.compare(expected, actual)
22+
assertThat(result).isInstanceOf(Differ.DiffResult.Identical::class.java)
23+
}
24+
25+
@Test
26+
fun `compare similar images using black actual and alpha expected`() {
27+
val expected = createImage(width = 1, height = 1, rgb = 0x00000000)
28+
val actual = createImage(width = 1, height = 1, rgb = 0xFF000000)
29+
val result = DeltaE2000.compare(expected, actual)
30+
assertThat(result).isInstanceOf(Differ.DiffResult.Identical::class.java)
31+
}
32+
33+
@Test
34+
fun `compare different images`() {
35+
val expected = createImage(width = 1, height = 1, rgb = 0x00000000)
36+
val actual = createImage(width = 1, height = 1)
37+
val result = DeltaE2000.compare(expected, actual)
38+
assertThat(result).isInstanceOf(Differ.DiffResult.Different::class.java)
39+
}
40+
}

‎paparazzi/src/test/java/app/cash/paparazzi/internal/differs/DiffersTest.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ class DiffersTest {
7777

7878
// Java heap space error (OOM)
7979

80-
// val siftResult2 = SiftDiffer.compare(linuxFullScreen, macosxFullScreen)
80+
// val siftResult2 = Sift.compare(linuxFullScreen, macosxFullScreen)
8181
// with(siftResult2) {
8282
// assertThat(this).isInstanceOf(DiffResult.Similar::class.java)
8383
// this as DiffResult.Similar

‎paparazzi/src/test/java/app/cash/paparazzi/internal/differs/FlipTest.kt

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
package app.cash.paparazzi.internal.differs
2+
3+
import app.cash.paparazzi.Differ
4+
import com.google.common.truth.Truth.assertThat
5+
import org.junit.Test
6+
import java.awt.image.BufferedImage
7+
8+
class FlipTest {
9+
@Test
10+
fun `compare identical images`() {
11+
val expected = createImage(width = 1, height = 1)
12+
val actual = createImage(width = 1, height = 1)
13+
val result = Flip.compare(expected, actual)
14+
assertThat(result).isInstanceOf(Differ.DiffResult.Identical::class.java)
15+
}
16+
17+
@Test
18+
fun `compare similar images`() {
19+
val expected = createImage(width = 1, height = 1, rgb = 0xFFFFFFFE)
20+
val actual = createImage(width = 1, height = 1)
21+
val result = Flip.compare(expected, actual)
22+
assertThat(result).isInstanceOf(Differ.DiffResult.Identical::class.java)
23+
}
24+
25+
@Test
26+
fun `compare similar images using black actual and alpha expected`() {
27+
val expected = createImage(width = 1, height = 1, rgb = 0x00000000)
28+
val actual = createImage(width = 1, height = 1, rgb = 0xFF000000)
29+
val result = Flip.compare(expected, actual)
30+
assertThat(result).isInstanceOf(Differ.DiffResult.Identical::class.java)
31+
}
32+
33+
@Test
34+
fun `compare different images`() {
35+
val expected = createImage(width = 1, height = 1, rgb = 0x00000000)
36+
val actual = createImage(width = 1, height = 1)
37+
val result = Flip.compare(expected, actual)
38+
assertThat(result).isInstanceOf(Differ.DiffResult.Different::class.java)
39+
}
40+
}

0 commit comments

Comments
 (0)