From c319d7077a145e52c870eca10fc2875a1f3d1898 Mon Sep 17 00:00:00 2001 From: Jules Kerssemakers Date: Wed, 17 Jan 2024 22:43:53 +0100 Subject: [PATCH 1/8] Refactor Theme: docu + comments --- .../java/com/nicobrailo/pianoli/Theme.java | 25 +++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/com/nicobrailo/pianoli/Theme.java b/app/src/main/java/com/nicobrailo/pianoli/Theme.java index e5e8a34..e665eba 100644 --- a/app/src/main/java/com/nicobrailo/pianoli/Theme.java +++ b/app/src/main/java/com/nicobrailo/pianoli/Theme.java @@ -8,6 +8,9 @@ public class Theme { public static final String PREFIX = "theme_"; + /** + * The sequence of colors to render; repeats from the start if there are more keys than array entries. + */ private final KeyColor[] colors; public static Theme fromPreferences(Context context) { @@ -84,13 +87,32 @@ private Theme(KeyColor[] colors) { } public int getColorForKey(int keyIndex, boolean isPressed) { - final int col_idx = (keyIndex / 2) % colors.length; + final int col_idx = (keyIndex / 2) % colors.length; // divide by two to skip 'flat'/black keys at odd positions. final KeyColor color = colors[col_idx]; return isPressed ? color.pressed : color.normal; } + /** + * The render-colours for a big piano key: {@link #normal} and {@link #pressed}. + * + *

+ * Note that this only applies to big keys, the 'flat' keys are always black. + *

+ */ private static class KeyColor { + /** The normal rendering color, when the key is inactive */ public final int normal; + + /** + * The pressed/touched color of a key. + * + *

+ * Not automatically derived from {@link #normal}, because different hues need different amounts of + * real lightening for the same amount of subjective lightening. + *

+ * + * @see #createLighterWhenPressed(int, float) + */ public final int pressed; public KeyColor(int normal, int pressed) { @@ -102,5 +124,4 @@ public static KeyColor createLighterWhenPressed(int color, float blendWhiteFacto return new KeyColor(color, ColorUtils.blendARGB(color, Color.WHITE, blendWhiteFactor)); } } - } From 03ab015aa48a463fa0b4509552d771cca426904e Mon Sep 17 00:00:00 2001 From: Jules Kerssemakers Date: Wed, 17 Jan 2024 23:11:09 +0100 Subject: [PATCH 2/8] Refactor Theme: convert to enum. Since this shifts the color-evaluation to static-initializer time, we additionally need to mock Android.graphics.Color, so that our Theme-using tests continue to work. --- .../java/com/nicobrailo/pianoli/Theme.java | 102 +++++++++--------- app/src/test/java/android/graphics/Color.java | 60 +++++++++++ .../DynamicTranslationIdentifierTest.java | 8 +- 3 files changed, 110 insertions(+), 60 deletions(-) create mode 100644 app/src/test/java/android/graphics/Color.java diff --git a/app/src/main/java/com/nicobrailo/pianoli/Theme.java b/app/src/main/java/com/nicobrailo/pianoli/Theme.java index e665eba..4be15cc 100644 --- a/app/src/main/java/com/nicobrailo/pianoli/Theme.java +++ b/app/src/main/java/com/nicobrailo/pianoli/Theme.java @@ -5,7 +5,51 @@ import androidx.core.graphics.ColorUtils; -public class Theme { +public enum Theme { + /** + * Note that light green, orange and yellow have higher lightness than other colors, + * so adding just a little white doesn't have the desired effect. + * That is why they have a larger proportion of white added in. + */ + BOOMWHACKER(new KeyColor[] { + KeyColor.createLighterWhenPressed(Color.rgb(220, 0, 0), 0.5f), // Red + KeyColor.createLighterWhenPressed(Color.rgb(255, 135, 0), 0.6f), // Orange + KeyColor.createLighterWhenPressed(Color.rgb(255, 255, 0), 0.75f), // Yellow + KeyColor.createLighterWhenPressed(Color.rgb(80, 220, 20), 0.6f), // Light Green + KeyColor.createLighterWhenPressed(Color.rgb(0, 150, 150), 0.5f), // Dark Green + KeyColor.createLighterWhenPressed(Color.rgb(95, 70, 165), 0.5f), // Purple + KeyColor.createLighterWhenPressed(Color.rgb(213, 43, 149), 0.5f), // Pink + }), + + PASTEL(new KeyColor[] { + // https://colorbrewer2.org/#type=qualitative&scheme=Pastel1&n=8 + KeyColor.createLighterWhenPressed(0xfffbb4ae, 0.5f), + KeyColor.createLighterWhenPressed(0xffb3cde3, 0.5f), + KeyColor.createLighterWhenPressed(0xffccebc5, 0.5f), + KeyColor.createLighterWhenPressed(0xffdecbe4, 0.5f), + KeyColor.createLighterWhenPressed(0xfffed9a6, 0.5f), + KeyColor.createLighterWhenPressed(0xffffffcc, 0.5f), + KeyColor.createLighterWhenPressed(0xffe5d8bd, 0.5f), + }), + + RAINBOW(new KeyColor[] { + KeyColor.createLighterWhenPressed(0xff001caf, 0.5f), // darkblue + KeyColor.createLighterWhenPressed(0xff0099ff, 0.5f), // lightblue + KeyColor.createLighterWhenPressed(0xff63c624, 0.5f), // darkgreen + KeyColor.createLighterWhenPressed(0xffbde53d, 0.5f), // lightgreen + KeyColor.createLighterWhenPressed(0xfffcc000, 0.5f), // yellow + KeyColor.createLighterWhenPressed(0xffff810a, 0.5f), // lightorange + KeyColor.createLighterWhenPressed(0xffff5616, 0.5f), // darkorange + KeyColor.createLighterWhenPressed(0xffd51016, 0.5f), // red + }), + + BLACK_AND_WHITE(new KeyColor[] { + new KeyColor( + Color.rgb(240, 240, 240), + Color.rgb(200, 200, 200) + ) + }); + public static final String PREFIX = "theme_"; /** @@ -30,59 +74,9 @@ public static Theme fromPreferences(Context context) { } } - /** - * Note that light green, orange and yellow have higher lightness than other colors, - * so adding just a little white doesn't have the desired effect. - * That is why they have a larger proportion of white added in. - */ - private static final Theme BOOMWHACKER = new Theme( - new KeyColor[] { - KeyColor.createLighterWhenPressed(Color.rgb(220, 0, 0), 0.5f), // Red - KeyColor.createLighterWhenPressed(Color.rgb(255, 135, 0), 0.6f), // Orange - KeyColor.createLighterWhenPressed(Color.rgb(255, 255, 0), 0.75f), // Yellow - KeyColor.createLighterWhenPressed(Color.rgb(80, 220, 20), 0.6f), // Light Green - KeyColor.createLighterWhenPressed(Color.rgb(0, 150, 150), 0.5f), // Dark Green - KeyColor.createLighterWhenPressed(Color.rgb(95, 70, 165), 0.5f), // Purple - KeyColor.createLighterWhenPressed(Color.rgb(213, 43, 149), 0.5f), // Pink - } - ); - - private static final Theme PASTEL = new Theme( - new KeyColor[] { - // https://colorbrewer2.org/#type=qualitative&scheme=Pastel1&n=8 - KeyColor.createLighterWhenPressed(0xfffbb4ae, 0.5f), - KeyColor.createLighterWhenPressed(0xffb3cde3, 0.5f), - KeyColor.createLighterWhenPressed(0xffccebc5, 0.5f), - KeyColor.createLighterWhenPressed(0xffdecbe4, 0.5f), - KeyColor.createLighterWhenPressed(0xfffed9a6, 0.5f), - KeyColor.createLighterWhenPressed(0xffffffcc, 0.5f), - KeyColor.createLighterWhenPressed(0xffe5d8bd, 0.5f), - } - ); - - private static final Theme RAINBOW = new Theme( - new KeyColor[] { - KeyColor.createLighterWhenPressed(0xff001caf, 0.5f), // darkblue - KeyColor.createLighterWhenPressed(0xff0099ff, 0.5f), // lightblue - KeyColor.createLighterWhenPressed(0xff63c624, 0.5f), // darkgreen - KeyColor.createLighterWhenPressed(0xffbde53d, 0.5f), // lightgreen - KeyColor.createLighterWhenPressed(0xfffcc000, 0.5f), // yellow - KeyColor.createLighterWhenPressed(0xffff810a, 0.5f), // lightorange - KeyColor.createLighterWhenPressed(0xffff5616, 0.5f), // darkorange - KeyColor.createLighterWhenPressed(0xffd51016, 0.5f), // red - } - ); - - private static final Theme BLACK_AND_WHITE = new Theme( - new KeyColor[] { - new KeyColor( - Color.rgb(240, 240, 240), - Color.rgb(200, 200, 200) - ) - } - ); - - private Theme(KeyColor[] colors) { + + + Theme(KeyColor[] colors) { this.colors = colors; } diff --git a/app/src/test/java/android/graphics/Color.java b/app/src/test/java/android/graphics/Color.java new file mode 100644 index 0000000..08fbe01 --- /dev/null +++ b/app/src/test/java/android/graphics/Color.java @@ -0,0 +1,60 @@ +package android.graphics; + +/** + * Minimal test double for colour handling, so we can classload the {@link com.nicobrailo.pianoli.Theme} enum. + * + *

+ * When test-compiling, this will shadow the default (not-mocked-exception-throwing) implementation + * provided by the Android Gradle Plugin.
+ * This is a low-tech way of avoiding a full-featured mocking dependency like Powermock or Mockito. + *

+ */ +public class Color { + /** + * Needed for {@link com.nicobrailo.pianoli.Theme#BLACK_AND_WHITE} + * @noinspection unused + */ + public static int rgb(int red, int green, int blue) { + return 0; + } + + /** + * Used internally in {@link androidx.core.graphics.ColorUtils}, which is used by {@link com.nicobrailo.pianoli.Theme.KeyColor#createLighterWhenPressed(int, float)} + * @noinspection unused + */ + public static int alpha(int a) { + return 0; + } + + /** + * Used internally in {@link androidx.core.graphics.ColorUtils}, which is used by {@link com.nicobrailo.pianoli.Theme.KeyColor#createLighterWhenPressed(int, float)} + * @noinspection unused + */ + public static int red(int a) { + return 0; + } + + /** + * Used internally in {@link androidx.core.graphics.ColorUtils}, which is used by {@link com.nicobrailo.pianoli.Theme.KeyColor#createLighterWhenPressed(int, float)} + * @noinspection unused + */ + public static int green(int a) { + return 0; + } + + /** + * Used internally in {@link androidx.core.graphics.ColorUtils}, which is used by {@link com.nicobrailo.pianoli.Theme.KeyColor#createLighterWhenPressed(int, float)} + * @noinspection unused + */ + public static int blue(int a) { + return 0; + } + + /** + * Used internally in {@link androidx.core.graphics.ColorUtils}, which is used by {@link com.nicobrailo.pianoli.Theme.KeyColor#createLighterWhenPressed(int, float)} + * @noinspection unused + */ + public static int argb(int a, int r, int g, int b) { + return 0; + } +} diff --git a/app/src/test/java/com/nicobrailo/pianoli/DynamicTranslationIdentifierTest.java b/app/src/test/java/com/nicobrailo/pianoli/DynamicTranslationIdentifierTest.java index 8a5b549..c66adac 100644 --- a/app/src/test/java/com/nicobrailo/pianoli/DynamicTranslationIdentifierTest.java +++ b/app/src/test/java/com/nicobrailo/pianoli/DynamicTranslationIdentifierTest.java @@ -10,7 +10,6 @@ import java.io.IOException; import java.lang.reflect.Field; -import java.lang.reflect.Modifier; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; @@ -213,11 +212,8 @@ public static List getThemeTranslations() { * @see Theme */ public static List getThemes() { - Field[] fields = Theme.class.getDeclaredFields(); - return Arrays.stream(fields) - .filter(field -> Theme.class.equals(field.getType())) - .filter(field -> Modifier.isStatic(field.getModifiers())) - .map(Field::getName) + return Arrays.stream(Theme.values()) + .map(Enum::name) .collect(Collectors.toList()); } From ee1f04bf9a5ffeb8c1c3ebb3ca476a18daf75a92 Mon Sep 17 00:00:00 2001 From: Jules Kerssemakers Date: Thu, 18 Jan 2024 00:38:23 +0100 Subject: [PATCH 3/8] Decouple Theme from android Context, to make it testable. Move the Context-dependence 'up' to PianoCanvas for now. --- .../com/nicobrailo/pianoli/PianoCanvas.java | 4 +- .../com/nicobrailo/pianoli/Preferences.java | 2 +- .../java/com/nicobrailo/pianoli/Theme.java | 4 +- .../DynamicTranslationIdentifierTest.java | 2 +- .../nicobrailo/pianoli/PreferencesTest.java | 12 ++++ .../com/nicobrailo/pianoli/ThemeTest.java | 69 +++++++++++++++++++ 6 files changed, 87 insertions(+), 6 deletions(-) create mode 100644 app/src/test/java/com/nicobrailo/pianoli/PreferencesTest.java create mode 100644 app/src/test/java/com/nicobrailo/pianoli/ThemeTest.java diff --git a/app/src/main/java/com/nicobrailo/pianoli/PianoCanvas.java b/app/src/main/java/com/nicobrailo/pianoli/PianoCanvas.java index c70a23a..6f4c969 100644 --- a/app/src/main/java/com/nicobrailo/pianoli/PianoCanvas.java +++ b/app/src/main/java/com/nicobrailo/pianoli/PianoCanvas.java @@ -98,7 +98,9 @@ public PianoCanvas(Context context, AttributeSet as, int defStyle) { public void reInitPiano(Context context, String prefSoundset) { Log.i("PianOli::PianoCanvas", "re-initialising Piano"); this.piano = new Piano(screen_size_x, screen_size_y); - this.theme = Theme.fromPreferences(context); + + String prefTheme = Preferences.selectedTheme(context); + this.theme = Theme.fromPreference(prefTheme); // for config trigger updates piano.addListener(appConfigTrigger); diff --git a/app/src/main/java/com/nicobrailo/pianoli/Preferences.java b/app/src/main/java/com/nicobrailo/pianoli/Preferences.java index a3367b0..a76e53f 100644 --- a/app/src/main/java/com/nicobrailo/pianoli/Preferences.java +++ b/app/src/main/java/com/nicobrailo/pianoli/Preferences.java @@ -20,7 +20,7 @@ public class Preferences { private final static String PREF_SELECTED_SOUND_SET = "selectedSoundSet"; private final static String PREF_SELECTED_MELODIES = "selectedMelodies"; private final static String PREF_ENABLE_MELODIES = "enableMelodies"; - private static final String DEFAULT_THEME = "rainbow"; + public static final String DEFAULT_THEME = "rainbow"; private final static String PREF_THEME = "theme"; /** diff --git a/app/src/main/java/com/nicobrailo/pianoli/Theme.java b/app/src/main/java/com/nicobrailo/pianoli/Theme.java index 4be15cc..11d76ff 100644 --- a/app/src/main/java/com/nicobrailo/pianoli/Theme.java +++ b/app/src/main/java/com/nicobrailo/pianoli/Theme.java @@ -1,6 +1,5 @@ package com.nicobrailo.pianoli; -import android.content.Context; import android.graphics.Color; import androidx.core.graphics.ColorUtils; @@ -57,8 +56,7 @@ public enum Theme { */ private final KeyColor[] colors; - public static Theme fromPreferences(Context context) { - String selectedTheme = Preferences.selectedTheme(context); + public static Theme fromPreference(String selectedTheme) { switch (selectedTheme) { case "black_and_white": return BLACK_AND_WHITE; diff --git a/app/src/test/java/com/nicobrailo/pianoli/DynamicTranslationIdentifierTest.java b/app/src/test/java/com/nicobrailo/pianoli/DynamicTranslationIdentifierTest.java index c66adac..5e0878f 100644 --- a/app/src/test/java/com/nicobrailo/pianoli/DynamicTranslationIdentifierTest.java +++ b/app/src/test/java/com/nicobrailo/pianoli/DynamicTranslationIdentifierTest.java @@ -90,7 +90,7 @@ public void testNoLeftoverSoundSetTranslations(String translationIdentifier) thr * * @see Theme * @see Theme#PREFIX - * @see Theme#fromPreferences(Context) + * @see Theme#fromPreference(String) * @see Preferences#selectedTheme(Context) * @see R.xml#root_preferences */ diff --git a/app/src/test/java/com/nicobrailo/pianoli/PreferencesTest.java b/app/src/test/java/com/nicobrailo/pianoli/PreferencesTest.java new file mode 100644 index 0000000..b0eb3b5 --- /dev/null +++ b/app/src/test/java/com/nicobrailo/pianoli/PreferencesTest.java @@ -0,0 +1,12 @@ +package com.nicobrailo.pianoli; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.*; + +class PreferencesTest { + @Test + public void defaultThemeExists() { + assertEquals(Theme.RAINBOW, Theme.fromPreference(Preferences.DEFAULT_THEME)); + } +} diff --git a/app/src/test/java/com/nicobrailo/pianoli/ThemeTest.java b/app/src/test/java/com/nicobrailo/pianoli/ThemeTest.java new file mode 100644 index 0000000..db6f947 --- /dev/null +++ b/app/src/test/java/com/nicobrailo/pianoli/ThemeTest.java @@ -0,0 +1,69 @@ +package com.nicobrailo.pianoli; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; +import org.junit.jupiter.params.provider.EnumSource; +import org.junit.jupiter.params.provider.NullAndEmptySource; +import org.junit.jupiter.params.provider.ValueSource; + +import java.util.Locale; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +class ThemeTest { + /** + * Checks if theme values from historical versions of the app can be decoded. + * + *

+ * Tests against a hardcoded copy of {@link R.array#theme_entryValues}, in order to preserve historical values, + * even if we remove some themes in the future. + *

+ * + * @see Theme#fromPreference(String) + * @see R.array#theme_entryValues + */ + @ParameterizedTest(name = "[{index}] pref {0} -> Theme.{1}") + @CsvSource({ + "rainbow,RAINBOW", + "pastel,PASTEL", + "black_and_white,BLACK_AND_WHITE", + "boomwhacker,BOOMWHACKER", + }) + public void possiblyPersistedThemePreferencesWork(String persistedName, String expectedTheme) { + // test the intended, productive mapping + Theme decodedPref = Theme.fromPreference(persistedName); + assertEquals(expectedTheme, decodedPref.name(), + "mapping the possibly-persisted theme-preference '" + persistedName + "' yielded a surprising Theme-implementation: " + decodedPref); + } + + /** + * Checks if all currently implemented {@link Theme}s can be reached via their preference-identifier. + * + *

+ * This explicitly does not test if the app-UI will actually offer the theme in the settings UI. + * That kind of android UI-dependant test is out of scope here. + *

+ * + * @see Theme#fromPreference(String) + */ + @ParameterizedTest + @EnumSource(Theme.class) + public void fromPreferenceRoundTrip(Theme theme) { + String prefName = theme.name().toLowerCase(Locale.ROOT); // Root-locale should be sufficient for java identifiers. + + assertEquals(theme, Theme.fromPreference(prefName), + "Theme not roundtrippable via preferences; see Theme.fromPreference()"); + } + + /** + * In case we ever mess up our preferences-handling, it is better to forget the user's color preference, + * than it is to crash the app. + */ + @ParameterizedTest + @NullAndEmptySource + @ValueSource(strings = "non-existing-test-theme") + public void unknownPreferencesShouldFallback(String brokenPref) { + assertEquals(Theme.RAINBOW, Theme.fromPreference(brokenPref)); + } +} From 4bd3acdb44360f6db723427efc2f70ac575cda2a Mon Sep 17 00:00:00 2001 From: Jules Kerssemakers Date: Thu, 18 Jan 2024 01:43:52 +0100 Subject: [PATCH 4/8] Make Theme-from-Preference null-safe. This will prevent app-crashes if we ever seriously mishandle preferences in a future release. --- app/src/main/java/com/nicobrailo/pianoli/Theme.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/app/src/main/java/com/nicobrailo/pianoli/Theme.java b/app/src/main/java/com/nicobrailo/pianoli/Theme.java index 11d76ff..e5b2ca1 100644 --- a/app/src/main/java/com/nicobrailo/pianoli/Theme.java +++ b/app/src/main/java/com/nicobrailo/pianoli/Theme.java @@ -57,6 +57,12 @@ public enum Theme { private final KeyColor[] colors; public static Theme fromPreference(String selectedTheme) { + // defensive programming: if we ever mess up our preferences handling, it's better to fall back to default, + // than to crash the app. + if (selectedTheme == null) { + return RAINBOW; + } + switch (selectedTheme) { case "black_and_white": return BLACK_AND_WHITE; From f319c160771304c872ab28eb132b32ef4feac379 Mon Sep 17 00:00:00 2001 From: Jules Kerssemakers Date: Thu, 18 Jan 2024 01:48:14 +0100 Subject: [PATCH 5/8] Oops: don't attempt to draw non-existing small keys. Bug introduced in 2018 with commit cf8bd944. That converted a null-return to KeyArea(0,0,0,0), but forgot to update the matching check in the rendering logic. --- app/src/main/java/com/nicobrailo/pianoli/PianoCanvas.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/java/com/nicobrailo/pianoli/PianoCanvas.java b/app/src/main/java/com/nicobrailo/pianoli/PianoCanvas.java index 6f4c969..0c84621 100644 --- a/app/src/main/java/com/nicobrailo/pianoli/PianoCanvas.java +++ b/app/src/main/java/com/nicobrailo/pianoli/PianoCanvas.java @@ -156,7 +156,7 @@ private void drawSmallKeys(Canvas canvas) { for (int i = 1; i < piano.get_keys_count(); i += 2) { Paint paint = new Paint(); paint.setColor(piano.is_key_pressed(i) ? Color.GRAY : 0xFF333333); - if (piano.get_area_for_flat_key(i) != null) { + if (piano.get_area_for_flat_key(i) != Key.CANT_TOUCH_THIS) { draw_key(canvas, piano.get_area_for_flat_key(i), paint); } } From b42299ba86df61aaab29844478dd9527af1a5c44 Mon Sep 17 00:00:00 2001 From: Jules Kerssemakers Date: Thu, 18 Jan 2024 01:46:00 +0100 Subject: [PATCH 6/8] Move color decision for small keys into Theme. This: - makes small keys theme-able in the future, and - brings the drawing implementation for both key-types closer together. --- .../java/com/nicobrailo/pianoli/PianoCanvas.java | 13 ++++++------- app/src/main/java/com/nicobrailo/pianoli/Theme.java | 4 ++++ 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/app/src/main/java/com/nicobrailo/pianoli/PianoCanvas.java b/app/src/main/java/com/nicobrailo/pianoli/PianoCanvas.java index 0c84621..518e9ac 100644 --- a/app/src/main/java/com/nicobrailo/pianoli/PianoCanvas.java +++ b/app/src/main/java/com/nicobrailo/pianoli/PianoCanvas.java @@ -146,18 +146,14 @@ private static void resetCanvas(Canvas canvas) { private void drawBigKeys(Canvas canvas) { for (int i = 0; i < piano.get_keys_count(); i += 2) { - Paint paint = new Paint(); - paint.setColor(theme.getColorForKey(i, piano.is_key_pressed(i))); - draw_key(canvas, piano.get_area_for_key(i), paint); + draw_key(canvas, piano.get_area_for_key(i), i); } } private void drawSmallKeys(Canvas canvas) { for (int i = 1; i < piano.get_keys_count(); i += 2) { - Paint paint = new Paint(); - paint.setColor(piano.is_key_pressed(i) ? Color.GRAY : 0xFF333333); if (piano.get_area_for_flat_key(i) != Key.CANT_TOUCH_THIS) { - draw_key(canvas, piano.get_area_for_flat_key(i), paint); + draw_key(canvas, piano.get_area_for_flat_key(i), i); } } } @@ -177,7 +173,10 @@ void drawConfigGears(Canvas androidCanvas) { draw_icon_on_black_key(androidCanvas, gearIcon, appConfigTrigger.getNextExpectedKey(), normalSize, normalSize); } - void draw_key(final Canvas canvas, final Key rect, final Paint p) { + void draw_key(final Canvas canvas, final Key rect, int i) { + Paint p = new Paint(); + p.setColor(theme.getColorForKey(i, piano.is_key_pressed(i))); + // Draw the main (solid) background of the key. Rect r = new Rect(); diff --git a/app/src/main/java/com/nicobrailo/pianoli/Theme.java b/app/src/main/java/com/nicobrailo/pianoli/Theme.java index e5b2ca1..2ccd5a2 100644 --- a/app/src/main/java/com/nicobrailo/pianoli/Theme.java +++ b/app/src/main/java/com/nicobrailo/pianoli/Theme.java @@ -85,6 +85,10 @@ public static Theme fromPreference(String selectedTheme) { } public int getColorForKey(int keyIndex, boolean isPressed) { + if ((keyIndex & 1) == 1) { // odd index = black/flat/small key + return isPressed ? Color.GRAY : 0xFF333333; // hardcoded "black" for now, but theme-able in the future. + } + final int col_idx = (keyIndex / 2) % colors.length; // divide by two to skip 'flat'/black keys at odd positions. final KeyColor color = colors[col_idx]; return isPressed ? color.pressed : color.normal; From 494aa49a9e9092a519deb7c4f99029373889c1b9 Mon Sep 17 00:00:00 2001 From: Jules Kerssemakers Date: Thu, 18 Jan 2024 02:46:59 +0100 Subject: [PATCH 7/8] Move all area logic to Piano privates, to simplify drawing logic. Logic outside of Piano shouldn't have to care about the relationship between key-index odd/even and big-vs-small-ness of the keys. We're not completely there yet, due to the z-index draw ordering, but it's definitely an improvement. - Style: convert snake_case names to proper camelCase, where touched. --- .../java/com/nicobrailo/pianoli/Piano.java | 27 ++++++++++---- .../com/nicobrailo/pianoli/PianoCanvas.java | 37 +++++++++---------- 2 files changed, 37 insertions(+), 27 deletions(-) diff --git a/app/src/main/java/com/nicobrailo/pianoli/Piano.java b/app/src/main/java/com/nicobrailo/pianoli/Piano.java index 21be151..4ab2b50 100644 --- a/app/src/main/java/com/nicobrailo/pianoli/Piano.java +++ b/app/src/main/java/com/nicobrailo/pianoli/Piano.java @@ -165,11 +165,11 @@ int pos_to_key_idx(float pos_x, float pos_y) { if (pos_y > keys_flats_height) return big_key_idx; // Check if press is inside rect of flat key - Key flat = get_area_for_flat_key(big_key_idx); + Key flat = getAreaForSmallKey(big_key_idx + 1); if (flat.contains(pos_x, pos_y)) return big_key_idx + 1; if (big_key_idx > 0) { - Key prev_flat = get_area_for_flat_key(big_key_idx - 2); + Key prev_flat = getAreaForSmallKey(big_key_idx - 1); if (prev_flat.contains(pos_x, pos_y)) return big_key_idx - 1; } @@ -177,20 +177,31 @@ int pos_to_key_idx(float pos_x, float pos_y) { return big_key_idx; } - Key get_area_for_key(int key_idx) { - int x_i = key_idx / 2 * keys_width; + @NonNull + Key getAreaForKey(int keyIdx) { + if ((keyIdx & 1) == 0) { // even positions are the full, big keys + return getAreaForBigKey(keyIdx); + } else { + return getAreaForSmallKey(keyIdx); // odd positions are the small/black/flat keys. + } + } + + @NonNull + private Key getAreaForBigKey(int keyIdx) { + int x_i = keyIdx / 2 * keys_width; return new Key(x_i, x_i + keys_width, 0, keys_height); } - Key get_area_for_flat_key(int key_idx) { - final int octave_idx = (key_idx / 2) % 7; - if (octave_idx == 2 || octave_idx == 6) { + @NonNull + private Key getAreaForSmallKey(int keyIdx) { + final int octaveIdx = (keyIdx / 2) % 7; + if (octaveIdx == 2 || octaveIdx == 6) { // Keys without flat get a null-area return Key.CANT_TOUCH_THIS; } final int offset = keys_width - (keys_flat_width / 2); - int x_i = (key_idx / 2) * keys_width + offset; + int x_i = (keyIdx / 2) * keys_width + offset; return new Key(x_i, x_i + keys_flat_width, 0, keys_flats_height); } } diff --git a/app/src/main/java/com/nicobrailo/pianoli/PianoCanvas.java b/app/src/main/java/com/nicobrailo/pianoli/PianoCanvas.java index 518e9ac..9fc5b59 100644 --- a/app/src/main/java/com/nicobrailo/pianoli/PianoCanvas.java +++ b/app/src/main/java/com/nicobrailo/pianoli/PianoCanvas.java @@ -144,20 +144,6 @@ private static void resetCanvas(Canvas canvas) { canvas.drawPaint(p); } - private void drawBigKeys(Canvas canvas) { - for (int i = 0; i < piano.get_keys_count(); i += 2) { - draw_key(canvas, piano.get_area_for_key(i), i); - } - } - - private void drawSmallKeys(Canvas canvas) { - for (int i = 1; i < piano.get_keys_count(); i += 2) { - if (piano.get_area_for_flat_key(i) != Key.CANT_TOUCH_THIS) { - draw_key(canvas, piano.get_area_for_flat_key(i), i); - } - } - } - /** * Overlays gear icons onto the currently-held and next expected flat keys. */ @@ -173,7 +159,12 @@ void drawConfigGears(Canvas androidCanvas) { draw_icon_on_black_key(androidCanvas, gearIcon, appConfigTrigger.getNextExpectedKey(), normalSize, normalSize); } - void draw_key(final Canvas canvas, final Key rect, int i) { + void drawKey(final Canvas canvas, final int i) { + Key rect = piano.getAreaForKey(i); + if (rect == Key.CANT_TOUCH_THIS) { + return; // don't waste performance drawing the skipped black keys. + } + Paint p = new Paint(); p.setColor(theme.getColorForKey(i, piano.is_key_pressed(i))); @@ -253,7 +244,7 @@ void draw_key(final Canvas canvas, final Key rect, int i) { */ void draw_icon_on_black_key(final Canvas canvas, final Drawable icon, int key_idx, final int icon_width, final int icon_height) { - final Key key = piano.get_area_for_flat_key(key_idx); + final Key key = piano.getAreaForKey(key_idx); int icon_x = ((key.x_f - key.x_i) / 2) + key.x_i; int icon_y = icon_height; @@ -285,9 +276,17 @@ public void redraw(SurfaceHolder surfaceHolder) { if (canvas == null) return; resetCanvas(canvas); - drawBigKeys(canvas); - // Small keys drawn after big keys to ensure z-index - drawSmallKeys(canvas); + + // draw main, big keys (even key index) + for (int i = 0; i < piano.get_keys_count(); i += 2) { + drawKey(canvas, i); + } + + // Small keys drawn after big keys to ensure z-index (odd key index) + for (int i = 1; i < piano.get_keys_count(); i += 2) { + drawKey(canvas, i); + } + // Gear icons drawn after small keys, since they go on top of those. drawConfigGears(canvas); From 252fac1b57713e9de8e39fad3efdb03a4a5f9f1f Mon Sep 17 00:00:00 2001 From: Jules Kerssemakers Date: Mon, 22 Jan 2024 13:58:04 +0100 Subject: [PATCH 8/8] Theme javadoc, round 2. --- .../java/com/nicobrailo/pianoli/Theme.java | 63 +++++++++++++++---- 1 file changed, 50 insertions(+), 13 deletions(-) diff --git a/app/src/main/java/com/nicobrailo/pianoli/Theme.java b/app/src/main/java/com/nicobrailo/pianoli/Theme.java index 2ccd5a2..f7fdc8b 100644 --- a/app/src/main/java/com/nicobrailo/pianoli/Theme.java +++ b/app/src/main/java/com/nicobrailo/pianoli/Theme.java @@ -1,14 +1,36 @@ package com.nicobrailo.pianoli; import android.graphics.Color; - import androidx.core.graphics.ColorUtils; + +/** + * Switchable key-colouring decisions for {@link PianoCanvas}. + * + *

+ * Whenever {@link PianoCanvas} renders a key, it asks the current Theme-variant for the paint colour. + * This allows us to switch palettes via preferences. + *

+ * + * @see Preferences#selectedTheme(android.content.Context) + * @see PianoCanvas#drawKey(android.graphics.Canvas, int) + */ public enum Theme { /** + * Boomwhackers are colour-coded pipes that produce a (tuned) note when hit. + * Their popularity in educational circles cemented their colour-to-note mapping as a de-facto international standard. + * + *

+ * See also: wikipedia: Boomwhacker, and + * boomwhackers.com. + *

+ * + * + *

* Note that light green, orange and yellow have higher lightness than other colors, * so adding just a little white doesn't have the desired effect. * That is why they have a larger proportion of white added in. + *

*/ BOOMWHACKER(new KeyColor[] { KeyColor.createLighterWhenPressed(Color.rgb(220, 0, 0), 0.5f), // Red @@ -20,17 +42,22 @@ public enum Theme { KeyColor.createLighterWhenPressed(Color.rgb(213, 43, 149), 0.5f), // Pink }), + /** + * Soft pastel tones, derived from Colorbrewer2.org: Pastel. + */ PASTEL(new KeyColor[] { - // https://colorbrewer2.org/#type=qualitative&scheme=Pastel1&n=8 - KeyColor.createLighterWhenPressed(0xfffbb4ae, 0.5f), - KeyColor.createLighterWhenPressed(0xffb3cde3, 0.5f), - KeyColor.createLighterWhenPressed(0xffccebc5, 0.5f), - KeyColor.createLighterWhenPressed(0xffdecbe4, 0.5f), - KeyColor.createLighterWhenPressed(0xfffed9a6, 0.5f), - KeyColor.createLighterWhenPressed(0xffffffcc, 0.5f), - KeyColor.createLighterWhenPressed(0xffe5d8bd, 0.5f), + KeyColor.createLighterWhenPressed(0xfffbb4ae, 0.5f), // dark pink + KeyColor.createLighterWhenPressed(0xffb3cde3, 0.5f), // powder blue + KeyColor.createLighterWhenPressed(0xffccebc5, 0.5f), // pistachio green + KeyColor.createLighterWhenPressed(0xffdecbe4, 0.5f), // lavender + KeyColor.createLighterWhenPressed(0xfffed9a6, 0.5f), // orange + KeyColor.createLighterWhenPressed(0xffffffcc, 0.5f), // pale yellow + KeyColor.createLighterWhenPressed(0xffe5d8bd, 0.5f), // light pink }), + /** + * All the colours of the rainbow, C1 dark blue, C2 red, then looping back to blue. + */ RAINBOW(new KeyColor[] { KeyColor.createLighterWhenPressed(0xff001caf, 0.5f), // darkblue KeyColor.createLighterWhenPressed(0xff0099ff, 0.5f), // lightblue @@ -42,13 +69,23 @@ public enum Theme { KeyColor.createLighterWhenPressed(0xffd51016, 0.5f), // red }), + /** + * "classic" Ivory and Black. + */ BLACK_AND_WHITE(new KeyColor[] { - new KeyColor( - Color.rgb(240, 240, 240), - Color.rgb(200, 200, 200) + new KeyColor( // white, lighter + Color.rgb(240, 240, 240), // normal: slightly muted white; + Color.rgb(200, 200, 200) // darker gray when pressed ) - }); + }); // note that the black flat keys are implicit and hardcoded for all themes at the moment. + /** + * Prefix for preferences-values and translation strings. + * + *

+ * Often used implicitly, so don't forget to do full-text searches across the project when changing this. + *

+ */ public static final String PREFIX = "theme_"; /**