Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Display rotated keys in the configuration screens #725

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kbrock
Copy link
Contributor

@kbrock kbrock commented Apr 30, 2020

Overview

QMK renders a representation of the keyboard in the configurator. If QMK could render keyboards with rotated keys, they would look more similar to the physical keyboard.

File format

QMK needs to store this data somewhere.

The current layout section of info.json has fields for specifying rotation: r, rx, ry and the future info.json format has that place available as well.

Currently no info.json files in the qmk_firmware/keyboards have rotational data stored. One reason is because the files currently render better when no rotational data is populated.

Rendering

Currently the configurator renders the keyboard without rotated keys. It needs to be updated to take rotational data into account. It is imperative that the configurator renders existing info.json files the same way as it does before this change.

Rotation (r) currently defaults to 0 or no rotation. This makes the transition easier. Reading older info.json files will default the keys to no rotation. Which is the current behavior before this change is made.

This should be committed before the other changes are made. Introducing this change by itself results in no change. Which is good in a transitional process.

Populate info.json files

The configurator can only render rotated keys if the info.json has that rotational data.

A popular layout editor, keyboard layout editor, has a raw format, kle.json, that has fields for specifying rotation as well. (also r, rx, and ry). This file is often a good starting point for creating keyboard layouts.

Currently [kle2json] converts from kle.json to info.json format but does not transfer the rotational data across. The kle.json files typically have rotational data, so this is a good place to start and help developers understand how these values work. It is also similar to the keyboard layout editor, so the developers are probably familiar with these concept.

This change needs to be made after the configurator changes. Populating data in the info file when the results can't be seen, or worse when the results look bad is not a good situation.

please see kle2json PR

note: the info.json files do not need to be changed. Developers only change them if they want the added rotational feature.


configurator changes

Defining the rotations

alice keyboard kle and an alternate kle were developed in keyboard layout configurator.
The alternate was mostly developed in the raw pane, which is not typical for developing a kle.json

getting the info.json file

The above kle.json files were passed throghkle2json with the changes from kle2json PR applied. No modifications were made to these info.json files. Typically, they will be tweaked by the keyboard developer.

Results

Original Alice info.json

The original alice info.json still renders the same.

configurator_orig_info_json

alice keyboard kle

The new alice info.json displays rotated keys:

orig_kle_configuration

Alternate kle.json

The hand generated info.json also displays rotated keys:

alice_confiturator_drag_drop

@yanfali
Copy link
Collaborator

yanfali commented Apr 30, 2020

This is a great conversation starter. There's some interesting ideas here. It actually points to some refactoring work I'd like to do around BaseKey that makes it less efficient that it really should be. Especially as some earlier work has now made rendering much slower than I think it should be.

I'll let @skullydazed weigh in on the JSON changes. I think this actually probably goes counter to some work he's doing right now to support multiple layouts. I think transform is the right way to do this, and I just ran an experiment where I replaced top and left with transform and it works largely the same.

Having said that, width and height per key have to go, and I have an idea how to do that, involving a new CSS spec called css custom variables. This should clean up the DOM significantly. It should make rendering more efficient as we can use CSS classes instead of per element styling.

@kbrock
Copy link
Contributor Author

kbrock commented Apr 30, 2020

yea, transform rx,ry rotate(r) transform y,x will work for all. I was trying to change as little as possible with the top/bottom

also, I'm not sure why you don't want 0's (i.e.: top:0) in the style. removing that restriction would simplify the vue code. (but you are more aware of these tradeoffs than I am.)

@yanfali
Copy link
Collaborator

yanfali commented Apr 30, 2020

yea, transform rx,ry rotate(r) transform y,x will work for all. I was trying to change as little as possible with the top/bottom

also, I'm not sure why you don't want 0's (i.e.: top:0) in the style. removing that restriction would simplify the vue code. (but you are more aware of these tradeoffs than I am.)

0 is the default, making the style parse code that doesn't do much isn't necessarily helpful. It's a micro optimization at best. transform has one downside, the transition code doesn't seem to know what to do with it, so we lose some of the snazzy animation we got for free by using simple css attributes.

I'm also thinking of doing away with the calculations and just using transform: scale

@kbrock
Copy link
Contributor Author

kbrock commented Apr 30, 2020

I tried hard to follow the layout project guidelines and discussed the rotation ideas with him.

This is my first time with vue, and my javascript is average. So any direction you want me to take, I'm all ears.

@yanfali
Copy link
Collaborator

yanfali commented Apr 30, 2020

This https://github.com/qmk/qmk_configurator/pull/726/files might interest you. I'm thinking about how to get rid of the height and width explicit values and replace them with css classes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants