kicad-developers team mailing list archive

[Wayne Stambaugh <stambaughw@xxxxxxxxx>]

This patch doesn't seem to break anything on windows.  However, I
noticed that the touchpad scroll behavior is wrong in the 3D viewer.
The touchpad horizontal scroll does nothing and the touchpad vertical
scroll zooms.  We should probably fix this as well.  I commit this patch
as is r6606.  Thanks.

On 3/1/2016 2:13 PM, Bernhard Stegmaier wrote:
> It was the one you responded with
> <<<
> I believe the axis is for mice with multiple wheels where the 0 axis is
> the scroll wheel typically used for vertical scrolling. ...
>>>>
> 
> I did forget to mark it as [PATCH] in the title.
> Sorry, my fault.
> 
> For your convenience, again attached to this mail.
> 
> 
> 
> 
> 
> 
> Regards,
> Bernhard
> 
>> On 01 Mar 2016, at 20:05, Wayne Stambaugh <stambaughw@xxxxxxxxx> wrote:
>>
>> What patch?  Did I miss an email in this thread at some point?
>>
>> On 3/1/2016 1:42 PM, Bernhard Stegmaier wrote:
>>> With my patch it should behave the same way on all platforms.
>>> It just does ignore axis then…
>>>
>>> GAL does things already differently and apparently it is fine on Windows/Linux 
>>> and Mac with that code.
>>> So, just using my patch should be the easiest way to get things consistent again
>>> on all platforms for now.
>>>
>>>> On 01.03.2016, at 19:35, Wayne Stambaugh <stambaughw@xxxxxxxxx> wrote:
>>>>
>>>> On 3/1/2016 1:25 PM, Bernhard Stegmaier wrote:
>>>>> I did that for a PC mouse with my Mac.
>>>>> As I said in the beginning… Whoever does this, I get axis 0 when nothing 
>>>>> is pressed, and axis 1 when shift is pressed (using the same mouse, the 
>>>>> same mousewheel, no touchpad).
>>>>> That’s why it only did scroll horizontal with the original code (obviously only 
>>>>> on OS X).
>>>>
>>>> It sounds like this is not what is happen on linux.  We may have to
>>>> normalize the axis/shift key behavior to account for platform differences.
>>>>
>>>>>
>>>>> Of course, the axis is for things like touchpads and that’s exactly what gets 
>>>>> used in touchpad-panning mode.
>>>>> That’s exactly what is the really nice thing about the automatic axis change:
>>>>> With touchpad-panning the same piece of code works with a touchpad and
>>>>> a normal mouse.
>>>>> Unmodifed wheel scrolls up/down, shift-wheel scrolls left/right.
>>>>> Kicad doesn’t have to know anything about which device is there, it just
>>>>> works… :)
>>>>>
>>>>>
>>>>>> On 01.03.2016, at 19:15, Wayne Stambaugh <stambaughw@xxxxxxxxx> wrote:
>>>>>>
>>>>>> I believe the axis is for mice with multiple wheels where the 0 axis is
>>>>>> the scroll wheel typically used for vertical scrolling.  I do not
>>>>>> believe the shift or control keys have anything to do with the axis in
>>>>>> the wxMouseEvent but I have looked at the wxWidget code to see if that
>>>>>> is the case.  Perhaps there are some inconsistencies between platform as
>>>>>> to what these axis mean.  The easiest way to find this out is to trace
>>>>>> the mouse events and see which axis is being sent in the wxMouseEvent
>>>>>> inside the OnScroll function.
>>>>>>
>>>>>> On 3/1/2016 12:44 PM, Bernhard Stegmaier wrote:
>>>>>>> Yes, but documentation doesn’t state anything about changing axis with 
>>>>>>> shift/ctrl (at least, I didn’t see it).
>>>>>>>
>>>>>>> If the non-touchpad scroll code shall ignore ignore any additional axis, 
>>>>>>> then the current implementation is wrong (it doesn’t ignore axis).
>>>>>>>
>>>>>>> Attached a patch to ignore axis in non-touchpad scroll code (as it was 
>>>>>>> before rev4505).
>>>>>>> Therewith, also shift/ctrl panning again works on OS X (though it might not
>>>>>>> conform to Apple UI guidelines where shift-wheel does a horizontal scroll).
>>>>>>>
>>>>>>>
>>>>>>> Regards,
>>>>>>> Bernhard
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> PS:
>>>>>>> Mac users using a PC mouse and a Apple mouse in parallel might notice
>>>>>>> that the
>>>>>>> scroll direction of PC mouse is just the other way round than for Mac
>>>>>>> mouse (and
>>>>>>> maybe touchpad).
>>>>>>> Yes. I don’t want to investigate if I can find out which device is
>>>>>>> connected, so it
>>>>>>> is as it is. Don’t use a PC mouse on a Mac… :) 
>>>>>>>
>>>>>>>> On 29.02.2016, at 01:24, Wayne Stambaugh <stambaughw@xxxxxxxxx
>>>>>>>> <mailto:stambaughw@xxxxxxxxx>> wrote:
>>>>>>>>
>>>>>>>> Take a look at the documentation for wxMouseEvent.GetWheelAxis()[1].
>>>>>>>> Some mice and/or trackpads can generate additional axis for horizontal
>>>>>>>> scroll support.  That's why the scroll code ignores any additional axis.
>>>>>>>> Prior to 2.94, GetWheelAxis() returned an int where 0 was the vertical
>>>>>>>> scroll.  Now GetWheelAxis() returns wxMOUSE_WHEEL_VERTICAL and
>>>>>>>> wxMOUSE_WHEEL_HORIZONTAL which are enums.  I didn't look but I'm
>>>>>>>> assuming they map to 0 and 1 but it would be more correct to use the
>>>>>>>> enums rather than 0.
>>>>>>>>
>>>>>>>> [1]:
>>>>>>>> http://docs.wxwidgets.org/3.0/classwx_mouse_event.html#a6e455a3fa3708031d8571e42489d453e
>>>>>>>>
>>>>>>>> On 2/28/2016 6:22 PM, Bernhard Stegmaier wrote:
>>>>>>>>> Hi Garth,
>>>>>>>>>
>>>>>>>>> thanks for the link, do you remember what this patch does actually fix?
>>>>>>>>> I just checked and the bug is closed… and is integrated both in current
>>>>>>>>> 3.0.x branch and master (I am using master).
>>>>>>>>> If I understand the ticket correctly, this fix is only about the
>>>>>>>>> not-working event.skip() or some duplicate handling.
>>>>>>>>>
>>>>>>>>> So, even with this fix the current scrolling is broken due to (obviously
>>>>>>>>> OS X only) change in axis when shift is pressed.
>>>>>>>>> But, as you said, this might be on purpose, because it just leads to
>>>>>>>>> consistent behaviour of how it is handled in native OS X applications
>>>>>>>>> without having to change code or knowing which device is used.
>>>>>>>>>
>>>>>>>>> On OS X the only thing you would want to use is some Apple device with
>>>>>>>>> touchpad-panning anyway… :)
>>>>>>>>>
>>>>>>>>> I still would consider this a wxWidgets bug, because I would expect that
>>>>>>>>> a platform-independent application framework doesn’t show different
>>>>>>>>> platform specific behavior like this… but that’s another story.
>>>>>>>>>
>>>>>>>>> Anyway… this still leaves the question open on how to fix it - assuming
>>>>>>>>> that a consistent KiCad behavior on all platforms is the goal.
>>>>>>>>> Change axis back on OS X or remove the piece of code that checks axis
>>>>>>>>> (if somebody can remember why it is done this way)?
>>>>>>>>> It came in with
>>>>>>>>> http://bazaar.launchpad.net/~kicad-product-committers/kicad/product/revision/4505
>>>>>>>>> … but there is no hint why this was changed.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Regards,
>>>>>>>>> Bernhard
>>>>>>>>>
>>>>>>>>>> On 28 Feb 2016, at 23:18, Garth Corral <gcorral@xxxxxxxxx
>>>>>>>>>> <mailto:gcorral@xxxxxxxxx>
>>>>>>>>>> <mailto:gcorral@xxxxxxxxx>> wrote:
>>>>>>>>>>
>>>>>>>>>> Hi, Bernhard
>>>>>>>>>>
>>>>>>>>>> Yes, you have it right on both counts.   That was, and still is, my
>>>>>>>>>> assessment.  The existing behavior is not correct, at least as far as
>>>>>>>>>> OS X (and probably Linux) is concerned.  I don’t know about windows.  
>>>>>>>>>> That is the patch that fixes the issue to which I referred in that post.
>>>>>>>>>>
>>>>>>>>>> I think it might be this issue:
>>>>>>>>>>
>>>>>>>>>> http://trac.wxwidgets.org/ticket/15684
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>> On Feb 28, 2016, at 11:25 AM, Bernhard Stegmaier
>>>>>>>>>>> <stegmaier@xxxxxxxxxxxxx
>>>>>>>>>>> <mailto:stegmaier@xxxxxxxxxxxxx> <mailto:stegmaier@xxxxxxxxxxxxx>>
>>>>>>>>>>> wrote:
>>>>>>>>>>>
>>>>>>>>>>> FYI, meanwhile I found an old mail from Garth on this topic (March
>>>>>>>>>>> last year):
>>>>>>>>>>> <<<
>>>>>>>>>>> Shift-modified scrollwheel events in wxWidgets (at least on OS X)
>>>>>>>>>>> have their axis modified (by wxWidgets) to be horizontal.  This
>>>>>>>>>>> matches the behavior of native OS X applications as well.  So making
>>>>>>>>>>> shift-scrollwheel do vertical scrolling is wrong (and exposed a
>>>>>>>>>>> wxWidgets bug for which I committed a patch).
>>>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Maybe it is this patch
>>>>>>>>>>> patches/wxwidgets-3.0.0_macosx_scrolledwindow.patch
>>>>>>>>>>> but I didn’t find a wxWidgets bug for it…
>>>>>>>>>>>
>>>>>>>>>>> So, regardless if the scrolling direction with shift is right or not,
>>>>>>>>>>> another wxWidgets bug… should we keep on patching wxWidgets (and file
>>>>>>>>>>> a bug report and hope for the best)?
>>>>>>>>>>> The change I proposed below would at least make it fault tolerant
>>>>>>>>>>> without any additional workaround code.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Regards,
>>>>>>>>>>> Bernhard
>>>>>>>>>>>
>>>>>>>>>>>> On 28 Feb 2016, at 19:33, Bernhard Stegmaier
>>>>>>>>>>>> <stegmaier@xxxxxxxxxxxxx
>>>>>>>>>>>> <mailto:stegmaier@xxxxxxxxxxxxx> <mailto:stegmaier@xxxxxxxxxxxxx>>
>>>>>>>>>>>> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>> Hi,
>>>>>>>>>>>>
>>>>>>>>>>>> while testing the touchpad-panning for the recent 3d-viewer fixes I
>>>>>>>>>>>> noticed, that non-touchpad-panning using shift/ctrl-wheel and a
>>>>>>>>>>>> normal PC mouse seems to be broken on OS X with legacy canvas.
>>>>>>>>>>>>
>>>>>>>>>>>> It only pans horizontal regardless whether you press shift or ctrl.
>>>>>>>>>>>> This is already the case in the official 4.0.1, so it has nothing to
>>>>>>>>>>>> do with my changes.
>>>>>>>>>>>>
>>>>>>>>>>>> Debugging this I noticed that obviously wxWidgets or OS X change the
>>>>>>>>>>>> wheel axis from vertical (0) without shift (or, while pressing ctrl)
>>>>>>>>>>>> to horizontal (1) when pressing shift.
>>>>>>>>>>>>
>>>>>>>>>>>> So, this piece of code (draw_panel.cpp, line 985ff):
>>>>>>>>>>>>
>>>>>>>>>>>> if(event.ShiftDown()&&!event.ControlDown())
>>>>>>>>>>>> {
>>>>>>>>>>>> if(axis==0)
>>>>>>>>>>>> cmd.SetId(ID_PAN_UP);
>>>>>>>>>>>> else
>>>>>>>>>>>> cmd.SetId(ID_PAN_RIGHT);
>>>>>>>>>>>> }
>>>>>>>>>>>> elseif(event.ControlDown()&&!event.ShiftDown())
>>>>>>>>>>>> cmd.SetId(ID_PAN_LEFT);
>>>>>>>>>>>>
>>>>>>>>>>>> will do a pan left/right when shift is pressed (because axis==1)
>>>>>>>>>>>> instead of horizontal as it should.
>>>>>>>>>>>>
>>>>>>>>>>>> Question is… why is this “if( axis == 0 )” in there (and, only for
>>>>>>>>>>>> the shift case)?
>>>>>>>>>>>> Any special purpose?
>>>>>>>>>>>>
>>>>>>>>>>>> Of course, I could fix this by adding a OS X specific #ifdef, which
>>>>>>>>>>>> reverts the change of the axis on shift being pressed.
>>>>>>>>>>>> However, if the axis would be just ignored in the shift-case like in
>>>>>>>>>>>> the ctrl-case, it would work without any #ifdef (not tested):
>>>>>>>>>>>>
>>>>>>>>>>>> if(event.ShiftDown()&&!event.ControlDown())
>>>>>>>>>>>> cmd.SetId(ID_PAN_UP);
>>>>>>>>>>>> elseif(event.ControlDown()&&!event.ShiftDown())
>>>>>>>>>>>> cmd.SetId(ID_PAN_LEFT);
>>>>>>>>>>>>
>>>>>>>>>>>> That’s basically what GAL does (ignoring axis, just looking at
>>>>>>>>>>>> shift/ctrl modifiers) and that’s why it works there.
>>>>>>>>>>>> However, I don’t know if any intended functionality gets lost with
>>>>>>>>>>>> that change.
>>>>>>>>>>>> Maybe something like touchpad-panning when pressing shift, but zoom
>>>>>>>>>>>> without any modifier?
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Regards,
>>>>>>>>>>>> Bernhard
>>>>>>>>>>>> _______________________________________________
>>>>>>>>>>>> Mailing list: https://launchpad.net/~kicad-developers
>>>>>>>>>>>> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>>>>>>>>>>>> <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>
>>>>>>>>>>>> <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>
>>>>>>>>>>>> Unsubscribe : https://launchpad.net/~kicad-developers
>>>>>>>>>>>> More help   : https://help.launchpad.net/ListHelp
>>>>>>>>>>>
>>>>>>>>>>> _______________________________________________
>>>>>>>>>>> Mailing list: https://launchpad.net/~kicad-developers
>>>>>>>>>>> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>>>>>>>>>>> <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>
>>>>>>>>>>> <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>
>>>>>>>>>>> Unsubscribe : https://launchpad.net/~kicad-developers
>>>>>>>>>>> More help   : https://help.launchpad.net/ListHelp
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> _______________________________________________
>>>>>>>>> Mailing list: https://launchpad.net/~kicad-developers
>>>>>>>>> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>>>>>>>>> <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>
>>>>>>>>> Unsubscribe : https://launchpad.net/~kicad-developers
>>>>>>>>> More help   : https://help.launchpad.net/ListHelp
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> _______________________________________________
>>>>>>>> Mailing list: https://launchpad.net/~kicad-developers
>>>>>>>> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>>>>>>>> <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>
>>>>>>>> Unsubscribe : https://launchpad.net/~kicad-developers
>>>>>>>> More help   : https://help.launchpad.net/ListHelp
>>>>>>>
>>>>>
>>>
>