kicad-developers team mailing list archive

[Wayne Stambaugh <stambaughw@xxxxxxxxx>]

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
>>>>>
>>>
>