In the sdl3 demo, the way that ctrl+key functions, select_all, undo, start of line, end of line, ..., etc., are managed causes issue with modded keyboards using macros or with android key boards.
Here's an example of what was happening, pressing a would erase all text and write the letter a instead, pressing b would write the letter b at the start of the text area and set the cursor in between the first and second character, pressing e would add the letter e at the end of the text area.
After some code investigation, I found out that calling nk_input_key would always increase ctx->input.keyboard.keys[key].clicked even if ctrl wasn't push, then if it was released while the clicked counter was still equals to 1, it would increase it to 2 triggering the nk_input_key_pressed due to the validation of if ((k->down && k->clicked) || (!k->down && k->clicked >= 2)).
In #879 there was a small discussion about it and @sleeptightAnsiC asked me to start an issue to duscuss about it.
I've looked at a few other demos and there's a few that are handled like the sld3 demo—mainly sdl demos—, but there's also a six demos —mainly windows demos—, that filter the nk_input_key with ctrl` first.
I see that they also added return 1 there, maybe I should add that to the pull requests too.
To answer one of the question from the pull request, The release would be handled only if the letter key is released before releasing control. The problem here is that we should only send a release when the combo was released.
There was also a mention that the nk_textedit_key should handle those mods and I'm not sure how this would work, should we call nk_textedit_key here instead? Because I know that calling calling nk_input_key alone will cause the same issue, unless we need a fix inside nuklear.h to manage it properly, does nk_input_key need to support a mods enum?
After looking at it in with a deeper understanding of the code, I'm starting to think that we should have something like nk_input_key_modded, it would be similar to nk_input_key but would change the key pressed key state only if the mod is true and setup the released state only if the key was pressed in the first place. What do you think @sleeptightAnsiC ?
Activity
30
sleeptightAnsiC9mo agoThanks for reporting this. I really need to dive deeper into the problem before answering your questions.
Current situation with demos is quite delicate: most of them have very similar code paths but with enough differences to make them confusing to deal with. For the demo/sdl3_renderer one, I had to figure out how to deal with SDL_TextInputActive and I feel it might be causing some issues (maybe that's even the case here, not sure).
I've looked at a few other demos and there's a few that are handled like the sld3 demo—mainly sdl demos—, but there's also a six demos —mainly windows demos—, that filter the nk_input_key with ctrl` first.
@belug23 I will ask again just to make sure: You tried to reproduce this with other demos, right? and this bug simply doesn't occur anywhere else? or maybe the other demos are impossible to be built for the affected platform (Android) ?
sleeptightAnsiC9mo agoThanks for reporting this. I really need to dive deeper into the problem before answering your questions.
Current situation with demos is quite delicate: most of them have very similar code paths but with enough differences to make them confusing to deal with. For the demo/sdl3_renderer one, I had to figure out how to deal with SDL_TextInputActive and I feel it might be causing some issues (maybe that's even the case here, not sure).
I've looked at a few other demos and there's a few that are handled like the sld3 demo—mainly sdl demos—, but there's also a six demos —mainly windows demos—, that filter the nk_input_key with ctrl` first.
@belug23 I will ask again just to make sure: You tried to reproduce this with other demos, right? and this bug simply doesn't occur anywhere else? or maybe the other demos are impossible to be built for the affected platform (Android) ?
sleeptightAnsiC9mo agoThanks for reporting this. I really need to dive deeper into the problem before answering your questions.
Current situation with demos is quite delicate: most of them have very similar code paths but with enough differences to make them confusing to deal with. For the demo/sdl3_renderer one, I had to figure out how to deal with SDL_TextInputActive and I feel it might be causing some issues (maybe that's even the case here, not sure).
I've looked at a few other demos and there's a few that are handled like the sld3 demo—mainly sdl demos—, but there's also a six demos —mainly windows demos—, that filter the nk_input_key with ctrl` first.
@belug23 I will ask again just to make sure: You tried to reproduce this with other demos, right? and this bug simply doesn't occur anywhere else? or maybe the other demos are impossible to be built for the affected platform (Android) ?
belug239mo agoI thought about it in the shower, but we could add a little bit of complexity to the nk_input_key function by letting it handle the "only go to 2 if it was pressed already" part, but this would be a change that could have a bigger impact on the code that uses it.
There was also a mention of other possible issues with the current code, I did get a weird issue where if I did ctrl+A and kept it pressed then at some point the repeat would trigger an insert of A, removing the rest of the text, even if ctrl was pressed, adding the return 1 didn't solve it.
belug239mo agoI thought about it in the shower, but we could add a little bit of complexity to the nk_input_key function by letting it handle the "only go to 2 if it was pressed already" part, but this would be a change that could have a bigger impact on the code that uses it.
There was also a mention of other possible issues with the current code, I did get a weird issue where if I did ctrl+A and kept it pressed then at some point the repeat would trigger an insert of A, removing the rest of the text, even if ctrl was pressed, adding the return 1 didn't solve it.
belug239mo agoI thought about it in the shower, but we could add a little bit of complexity to the nk_input_key function by letting it handle the "only go to 2 if it was pressed already" part, but this would be a change that could have a bigger impact on the code that uses it.
There was also a mention of other possible issues with the current code, I did get a weird issue where if I did ctrl+A and kept it pressed then at some point the repeat would trigger an insert of A, removing the rest of the text, even if ctrl was pressed, adding the return 1 didn't solve it.
belug239mo agoI thought about it in the shower, but we could add a little bit of complexity to the nk_input_key function by letting it handle the "only go to 2 if it was pressed already" part, but this would be a change that could have a bigger impact on the code that uses it.
There was also a mention of other possible issues with the current code, I did get a weird issue where if I did ctrl+A and kept it pressed then at some point the repeat would trigger an insert of A, removing the rest of the text, even if ctrl was pressed, adding the return 1 didn't solve it.
sleeptightAnsiC9mo ago@belug23 I will ask again just to make sure: You tried to reproduce this with other demos, right? and this bug simply doesn't occur anywhere else? or maybe the other demos are impossible to be built for the affected platform (Android) ?
Could you please answer this one?
sleeptightAnsiC9mo ago@belug23 I will ask again just to make sure: You tried to reproduce this with other demos, right? and this bug simply doesn't occur anywhere else? or maybe the other demos are impossible to be built for the affected platform (Android) ?
Could you please answer this one?
belug239mo agoAfter testing sdl_opengl3, sdl_vulkan and x11, they either fail the test or have the contrains in the code.
IMGui demo doesn't have the issue.
sdl_opengl3 has the ctrl contraint but only on a for the select all, but the b and e still bypass the need for ctrl since it's not check in the inputs section.
belug239mo agoAfter testing sdl_opengl3, sdl_vulkan and x11, they either fail the test or have the contrains in the code.
IMGui demo doesn't have the issue.
sdl_opengl3 has the ctrl contraint but only on a for the select all, but the b and e still bypass the need for ctrl since it's not check in the inputs section.
belug239mo agoAfter testing sdl_opengl3, sdl_vulkan and x11, they either fail the test or have the contrains in the code.
IMGui demo doesn't have the issue.
sdl_opengl3 has the ctrl contraint but only on a for the select all, but the b and e still bypass the need for ctrl since it's not check in the inputs section.
belug239mo agoAfter testing sdl_opengl3, sdl_vulkan and x11, they either fail the test or have the contrains in the code.
IMGui demo doesn't have the issue.
sdl_opengl3 has the ctrl contraint but only on a for the select all, but the b and e still bypass the need for ctrl since it's not check in the inputs section.
belug239mo agoLooking at the commit that added the 'Select all' to a lot of the demos, they all have that ctrl pressed validation.
#762
p.s. for the glfw demos, it's not visible directly, but it's in an if statement checking ctrl for all that batch of inputs.
belug239mo agoLooking at the commit that added the 'Select all' to a lot of the demos, they all have that ctrl pressed validation.
#762
p.s. for the glfw demos, it's not visible directly, but it's in an if statement checking ctrl for all that batch of inputs.
belug239mo agoLooking at the commit that added the 'Select all' to a lot of the demos, they all have that ctrl pressed validation.
#762
p.s. for the glfw demos, it's not visible directly, but it's in an if statement checking ctrl for all that batch of inputs.
belug239mo agoLooking at the commit that added the 'Select all' to a lot of the demos, they all have that ctrl pressed validation.
#762
p.s. for the glfw demos, it's not visible directly, but it's in an if statement checking ctrl for all that batch of inputs.
sleeptightAnsiC9mo agoAfter testing sdl_opengl3, sdl_vulkan and x11, they either fail the test or have the contrains in the code. sdl_opengl3 has the ctrl contraint but only on a for the select all, but the b and e still bypass the need for ctrl since it's not check in the inputs section.
Well, we will need to change most of those demos then.
I'm still a bit afraid that with the code from https://github.com/Immediate-Mode-UI/Nuklear/pull/879, it will not work in cases like: hold CTRL > hold A > release CTRL > release A (the TEXT_SELECT_ALL state will still be DOWN, while it should not, am I right?)
I'm a bit confused, why this function generates "clicks" even when you repeatedly send UP states (down==false) to it, but it seems to be intentional:
https://github.com/Immediate-Mode-UI/Nuklear/blob/b5f67b88889e77acde49622747fd73888488db73/src/nuklear_input.c#L56-L70
sleeptightAnsiC9mo agoAfter testing sdl_opengl3, sdl_vulkan and x11, they either fail the test or have the contrains in the code. sdl_opengl3 has the ctrl contraint but only on a for the select all, but the b and e still bypass the need for ctrl since it's not check in the inputs section.
Well, we will need to change most of those demos then.
I'm still a bit afraid that with the code from https://github.com/Immediate-Mode-UI/Nuklear/pull/879, it will not work in cases like: hold CTRL > hold A > release CTRL > release A (the TEXT_SELECT_ALL state will still be DOWN, while it should not, am I right?)
I'm a bit confused, why this function generates "clicks" even when you repeatedly send UP states (down==false) to it, but it seems to be intentional:
https://github.com/Immediate-Mode-UI/Nuklear/blob/b5f67b88889e77acde49622747fd73888488db73/src/nuklear_input.c#L56-L70
Thanks for reporting this. I really need to dive deeper into the problem before answering your questions.
Current situation with demos is quite delicate: most of them have very similar code paths but with enough differences to make them confusing to deal with. For the
demo/sdl3_rendererone, I had to figure out how to deal withSDL_TextInputActiveand I feel it might be causing some issues (maybe that's even the case here, not sure).@belug23 I will ask again just to make sure: You tried to reproduce this with other demos, right? and this bug simply doesn't occur anywhere else? or maybe the other demos are impossible to be built for the affected platform (Android) ?