Page MenuHome

Keymaps: move builtin keymaps from C to Python.
ClosedPublic

Authored by Brecht Van Lommel (brecht) on Nov 6 2018, 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

Event Timeline

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.Nov 8 2018, 11:57 PM
  • Cleanup: separate selectmouse/actionmouse vars w/ underscore
  • Move keymap data into presets, make parameters optional