Fix a bug where a BitmapFont could become zero width or height (#56)

* Fix a bug where a BitmapFont could become zero width or height

* Ensure we don't scale imeages that don't need scaling

Previously images that didn't need to be scaled where sometimes
incorrectly scaled. This patch fixes that.

tools/wff-optimizer/src/main/java/com/google/android/clockwork/wff/optimizer/BitmapFonts.kt

@@ -104,6 +104,7 @@ class BitmapFont(val name: String, val characters: Map<String, Character>) {
                         nonTransparentBounds.width(),
                         nonTransparentBounds.height()
                     )
+                image.cropped = true
                 optimizationApplied = true
 
                 if (imageLoader.settings.verbose) {
@@ -119,66 +120,69 @@ class BitmapFont(val name: String, val characters: Map<String, Character>) {
             val aspectRatio =
                 image.bufferedImage.width.toDouble() / image.bufferedImage.height.toDouble()
 
-            if (maxHeight > image.bufferedImage.height) {
-                maxHeight = image.bufferedImage.height
-            }
-
-            val maxWidth = maxHeight.toDouble() * aspectRatio
-            val scaleX = maxWidth / image.bufferedImage.width.toDouble()
-            val scaleY = maxHeight.toDouble() / image.bufferedImage.height.toDouble()
-            val newWidth = (nonTransparentBounds.width().toDouble() * scaleX).toInt()
-            val newHeight = (nonTransparentBounds.height().toDouble() * scaleY).toInt()
             var marginLeft = nonTransparentBounds.left
             var marginTop = nonTransparentBounds.top
             var marginRight = image.bufferedImage.width - nonTransparentBounds.right
             var marginBottom = image.bufferedImage.height - nonTransparentBounds.bottom
 
-            // If the resized area is smaller, then scale image and bounds.
-            if (
-                newWidth * newHeight < nonTransparentBounds.width() * nonTransparentBounds.height()
-            ) {
-                if (imageLoader.settings.verbose) {
-                    System.out.println(
-                        "Scaling image ${character.resourceId}: " +
-                            "${croppedImage.getWidth()}x${croppedImage.getHeight()} -> " +
-                            "${newWidth}x${newHeight}"
+            // If maxHeight is smaller, then down scale the image.
+            if (maxHeight < image.bufferedImage.height) {
+                val maxWidth = maxHeight.toDouble() * aspectRatio
+                val scaleX = maxWidth / image.bufferedImage.width.toDouble()
+                val scaleY = maxHeight.toDouble() / image.bufferedImage.height.toDouble()
+
+                val newWidth = Math.ceil(nonTransparentBounds.width().toDouble() * scaleX).toInt()
+                val newHeight = Math.ceil(nonTransparentBounds.height().toDouble() * scaleY).toInt()
+
+                // If the resized area is smaller, then scale image and bounds.
+                if (newWidth * newHeight <
+                    nonTransparentBounds.width() * nonTransparentBounds.height()
+                ) {
+                    if (imageLoader.settings.verbose) {
+                        System.out.println(
+                            "Scaling image ${character.resourceId}: " +
+                                    "${croppedImage.getWidth()}x${croppedImage.getHeight()} -> " +
+                                    "${newWidth}x${newHeight}"
+                        )
+                    }
+
+                    val scaledImage =
+                        BufferedImage(newWidth, newHeight, BufferedImage.TYPE_INT_ARGB)
+                    val graphics = scaledImage.createGraphics()
+                    graphics.setRenderingHint(
+                        RenderingHints.KEY_INTERPOLATION,
+                        RenderingHints.VALUE_INTERPOLATION_BILINEAR
                     )
+                    graphics.drawImage(
+                        croppedImage,
+                        0,
+                        0,
+                        newWidth,
+                        newHeight,
+                        0,
+                        0,
+                        croppedImage.getWidth(),
+                        croppedImage.getHeight(),
+                        null
+                    )
+                    graphics.dispose()
+
+                    marginLeft = (marginLeft.toDouble() * scaleX).toInt()
+                    marginTop = (marginTop.toDouble() * scaleY).toInt()
+                    marginRight = (marginRight.toDouble() * scaleX).toInt()
+                    marginBottom = (marginBottom.toDouble() * scaleY).toInt()
+                    croppedImage = scaledImage
+                    image.scaled = true
+                    character.element.setAttribute(
+                        "width",
+                        (newWidth + marginLeft + marginRight).toString()
+                    )
+                    character.element.setAttribute(
+                        "height",
+                        (newHeight + marginTop + marginBottom).toString()
+                    )
+                    optimizationApplied = true
                 }
-
-                val scaledImage = BufferedImage(newWidth, newHeight, BufferedImage.TYPE_INT_ARGB)
-                val graphics = scaledImage.createGraphics()
-                graphics.setRenderingHint(
-                    RenderingHints.KEY_INTERPOLATION,
-                    RenderingHints.VALUE_INTERPOLATION_BILINEAR
-                )
-                graphics.drawImage(
-                    croppedImage,
-                    0,
-                    0,
-                    newWidth,
-                    newHeight,
-                    0,
-                    0,
-                    croppedImage.getWidth(),
-                    croppedImage.getHeight(),
-                    null
-                )
-                graphics.dispose()
-
-                marginLeft = (marginLeft.toDouble() * scaleX).toInt()
-                marginTop = (marginTop.toDouble() * scaleY).toInt()
-                marginRight = (marginRight.toDouble() * scaleX).toInt()
-                marginBottom = (marginBottom.toDouble() * scaleY).toInt()
-                croppedImage = scaledImage
-                character.element.setAttribute(
-                    "width",
-                    (newWidth + marginLeft + marginRight).toString()
-                )
-                character.element.setAttribute(
-                    "height",
-                    (newHeight + marginTop + marginBottom).toString()
-                )
-                optimizationApplied = true
             }
 
             // Save margins.

tools/wff-optimizer/src/main/java/com/google/android/clockwork/wff/optimizer/ImageLoader.kt

@@ -143,6 +143,8 @@ open class Image(
 ) {
     val referencingElements = ArrayList<Element>()
     var optimizedImage: BufferedImage? = null
+    var cropped = false
+    var scaled = false
 
     open fun maybeWriteOptimizedImage() {
         optimizedImage?.let { ImageIO.write(it, "png", file) }

tools/wff-optimizer/src/test/java/com/google/android/clockwork/wff/optimizer/ImageLoaderTest.kt

@@ -102,7 +102,57 @@ class ImageLoaderTest {
             fixture.document.transformToString()
         )
         assertThat(fixture.imageLoader.optimizedImagesSummary())
-            .containsExactly("a 40 x 44", "b 28 x 44")
+            .containsExactly("a 40 x 44 cropped", "b 28 x 44 cropped")
+    }
+
+    @Test
+    fun bitmapFontCropping2() {
+        val fixture =
+            load(
+                "src/test/resources/bitmapFontCropTest2",
+                "src/test/resources/bitmapFontCropTest2/res/raw/watchface.xml"
+            )
+
+        fixture.optimizer.bitmapFonts.optimize(fixture.imageLoader)
+        fixture.imageLoader.dedupeAndWriteOptimizedImages()
+
+        // The bitmap fonts should be cropped with margins added to correctly place the image.
+        assertEquals(
+            getResource("bitmapFontCropTest2/res/raw/watchface_expected.xml"),
+            fixture.document.transformToString()
+        )
+        assertThat(fixture.imageLoader.optimizedImagesSummary())
+            .containsExactly(
+                "wfs_4_478cdc4e_a2d3_4a0b_a363_0141236a4fb2 60 x 109 cropped",
+                "wfs_6_82227af2_ab99_4635_a282_f18e61f6a4e5 60 x 109 cropped",
+                "wfs_1_7fd1e3ba_f9e5_4114_95d0_404fd95b0f1c 60 x 109 cropped",
+                "wfs_9_142c8d9c_ac84_4358_8512_a0a23bafa6bc 60 x 109 cropped",
+                "wfs__bcea73dd_7a37_467f_94c8_f57f413c1618 30 x 80 cropped",
+                "wfs_0_84beb937_1dd8_4ca7_afc6_9d52ee68188b 60 x 109 cropped",
+                "wfs_8_f5b0994f_24cc_43d3_823c_b4d12dbea6f8 60 x 109 cropped",
+                "wfs_2_1cd08553_4def_4929_ae79_e4e95dc8e7d2 60 x 109 cropped",
+                "wfs_3_f50b11ef_1120_43f1_8c23_ae8f4d968b77 60 x 109 cropped",
+                "wfs_5_6060f0fa_b314_403c_8ef3_8b2127cdf8a5 60 x 109 cropped",
+                "wfs_7_3bc978df_2037_4d89_b3bd_a52b01f702f0 60 x 109 cropped"
+            )
+    }
+
+    @Test
+    fun bitmapFontCropAndMargins_minimumWidth() {
+        val fixture =
+            load(
+                "src/test/resources/narrowBitmapFont",
+                "src/test/resources/narrowBitmapFont/res/raw/watchface.xml"
+            )
+
+        fixture.optimizer.bitmapFonts.optimize(fixture.imageLoader)
+        fixture.imageLoader.dedupeAndWriteOptimizedImages()
+
+        // Check that we don't create a zero width image when downsizeing.
+        assertThat(fixture.imageLoader.optimizedImagesSummary())
+            .containsExactly(
+                "wfs_sec_decorative_3e6538b4_6b9c_4b20_9c09_0e7ae4babb1c 1 x 13 cropped scaled"
+            )
     }
 
     @Test
@@ -116,14 +166,14 @@ class ImageLoaderTest {
         fixture.optimizer.bitmapFonts.optimize(fixture.imageLoader)
         fixture.imageLoader.dedupeAndWriteOptimizedImages()
 
-        // The source images are 100x100 but they're only ever rendered at 50x50 so we should scale
-        // them down as well as crop to the visible pixels.
+        // The source images are 100x100, however they're rendered at 200x200 and we don't want to
+        // increace runtime memory usage so we should crop but not scale.
         assertEquals(
             getResource("bitmapFontCropTest/res/raw/too_large_expected.xml"),
             fixture.document.transformToString()
         )
         assertThat(fixture.imageLoader.optimizedImagesSummary())
-            .containsExactly("a 40 x 44", "b 28 x 44")
+            .containsExactly("a 40 x 44 cropped", "b 28 x 44 cropped")
     }
 
     @Test
@@ -138,14 +188,14 @@ class ImageLoaderTest {
 
         fixture.imageLoader.dedupeAndWriteOptimizedImages()
 
-        // The source images are 100x100, however they're rendered at 200x200 and we don't want to
-        // increace runtime memory usage so we should crop but not scale.
+        // The source images are 100x100 but they're only ever rendered at 50x50 so we should scale
+        // them down as well as crop to the visible pixels.
         assertEquals(
             getResource("bitmapFontCropTest/res/raw/too_small_expected.xml"),
             fixture.document.transformToString()
         )
         assertThat(fixture.imageLoader.optimizedImagesSummary())
-            .containsExactly("a 20 x 22", "b 14 x 22")
+            .containsExactly("a 20 x 22 cropped scaled", "b 14 x 22 cropped scaled")
     }
 
     @Test
@@ -231,7 +281,14 @@ class TestImage(
 
     fun summary(): String {
         val image = optimizedImage ?: bufferedImage
-        return "$name ${image.width} x ${image.height}"
+        var result = "$name ${image.width} x ${image.height}"
+        if (cropped) {
+            result = result + " cropped"
+        }
+        if (scaled) {
+            result = result + " scaled"
+        }
+        return result
     }
 }
 

tools/wff-optimizer/src/test/resources/bitmapFontCropTest2/res/raw/watchface.xml

@@ -0,0 +1,138 @@
+<WatchFace
+    clipShape="CIRCLE"
+    height="450"
+    width="450">
+    <Metadata
+        key="CLOCK_TYPE"
+        value="ANALOG" />
+    <BitmapFonts>
+        <BitmapFont name="Bitmap font 1">
+            <Character
+                name=":"
+                height="100"
+                resource="wfs__bcea73dd_7a37_467f_94c8_f57f413c1618"
+                width="100" />
+            <Character
+                name="0"
+                height="110"
+                resource="wfs_0_84beb937_1dd8_4ca7_afc6_9d52ee68188b"
+                width="60" />
+            <Character
+                name="1"
+                height="110"
+                resource="wfs_1_7fd1e3ba_f9e5_4114_95d0_404fd95b0f1c"
+                width="60" />
+            <Character
+                name="2"
+                height="110"
+                resource="wfs_2_1cd08553_4def_4929_ae79_e4e95dc8e7d2"
+                width="60" />
+            <Character
+                name="3"
+                height="110"
+                resource="wfs_3_f50b11ef_1120_43f1_8c23_ae8f4d968b77"
+                width="60" />
+            <Character
+                name="4"
+                height="110"
+                resource="wfs_4_478cdc4e_a2d3_4a0b_a363_0141236a4fb2"
+                width="60" />
+            <Character
+                name="5"
+                height="110"
+                resource="wfs_5_6060f0fa_b314_403c_8ef3_8b2127cdf8a5"
+                width="60" />
+            <Character
+                name="6"
+                height="110"
+                resource="wfs_6_82227af2_ab99_4635_a282_f18e61f6a4e5"
+                width="60" />
+            <Character
+                name="7"
+                height="110"
+                resource="wfs_7_3bc978df_2037_4d89_b3bd_a52b01f702f0"
+                width="60" />
+            <Character
+                name="8"
+                height="110"
+                resource="wfs_8_f5b0994f_24cc_43d3_823c_b4d12dbea6f8"
+                width="60" />
+            <Character
+                name="9"
+                height="110"
+                resource="wfs_9_142c8d9c_ac84_4358_8512_a0a23bafa6bc"
+                width="60" />
+        </BitmapFont>
+    </BitmapFonts>
+    <Scene backgroundColor="#ff000000">
+        <DigitalClock
+            alpha="255"
+            height="134"
+            pivotX="0.5"
+            pivotY="0.5"
+            width="420"
+            x="12"
+            y="106">
+            <TimeText
+                align="CENTER"
+                alpha="255"
+                format="hh:mm"
+                height="134"
+                hourFormat="SYNC_TO_DEVICE"
+                width="420"
+                x="0"
+                y="0">
+                <BitmapFont
+                    color="#ffffffff"
+                    family="Bitmap font 1"
+                    size="120" />
+            </TimeText>
+        </DigitalClock>
+        <DigitalClock
+            alpha="255"
+            height="134"
+            pivotX="0.5"
+            pivotY="0.5"
+            width="420"
+            x="12"
+            y="234">
+            <TimeText
+                align="CENTER"
+                alpha="255"
+                format="hh:mm"
+                height="134"
+                hourFormat="SYNC_TO_DEVICE"
+                width="420"
+                x="0"
+                y="0">
+                <BitmapFont
+                    color="#ffffffff"
+                    family="Bitmap font 1"
+                    size="110" />
+            </TimeText>
+        </DigitalClock>
+        <DigitalClock
+            alpha="255"
+            height="134"
+            pivotX="0.5"
+            pivotY="0.5"
+            width="420"
+            x="12"
+            y="0">
+            <TimeText
+                align="CENTER"
+                alpha="255"
+                format="hh:mm"
+                height="134"
+                hourFormat="SYNC_TO_DEVICE"
+                width="420"
+                x="0"
+                y="0">
+                <BitmapFont
+                    color="#ffffffff"
+                    family="Bitmap font 1"
+                    size="40" />
+            </TimeText>
+        </DigitalClock>
+    </Scene>
+</WatchFace>