Restrict the input from the gyro to the same range as desktop version. #41

Merged
merged 1 commit into from Apr 30, 2013

Conversation

Projects
None yet
2 participants
Contributor

magebarf commented Apr 26, 2013

When merging holman/gyro (commit 67997a7) the range restriction that was applied on the old accelerometer code was lost.

With the gyro code, there is no actual restriction on the x/y values (other than the -180 to 180 divided by 30 range) that can be the result after the motion event calculation.

Right now, it is possible to get values quite wide in range, thus getting a parallax offset far larger than the ranges defined for each layer.

In the old accelerometer code, the min and max values was restricted to -1 to 1, and then "normalized" to the range 0 to 1, which is what I've reapplied with slightly rewritten code.

Another side effect by the code in place before the suggested change is that when using gyro, the starting position of the images would always correspond to pointing at the top left corner, when using the desktop methods. With the suggested change, starting position will be corresponding to pointing at the middle of the image instead.

As I'm unsure what thoughts there are on backwards compatibility, I'm very open to suggestions for this pull request, and as I see it there are three possible ways forward;

  • Merge as is, considering the expanded ranges introduced with gyro changes as a bug.
  • Condition the fix as an option to enable(). Does not break anything for existing users.
  • Add an option to enable() to use the "unrestricted" behavior as it is today, but default option being that the fix is applied.

The main question I guess is how many users are aware of the behavior when using gyro. Looking at the 404 and 500 page of github it's apparent that it's been tested quite thoroughly on desktop, but load it up on a phone and you see quite a few artifacts/unexpected edges when you are tilting a bit (not necessarily that much, considering the starting position).

@magebarf magebarf Restrict the input from the gyro to the same range as desktop version.
Also ensures that starting position when using gyro is "in the middle", and not the position corresponding to the top left corner when using mouse.
253f033
Owner

cameronmcefee commented Apr 30, 2013

⚡️ Thanks. This is totally necessary.

  • Merge as is, considering the expanded ranges introduced with gyro changes as a bug.

This is my vote. My guess is that there are very few people that keep plax up-to date. Considering the motion support is passible at best as it stands, I'm down to just merge this and deal with any flak as it comes.

The main question I guess is how many users are aware of the behavior when using gyro. Looking at the 404 and 500 page of github it's apparent that it's been tested quite thoroughly on desktop, but load it up on a phone and you see quite a few artifacts/unexpected edges when you are tilting a bit (not necessarily that much, considering the starting position).

Plax was originally a quick plugin I wrote as part of my contract-to-hire agreement to get my job at GitHub. Since then it's fallen to the back burner, so the poor gyro support is really just a result of it being a low priority project. It's a shame and I'd like to build it out to be more robust. I just haven't had the time to make it happen.

@cameronmcefee cameronmcefee pushed a commit that referenced this pull request Apr 30, 2013

Cameron McEfee Merge pull request #41 from magebarf/gyrorange
Restrict the input from the gyro to the same range as desktop version.
d4f9b33

@cameronmcefee cameronmcefee merged commit d4f9b33 into cameronmcefee:master Apr 30, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment