914776 - Touch coordinates are wrong when screen is rotated

Status: RESOLVED FIXED

(Core Graveyard :: Widget: Gonk, defect)

Description

[gbennett]

Attachment: LandscapeHomeButtonLog.txt

Description:
After launching an app and changing orientation to landscape mode pressing the home button does not work. This does not repro when holding the device sideways within an app that cannot change orientation (contacts).

Repro Steps:
1) Updated Buri 1.2 to Build ID: 20130910040201
2) Open Browser.
3) Change orientation to landscape.
4) Press home button.

Actual:
Home button does not vibrate or bring user to the home screen when pressed.

Expected:
Home button vibrates and brings the user to the home screen when pressed.

Environmental Variables
Build ID: 20130910040201
Gecko: http://hg.mozilla.org/mozilla-central/rev/be1053dc223b
Gaia: 6deda9d7c51f278443f704217eaed722044a03e7
Platform Version: 26.0a1

Notes: I repro'd this using gallery, browser, and video.
Repro frequency: 100%, 6/6
See attached: LandscapeHomeButtonLog.txt

Comment 1

[Deepa Subramanian]

When the user changes the orientation on the gallery/video app. The tool bar icons found on these apps on landscape mode becomes non functional. 

Please see the video of the issue http://youtu.be/z26PcVpn8fM

Environmental Variables
Buri Build ID: 20130910040201
Gecko: http://hg.mozilla.org/mozilla-central/rev/be1053dc223b
Gaia: 6deda9d7c51f278443f704217eaed722044a03e7
Platform Version: 26.0a1

Comment 2

[Natalya Kot [:nkot]]

Regression range:

Build ID: 20130906040204 - Does NOT Reproduce
Gecko: http://hg.mozilla.org/mozilla-central/rev/ab5f29823236
Gaia: 94e5f269874b02ac0ea796b64ab995fce9efa4b3
Platform Version: 26.0a1

Build ID: 20130909114657 - Reproduces
Gecko: http://hg.mozilla.org/mozilla-central/rev/218d4334d29e
Gaia: aa4180e9286d385fa6b62d236f30fb24cd8b93e9
Platform Version: 26.0a1

Comment 3

[Jason Smith [:jsmith]]

The Gaia commit log shows evidence that Alive was doing changes here recently in the System app.

Alive - Any ideas on what's failing here?

Comment 4

[(inactive after 6/18) Alive Kuo [:alive]]

This sounds terrible.

Jason, I couldn't repro on latest gaia: efd21eddc030c2232b2e4707650995ed7714c38e
My device is inari.

Also from the video, the phone seems totally hang...

Comment 5

[Jason Smith [:jsmith]]

For those watching the bug - I confirmed a reproduction of this on 9/10/2013 unagi build. Alive has borrowed my device to debug this.

Comment 6

[(inactive after 6/18) Alive Kuo [:alive]]

I do think this is a gecko issue. I don't get mozChromeEvent once device is landscape-oriented.

Comment 7

[(inactive after 6/18) Alive Kuo [:alive]]

Lemme debug from shell.js...

Comment 8

[(inactive after 6/18) Alive Kuo [:alive]]

