From 8dda28f86b7ccf0ea5c9e3155aa4b5f94169daaa Mon Sep 17 00:00:00 2001 From: Michael Richters Date: Tue, 18 May 2021 15:07:09 -0500 Subject: [PATCH] Separate tracking of layer shifts & locks This (hopefully) makes layer changes more intuitive for users, and more straightforward to explain and reason about. I believe this is a natural extension of the activation-order layer stack, and away from the fixed-order stack of old. Specifically, it changes things so that layer lock keys will deactivate the target layer if it is at the top of the active layer stack, but otherwise it will bring the target layer to the top of the stack, regardless of whether or not it was previously active. This ought to provide less surprising behaviour in the case where a layer was active, but hidden (possibly for long enough that the user can't recall its status). Pressing the layer lock key is now guaranteed to produce a user-visible change (unless the target layer is 100% transparent). Layer shift behaviour is also changed, but not quite in the same way. A layer shift key will now activate the target layer, as before, but pressing a second layer shift key for the same target layer won't bring the target layer to the top of the stack. The idea is that pressing a second identical layer shift should be interpreted as a continuation of the already-active shift, probably so that the user can change hands. A user could still forget about a shifted layer, but it would have to be a sticky OneShot layer shift, so I think this is reasonable. The major change is that locked and shifted layers are now tracked separately on the layer stack (using an offset), which means that locking a layer, then pressing and releasing a layer shift key for that layer will not result in the layer being unlocked. Closes #1009 Signed-off-by: Michael Richters --- src/kaleidoscope/layers.cpp | 150 +++++++++++++++++++----------------- src/kaleidoscope/layers.h | 9 ++- 2 files changed, 86 insertions(+), 73 deletions(-) diff --git a/src/kaleidoscope/layers.cpp b/src/kaleidoscope/layers.cpp index 414ce5af..235ce39f 100644 --- a/src/kaleidoscope/layers.cpp +++ b/src/kaleidoscope/layers.cpp @@ -30,9 +30,6 @@ #include "kaleidoscope/layers.h" // for Layer_, Layer, Layer_::GetKeyFunction, Layer_:... #include "kaleidoscope_internal/device.h" // for device -// The maximum number of layers allowed. -#define MAX_LAYERS 32; - // The following definitions of layer_count and keymaps_linear // are used if the user does not define a keymap within the sketch // by means of the KEYMAP macro. @@ -48,7 +45,7 @@ __attribute__((weak)) extern constexpr Key keymaps_linear[][kaleidoscope_interna namespace kaleidoscope { uint8_t Layer_::active_layer_count_ = 1; -int8_t Layer_::active_layers_[31]; +int8_t Layer_::active_layers_[MAX_ACTIVE_LAYERS]; uint8_t Layer_::active_layer_keymap_[kaleidoscope_internal::device.numKeys()]; Layer_::GetKeyFunction Layer_::getKey = &Layer_::getKeyFromPROGMEM; @@ -75,53 +72,39 @@ void Layer_::handleLayerKeyEvent(const KeyEvent &event) { target_layer = key_code - LAYER_SHIFT_OFFSET; switch (target_layer) { - case KEYMAP_NEXT: - // Key_KeymapNext_Momentary - if (keyToggledOn(event.state)) - activateNext(); - else - deactivateMostRecent(); - break; + case KEYMAP_NEXT: case KEYMAP_PREVIOUS: - // Key_KeymapPrevious_Momentary - if (keyToggledOn(event.state)) - deactivateMostRecent(); - else - activateNext(); + if (keyToggledOn(event.state)) { + uint8_t top_layer = active_layers_[active_layer_count_ - 1]; + if (target_layer == KEYMAP_NEXT) { + // Key_KeymapNext_Momentary + target_layer = top_layer + 1; + } else { + // Key_KeymapPrevious_Momentary + target_layer = top_layer - 1; + } + if (target_layer >= layer_count) { + live_keys.mask(event.addr); + return; + } + uint8_t target_layer_shifted = target_layer + LAYER_SHIFT_OFFSET; + activate(target_layer_shifted); + // We can't just change `event.key` here because `live_keys[]` has + // already been updated by the time `handleLayerKeyEvent()` gets called. + live_keys[event.addr] = ShiftToLayer(target_layer); + } break; default: // ShiftToLayer() - /* The default case is when we are switching to a layer by its number, and - * is a bit more complicated than switching there when the key toggles on, - * and away when it toggles off. - * - * We want to handle the case where we have more than one momentary layer - * key on our keymap that point to the same target layer, and we hold - * both, and release one. In this case, the layer should remain active, - * because the second momentary key is still held. - * - * To do this, we turn the layer back on if the switcher key is still - * held, not only when it toggles on. So when one of them is released, - * that does turn the layer off, but with the other still being held, the - * layer will toggle back on in the same cycle. - */ + target_layer += LAYER_SHIFT_OFFSET; + if (keyToggledOn(event.state)) { - // Re-think this: maybe we want to bring an already-active layer to the - // top when a layer shift key is pressed. - if (!isActive(target_layer)) + if (stackPosition(target_layer) < 0) { activate(target_layer); - } else { - // If there's another layer shift key keeping the target layer active, - // we need to abort before deactivating it. - for (Key key : live_keys.all()) { - if (key == event.key) { - return; - } } - // No other layer shift key for the target layer is pressed; deactivate - // it now. + } else { deactivate(target_layer); } break; @@ -129,11 +112,12 @@ void Layer_::handleLayerKeyEvent(const KeyEvent &event) { } else if (keyToggledOn(event.state)) { // LockLayer()/UnlockLayer() target_layer = key_code; - // switch keymap and stay there - if (isActive(target_layer)) + + if (stackPosition(target_layer) == active_layer_count_ - 1) { deactivate(target_layer); - else + } else { activate(target_layer); + } } } @@ -150,9 +134,9 @@ void Layer_::updateActiveLayers(void) { // of the top active layer that has a non-transparent entry for that address. for (auto key_addr : KeyAddr::all()) { for (uint8_t i = active_layer_count_; i > 0; --i) { - uint8_t layer = active_layers_[i - 1]; - Key key = (*getKey)(layer, key_addr); + uint8_t layer = unshifted(active_layers_[i - 1]); + Key key = (*getKey)(layer, key_addr); if (key != Key_Transparent) { active_layer_keymap_[key_addr.toInt()] = layer; break; @@ -170,9 +154,9 @@ void Layer_::move(uint8_t layer) { // We do pretty much what activate() does, except we do everything // unconditionally, to make sure all parts of the firmware are aware of the // layer change. - if (layer >= layer_count) { - layer = 0; - } + if (layer >= layer_count) + return; + active_layer_count_ = 1; active_layers_[0] = layer; @@ -185,13 +169,22 @@ void Layer_::move(uint8_t layer) { void Layer_::activate(uint8_t layer) { // If we're trying to turn on a layer that doesn't exist, abort (but // if the keymap wasn't defined using the KEYMAPS() macro, proceed anyway - if (layer >= layer_count) - return; + uint8_t layer_unshifted = unshifted(layer); - // If the target layer was already on, return - if (isActive(layer)) + if (layer_unshifted >= layer_count) return; + int8_t old_pos = stackPosition(layer); + if (old_pos >= 0) { + remove(old_pos); + } + + // Guarantee that we don't overflow by removing layers from the bottom if + // we're about to exceed the size of the active layers array. + while (active_layer_count_ >= MAX_ACTIVE_LAYERS) { + remove(0); + } + // Otherwise, push it onto the active layer stack active_layers_[active_layer_count_++] = layer; @@ -202,10 +195,17 @@ void Layer_::activate(uint8_t layer) { kaleidoscope::Hooks::onLayerChange(); } +void Layer_::remove(uint8_t i) { + memmove(&active_layers_[i], &active_layers_[i + 1], active_layer_count_ - (i + 1)); + --active_layer_count_; +} + // Deactivate a given layer void Layer_::deactivate(uint8_t layer) { + int8_t current_pos = stackPosition(layer); + // If the target layer was already off, return - if (!isActive(layer)) + if (current_pos < 0) return; // If the sole active layer is being deactivated, turn on the base layer and @@ -216,44 +216,50 @@ void Layer_::deactivate(uint8_t layer) { } // Remove the target layer from the active layer stack, and shift any layers - // above it down to fill in the gap - for (uint8_t i = 0; i < active_layer_count_; ++i) { - if (active_layers_[i] == layer) { - memmove(&active_layers_[i], &active_layers_[i + 1], active_layer_count_ - (i + 1)); - --active_layer_count_; - break; - } - } + // above it down to fill in the gap. + remove(current_pos); - // Update the keymap cache (but not live_composite_keymap_; that gets - // updated separately, when keys toggle on or off. See layers.h) + // Update the keymap cache. updateActiveLayers(); kaleidoscope::Hooks::onLayerChange(); } -bool Layer_::isActive(uint8_t layer) { +boolean Layer_::isActive(uint8_t layer) { + return ((stackPosition(layer) >= 0) || + (stackPosition(layer + LAYER_SHIFT_OFFSET) >= 0)); +} + +int8_t Layer_::stackPosition(uint8_t layer) { for (int8_t i = 0; i < active_layer_count_; ++i) { if (active_layers_[i] == layer) - return true; + return i; } - return false; + return -1; } -void Layer_::activateNext(void) { +void Layer_::activateNext() { activate(active_layers_[active_layer_count_ - 1] + 1); } -void Layer_::deactivateMostRecent(void) { - deactivate(active_layers_[active_layer_count_ - 1]); +void Layer_::deactivateMostRecent() { + uint8_t layer = unshifted(active_layers_[active_layer_count_ - 1]); + deactivate(layer); } void Layer_::forEachActiveLayer(forEachHandler h) { for (uint8_t i = 0; i < active_layer_count_; i++) { - (*h)(i, active_layers_[i]); + uint8_t layer = unshifted(active_layers_[i]); + (*h)(i, layer); } } +uint8_t Layer_::unshifted(uint8_t layer) { + if (layer >= LAYER_SHIFT_OFFSET) + layer -= LAYER_SHIFT_OFFSET; + return layer; +} + } // namespace kaleidoscope kaleidoscope::Layer_ Layer; diff --git a/src/kaleidoscope/layers.h b/src/kaleidoscope/layers.h index d0dd9027..b4bc611f 100644 --- a/src/kaleidoscope/layers.h +++ b/src/kaleidoscope/layers.h @@ -30,6 +30,9 @@ #include "kaleidoscope_internal/sketch_exploration/sketch_exploration.h" // for _INIT_SKETCH_EX... // clang-format off +#ifndef MAX_ACTIVE_LAYERS +#define MAX_ACTIVE_LAYERS 16 +#endif #define START_KEYMAPS __NL__ \ constexpr Key keymaps_linear[][kaleidoscope_internal::device.matrix_rows * kaleidoscope_internal::device.matrix_columns] PROGMEM = { @@ -120,8 +123,12 @@ class Layer_ { private: static uint8_t active_layer_count_; - static int8_t active_layers_[31]; + static int8_t active_layers_[MAX_ACTIVE_LAYERS]; static uint8_t active_layer_keymap_[kaleidoscope_internal::device.numKeys()]; + + static int8_t stackPosition(uint8_t layer); + static void remove(uint8_t stack_index); + static uint8_t unshifted(uint8_t layer); }; } // namespace kaleidoscope