Incorrect key positioning for rotated keys

[joelspadin]

The layout at https://github.com/eli2c/zmk-config-ergo_wireless/blob/7fb5f574b039c9eb11149db2559ec22d1ac2ea25/config/boards/shields/ergo_wireless/ergo_wireless.dtsi#L1 should look like this

however, the creator of that config repo reported on Discord that it looks like this instead

I noticed that the rx and ry values are both zero.

zmk-studio/src/keyboard/PhysicalLayout.tsx

Lines 57 to 58 in e5ab3b5

	let transformX = ((rx || x) - x) * oneU;
	let transformY = ((ry || y) - y) * oneU;

modifies the positioning if rx or ry are zero, which seems suspect.

[mhantsch]

But rx=ry=0 would be rotating around the top left corner of the whole keyboard, wouldn't it? If the keys are supposed to be rotated around their position, then the rx and ry should specify that position -- the centre of rotation.

I created a layout for an ErgoTravel keyboard, and to get the rotated keys to display correctly I had to make sure the rx and ry values are set to the correct rotation centre. See here: https://github.com/mhantsch/ergotravel-zmk-module/blob/main/boards/shields/ergotravel/ergotravel-layouts.dtsi

        keys  //                     w   h    x    y     rot    rx    ry
            = <&key_physical_attrs 100 100    0   38       0     0     0>
            , <&key_physical_attrs 100 100  100   25       0     0     0>
            , <&key_physical_attrs 100 100  200   13       0     0     0>
            , <&key_physical_attrs 100 100  300    0       0     0     0>
            , <&key_physical_attrs 100 100  400   13       0     0     0>
            , <&key_physical_attrs 100 100  500   25       0     0     0>
            , <&key_physical_attrs 100 100  600   50       0     0     0>
            , <&key_physical_attrs 100 100  900   50       0     0     0>
            , <&key_physical_attrs 100 100 1000   25       0     0     0>
            , <&key_physical_attrs 100 100 1100   13       0     0     0>
            , <&key_physical_attrs 100 100 1200    0       0     0     0>
            , <&key_physical_attrs 100 100 1300   13       0     0     0>
            , <&key_physical_attrs 100 100 1400   25       0     0     0>
            , <&key_physical_attrs 100 100 1500   38       0     0     0>
            , <&key_physical_attrs 100 100    0  138       0     0     0>
            , <&key_physical_attrs 100 100  100  125       0     0     0>
            , <&key_physical_attrs 100 100  200  113       0     0     0>
            , <&key_physical_attrs 100 100  300  100       0     0     0>
            , <&key_physical_attrs 100 100  400  113       0     0     0>
            , <&key_physical_attrs 100 100  500  125       0     0     0>
            , <&key_physical_attrs 100 100  600  150       0     0     0>
            , <&key_physical_attrs 100 100  900  150       0     0     0>
            , <&key_physical_attrs 100 100 1000  125       0     0     0>
            , <&key_physical_attrs 100 100 1100  113       0     0     0>
            , <&key_physical_attrs 100 100 1200  100       0     0     0>
            , <&key_physical_attrs 100 100 1300  113       0     0     0>
            , <&key_physical_attrs 100 100 1400  125       0     0     0>
            , <&key_physical_attrs 100 100 1500  138       0     0     0>
            , <&key_physical_attrs 100 100    0  238       0     0     0>
            , <&key_physical_attrs 100 100  100  225       0     0     0>
            , <&key_physical_attrs 100 100  200  213       0     0     0>
            , <&key_physical_attrs 100 100  300  200       0     0     0>
            , <&key_physical_attrs 100 100  400  213       0     0     0>
            , <&key_physical_attrs 100 100  500  225       0     0     0>
            , <&key_physical_attrs 100 100  600  300    3000   700   350>
            , <&key_physical_attrs 100 100  900  300  (-3000)  900   350>
            , <&key_physical_attrs 100 100 1000  225       0     0     0>
            , <&key_physical_attrs 100 100 1100  213       0     0     0>
            , <&key_physical_attrs 100 100 1200  200       0     0     0>
            , <&key_physical_attrs 100 100 1300  213       0     0     0>
            , <&key_physical_attrs 100 100 1400  225       0     0     0>
            , <&key_physical_attrs 100 100 1500  238       0     0     0>
            , <&key_physical_attrs 100 100    0  338       0     0     0>
            , <&key_physical_attrs 100 100  100  325       0     0     0>
            , <&key_physical_attrs 100 100  200  313       0     0     0>
            , <&key_physical_attrs 100 100  300  300       0     0     0>
            , <&key_physical_attrs 125 100  425  325    1500   487   375>
            , <&key_physical_attrs 100 100  600  400    3000   700   350>
            , <&key_physical_attrs 100 100  700  360       0     0     0>   // XtraLeft
            , <&key_physical_attrs 100 100  800  360       0     0     0>   // XtraRight
            , <&key_physical_attrs 100 100  900  400  (-3000)  900   350>
            , <&key_physical_attrs 125 100 1050  325  (-1500) 1112   375>
            , <&key_physical_attrs 100 100 1200  300       0     0     0>
            , <&key_physical_attrs 100 100 1300  313       0     0     0>
            , <&key_physical_attrs 100 100 1400  325       0     0     0>
            , <&key_physical_attrs 100 100 1500  338       0     0     0>
            ;

[caksoylar]

The intention is to default to key's own top-left (i.e. x, y) for rx, ry if they aren't specified at all. But because in the devicetree-format description 0 is also used for defaults, this creates a problem if you actually want rx, ry to be at 0, 0.

Since we can't distinguish 0 from undefined, it seems wise to assume that rx and ry should always be specified correctly if rotation is not zero, and not implement a fallback.