(In reply to Alive Kuo [:alive] from comment #7)
> Lemme debug from shell.js...

There's no keydown/keyup/keypress event in landscape mode. Only for home button. Very strange...
Anyway I was able to repro after flashing latest pvt build to my inari.

Jason, I need regression window on this one -- for mozilla-central. Also you could bring your device back.

Comment 9

[(inactive after 6/18) Alive Kuo [:alive]]

http://mxr.mozilla.org/mozilla-central/source/b2g/chrome/content/shell.js#447
No event is fired here.
Also the touch screen seems totally failing - UI is unresponsive.

Comment 10

[Jason Smith [:jsmith]]

Fabrice - Do you know who on the gecko side could look into something like this issue?

Comment 11

[(inactive after 6/18) Alive Kuo [:alive]]

This shall be caused by changes very recently. I flashed my inari from pvtbuild yesterday (afternoon?) and everything is fine so I wrote comment 4.

My working sources.xml
https://gist.github.com/alivedise/6522946

Comment 12

[Benoit Jacob [:bjacob] (mostly away)]

(In reply to nkot from comment #2)
> Regression range:
> 
> Build ID: 20130906040204 - Does NOT Reproduce
> Gecko: http://hg.mozilla.org/mozilla-central/rev/ab5f29823236
> Gaia: 94e5f269874b02ac0ea796b64ab995fce9efa4b3
> Platform Version: 26.0a1
> 
> Build ID: 20130909114657 - Reproduces
> Gecko: http://hg.mozilla.org/mozilla-central/rev/218d4334d29e
> Gaia: aa4180e9286d385fa6b62d236f30fb24cd8b93e9
> Platform Version: 26.0a1

Which gives this list of changesets:

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=ab5f29823236&tochange=218d4334d29e

Comment 13

[Benoit Jacob [:bjacob] (mostly away)]

What this bug really seems to need is further bisection down to one changeset.

Comment 14

[Jason Smith [:jsmith]]

(In reply to Benoit Jacob [:bjacob] from comment #13)
> What this bug really seems to need is further bisection down to one
> changeset.

We could get down to a build within a 12 hour range, but after that, we don't have any release builds to get a deeper regression range.

Comment 15

[Benoit Jacob [:bjacob] (mostly away)]

Right, so someone needs to make builds from sources, by hand. I'm not signing up for this labor-intensive tasks given that I have other bugs to work on :-)

Comment 16

[Jason Smith [:jsmith]]

(In reply to Jason Smith [:jsmith] from comment #14)
> (In reply to Benoit Jacob [:bjacob] from comment #13)
> > What this bug really seems to need is further bisection down to one
> > changeset.
> 
> We could get down to a build within a 12 hour range, but after that, we
> don't have any release builds to get a deeper regression range.

Actually I'm not right here - https://pvtbuilds.mozilla.org/pub/mozilla.org/b2g/nightly/mozilla-central-unagi/2013/09/ shows that 9/6 to 9/9 is the deepest we can go by rel eng builds.

Comment 17

[Milan Sreckovic [:milan] (needinfo for best results)]

Link above is for unagi, but hamachi builds have the same gap: https://pvtbuilds.mozilla.org/pvt/mozilla.org/b2gotoro/nightly/mozilla-central-hamachi/2013/09/

Comment 18

[Milan Sreckovic [:milan] (needinfo for best results)]

Is there a way to reproduce this on an emulator?

Comment 19

[Benoit Jacob [:bjacob] (mostly away)]

Apparently yes, Ctrl+F11 and Ctrl+F12 should both tell the emulator to switch orientation.

Here for me, Ctrl+F11 works (while Ctrl+F12 is intercepted by my window manager).

http://stackoverflow.com/questions/1991318/how-to-change-emulator-screen-orientation

Comment 20

[Benoit Jacob [:bjacob] (mostly away)]

Here on emulator, I can reproduce the issue that in the Gallery app, in landscape mode, the 'camera' button does not do anything.

I cannot reproduce issues with the home button. The home button works for me.

Comment 21

[Benoit Jacob [:bjacob] (mostly away)]

Self-assigning, bisecting.

Comment 22

[Benoit Jacob [:bjacob] (mostly away)]

I'm half-way through bisection, made harder by being near a revision range that fails to bool, here is what I have:

145771:ab5f29823236 good
145953:028b937786cb good
145955:36cc6b1669b4 fails to boot
145996:8a73c481e49e fails to boot
146004:ba0ecbba9add bad
146068:4ca898d7db5f bad
146137:218d4334d29e bad

Comment 23

[Benoit Jacob [:bjacob] (mostly away)]

Regarding this component change --- there is still no definite reason to consider this bug graphics related. But let me finish bisecting...

Comment 24

[Benoit Jacob [:bjacob] (mostly away)]

Attachment: Fix the boot, needed for bisecting into the range of changesets that don't boot

Comment 25

[Benoit Jacob [:bjacob] (mostly away)]

Finished bisecting, the first bad changeset is:

http://hg.mozilla.org/mozilla-central/rev/0b914f0e61ab

Bug 908797 - Update libui to the latest input code from JB MR2

-> blocking bug 908797, un-blocking the gfx tracking bug. Not a graphics bug.

Comment 26

[Benoit Jacob [:bjacob] (mostly away)]

Note: there have been multiple symptoms discussed above. The specific one I'm talking about is that in the Gallery app, in landscape mode, the 'camera' button (which should allow switching back to the camera mode to take new pictures) does nothing.

Comment 27

[Benoit Jacob [:bjacob] (mostly away)]

(In reply to Deepa Subramanian from comment #1)
> When the user changes the orientation on the gallery/video app. The tool bar
> icons found on these apps on landscape mode becomes non functional. 

^ that is what I am talking about (the only thing that I was able to reproduce).

Comment 28

[viral [:viralwang]]

It looks like the mXScale and mYScale is not correct in landscape mode, it will cause the touch event mapping incorrect.
I think problem comes from the mSurfaceWidth and mSurfaceHeight.

    // The surface orientation, width and height set by configureSurface().
    // The width and height are derived from the viewport but are specified
    // in the natural orientation.
    int32_t mSurfaceWidth;
    int32_t mSurfaceHeight;

Take N4 for example, it should be 768(width)*1280(height) no matter it's landscape or portrait.
But we have 768*1280 in portrait and 1280*768 in landscape mode now.

Since we always setDisplayInfo with gScreenBounds.width and gScreenBounds.height in nsAppShell, I'm thinking we should used it directly as the mSurfacewidth and mSurfaceHeight in TouchInputMapper::configureSurface without considering the orientation. 

I would like to fix the mXScale and mYScale in configureSurface, and we can have correct coordination in cookPointerData

Comment 29

[viral [:viralwang]]

Attachment: fix the mXScale and mYScale in configureSurface and leave the coordination in cookPointerData. (v1)

This patch will keep the correct mXScale/mYScale, and we can have the correct touch panel points for applications

Comment 30

[Michael Wu [:mwu]]

(In reply to viral [:viralwang] from comment #29)
> Created attachment 808483 [details] [diff] [review]
> fix the mXScale and mYScale in configureSurface and leave the coordination
> in cookPointerData. (v1)
> 
> This patch will keep the correct mXScale/mYScale, and we can have the
> correct touch panel points for applications

As mentioned in bug 908797, I would like to minimize changes to any code in widget/gonk/libui. We should be able to fix this in nsAppShell.cpp, right?

Comment 31

[Michael Wu [:mwu]]

Attachment: Fix touch events when rotated

Sorry - gonna steal this bug. This is a pretty bad regression and I'd like to get it fixed sooner.

Comment 32

[Michael Wu [:mwu]]

http://androidxref.com/4.3_r2.1/xref/frameworks/base/services/java/com/android/server/display/DisplayViewport.java has some explanation for the different fields in the C++ version of DisplayViewport.

Comment 33

[Michael Vines [:m1] [:evilmachines]]

Comment on attachment 809457 [details] [diff] [review]
Fix touch events when rotated

Review of attachment 809457 [details] [diff] [review]:
-----------------------------------------------------------------

lgtm.

Comment 34

[Michael Wu [:mwu]]

Thanks!

https://hg.mozilla.org/integration/b2g-inbound/rev/ddf051d653ec

Comment 35

[Michael Vines [:m1] [:evilmachines]]

ftfy

Comment 36

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/ddf051d653ec

Comment 37

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/997e9a86b388