Keymaps: move builtin keymaps from C to Python.
ClosedPublic

Authored by Brecht Van Lommel (brecht) on Tue, Nov 6, 7:28 PM.

Details

Summary

This should be purely an implementation change, for end users there should be
no functional difference.

The entire key configuration is in one file with ~5000 lines of code. I tried
to avoid code duplication and preserve comments and utility functions from the
C code. It's a bit long but for searching and editing it's also convenient to
have it all in one file.

I was hoping the default keyconfig code presets/keyconfig/blender.py folder,
so that multiple keymaps can be edited side by side more easily. But it seems
difficult to import from that location.

Not sure how to solve this, or if there is a different location that makes more
sense.

Diff Detail

Repository
rB Blender
Branch
arcpatch-D3907 (branched from blender2.8)
Build Status
Buildable 2455
Build 2455: arc lint + arc unit

Resolved conflict w/ 2.8 branch

Cleanup: literal fractions, use slots for KeymapParameters

Generally fine, I can't see any blocking issues for 2.8. Although we should avoid noisy commits since the file is so huge.

  • Am not so keen on putting keymap data in bpy_extras, but if we don't have a better alternative - would put in it's own submodule and name it more explicitly. bpy_extras.keyconfig_data.default (later we could have bpy_extras.keyconfig_data.minimal which gives a more usable is more easily extended).
  • The keymap expands some utility functions, eg (ED_keymap_template_select_all (done), ED_keymap_proportional_cycle, ED_keymap_proportional_editmode, ED_keymap_editmesh_elem_mode... possible others), should be marked as TODO (or handled before inclusion).
  • The keymap contains expanded loops which should be marked as TODO's.

    Even if they're committed as is, we could still define how these should be expressed, eg:
items.extend([
    ("mesh.select_mode", {"type": 'ONE', "value": 'PRESS'},
     {"properties": [("type", 'VERT')]}),
    ("mesh.select_mode", {"type": 'TWO', "value": 'PRESS'},
     {"properties": [("type", 'EDGE')]}),
    ("mesh.select_mode", {"type": 'THREE', "value": 'PRESS'},
     {"properties": [("type", 'FACE')]}),
    ("mesh.select_mode", {"type": 'ONE', "value": 'PRESS', "shift": True},
     {"properties": [("use_extend", True),("type", 'VERT')]}),
    ("mesh.select_mode", {"type": 'TWO', "value": 'PRESS', "shift": True},
     {"properties": [("use_extend", True),("type", 'EDGE')]}),
    ("mesh.select_mode", {"type": 'THREE', "value": 'PRESS', "shift": True},
     {"properties": [("use_extend", True),("type", 'FACE')]}),
    ("mesh.select_mode", {"type": 'ONE', "value": 'PRESS', "ctrl": True},
     {"properties": [("use_expand", True),("type", 'VERT')]}),
    ("mesh.select_mode", {"type": 'TWO', "value": 'PRESS', "ctrl": True},
     {"properties": [("use_expand", True),("type", 'EDGE')]}),
    ("mesh.select_mode", {"type": 'THREE', "value": 'PRESS', "ctrl": True},
     {"properties": [("use_expand", True),("type", 'FACE')]}),
    ("mesh.select_mode", {"type": 'ONE', "value": 'PRESS', "shift": True, "ctrl": True},
     {"properties": [("use_extend", True),("use_expand", True),("type", 'VERT')]}),
    ("mesh.select_mode", {"type": 'TWO', "value": 'PRESS', "shift": True, "ctrl": True},
     {"properties": [("use_extend", True),("use_expand", True),("type", 'EDGE')]}),
    ("mesh.select_mode", {"type": 'THREE', "value": 'PRESS', "shift": True, "ctrl": True},
     {"properties": [("use_extend", True),("use_expand", True),("type", 'FACE')]}),

Can be expressed as:

NUMBERS = ('ONE', 'TWO', 'THREE', 'FOUR', 'FIVE', 'SIX', 'SEVEN', 'EIGHT', 'NINE', 'ZERO')

# -- snip ---

    items.extend([
        *[("mesh.select_mode",
           {"type": num, "value": 'PRESS', **key_expand, **key_extend},
           {"properties": [*prop_extend, *prop_expand, ("type", elem)]})
            for key_expand, prop_expand in (({}, ()), ({"ctrl": True}, (("use_expand", True),)))
            for key_extend, prop_extend in (({}, ()), ({"shift": True}, (("use_extend", True),)))
            for num, elem in zip(NUMBERS[:3], ('VERT', 'EDGE', 'FACE'))
        ],

... or it could be constructed in a more typical for loop too.

  • Tested time to load the keymap for an optimized build.
    • Cold start 0.11 seconds (no pyc).
    • Warm start 0.05 seconds.

      Seems acceptable :)
  • Add selection template function

Use underscore prefix for private API calls, inline function calls to lists

  • Cleanup: use comment headers which aren't a hassle to align :)
  • Add keymap item wrappers for menu/pie/panel popups

Use km_* prefix for all functions that return a keymap

This way we can tell at a glance if a function defines a keymap or is a utility.

Also added GPL header to avoid ambiguity since the file contains some code.

  • Add block for outputting keymap data to a file (for testing non-functional changes)
  • De-duplicate gizmo keymaps (use-template)
  • Quiet unused args warnings, add pylint example command
  • Use '_template_items_*' prefix for templates that generate keymap items

rebase on blender2.8 branch

  • Move keymap data into 'keyconfig_data' submodule.
  • Rename 'all_keymaps' -> 'generate_keymaps'

Have been de-duplicating keymap items and cleaning things up.

Besides some de-duplication which can be done later (as noted in initial review), think this is ready for the 2.8x branch.

This revision is now accepted and ready to land.Thu, Nov 8, 11:57 PM
  • Cleanup: separate selectmouse/actionmouse vars w/ underscore
  • Move keymap data into presets, make parameters optional