OpenTTD

Tasklist

FS#4760 - [OSX] Pinch to zoom support

Attached to Project: OpenTTD
Opened by Lee (leecbaker) - Monday, 05 September 2011, 13:35 GMT
Last edited by Michael Lutz (michi_cc) - Monday, 05 August 2013, 20:42 GMT
Type Patch
Category Interface
Status Closed
Assigned To No-one
Operating System Mac OS X
Severity Low
Priority Normal
Reported Version trunk
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No

Details

I've implemented the pinch-zoom gesture for zooming of the map. The gesture can be used to zoom the main view only.

I chose to directly call DoZoomInOutWindow on the main viewport (ID 0); as a result, the pinch zoom only works on the main viewport, and not secondary viewports or the map. Ideally, we would eventually determine out which window is active, etc, to properly zoom other viewports or the map if selected. Maybe this should be done before the patch is committed? I'm definitely open to suggestions here.

This functionality should work on 10.6 or higher, and is guarded to compile properly on earlier SDKs if necessary.
This task depends upon

Closed by  Michael Lutz (michi_cc)
Monday, 05 August 2013, 20:42 GMT
Reason for closing:  Implemented
Additional comments about closing:  In r25666.
Comment by frosch (frosch) - Monday, 05 September 2011, 17:39 GMT
I would suggest to add WKC_ZOOM_IN and WKC_ZOOM_OUT keycodes to gfx_type.h. Then the viewport and main window can react to that, just like they react to mousewheel events.
Comment by Lee (leecbaker) - Tuesday, 06 September 2011, 07:02 GMT
I'll look at that. Thanks.
Comment by Ingo von Borstel (planetmaker) - Wednesday, 07 September 2011, 10:52 GMT
Please mind also the coding style guidelines:
- comments in the same line always use //
- Objective C calls use spaces after and before [ and ] respectively.
- compiler conditions are indented to the same level as the code they apply to (but they don't change indetation level): http://hg.openttd.org/openttd/trunk.hg/rev/e395bb550a48#l11
- indentation of case / if / while / for / switch statements needs to be consistent: individual cases are indented one tab further, their code (if multi-line) another tab

Generally see https://wiki.openttd.org/Coding_style
Comment by Lee (leecbaker) - Sunday, 11 September 2011, 13:03 GMT
I think I've fixed all issues mentioned above. Also, planetmaker, I think in this patch I'm compliant with the OTTD coding style - can you verify?
Comment by frosch (frosch) - Sunday, 11 September 2011, 16:52 GMT
+ HandleKeypress(WKC_EQUALS<<16);

I suppose that shall be WKC_TOUCH_ZOOM_IN :)

Howver, I did not yet understood what kind of event this is. Is it a global event like a keypress, or is it specific to a screen position (i.e. to the mouse position)?

If it is specific to the mouseposition it should also apply to viewport windows, and not apply globally to the main viewport. (I guess it would be a mouseevent instead of a key event in that case.) Though I guess that would conflict with _current_magnification as well.
Comment by Ingo von Borstel (planetmaker) - Sunday, 11 September 2011, 18:07 GMT
I can't test it on my own hardware, it doesn't support it, but some people tested it.

From comparison with the normal zoom via scroll wheel / scroll gesture it seems that this gesture should work just the same on other views (i.e. the one under the mouse cursor, that's how it supposedly works in other applications) as well or it'll be a confusing and inconsistent GUI design.

wrt to coding style: it's missing spaces around the << operator at "WKC_TOUCH_ZOOM_OUT<<16" and you have pointless whitespace in two otherwise empty lines. I'd also space-align the enum-constants in gfx_type.h for the two gestures.
Otherwise it looks good.
Comment by Lee (leecbaker) - Monday, 12 September 2011, 06:31 GMT
I've made another pass, attempting to the formatting and WKC_EQUALS issues identified above. planetmaker, I don't see the two pointless blank lines you point out- the blank lines above each case follow the coding style of the rest of the switch statement (and the coding guidelines), and the blank lines above the while()s follow similar blocking in the rest of the file (not sure if this fits with guidelines).

Regarding this being a mouse versus keyboard event, my inclination was to follow the normal +/- zoom keys that I use regularly (WKC_EQUALS and WKC_MINUS), and to keep the functionality identical. I agree with your case for using this gesture for the minimap as well (or whatever the mouse is over). However, I don't actually use the minimap myself, and so I'm likely to spend my time fixing other bugs that have more of an impact on me (#4392), and possibly return to this issue later.
Comment by Ingo von Borstel (planetmaker) - Monday, 12 September 2011, 09:20 GMT
"pointless whitespace in otherwise empty lines" is not the same as "pointless blank lines". Blank lines should be empty and contain no tabs or spaces like the one preceeding + /* Touchpad gestures */ and + while(_current_magnification <= -1.f) {
Comment by Lee (leecbaker) - Monday, 12 September 2011, 14:24 GMT
I should have read more carefully- I apologize. Removed whitespace on 3 lines.

Loading...