[vtkusers] Question key conversion in QVTKRenderWindowInteractor.keyPressEvent

[Elvis Stansvik]

2016-06-16 14:44 GMT+02:00 David Gobbi <david.gobbi at gmail.com>:

> On Thu, Jun 16, 2016 at 6:32 AM, Elvis Stansvik <
> elvis.stansvik at orexplore.com> wrote:
>
>> 2016-06-16 14:12 GMT+02:00 David Gobbi <david.gobbi at gmail.com>:
>>
>>> Hi Elvis,
>>>
>>> The commit logs might hold the answers to some of your questions.
>>>
>>> Here's the history line for when QVTKRenderWindowInteractor.py was
>>> created:
>>>
>>> https://gitlab.kitware.com/vtk/vtk/commits/720b580c/Wrapping/Python/vtk/qt/QVTKRenderWindowInteractor.py
>>>
>>> The history continues here:
>>>
>>> https://gitlab.kitware.com/vtk/vtk/commits/3f70e5ff/Wrapping/Python/vtk/qt4/QVTKRenderWindowInteractor.py
>>>
>>> And finally comes to the present:
>>>
>>> https://gitlab.kitware.com/vtk/vtk/commits/master/Wrapping/Python/vtk/qt/QVTKRenderWindowInteractor.py
>>>
>>> Also, here is some pre-history:
>>>
>>> https://gitlab.kitware.com/vtk/vtk/commits/bae05540/Wrapping/Python/QVTKRenderWidget.py
>>> https://gitlab.kitware.com/vtk/vtk/tree/5b48ce33/Wrapping/Python
>>>
>>>
>> Thanks for the pointers, but it seems the code was like that when the Qt
>> 4 variant was initially contributed by Phil Thomson:
>>
>>
>> https://gitlab.kitware.com/vtk/vtk/commit/720b580c32b1ab002572fa59d796e409b5b0c346
>>
>
> Wrong.  You might need to practice your detective skills some more :)
>
> https://gitlab.kitware.com/vtk/vtk/commit/0ab4f15f
>
>
Bah, indeed :) Sorry for my sloppiness.


> But even if we forget about history/origin, doesn't it seem strange to use
>> str(ev.text()) in the keyPressEvent?
>>
>
> Yes, it seems strange, but there might be a reason.  You'll have to
> experiment with Qt to see what str(ev.text()) and chr(ev.key()) return
> under various situations.
>

Okay, that was easy:

str(ev.text()): d
chr(ev.key()): D
str(ev.text()): f
chr(ev.key()): F
str(ev.text()): g
chr(ev.key()): G
str(ev.text()): h
chr(ev.key()): H
str(ev.text()): j
chr(ev.key()): J
...

So that fix was indeed correct, and from the Qt docs for key():

"Note that this function does not distinguish between capital and
non-capital letters, use the text
<http://doc.qt.io/qt-5/qkeyevent.html#text>() function (returning the
Unicode text the key generated) for this purpose."

So I guess keyReleaseEvent should be changed to also use str(ev.text()).
But thinking more about how Qt's key() is used in
QVTKRenderWindowInteractor, for example:

if ev.key() < 256:
    ...

Is it not wrong to make assumptions about the .key() as a number like this?
I don't think Qt gives any guarantee that the Qt::Key enum values
correspond to ASCII values? (even if they currently do). Shouldn't the code
first call .text() and then check that it's a single byte string, instead
of making assumptions about the layout of the Qt::Key enum?

Elvis


>  - David
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://public.kitware.com/pipermail/vtkusers/attachments/20160616/e3cfacde/attachment.html>