From b8f8c9b3d5f01ebb89ba125d1296284f1d83c8ee Mon Sep 17 00:00:00 2001 From: Gergely Nagy Date: Sat, 12 Aug 2017 08:05:16 +0200 Subject: [PATCH 1/5] Smarter Layer.on / Layer.off Only update the keymap cache if the layer state changed for real. If we turn a layer that was already on, on again, we do not need to update. Same for turning them off. This results in a tiny speedup if we have code that calls `Layer.on()` or `Layer.off()` often. Signed-off-by: Gergely Nagy --- src/layers.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/layers.cpp b/src/layers.cpp index ecce13a0..53254889 100644 --- a/src/layers.cpp +++ b/src/layers.cpp @@ -103,23 +103,29 @@ void Layer_::move(uint8_t layer) { } void Layer_::on(uint8_t layer) { + bool wasOn = isOn(layer); + bitSet(LayerState, layer); if (layer > highestLayer) highestLayer = layer; // Update the key cache, so that if anything depends on knowing the active // layout, the layout will be in sync. - updateKeymapCache(); + if (!wasOn) + updateKeymapCache(); } void Layer_::off(uint8_t layer) { + bool wasOn = isOn(layer); + bitClear(LayerState, layer); if (layer == highestLayer) highestLayer = top(); // Update the key cache, so that if anything depends on knowing the active // layout, the layout will be in sync. - updateKeymapCache(); + if (wasOn) + updateKeymapCache(); } boolean Layer_::isOn(uint8_t layer) { From 29a8b95615ad641e9d6c335d4c56b0b559b273cf Mon Sep 17 00:00:00 2001 From: Gergely Nagy Date: Sat, 12 Aug 2017 08:27:07 +0200 Subject: [PATCH 2/5] layers: Implement a two-stage cache With the new implementation, there are two lookup functions, because we have two caches, and different parts of the firmware will want to use either this or that (or perhaps both, in rare cases). First of all, we use caches because looking up a key through all the layers is costy, and the cost increases dramatically the more layers we have. Then, we have the `effectiveKeymapCache`, because to have layer behaviours we want, that is, if you hold a key on a layer, release the layer key but continue holding the other, we want for the layered keycode to continue repeating. At the same time, we want other keys to not be affected by the now-turned-off layer. So we update the keycode in the cache on-demand, when the key is pressed or released. (see the top of `handleKeyswitchEvent`). On the other hand, we also have plugins that scan the whole keymap, and do things based on that information, such as highlighting keys that changed between layers. These need to be able to look at a state of where the keymap *should* be, not necessarily where it is. The `effectiveKeymapCache` is not useful here. So we use a `keymapCache` which we update whenever layers change (see `Layer.on` and `Layer.off`), and it updates the cache to show how the keymap should look, without the `effectiveKeymapCache`-induced behaviour. Thus, if we are curious about what a given key will do, use `lookup`. If we are curious what the active layer state describes the key as, use `lookupUncached`. Signed-off-by: Gergely Nagy --- src/key_events.cpp | 4 +++- src/layers.cpp | 45 +++++++++++++++++++++++---------------------- src/layers.h | 39 ++++++++++++++++++++++++++++++++++++--- 3 files changed, 62 insertions(+), 26 deletions(-) diff --git a/src/key_events.cpp b/src/key_events.cpp index 9d5160a6..782e301b 100644 --- a/src/key_events.cpp +++ b/src/key_events.cpp @@ -44,8 +44,10 @@ static bool handleKeyswitchEventDefault(Key mappedKey, byte row, byte col, uint8 } void handleKeyswitchEvent(Key mappedKey, byte row, byte col, uint8_t keyState) { + /* If a key had an on or off event, we update the effective keymap cache. See + * layers.h for an explanation about the different caches we have. */ if (keyToggledOn(keyState) || keyToggledOff(keyState)) - Layer.updateKeymapCache(row, col); + Layer.updateEffectiveKeymapCache(row, col); /* If the key we are dealing with is masked, ignore it until it is released. * When releasing it, clear the mask, so future key events can be handled diff --git a/src/layers.cpp b/src/layers.cpp index 53254889..ea20e201 100644 --- a/src/layers.cpp +++ b/src/layers.cpp @@ -4,7 +4,8 @@ static uint8_t DefaultLayer; static uint32_t LayerState; uint8_t Layer_::highestLayer; -Key Layer_::keyMap[ROWS][COLS]; +Key Layer_::effectiveKeymapCache[ROWS][COLS]; +uint8_t Layer_::keymapCache[ROWS][COLS]; Key(*Layer_::getKey)(uint8_t layer, byte row, byte col) = Layer.getKeyFromPROGMEM; static void handleKeymapKeyswitchEvent(Key keymapEntry, uint8_t keyState) { @@ -62,29 +63,29 @@ Layer_::getKeyFromPROGMEM(uint8_t layer, byte row, byte col) { } void -Layer_::updateKeymapCache(byte row, byte col) { - int8_t layer = highestLayer; - - if (row >= ROWS || col >= COLS) - return; - - for (layer = highestLayer; layer >= DefaultLayer; layer--) { - if (Layer.isOn(layer)) { - Key mappedKey = (*getKey)(layer, row, col); - - if (mappedKey != Key_Transparent) { - keyMap[row][col] = mappedKey; - break; - } - } - } +Layer_::updateEffectiveKeymapCache(byte row, byte col) { + int8_t layer = keymapCache[row][col]; + effectiveKeymapCache[row][col] = (*getKey)(layer, row, col); } void Layer_::updateKeymapCache(void) { + memset(keymapCache, DefaultLayer, ROWS * COLS); for (byte row = 0; row < ROWS; row++) { for (byte col = 0; col < COLS; col++) { - updateKeymapCache(row, col); + int8_t layer = highestLayer; + + while (layer > DefaultLayer) { + if (Layer.isOn(layer)) { + Key mappedKey = (*getKey)(layer, row, col); + + if (mappedKey != Key_Transparent) { + keymapCache[row][col] = layer; + break; + } + } + layer--; + } } } } @@ -109,8 +110,8 @@ void Layer_::on(uint8_t layer) { if (layer > highestLayer) highestLayer = layer; - // Update the key cache, so that if anything depends on knowing the active - // layout, the layout will be in sync. + /* If the layer did turn on, update the keymap cache. See layers.h for an + * explanation about the caches we have. */ if (!wasOn) updateKeymapCache(); } @@ -122,8 +123,8 @@ void Layer_::off(uint8_t layer) { if (layer == highestLayer) highestLayer = top(); - // Update the key cache, so that if anything depends on knowing the active - // layout, the layout will be in sync. + /* If the layer did turn off, update the keymap cache. See layers.h for an + * explanation about the caches we have. */ if (wasOn) updateKeymapCache(); } diff --git a/src/layers.h b/src/layers.h index 091b1417..8990c6e1 100644 --- a/src/layers.h +++ b/src/layers.h @@ -8,9 +8,41 @@ class Layer_ { public: Layer_(void); + /* There are two lookup functions, because we have two caches, and different + * parts of the firmware will want to use either this or that (or perhaps + * both, in rare cases). + * + * First of all, we use caches because looking up a key through all the layers + * is costy, and the cost increases dramatically the more layers we have. + * + * Then, we have the `effectiveKeymapCache`, because to have layer behaviours + * we want, that is, if you hold a key on a layer, release the layer key but + * continue holding the other, we want for the layered keycode to continue + * repeating. At the same time, we want other keys to not be affected by the + * now-turned-off layer. So we update the keycode in the cache on-demand, when + * the key is pressed or released. (see the top of `handleKeyswitchEvent`). + * + * On the other hand, we also have plugins that scan the whole keymap, and do + * things based on that information, such as highlighting keys that changed + * between layers. These need to be able to look at a state of where the + * keymap *should* be, not necessarily where it is. The `effectiveKeymapCache` + * is not useful here. So we use a `keymapCache` which we update whenever + * layers change (see `Layer.on` and `Layer.off`), and it updates the cache to + * show how the keymap should look, without the `effectiveKeymapCache`-induced + * behaviour. + * + * Thus, if we are curious about what a given key will do, use `lookup`. If we + * are curious what the active layer state describes the key as, use + * `lookupUncached`. + */ static Key lookup(byte row, byte col) { - return keyMap[row][col]; + return effectiveKeymapCache[row][col]; } + static Key lookupUncached(byte row, byte col) { + uint8_t layer = keymapCache[row][col]; + return (*getKey)(layer, row, col); + } + static void on(uint8_t layer); static void off(uint8_t layer); static void move(uint8_t layer); @@ -32,12 +64,13 @@ class Layer_ { static Key getKeyFromPROGMEM(uint8_t layer, byte row, byte col); - static void updateKeymapCache(byte row, byte col); + static void updateEffectiveKeymapCache(byte row, byte col); static void updateKeymapCache(void); private: static uint8_t highestLayer; - static Key keyMap[ROWS][COLS]; + static Key effectiveKeymapCache[ROWS][COLS]; + static uint8_t keymapCache[ROWS][COLS]; }; Key layer_getKey(uint8_t layer, uint8_t r, uint8_t c); From b82b6910ea76a5f819b6f6e737a169cbba76988c Mon Sep 17 00:00:00 2001 From: Jesse Vincent Date: Sat, 12 Aug 2017 22:22:33 -0700 Subject: [PATCH 3/5] Rename 'keymapCache' to 'activeLayers' to better describe what it is Rename effectiveKeymapCache to liveCompositeKeymap to try to describe what it does just a tiny bit better --- src/Kaleidoscope.cpp | 2 +- src/key_events.cpp | 4 ++-- src/layers.cpp | 20 ++++++++++---------- src/layers.h | 24 +++++++++++++----------- 4 files changed, 26 insertions(+), 24 deletions(-) diff --git a/src/Kaleidoscope.cpp b/src/Kaleidoscope.cpp index 4e8e5b98..830ea571 100644 --- a/src/Kaleidoscope.cpp +++ b/src/Kaleidoscope.cpp @@ -18,7 +18,7 @@ Kaleidoscope_::setup(void) { handleKeyswitchEvent(Key_NoKey, 255, 255, 0); // Update the keymap cache, so we start with a non-empty state. - Layer.updateKeymapCache(); + Layer.updateActiveLayers(); } void diff --git a/src/key_events.cpp b/src/key_events.cpp index 782e301b..1004d855 100644 --- a/src/key_events.cpp +++ b/src/key_events.cpp @@ -44,10 +44,10 @@ static bool handleKeyswitchEventDefault(Key mappedKey, byte row, byte col, uint8 } void handleKeyswitchEvent(Key mappedKey, byte row, byte col, uint8_t keyState) { - /* If a key had an on or off event, we update the effective keymap cache. See + /* If a key had an on or off event, we update the live composite keymap. See * layers.h for an explanation about the different caches we have. */ if (keyToggledOn(keyState) || keyToggledOff(keyState)) - Layer.updateEffectiveKeymapCache(row, col); + Layer.updateLiveCompositeKeymap(row, col); /* If the key we are dealing with is masked, ignore it until it is released. * When releasing it, clear the mask, so future key events can be handled diff --git a/src/layers.cpp b/src/layers.cpp index ea20e201..765615e1 100644 --- a/src/layers.cpp +++ b/src/layers.cpp @@ -4,8 +4,8 @@ static uint8_t DefaultLayer; static uint32_t LayerState; uint8_t Layer_::highestLayer; -Key Layer_::effectiveKeymapCache[ROWS][COLS]; -uint8_t Layer_::keymapCache[ROWS][COLS]; +Key Layer_::liveCompositeKeymap[ROWS][COLS]; +uint8_t Layer_::activeLayers[ROWS][COLS]; Key(*Layer_::getKey)(uint8_t layer, byte row, byte col) = Layer.getKeyFromPROGMEM; static void handleKeymapKeyswitchEvent(Key keymapEntry, uint8_t keyState) { @@ -63,14 +63,14 @@ Layer_::getKeyFromPROGMEM(uint8_t layer, byte row, byte col) { } void -Layer_::updateEffectiveKeymapCache(byte row, byte col) { - int8_t layer = keymapCache[row][col]; - effectiveKeymapCache[row][col] = (*getKey)(layer, row, col); +Layer_::updateLiveCompositeKeymap(byte row, byte col) { + int8_t layer = activeLayers[row][col]; + liveCompositeKeymap[row][col] = (*getKey)(layer, row, col); } void -Layer_::updateKeymapCache(void) { - memset(keymapCache, DefaultLayer, ROWS * COLS); +Layer_::updateActiveLayers(void) { + memset(activeLayers, DefaultLayer, ROWS * COLS); for (byte row = 0; row < ROWS; row++) { for (byte col = 0; col < COLS; col++) { int8_t layer = highestLayer; @@ -80,7 +80,7 @@ Layer_::updateKeymapCache(void) { Key mappedKey = (*getKey)(layer, row, col); if (mappedKey != Key_Transparent) { - keymapCache[row][col] = layer; + activeLayers[row][col] = layer; break; } } @@ -113,7 +113,7 @@ void Layer_::on(uint8_t layer) { /* If the layer did turn on, update the keymap cache. See layers.h for an * explanation about the caches we have. */ if (!wasOn) - updateKeymapCache(); + updateActiveLayers(); } void Layer_::off(uint8_t layer) { @@ -126,7 +126,7 @@ void Layer_::off(uint8_t layer) { /* If the layer did turn off, update the keymap cache. See layers.h for an * explanation about the caches we have. */ if (wasOn) - updateKeymapCache(); + updateActiveLayers(); } boolean Layer_::isOn(uint8_t layer) { diff --git a/src/layers.h b/src/layers.h index 8990c6e1..0e503666 100644 --- a/src/layers.h +++ b/src/layers.h @@ -15,20 +15,22 @@ class Layer_ { * First of all, we use caches because looking up a key through all the layers * is costy, and the cost increases dramatically the more layers we have. * - * Then, we have the `effectiveKeymapCache`, because to have layer behaviours + * Then, we have the `liveCompositeKeymap`, because to have layer behaviours * we want, that is, if you hold a key on a layer, release the layer key but * continue holding the other, we want for the layered keycode to continue - * repeating. At the same time, we want other keys to not be affected by the + * repeating. + * + * At the same time, we want other keys to not be affected by the * now-turned-off layer. So we update the keycode in the cache on-demand, when * the key is pressed or released. (see the top of `handleKeyswitchEvent`). * * On the other hand, we also have plugins that scan the whole keymap, and do * things based on that information, such as highlighting keys that changed * between layers. These need to be able to look at a state of where the - * keymap *should* be, not necessarily where it is. The `effectiveKeymapCache` - * is not useful here. So we use a `keymapCache` which we update whenever + * keymap *should* be, not necessarily where it is. The `liveCompositeKeymap` + * is not useful here. So we use `activeLayers` which we update whenever * layers change (see `Layer.on` and `Layer.off`), and it updates the cache to - * show how the keymap should look, without the `effectiveKeymapCache`-induced + * show how the keymap should look, without the `liveCompositeKeymap`-induced * behaviour. * * Thus, if we are curious about what a given key will do, use `lookup`. If we @@ -36,10 +38,10 @@ class Layer_ { * `lookupUncached`. */ static Key lookup(byte row, byte col) { - return effectiveKeymapCache[row][col]; + return liveCompositeKeymap[row][col]; } static Key lookupUncached(byte row, byte col) { - uint8_t layer = keymapCache[row][col]; + uint8_t layer = activeLayers[row][col]; return (*getKey)(layer, row, col); } @@ -64,13 +66,13 @@ class Layer_ { static Key getKeyFromPROGMEM(uint8_t layer, byte row, byte col); - static void updateEffectiveKeymapCache(byte row, byte col); - static void updateKeymapCache(void); + static void updateLiveCompositeKeymap(byte row, byte col); + static void updateActiveLayers(void); private: static uint8_t highestLayer; - static Key effectiveKeymapCache[ROWS][COLS]; - static uint8_t keymapCache[ROWS][COLS]; + static Key liveCompositeKeymap[ROWS][COLS]; + static uint8_t activeLayers[ROWS][COLS]; }; Key layer_getKey(uint8_t layer, uint8_t r, uint8_t c); From f738c4ea1eec657fd5621622a02eae35f9bd99d8 Mon Sep 17 00:00:00 2001 From: Jesse Vincent Date: Sat, 12 Aug 2017 23:14:36 -0700 Subject: [PATCH 4/5] rename lookupUncached to lookupOnActiveLayer --- src/layers.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/layers.h b/src/layers.h index 0e503666..329c777b 100644 --- a/src/layers.h +++ b/src/layers.h @@ -35,12 +35,12 @@ class Layer_ { * * Thus, if we are curious about what a given key will do, use `lookup`. If we * are curious what the active layer state describes the key as, use - * `lookupUncached`. + * `lookupOnActiveLayer`. */ static Key lookup(byte row, byte col) { return liveCompositeKeymap[row][col]; } - static Key lookupUncached(byte row, byte col) { + static Key lookupOnActiveLayer(byte row, byte col) { uint8_t layer = activeLayers[row][col]; return (*getKey)(layer, row, col); } From 3199594181e5de9b3dcffa76bf47876b90b99929 Mon Sep 17 00:00:00 2001 From: Jesse Vincent Date: Sat, 12 Aug 2017 23:15:27 -0700 Subject: [PATCH 5/5] astyle --- src/layers.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/layers.h b/src/layers.h index 329c777b..b91889cc 100644 --- a/src/layers.h +++ b/src/layers.h @@ -18,7 +18,7 @@ class Layer_ { * Then, we have the `liveCompositeKeymap`, because to have layer behaviours * we want, that is, if you hold a key on a layer, release the layer key but * continue holding the other, we want for the layered keycode to continue - * repeating. + * repeating. * * At the same time, we want other keys to not be affected by the * now-turned-off layer. So we update the keycode in the cache on-demand, when