From 226165708c1434918377b887fcb43b1df59bafa9 Mon Sep 17 00:00:00 2001 From: "Florine W. Dekker" Date: Tue, 15 Oct 2024 13:04:56 +0200 Subject: [PATCH 1/2] Defer icon size validation --- CHANGELOG.md | 3 +- .../kotlin/com/fwdekker/randomness/Icons.kt | 47 ++++++------ .../com/fwdekker/randomness/IconsTest.kt | 74 ++++++++++--------- 3 files changed, 69 insertions(+), 55 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a6255c74b..13be7a04e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,8 @@ # Changelog ## 9.9.9-unreleased ### Fixed -* Fixed bug reported to also check closed issues when checking for duplicates. +* Hopefully fix "Must be not computed before that call" bug by deferring icon validation to painting. ([#R1](https://github.com/FWDekkerBot/intellij-randomness-issues/issues/1)) ([#R13](https://github.com/FWDekkerBot/intellij-randomness-issues/issues/13)) ([IJPL-163887](https://youtrack.jetbrains.com/issue/IJPL-163887/)) +* Fixed bug reporter to also check closed issues when checking for duplicates. ## 3.3.2 -- 2024-09-28 diff --git a/src/main/kotlin/com/fwdekker/randomness/Icons.kt b/src/main/kotlin/com/fwdekker/randomness/Icons.kt index 018a31a87..52bf95fc7 100644 --- a/src/main/kotlin/com/fwdekker/randomness/Icons.kt +++ b/src/main/kotlin/com/fwdekker/randomness/Icons.kt @@ -229,27 +229,10 @@ data class OverlayIcon(val base: Icon, val background: Icon? = null) : Icon { * @property overlays The various icons that are overlayed on top of [base]. */ data class OverlayedIcon(val base: Icon, val overlays: List = emptyList()) : Icon { - init { - val baseWidth = base.iconWidth - val baseHeight = base.iconHeight - - require(baseWidth == baseHeight) { - Triple( - "Base must be square, but was ${baseWidth}x$baseHeight. " + - "Repeat: ${base.iconWidth}x${base.iconHeight}. " + - "Repeat: ${base.iconWidth}x${base.iconHeight}.", - base, - overlays - ) - } - overlays.forEachIndexed { idx, item -> - require(item.iconWidth == item.iconHeight) { - Triple("Overlay $idx must be square, but was ${item.iconWidth}x${item.iconHeight}.", base, overlays) - } - } - require(overlays.all { it.iconWidth == it.iconHeight }) { "Overlays must be square." } - require(overlays.map { it.iconWidth }.toSet().size <= 1) { "All overlays must have same size." } - } + /** + * @see validate + */ + private var validated: Boolean = false /** @@ -269,6 +252,8 @@ data class OverlayedIcon(val base: Icon, val overlays: List = emptyList()) override fun paintIcon(c: Component?, g: Graphics?, x: Int, y: Int) { if (c == null || g == null) return + validate() + base.paintIcon(c, g, x, y) overlays.forEachIndexed { i, overlay -> val overlaySize = iconWidth.toFloat() / OVERLAYS_PER_ROW @@ -290,6 +275,26 @@ data class OverlayedIcon(val base: Icon, val overlays: List = emptyList()) override fun getIconHeight() = base.iconHeight + /** + * Lazily validates the relative sizes of the [base] and the [overlays]. + * + * This code must *not* be called in the constructor; it should be deferred to some later point. Calling this in the + * constructor *will* cause exceptions, because the constructor is called in the constructor of various actions, and + * actions are constructor before UI scaling is initialized. + * + * See also https://youtrack.jetbrains.com/issue/IJPL-163887/ + */ + private fun validate() { + if (validated) return + + require(base.iconWidth == base.iconHeight) { "Base must be square." } + require(overlays.all { it.iconWidth == it.iconHeight }) { "All overlays must be square." } + require(overlays.map { it.iconWidth }.toSet().size <= 1) { "All overlays must have same size." } + + validated = true + } + + /** * Holds constants. */ diff --git a/src/test/kotlin/com/fwdekker/randomness/IconsTest.kt b/src/test/kotlin/com/fwdekker/randomness/IconsTest.kt index d268cf2ca..9be5dd590 100644 --- a/src/test/kotlin/com/fwdekker/randomness/IconsTest.kt +++ b/src/test/kotlin/com/fwdekker/randomness/IconsTest.kt @@ -3,6 +3,7 @@ package com.fwdekker.randomness import com.fwdekker.randomness.testhelpers.Tags +import com.fwdekker.randomness.testhelpers.afterNonContainer import com.fwdekker.randomness.testhelpers.beforeNonContainer import com.fwdekker.randomness.testhelpers.guiGet import io.kotest.assertions.throwables.shouldThrow @@ -15,12 +16,12 @@ import io.kotest.matchers.should import io.kotest.matchers.shouldBe import io.kotest.matchers.shouldNot import io.kotest.matchers.shouldNotBe -import io.kotest.matchers.string.shouldStartWith import io.kotest.matchers.types.shouldBeSameInstanceAs import org.assertj.swing.edt.FailOnThreadViolationRepaintManager import java.awt.Color import java.awt.Component import java.awt.Graphics +import java.awt.Graphics2D import java.awt.image.BufferedImage import javax.swing.Icon import javax.swing.JLabel @@ -212,19 +213,47 @@ object OverlayIconTest : FunSpec({ * Unit tests for [OverlayedIcon]. */ object OverlayedIconTest : FunSpec({ - context("constructor") { + lateinit var image: BufferedImage + lateinit var graphics: Graphics2D + + + beforeSpec { + FailOnThreadViolationRepaintManager.install() + } + + afterSpec { + FailOnThreadViolationRepaintManager.uninstall() + } + + beforeNonContainer { + image = BufferedImage(32, 32, BufferedImage.TYPE_INT_ARGB) + graphics = image.createGraphics() + } + + afterNonContainer { + graphics.dispose() + } + + + context("deferred validation") { test("fails if the base image is not square") { - shouldThrow { OverlayedIcon(PlainIcon(186, 132), emptyList()) } - .message shouldStartWith "(Base must be square, but was" + val icon = OverlayedIcon(PlainIcon(186, 132), emptyList()) + + shouldThrow { icon.paintIcon(guiGet { JLabel() }, graphics, 0, 0) } + .message shouldBe "Base must be square." } test("fails if an overlay is not square") { - shouldThrow { OverlayedIcon(PlainIcon(), listOf(PlainIcon(), PlainIcon(38, 40))) } - .message shouldStartWith Regex("\\(Overlay [1-9] must be square, but was") + val icon = OverlayedIcon(PlainIcon(), listOf(PlainIcon(), PlainIcon(38, 40))) + + shouldThrow { icon.paintIcon(guiGet { JLabel() }, graphics, 0, 0) } + .message shouldBe "All overlays must be square." } test("fails if overlays have different sizes") { - shouldThrow { OverlayedIcon(PlainIcon(), listOf(PlainIcon(), PlainIcon(34, 34))) } + val icon = OverlayedIcon(PlainIcon(), listOf(PlainIcon(), PlainIcon(34, 34))) + + shouldThrow { icon.paintIcon(guiGet { JLabel() }, graphics, 0, 0) } .message shouldBe "All overlays must have same size." } } @@ -241,45 +270,24 @@ object OverlayedIconTest : FunSpec({ context("paintIcon").config(tags = setOf(Tags.SWING)) { - lateinit var image: BufferedImage - - - beforeContainer { - FailOnThreadViolationRepaintManager.install() - } - - beforeNonContainer { - image = BufferedImage(32, 32, BufferedImage.TYPE_INT_ARGB) - } - - test("paints nothing if the component is null") { - with(image.createGraphics()) { - OverlayedIcon(PlainIcon(), listOf(PlainIcon())).paintIcon(null, this, 0, 0) - this.dispose() - } + OverlayedIcon(PlainIcon(), listOf(PlainIcon())).paintIcon(null, graphics, 0, 0) image.getRGB(0, 0, 32, 32, null, 0, 32).forEach { it shouldBe 0 } } test("paints the base icon only if no overlays are specified") { - with(image.createGraphics()) { - val label = JLabel() + val label = guiGet { JLabel() } - OverlayedIcon(PlainIcon(color = Color.GREEN.rgb)).paintIcon(label, this, 0, 0) - this.dispose() - } + OverlayedIcon(PlainIcon(color = Color.GREEN.rgb)).paintIcon(label, graphics, 0, 0) image.getRGB(0, 0, 32, 32, null, 0, 32).forEach { it shouldBe Color.GREEN.rgb } } test("paints the overlays starting in the top-left corner") { - with(image.createGraphics()) { - val label = JLabel().also { it.background = Color.BLUE } + val label = guiGet { JLabel().also { it.background = Color.BLUE } } - OverlayedIcon(PlainIcon(), listOf(PlainIcon(color = Color.BLUE.rgb))).paintIcon(label, this, 0, 0) - this.dispose() - } + OverlayedIcon(PlainIcon(), listOf(PlainIcon(color = Color.BLUE.rgb))).paintIcon(label, graphics, 0, 0) image.getRGB(0, 0, 16, 16, null, 0, 16).forEach { it shouldBe Color.BLUE.rgb } image.getRGB(16, 16, 16, 16, null, 0, 16).forEach { it shouldNotBe Color.BLUE.rgb } From e19e0ffcff8ae6494b9cb24e8416abeb0c6569c6 Mon Sep 17 00:00:00 2001 From: "Florine W. Dekker" Date: Tue, 15 Oct 2024 16:31:33 +0200 Subject: [PATCH 2/2] Change template icon scheme color order --- CHANGELOG.md | 5 ++++- .../kotlin/com/fwdekker/randomness/Icons.kt | 14 +++++++------- .../com/fwdekker/randomness/IconsTest.kt | 19 +++++++++---------- 3 files changed, 20 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 13be7a04e..b4f3e5f95 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,10 @@ # Changelog ## 9.9.9-unreleased +### Changed +* In template icons, change the order of scheme colors to be clockwise starting from the top, instead of counterclockwise starting on the right. + ### Fixed -* Hopefully fix "Must be not computed before that call" bug by deferring icon validation to painting. ([#R1](https://github.com/FWDekkerBot/intellij-randomness-issues/issues/1)) ([#R13](https://github.com/FWDekkerBot/intellij-randomness-issues/issues/13)) ([IJPL-163887](https://youtrack.jetbrains.com/issue/IJPL-163887/)) +* (Hopefully) fix "Must be not computed before that call" bug by deferring icon validation to painting. ([#R1](https://github.com/FWDekkerBot/intellij-randomness-issues/issues/1)) ([#R13](https://github.com/FWDekkerBot/intellij-randomness-issues/issues/13)) ([IJPL-163887](https://youtrack.jetbrains.com/issue/IJPL-163887/)) * Fixed bug reporter to also check closed issues when checking for duplicates. diff --git a/src/main/kotlin/com/fwdekker/randomness/Icons.kt b/src/main/kotlin/com/fwdekker/randomness/Icons.kt index 52bf95fc7..07c8ca4f5 100644 --- a/src/main/kotlin/com/fwdekker/randomness/Icons.kt +++ b/src/main/kotlin/com/fwdekker/randomness/Icons.kt @@ -340,21 +340,21 @@ class RadialColorReplacementFilter( /** - * Returns [toShift] which has its alpha multiplied by that of [shiftBy]. + * Returns [base] after multiplying its alpha by the alpha of [shiftBy]. */ - private fun shiftAlpha(toShift: Color, shiftBy: Color) = - ColorUtil.withAlpha(toShift, asFraction(toShift.alpha) * asFraction(shiftBy.alpha)) + private fun shiftAlpha(base: Color, shiftBy: Color): Color = + ColorUtil.withAlpha(base, asFraction(base.alpha) * asFraction(shiftBy.alpha)) /** * Represents a [number] in the range `[0, 256)` as a fraction of that range. */ - private fun asFraction(number: Int) = number / COMPONENT_MAX.toDouble() + private fun asFraction(number: Int): Double = number / COMPONENT_MAX /** * Returns the appropriate color from [colors] for an [offset] relative to the [center]. */ private fun positionToColor(offset: Pair): Color { - val angle = 2 * Math.PI - (atan2(offset.second.toDouble(), offset.first.toDouble()) + STARTING_ANGLE) + val angle = 2 * Math.PI + STARTING_ANGLE + atan2(offset.second.toDouble(), offset.first.toDouble()) val index = angle / (2 * Math.PI / colors.size) return colors[Math.floorMod(index.toInt(), colors.size)] } @@ -367,11 +367,11 @@ class RadialColorReplacementFilter( /** * Maximum value for an RGB component. */ - const val COMPONENT_MAX = 255 + const val COMPONENT_MAX: Double = 255.0 /** * The angle in radians at which the first color should start being displayed. */ - const val STARTING_ANGLE = -(3 * Math.PI / 4) + const val STARTING_ANGLE: Double = .25 * (2 * Math.PI) } } diff --git a/src/test/kotlin/com/fwdekker/randomness/IconsTest.kt b/src/test/kotlin/com/fwdekker/randomness/IconsTest.kt index 9be5dd590..38c5a701e 100644 --- a/src/test/kotlin/com/fwdekker/randomness/IconsTest.kt +++ b/src/test/kotlin/com/fwdekker/randomness/IconsTest.kt @@ -340,16 +340,15 @@ object RadialColorReplacementFilterTest : FunSpec({ Color(filtered, true).alpha shouldBe 12 } - test("returns the first of four colors if a position above the center is given") { - val filter = RadialColorReplacementFilter(listOf(Color.RED, Color.BLUE, Color.PINK, Color.GRAY), Pair(0, 0)) - - filter.filterRGB(0, 12, Color.MAGENTA.rgb) shouldBe Color.RED.rgb - } - - test("returns the second color of four colors if a position to the right of the center is given") { - val filter = RadialColorReplacementFilter(listOf(Color.RED, Color.BLUE, Color.PINK, Color.GRAY), Pair(0, 0)) - - filter.filterRGB(104, 0, Color.PINK.rgb) shouldBe Color.BLUE.rgb + test("arranges four colors in clockwise order starting from the top") { + val colors = listOf(Color.RED, Color.BLUE, Color.PINK, Color.GRAY) + val filter = RadialColorReplacementFilter(colors, Pair(0, 0)) + + // Uses pixel coordinates, so "up" is negative y and "down" is positive y + filter.filterRGB(5, -5, Color.MAGENTA.rgb) shouldBe colors[0].rgb + filter.filterRGB(5, 5, Color.MAGENTA.rgb) shouldBe colors[1].rgb + filter.filterRGB(-5, 5, Color.MAGENTA.rgb) shouldBe colors[2].rgb + filter.filterRGB(-5, -5, Color.MAGENTA.rgb) shouldBe colors[3].rgb } test("shifts the color") {