--[BUGFIX] query both x and y offset in mouseScrollEvent for an active scroll.

[jturner65]

Motivation and Context

This PR fixes a bug with how viewer processes mouseScrollEvent offsets on a mac by looking at both x and y offsets if a mouseScrollEvent is processed. If shift is pressed, the scroll is defaulted to be horizontal on a mac, whereas previously in viewer we only examined the y offset to determine if a valid scroll has occurred, and in what direction.

How Has This Been Tested

C++ and python

Types of changes

• Docs change / refactoring / dependency upgrade
• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have completed my CLA (see CONTRIBUTING)
• I have added tests to cover my changes.
• All new and existing tests passed.

[mosra]

As we discussed on Slack -- I'd rather want to see this fixed / worked around / configured at some upper level than in the application code...

[jturner65]

The question remains, though, should we intercept it at a lower level? I would imagine folks might want to see/use this behavior in other UI applications (if any existed -shrug-)

[jturner65]

I'm not a chronic mac user, and I don't know if we would ever implement something in sim/viewer that would take advantage of horizontal scrolling explicitly, so I figured this was the route of least commitment :)

[mosra]

intercept it at a lower level?

Yeah, depends. If I would patch this in Magnum, I would add some window flag to the Application to toggle this behavior (I'm neither a fan of Apple forcing this to everyone nor a library reverting that behavior unconditionally).

I would also need to check how this behaves in SDL, if it's already accounted for in some way or not.

[mosra]

To limit this weird workaround a bit, I'd suggest to wrap it in #ifdef CORRADE_TARGET_APPLE, at least.

[mosra]

Suggested change

	float scrollModVal = event.offset().y() + event.offset().x();
	float scrollModVal = event.offset().max();

shorter / less error-prone, maybe? On touchpads you can have both directions and so the two could cancel each other out. I think the max() could work reasonably well there too.

[jturner65]

is it an abs max? Can be + or -

[jturner65]

Also, the x displacement is roughly 10x as big as the y, which was why i added them, but as you say, max would work too, if it is a magnitude max.

[mosra]

Ah, no it's not, good point. Mn::Math::abs(event.offset()).max() then :) blargh, that's also wrong

Mn::Math::sign(event.offset())*Mn::Math::abs(event.offset()).max() 😅 😅

[jturner65]

How about this :

  float scrollModVal = abs(event.offset().y()) > abs(event.offset().x())
                           ? event.offset().y()
                           : event.offset().x();

[jturner65]

This way then any mouse scrolling on 1d or 2d mice, platform independent, will accomplish something expected, maybe?

[mosra]

Yeah let's do that :)