Re: [Kicad-developers] [PATCH] GTK+3 zooming
Am 2018-11-23 15:38, schrieb John Beard: Hi Seth, Sure thing, should it also be a constructor parameter? Or construct to a default then set it? Hi John- Probably an overridable default constructor parameter. We'll need the get/set pair regardless to respond to the preferences change as we don't want to tear down the controller for each change. -Seth ___ Mailing list: https://launchpad.net/~kicad-developers Post to : kicad-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~kicad-developers More help : https://help.launchpad.net/ListHelp
Re: [Kicad-developers] Adding global mouse shortcuts in GAL tool framework
Hi Orson, Thanks for the idea - seems like that could be a fairly flexible way to generalise action shortcuts. Perhaps such a refactor would be better after the hotkeys aren't synced with legacy, though. Cheers, John On Fri, Nov 23, 2018 at 8:58 AM Maciej Sumiński wrote: > > Hi John, > > One possible way of solving the problem is to change the m_defaultHotkey > in TOOL_ACTION class to a TOOL_EVENT field which says what triggers the > action. Then ACTION_MANAGER instead of comparing hotkeys should run > TOOL_EVENT::Matches() to check whether a processed TOOL_EVENT is > supposed to fire an action. > > Cheers, > Orson > > On 11/21/18 11:15 AM, John Beard wrote: > > Hi, > > > > A little technical query provoked by someone else's question, which I > > was unable to answer. > > > > Say one wanted to add a global mouse shortcut, for example, double > > middle-click to do "zoom-to-fit" [1]. > > > > The current mouse events are handled on a contextual basis in the > > event loops for each tool. Thus, each tool handles its own clicks, > > drags, etc, which generally makes sense for the basic left/right > > clicks as what that does is usually highly tool-dependent. Global key > > shortcuts for ACTIONs are handled at a higher level and and either > > trigger a transition set by setTransitions(), or translated to an > > event and caught by the current tool's event loop as their own event > > (checked with evt->IsAction()) > > > > However, there appears to be no way to add a mouse click pattern to an > > ACTION, thus you can't have a global double-middle-click, unless you > > add an "if( evt->IsDblClick( BTN_MIDDLE))" to every event loop. > > > > What would be the right way (if any) to approach this? I'm ignoring > > for now the UI implications of presenting such a shortcut in the > > hotkey dialog, implementing the "set hotkey" action, and any problems > > that might arise in having it co-exist with legacy hotkeys. > > > > Cheers, > > > > John > > > > [1]: https://bugs.launchpad.net/kicad/+bug/1480868 > > ___ > Mailing list: https://launchpad.net/~kicad-developers > Post to : kicad-developers@lists.launchpad.net > Unsubscribe : https://launchpad.net/~kicad-developers > More help : https://help.launchpad.net/ListHelp ___ Mailing list: https://launchpad.net/~kicad-developers Post to : kicad-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~kicad-developers More help : https://help.launchpad.net/ListHelp
Re: [Kicad-developers] [PATCH] GTK+3 zooming
Hi Seth, Sure thing, should it also be a constructor parameter? Or construct to a default then set it? BTW, that WX spew is now fixed upstream in the 3 branch, so a shout out to the WX people for a quick response! Cheers, John On 23 November 2018 19:09:35 GMT, Seth Hillbrand wrote: >Am 2018-11-23 08:26, schrieb John Beard: >> Hi, >> >> This is a patch to refactor the zooming of WX_VIEW_CONTROL. This is >> related to lp:1786515 [1], but it's not a fix, it's just a refactor >to >> help debug the problem, and also tidy the code. > >Hi John- > >This looks good. Works for me on Linux and Mac. After one of our MSW >devs can test for ill effects there, I think we should merge these. > >The only additional change I'd like to see here would be putting the >"500" magic number into the ACCELERATING_ZOOM_CONTROLLER's class as a >settable value that we can later hook up to a user preferences. > >-Seth ___ Mailing list: https://launchpad.net/~kicad-developers Post to : kicad-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~kicad-developers More help : https://help.launchpad.net/ListHelp
Re: [Kicad-developers] [PATCH] GTK+3 zooming
Am 2018-11-23 08:26, schrieb John Beard: Hi, This is a patch to refactor the zooming of WX_VIEW_CONTROL. This is related to lp:1786515 [1], but it's not a fix, it's just a refactor to help debug the problem, and also tidy the code. Hi John- This looks good. Works for me on Linux and Mac. After one of our MSW devs can test for ill effects there, I think we should merge these. The only additional change I'd like to see here would be putting the "500" magic number into the ACCELERATING_ZOOM_CONTROLLER's class as a settable value that we can later hook up to a user preferences. -Seth ___ Mailing list: https://launchpad.net/~kicad-developers Post to : kicad-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~kicad-developers More help : https://help.launchpad.net/ListHelp
[Kicad-developers] [PATCH] GTK+3 zooming
Hi,
This is a patch to refactor the zooming of WX_VIEW_CONTROL. This is
related to lp:1786515 [1], but it's not a fix, it's just a refactor to
help debug the problem, and also tidy the code.
The refactor is done to avoid big chunks of conditionally compiled
code kicking around in functions, and to make the platform behaviours
clearer. The platform-specific bit is still ifdeff'ed, as making that
run-time is a little bit more verbose than is required, I feel.
There's also a patch (#2) for the libcommon tests to init WX.This
means you can see WXTRACE messages. The teardown of WX prints some
glib stuff on GTK3 [2], but it should be harmless, and doing the
teardown keeps valgrind happier.
Cheers,
John
[1]: https://bugs.launchpad.net/kicad/+bug/1786515
[2]: http://trac.wxwidgets.org/ticket/18274
From 86529229172eab9af7cacbc0508d784cd058bccd Mon Sep 17 00:00:00 2001
From: John Beard
Date: Thu, 22 Nov 2018 16:24:17 +
Subject: [PATCH 1/4] Break zoom control into a self-contained controller
This is done to avoid a big chunk of conditionally-compiled code
in the middle of the event function.
Also separates the zoom logic from the WX_VIEW_CONTROLS object
and isolates it in a separate class behind a clearer interface.
Add some simple tests for sane steps on GTK+3-sized scroll
steps.
---
Documentation/development/testing.md| 1 +
common/CMakeLists.txt | 1 +
common/view/wx_view_controls.cpp| 65 ++
common/view/zoom_controller.cpp | 110
include/view/wx_view_controls.h | 14 +--
include/view/zoom_controller.h | 110
qa/common/CMakeLists.txt| 2 +
qa/common/view/test_zoom_controller.cpp | 90 +++
8 files changed, 347 insertions(+), 46 deletions(-)
create mode 100644 common/view/zoom_controller.cpp
create mode 100644 include/view/zoom_controller.h
create mode 100644 qa/common/view/test_zoom_controller.cpp
diff --git a/Documentation/development/testing.md b/Documentation/development/testing.md
index 81fd22701..7fddb2050 100644
--- a/Documentation/development/testing.md
+++ b/Documentation/development/testing.md
@@ -243,6 +243,7 @@ Some available masks:
* `GAL_CACHED_CONTAINER`
* `PNS`
* `CN`
+* `SCROLL_ZOOM` - for the scroll-wheel zooming logic in GAL
* Plugin-specific (including "standard" KiCad formats):
* `3D_CACHE`
* `3D_SG`
diff --git a/common/CMakeLists.txt b/common/CMakeLists.txt
index 924599753..7c1db83df 100644
--- a/common/CMakeLists.txt
+++ b/common/CMakeLists.txt
@@ -46,6 +46,7 @@ set( GAL_SRCS
view/view_controls.cpp
view/view_overlay.cpp
view/wx_view_controls.cpp
+view/zoom_controller.cpp
# OpenGL GAL
gal/opengl/opengl_gal.cpp
diff --git a/common/view/wx_view_controls.cpp b/common/view/wx_view_controls.cpp
index 81e0e5752..6ef378469 100644
--- a/common/view/wx_view_controls.cpp
+++ b/common/view/wx_view_controls.cpp
@@ -28,6 +28,7 @@
#include
#include
+#include
#include
#include
@@ -35,6 +36,20 @@ using namespace KIGFX;
const wxEventType WX_VIEW_CONTROLS::EVT_REFRESH_MOUSE = wxNewEventType();
+
+static std::unique_ptr GetZoomControllerForPlatform()
+{
+#ifdef __WXMAC__
+// On Apple pointer devices, wheel events occur frequently and with
+// smaller rotation values. For those devices, let's handle zoom
+// based on the rotation amount rather than the time difference.
+return std::make_unique( CONSTANT_ZOOM_CONTROLLER::MAC_SCALE );
+#else
+return std::make_unique( 500 );
+#endif
+}
+
+
WX_VIEW_CONTROLS::WX_VIEW_CONTROLS( VIEW* aView, wxScrolledCanvas* aParentPanel ) :
VIEW_CONTROLS( aView ), m_state( IDLE ), m_parentPanel( aParentPanel ),
m_scrollScale( 1.0, 1.0 ), m_cursorPos( 0, 0 ), m_updateCursor( true )
@@ -72,12 +87,19 @@ WX_VIEW_CONTROLS::WX_VIEW_CONTROLS( VIEW* aView, wxScrolledCanvas* aParentPanel
m_parentPanel->Connect( wxEVT_SCROLLWIN_PAGEDOWN,
wxScrollWinEventHandler( WX_VIEW_CONTROLS::onScroll ), NULL, this );
+m_zoomController = GetZoomControllerForPlatform();
+
m_panTimer.SetOwner( this );
this->Connect( wxEVT_TIMER,
wxTimerEventHandler( WX_VIEW_CONTROLS::onTimer ), NULL, this );
}
+WX_VIEW_CONTROLS::~WX_VIEW_CONTROLS()
+{
+}
+
+
void WX_VIEW_CONTROLS::onMotion( wxMouseEvent& aEvent )
{
bool isAutoPanning = false;
@@ -110,7 +132,6 @@ void WX_VIEW_CONTROLS::onMotion( wxMouseEvent& aEvent )
void WX_VIEW_CONTROLS::onWheel( wxMouseEvent& aEvent )
{
const double wheelPanSpeed = 0.001;
-const double zoomLevelScale = 1.2; // The minimal step value when changing the current zoom level
// mousewheelpan disabled:
// wheel + ctrl-> horizontal scrolling;
@@ -153,46 +174,8 @@ void WX_VIEW_CONTROLS::onWheel( wxMouseEvent& aEvent )
}
else
{
-// Zooming
-wxLongLong timeStamp =
Re: [Kicad-developers] Adding global mouse shortcuts in GAL tool framework
Hi John, One possible way of solving the problem is to change the m_defaultHotkey in TOOL_ACTION class to a TOOL_EVENT field which says what triggers the action. Then ACTION_MANAGER instead of comparing hotkeys should run TOOL_EVENT::Matches() to check whether a processed TOOL_EVENT is supposed to fire an action. Cheers, Orson On 11/21/18 11:15 AM, John Beard wrote: > Hi, > > A little technical query provoked by someone else's question, which I > was unable to answer. > > Say one wanted to add a global mouse shortcut, for example, double > middle-click to do "zoom-to-fit" [1]. > > The current mouse events are handled on a contextual basis in the > event loops for each tool. Thus, each tool handles its own clicks, > drags, etc, which generally makes sense for the basic left/right > clicks as what that does is usually highly tool-dependent. Global key > shortcuts for ACTIONs are handled at a higher level and and either > trigger a transition set by setTransitions(), or translated to an > event and caught by the current tool's event loop as their own event > (checked with evt->IsAction()) > > However, there appears to be no way to add a mouse click pattern to an > ACTION, thus you can't have a global double-middle-click, unless you > add an "if( evt->IsDblClick( BTN_MIDDLE))" to every event loop. > > What would be the right way (if any) to approach this? I'm ignoring > for now the UI implications of presenting such a shortcut in the > hotkey dialog, implementing the "set hotkey" action, and any problems > that might arise in having it co-exist with legacy hotkeys. > > Cheers, > > John > > [1]: https://bugs.launchpad.net/kicad/+bug/1480868 signature.asc Description: OpenPGP digital signature ___ Mailing list: https://launchpad.net/~kicad-developers Post to : kicad-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~kicad-developers More help : https://help.launchpad.net/ListHelp
