tmk / tmk_keyboard

Keyboard firmwares for Atmel AVR and Cortex-M
3.98k stars 1.7k forks source link

Knowing how many layers the layout has #523

Open alex-ong opened 6 years ago

alex-ong commented 6 years ago

In terms of performance, we can do a bit of optimization if we know how many layers there are. This involves:

We can then use the num_layers to convert uint32_t's to uint8_t's etc, since 99% of the time people have <8 layers. Another area that could be optimized is when it scans from layer 31 down to layer 0 - it could start at MAX_LAYER instead.

The change is fairly easy to implement, the main issue is whether adding this feature will start other performance related changes that will make the code harder to read.

Thoughts on this kind of feature/and or these kinds of optimizations in general?

tmk commented 6 years ago

Good idea! I don't want users to define NUM_LAYERS if possible. We can use sizeof for this purpose in core/{actionmap, unimap or keymap}.h or somewhere? Not sure that changing integer type of variables is so useful, but we can define type like layer_t instead of uint32_t?

As for optimizastion, we will have to find code section that consumes time first.

alex-ong commented 6 years ago

Not sure that changing integer type of variables is so useful, but we can define type like layer_t instead of uint32_t?

Yes that's what I meant - we use matrix_row_t based on how wide a row is, and therefore I feel like layer_t should also be defined by how many layers there are.

As for determining NUM_LAYERS, sizeof is a neat way of solving the problem.

alex-ong commented 6 years ago

NUM_LAYERS - should it be a #define, a const int or an int?

I've hit a brick wall for storing NUM_LAYERS as a const int.

You can't calculate NUM_LAYERS in any file but the file where the layout is defined. This is because calling sizeof in {actionmap,unimap,keymap}.c/h doesn't work because the keymap hasn't been defined yet. https://stackoverflow.com/questions/48413313/c-accessing-incomplete-types-in-linked-files/48413600#48413600

Solutions:

tmk commented 6 years ago

Ah I see. I coundn't find this problem when I wrote last post. Then I would ditch my idea of auto calc and go simple way, the fomer solution. tmk_core has default value of NUM_LAYERS (in action_layer.h?) and users can change it in their config.h if needed.

I scaned quickly keymaps files in qmk_keyboard and found a keymap with 13 layers! :D https://github.com/qmk/qmk_firmware/blob/master/keyboards/ergodox/keymaps/narze/keymap.c