Filter out combining character keysyms to add displayable characters to the lookup table. #14

Closed
Ray-V wants to merge 1 commits from feat/AltGr-combining-keysyms into master
Ray-V commented 3 years ago
Collaborator

Re: PR #13.

I tried both options, but then found out what ((keysym & 0xff000000) == 0x01000000) means and that gave me the method to filter out the keysyms.

The patch covers filtering out combining characters U0300 to U036f.
I've only added keysyms to the lookup table where I've identified the characters in the mapping files.
Whether they display or not, as with any other character, is dependent on whether the character glyph exists in the font being used.

I've tried to find characters closely matching the combining characters for displaying on the keys.
In a couple of instances, this hasn't been possible and I've used the 'replacement' character, which of course can be changed if any user has a better preference.

>[Re: PR #13](https://mirror.git.trinitydesktop.org/gitea/TDE/kvkbd/pulls/13#issuecomment-11812). I tried both options, but then found out what ((keysym & 0xff000000) == 0x01000000) means and that gave me the method to filter out the keysyms. The patch covers filtering out combining characters U0300 to U036f. I've only added keysyms to the lookup table where I've identified the characters in the mapping files. Whether they display or not, as with any other character, is dependent on whether the character glyph exists in the font being used. I've tried to find characters closely matching the combining characters for displaying on the keys. In a couple of instances, this hasn't been possible and I've used the 'replacement' character, which of course can be changed if any user has a better preference.
Ray-V added 1 commit 3 years ago
Owner

Hi Ray,
I tested this and it works fine.

I still think it would be better to use solution b. so it would be more flexible in case we add more characters in future. For example if we had to handle the case "0x0340" as per original logic, we would have to add yet anotehr if case.
I don't know how likely this could be, so perhaps I am overthinking. But solution "b" in #13 would take away this problem in full: if the character is in the array, we use the translated value, otherwise apply the original mask filter.

What do you think?

Hi Ray, I tested this and it works fine. I still think it would be better to use solution b. so it would be more flexible in case we add more characters in future. For example if we had to handle the case "0x0340" as per original logic, we would have to add yet anotehr if case. I don't know how likely this could be, so perhaps I am overthinking. But solution "b" in #13 would take away this problem in full: if the character is in the array, we use the translated value, otherwise apply the original mask filter. What do you think?
Owner

@Ray-V
On a separate note, it would be good to add the ability to use kvkbd on both the lock screen and on the tdm login screen. Would you be interested to work on this as further PRs?

@Ray-V On a separate note, it would be good to add the ability to use kvkbd on both the lock screen and on the tdm login screen. Would you be interested to work on this as further PRs?
Ray-V commented 3 years ago
Poster
Collaborator

But solution "b" in #13 would take away this problem in full: if the character is in the array, we use the translated value, otherwise apply the original mask filter.

If I understand solution 'b' correctly, it doesn't work for the 'combining' character range this PR addresses.
Looking at the previous example, for the Ukrainian AltGr+<TLDE> key, keysym 0x1000301, the key is blank.

The problem seems to be trying to match a 32-bit keysym in a 16-bit list [0x0301].
And when that isn't made, further on when the 'check for directly encoded 24-bit UCS characters' takes place, the returned keysym is the actual 24-bit keysym for the combining character.

So there will still be a need to filter these to 16-bit for the lookup table, at whatever point it's accessed.

For example if we had to handle the case "0x0340" as per original logic, we would have to add yet anotehr if case.

No we wouldn't. That is already in the range covered:
if (keysym >= 0x01000300 && keysym <= 0x0100036f)
It would need a new entry in the lookup table, but that would be the case either way.

I don't know how likely this could be

I don't think very likely. I've looked at the xkb/symbols files where the keyboard characters are defined and for the levels 1-4 [i.e. normal to AltGr+Shift] kvkbd now covers, I believe I've included them all.

it would be good to add the ability to use kvkbd on both the lock screen and on the tdm login screen. Would you be interested to work on this as further PRs?

Definitely, it would be a great feature, but if you mean the PRs should come from me, I don't think my programming ability is up to the task.
I believe the kdm issue was solved for the KDE4 version 0.6, but I couldn't pick out the relevant changes from the source code, the main problem being that it had been updated to Qt4. On the other hand, a fresh look might yield something ...
Happy to help in any way I can, though.

> But solution "b" in #13 would take away this problem in full: if the character is in the array, we use the translated value, otherwise apply the original mask filter. If I understand solution 'b' correctly, it doesn't work for the 'combining' character range this PR addresses. Looking at the previous example, for the Ukrainian AltGr+&lt;TLDE> key, keysym 0x1000301, the key is blank. The problem seems to be trying to match a 32-bit keysym in a 16-bit list [0x0301]. And when that isn't made, further on when the 'check for directly encoded 24-bit UCS characters' takes place, the returned keysym is the actual 24-bit keysym for the combining character. So there will still be a need to filter these to 16-bit for the lookup table, at whatever point it's accessed. > For example if we had to handle the case "0x0340" as per original logic, we would have to add yet anotehr if case. No we wouldn't. That is already in the range covered: `if (keysym >= 0x01000300 && keysym <= 0x0100036f)` It would need a new entry in the lookup table, but that would be the case either way. > I don't know how likely this could be I don't think very likely. I've looked at the xkb/symbols files where the keyboard characters are defined and for the levels 1-4 [i.e. normal to AltGr+Shift] kvkbd now covers, I believe I've included them all. > it would be good to add the ability to use kvkbd on both the lock screen and on the tdm login screen. Would you be interested to work on this as further PRs? Definitely, it would be a great feature, but if you mean the PRs should come from me, I don't think my programming ability is up to the task. I believe the kdm issue was solved for the KDE4 version 0.6, but I couldn't pick out the relevant changes from the source code, the main problem being that it had been updated to Qt4. On the other hand, a fresh look might yield something ... Happy to help in any way I can, though.
Owner

Hi @Ray-V

The problem seems to be trying to match a 32-bit keysym in a 16-bit list [0x0301].

Maybe I didn't explain clearly in my previous comment. I ended up making a quick commit and alternative PR #15 to explain what I meant.
This also solves another potential issue (again, maybe I am overthinking) where we could have for example 0x0340 and 0x01000340 characters that require different characters to be displayed. With PR #15 that will simply need two different entries in the array, while with the current code or PR #14 that would not be possible at all.

Definitely, it would be a great feature, but if you mean the PRs should come from me, I don't think my programming ability is up to the task.

Well, up to you if you want to give it a go or not. If you do, I will be happy to look at it 😄
In such case, I would suggest to start with the part related to the lock screen in kdesktop lock process. I know there is already some code there but I have not looked at it in detail.


If you could test out #15, let me know your opinion and if I introduced any mistakes/errors, that would be great!
Hi @Ray-V > The problem seems to be trying to match a 32-bit keysym in a 16-bit list [0x0301]. Maybe I didn't explain clearly in my previous comment. I ended up making a quick commit and alternative PR #15 to explain what I meant. This also solves another potential issue (again, maybe I am overthinking) where we could have for example 0x0340 and 0x01000340 characters that require different characters to be displayed. With PR #15 that will simply need two different entries in the array, while with the current code or PR #14 that would not be possible at all. > Definitely, it would be a great feature, but if you mean the PRs should come from me, I don't think my programming ability is up to the task. Well, up to you if you want to give it a go or not. If you do, I will be happy to look at it :smile: In such case, I would suggest to start with the part related to the lock screen in kdesktop lock process. I know there is already some code there but I have not looked at it in detail. <hr/> If you could test out #15, let me know your opinion and if I introduced any mistakes/errors, that would be great!
Owner

Closing as this has been replaced by #15.

Closing as this has been replaced by #15.
MicheleC closed this pull request 3 years ago
MicheleC deleted branch feat/AltGr-combining-keysyms 3 years ago
This pull request cannot be reopened because the branch was deleted.
Sign in to join this conversation.
No reviewers
No Milestone
No Assignees
2 Participants
Notifications
Due Date

No due date set.

Dependencies

No dependencies set.

Reference: TDE/kvkbd#14
Loading…
There is no content yet.