Skip to content

Commit d81d81a

Browse files
committed
Cleanup and fix OOM for SIFT differ
1 parent ced88ae commit d81d81a

File tree

5 files changed

+34
-35
lines changed

5 files changed

+34
-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/Sift.kt

Lines changed: 22 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> {
@@ -128,32 +130,34 @@ internal object Sift : Differ {
128130

129131
private fun buildDoGPyramid(gaussians: List<List<Array<DoubleArray>>>): List<List<Array<DoubleArray>>> {
130132
return gaussians.map { octave ->
131-
octave.zipWithNext { a, b -> subtractImages(b, a) }
133+
buildList(maxOf(0, octave.size - 1)) {
134+
for (i in 0 until octave.size - 1) {
135+
add(subtractImagesInPlace(octave[i], octave[i + 1]))
136+
}
137+
}
132138
}
133139
}
134140

135-
private fun subtractImages(a: Array<DoubleArray>, b: Array<DoubleArray>): Array<DoubleArray> {
141+
private fun subtractImagesInPlace(a: Array<DoubleArray>, b: Array<DoubleArray>): Array<DoubleArray> {
136142
val h = a.size
137143
val w = a[0].size
138-
val result = Array(h) { DoubleArray(w) }
144+
// Modify 'a' in place
139145
for (y in 0 until h) {
140146
for (x in 0 until w) {
141-
result[y][x] = a[y][x] - b[y][x]
147+
a[y][x] -= b[y][x]
142148
}
143149
}
144-
return result
150+
return a // Return the modified 'a'
145151
}
146152

147153
private fun downsample(image: Array<DoubleArray>): Array<DoubleArray> {
148154
val h = image.size / 2
149155
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]
156+
return Array(h) { y ->
157+
DoubleArray(w) { x ->
158+
image[y * 2][x * 2]
154159
}
155160
}
156-
return result
157161
}
158162

159163
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/DiffersTest.kt

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -75,14 +75,12 @@ class DiffersTest {
7575
val flipResult2 = Flip.compare(linuxFullScreen, macosxFullScreen)
7676
assertThat(flipResult2).isInstanceOf(DiffResult.Identical::class.java)
7777

78-
// Java heap space error (OOM)
79-
80-
// val siftResult2 = SiftDiffer.compare(linuxFullScreen, macosxFullScreen)
81-
// with(siftResult2) {
82-
// assertThat(this).isInstanceOf(DiffResult.Similar::class.java)
83-
// this as DiffResult.Similar
84-
// assertThat(numSimilarPixels).isEqualTo(293)
85-
// }
78+
val siftResult2 = Sift.compare(linuxFullScreen, macosxFullScreen)
79+
with(siftResult2) {
80+
assertThat(this).isInstanceOf(DiffResult.Similar::class.java)
81+
this as DiffResult.Similar
82+
assertThat(numSimilarPixels).isEqualTo(635)
83+
}
8684

8785
val de2000Result2 = DeltaE2000.compare(linuxFullScreen, macosxFullScreen)
8886
assertThat(de2000Result2).isInstanceOf(DiffResult.Identical::class.java)

0 commit comments

Comments
 (0)