kicad-developers team mailing list archive

[Bernhard Stegmaier <stegmaier@xxxxxxxxxxxxx>]

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 <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> 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 <http://trac.wxwidgets.org/ticket/15684>
> 
> 
>> On Feb 28, 2016, at 11:25 AM, Bernhard Stegmaier <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>> 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 );
>>>         }
>>>         else if( 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 );
>>>         else if( 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 <https://launchpad.net/~kicad-developers>
>>> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>
>>> Unsubscribe : https://launchpad.net/~kicad-developers <https://launchpad.net/~kicad-developers>
>>> More help   : https://help.launchpad.net/ListHelp <https://help.launchpad.net/ListHelp>
>> 
>> _______________________________________________
>> Mailing list: https://launchpad.net/~kicad-developers <https://launchpad.net/~kicad-developers>
>> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>
>> Unsubscribe : https://launchpad.net/~kicad-developers <https://launchpad.net/~kicad-developers>
>> More help   : https://help.launchpad.net/ListHelp <https://help.launchpad.net/ListHelp>
>