Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[OSX] Pinch to zoom support #4760

Closed
DorpsGek opened this issue Sep 5, 2011 · 10 comments
Closed

[OSX] Pinch to zoom support #4760

DorpsGek opened this issue Sep 5, 2011 · 10 comments
Labels
component: interface This is an interface issue flyspray This issue is imported from FlySpray (https://bugs.openttd.org/) patch from FlySpray This issue is in fact a Patch, but imported from FlySrpay

Comments

@DorpsGek
Copy link
Member

DorpsGek commented Sep 5, 2011

leecbaker opened the ticket and wrote:

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.

Attachments

Reported version: trunk
Operating system: Mac OS X


This issue was imported from FlySpray: https://bugs.openttd.org/task/4760
@DorpsGek
Copy link
Member Author

DorpsGek commented Sep 5, 2011

frosch wrote:

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.


This comment was imported from FlySpray: https://bugs.openttd.org/task/4760#comment10321

@DorpsGek
Copy link
Member Author

DorpsGek commented Sep 6, 2011

leecbaker wrote:

I'll look at that. Thanks.


This comment was imported from FlySpray: https://bugs.openttd.org/task/4760#comment10325

@DorpsGek
Copy link
Member Author

DorpsGek commented Sep 7, 2011

planetmaker wrote:

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 http://wiki.openttd.org/Coding_style


This comment was imported from FlySpray: https://bugs.openttd.org/task/4760#comment10332

@DorpsGek
Copy link
Member Author

leecbaker wrote:

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?

Attachments


This comment was imported from FlySpray: https://bugs.openttd.org/task/4760#comment10342

@DorpsGek
Copy link
Member Author

frosch wrote:

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


This comment was imported from FlySpray: https://bugs.openttd.org/task/4760#comment10343

@DorpsGek
Copy link
Member Author

planetmaker wrote:

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.


This comment was imported from FlySpray: https://bugs.openttd.org/task/4760#comment10344

@DorpsGek
Copy link
Member Author

leecbaker wrote:

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.

Attachments


This comment was imported from FlySpray: https://bugs.openttd.org/task/4760#comment10348

@DorpsGek
Copy link
Member Author

planetmaker wrote:

"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) {


This comment was imported from FlySpray: https://bugs.openttd.org/task/4760#comment10349

@DorpsGek
Copy link
Member Author

leecbaker wrote:

I should have read more carefully- I apologize. Removed whitespace on 3 lines.

Attachments


This comment was imported from FlySpray: https://bugs.openttd.org/task/4760#comment10351

@DorpsGek
Copy link
Member Author

DorpsGek commented Aug 5, 2013

michi_cc closed the ticket.

Reason for closing: Implemented

In r25666.


This comment was imported from FlySpray: https://bugs.openttd.org/task/4760

@DorpsGek DorpsGek closed this as completed Aug 5, 2013
@DorpsGek DorpsGek added component: interface This is an interface issue flyspray This issue is imported from FlySpray (https://bugs.openttd.org/) patch from FlySpray This issue is in fact a Patch, but imported from FlySrpay labels Apr 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: interface This is an interface issue flyspray This issue is imported from FlySpray (https://bugs.openttd.org/) patch from FlySpray This issue is in fact a Patch, but imported from FlySrpay
Projects
None yet
Development

No branches or pull requests

1 participant