Merge pull request #174 from keyboardio/h/layers/double-caching

layers: Use a two-stage cache

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

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 live composite keymap. See
+   * layers.h for an explanation about the different caches we have. */
   if (keyToggledOn(keyState) || keyToggledOff(keyState))
-    Layer.updateKeymapCache(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

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_::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) {
@@ -62,29 +63,29 @@ Layer_::getKeyFromPROGMEM(uint8_t layer, byte row, byte col) {
 }
 
 void
-Layer_::updateKeymapCache(byte row, byte col) {
-  int8_t layer = highestLayer;
+Layer_::updateLiveCompositeKeymap(byte row, byte col) {
+  int8_t layer = activeLayers[row][col];
+  liveCompositeKeymap[row][col] = (*getKey)(layer, row, col);
+}
 
-  if (row >= ROWS || col >= COLS)
-    return;
+void
+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;
 
-  for (layer = highestLayer; layer >= DefaultLayer; layer--) {
+      while (layer > DefaultLayer) {
         if (Layer.isOn(layer)) {
           Key mappedKey = (*getKey)(layer, row, col);
 
           if (mappedKey != Key_Transparent) {
-        keyMap[row][col] = mappedKey;
+            activeLayers[row][col] = layer;
             break;
           }
         }
+        layer--;
       }
-}
-
-void
-Layer_::updateKeymapCache(void) {
-  for (byte row = 0; row < ROWS; row++) {
-    for (byte col = 0; col < COLS; col++) {
-      updateKeymapCache(row, col);
     }
   }
 }
@@ -103,23 +104,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 the layer did turn on, update the keymap cache. See layers.h for an
+   * explanation about the caches we have. */
+  if (!wasOn)
+    updateActiveLayers();
 }
 
 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 the layer did turn off, update the keymap cache. See layers.h for an
+   * explanation about the caches we have. */
+  if (wasOn)
+    updateActiveLayers();
 }
 
 boolean Layer_::isOn(uint8_t layer) {

src/layers.h

@@ -8,9 +8,43 @@ 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 `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
+   * 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 `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 `liveCompositeKeymap`-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
+   * `lookupOnActiveLayer`.
+   */
   static Key lookup(byte row, byte col) {
-    return keyMap[row][col];
+    return liveCompositeKeymap[row][col];
   }
+  static Key lookupOnActiveLayer(byte row, byte col) {
+    uint8_t layer = activeLayers[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 +66,13 @@ class Layer_ {
 
   static Key getKeyFromPROGMEM(uint8_t layer, byte row, byte col);
 
-  static void updateKeymapCache(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 keyMap[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);