layers: Use a two-stage cache #174
Conversation
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 <algernon@madhouse-project.org>
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 <algernon@madhouse-project.org>
|
Hi! Thanks for pushing on this. 'effective' feels like a weird name to me. Are you using it to mean 'the current state of the keymap, including possible additional layers for keys that were being held down before we changed layers'? I think we want to name the lookup functions in a way that suggests when you might want one or the other. So, If I've got this right, 'lookup', would become something like 'lookup live keymap state' and 'lookupUncached' would become something like 'lookup key on active layer' |
Yep.
Agreed, though, part of me feels that one of them should remain called
Correct, indeed. |
Rename effectiveKeymapCache to liveCompositeKeymap to try to describe what it does just a tiny bit better
|
@algernon - I did some renamings. I'd love to know if they accurately describe the intent of the features. |
|
I'm merging this for now so that we get at least a day of soak time before I need to send it to the factory. |
|
At a first glance, I'm happy with the new names. Thanks! |
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 ofhandleKeyswitchEvent).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
effectiveKeymapCacheis not useful here. So we use akeymapCachewhich we update whenever layers change (seeLayer.onandLayer.off), and it updates the cache to show how the keymap should look, without theeffectiveKeymapCache-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, uselookupUncached.This is one way to fix #173, at the cost of ~128 bytes of code, 64 bytes of SRAM, and ~+0.07-0.1ms of cycle time.
Kaleidoscope-Numlockand any other plugin that wants the real keymap, will need to be updated to useLayer.lookupUncached(which we may want to rename, as it is looking up a cached value, just a different one).I will be opening another PR soon, to see how the other proposed behaviour turns out.