743247 - Layout shakes around

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Gian-Carlo Pascutto [:gcp]]

Native Fennec, Galaxy S2, current Nightly.

Visit krantenkoppen.be. Pan and scroll a bit. Scroll down and then to the top again.

Expected: Page responds to user input.

Reality: Browser starts a party. User is not invited.

Comment 1

[Kartikaya Gupta (email:[e-mail])]

Possibly the same issue as bug 737801, but I haven't looked into it or tried reproducing yet.

Comment 2

[Kartikaya Gupta (email:[e-mail])]

I don't see this on either the Galaxy Tab or the Galaxy Nexus running recent builds.

Comment 3

[Gian-Carlo Pascutto [:gcp]]

Still reproduces on current Nightly on Galaxy S2 2.3.6. Does not reproduce on Nexus S 4.0.4.

Comment 4

[Aaron Train [:aaronmt]]

I see the main body/column of the page jump and clip to the right side when scrolling partial way through (Nightly 04/11, Galaxy Nexus).

Comment 5

[JP Rosevear [:jpr]]

Aaron, can you demo to kats?

Comment 6

[Gian-Carlo Pascutto [:gcp]]

BTW, it's not clear that what Aaron describes is the same as what I'm seeing: I really see a violent, rapid shake that lasts several seconds.

Comment 7

[Aaron Train [:aaronmt]]

I actually dont see anything wrong with the page in testing it again today on Nightly (04/12), GN/4.0.4.

Comment 8

[Gian-Carlo Pascutto [:gcp]]

Still reproduces for me on todays Nightly. It seems to be related to the Flash on the page - I didn't get the shimmering until I allowed plugins to run.

Comment 9

[Gian-Carlo Pascutto [:gcp]]

http://youtu.be/qo5WA5KlHKg?hd=1 unfortunately the Nexus S that I used to capture this doesn't have enough FPS to show the shake very well.

The grey square is new.

Comment 10

[Kartikaya Gupta (email:[e-mail])]

Interesting. I've seen similar behaviour on envirochem.us/25 while investigating bug 739289 - I think both of these are caused by us getting incorrect page size information because of position:fixed elements. The bad page size might trigger a redraw from java which then moves the position:fixed element again. This gets stuck in a loop causing the jitter.

Comment 11

[Joe Drew (not getting mail)]

I can't reproduce this on a Nexus S, Galaxy Nexus, or Galaxy S 2 with the latest nightly.

I tried zooming in and out, panning around and turning on flash on both http://envirochem.us/25 and http://krantenkoppen.be.

Please reopen with STR if I'm doing something wrong!

Comment 12

[Kevin Brosnan [Ex-Mozilla]]

Saw this with the 23rd build. Talked with Joe he does not think any of the gfx changes landing soon or last night would have made any difference.

Comment 13

[Joe Drew (not getting mail)]

If QA can provide us with STR, that would be absolutely swell.

Comment 14

[Catalin Suciu [:csuciu]]

I cannot reproduce on Samsung Galaxy SII (2.3.4) using the the latest nightly (15.0a1 (2012-04-24)).
Kevin, what device/OS are you using.

Comment 15

[Martijn Wargers (dead)]

I can't reproduce the issues that are seen in comment 9 on the url and on htpp://envirochem.us/25 , using Samsung Galaxy SII latest trunk Native Fennec build.

Comment 16

[Tony Chung [:tchung]]

SV, can another person try taking a look on a different device?   if no one else can reproduce, we'll mark wfm

Comment 17

[Adrian Tamas (:AdrianT)]

I am unable to reproduce the issue on an HTC Desire (Android 2.2) running Nightly/15.0a1 2012-04-25.

Comment 18

[Cristian Nicolae (:xti)]

I am able to reproduce this issue on today's Nightly, but I think that this bug might be a dupe of Bug 739584 because there is the same behavior (I mean that the same is happening for 739584 until the device will freeze).

--
Firefox 15.0a1 (2012-04-25)
Device: Samsung Galaxy S (Captivate)
OS: Android 2.2

Comment 19

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

I'm not sure if it is a dup.  kevs3d.co.uk has webgl versus the other sites that have flash.  They might be handled differently.  Until there are fixes for one or the other and we can test both, I think we should keep both open.

Comment 20

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

I can't reproduce this on an Atrix with a build from last night, with or without plugins enabled.  However, if I enable plugins, panning has horrible pauses and is in general awful.

Comment 21

[Peter Retzer (:pretzer)]

I can also reproduce this. I see this on http://krantenkoppen.be and also on any Doodle-Poll. 
STR:
1. Go to  http://doodle.com
2. Hit 'View Example'
3. Scroll down and back to the top
4. See the layout shaking

Device: Samsung Galaxy S2
OS: Android 4.0.3

Comment 22

[Cristian Nicolae (:xti)]

This bug is reproducible for me as well using the str from comment #21 on Samsung Galaxy S (Captivate). I still think that is the same issue as in bug 739584.

Comment 23

[Aaron Train [:aaronmt]]

Ok reproduced on the SII here in Toronto

Comment 24

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Galaxy SII uses PowerVR SGX540.

Comment 25

[Tony Chung [:tchung]]

(In reply to Cristian Nicolae (:xti) from comment #22)
> This bug is reproducible for me as well using the str from comment #21 on
> Samsung Galaxy S (Captivate). I still think that is the same issue as in bug
> 739584.

The galaxy S is also a PowerVR SGX540 chip.

i'm unable to reproduce on Adreno GPUs (HTC incredible S, HTC Sensation)

Comment 26

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

I made a mistake .  I - 9100g uses PowerVR.  Aaron is right, I9100 uses Mali-400MP

Comment 27

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Galaxy Note uses Mali-400 as well.  Need to check to see if it reproduces on the Galaxy Note.

Comment 28

[Jeff Muizelaar [:jrmuizel]]

I took a look at this, it does indeed seem specific to the Mali. I recorded a trace of the shaking and when replaying it the shaking did not occur. My guess is that, for whatever reason, our drawing breaks and we end up swapping between two old buffers. This gives the appearance of shaking but were actually just switching between two old frames.

Comment 29

[Jeff Muizelaar [:jrmuizel]]

It looks like this is caused by or related to the scroll bars. Disabling them makes the problem go away. I'm working on bisecting the actual problem but it seems like it's also related to the iframe at the top of the screen.

Comment 30

[Cristian Nicolae (:xti)]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #28)
> I took a look at this, it does indeed seem specific to the Mali. 

Actually it seems that nobody consider any of my comments. This issue occurs not only on Mali GPU, but on PoweVR as well (Samsung Captivate is a PowerVR: http://www.gsmarena.com/samsung_i897_captivate-3408.php)

The same issue is described in bug bug 739584 (as I already mentioned in comment #18 and comment #22).

Comment 31

[Jeff Muizelaar [:jrmuizel]]

Looking further, it's not the scroll bars that are the problem it's drawing anything afterwards that causes bad. It also has to do with the canvas tag being used in the ad. I've put up a reduced test case at http://people.mozilla.org/~jmuizelaar/doodle.html

Comment 32

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

XTI, we mentioned the Mali because it's easy to reproduce with a Mali device. 
We are not discounting what you stated in your comment.

We could not easily reproduce with the Captivate.

Comment 33

[Jeff Muizelaar [:jrmuizel]]

Attachment: Reduced test case

Comment 34

[Jeff Muizelaar [:jrmuizel]]

Attachment: Make sure we don't have an array buffer bound when drawing

apitrace saves the day. This may fix other problems as well.

Comment 35

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 619228 [details] [diff] [review]
Make sure we don't have an array buffer bound when drawing

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

I don't know enough about GL to meaningfully r+ this, but it looks straightforward enough to me.

Comment 36

[JP Rosevear [:jpr]]

Snorp, you may want to test this for the "black triangle" bug.

Comment 37

[Mozilla RelEng Bot]

Autoland Patchset:
	Patches: 619228
	Branch: mozilla-central => try
	Destination: http://hg.mozilla.org/try/pushloghtml?changeset=190815168392
Try run started, revision 190815168392. To cancel or monitor the job, see: https://tbpl.mozilla.org/?tree=Try&rev=190815168392

Comment 38

[Bas Schouten (:bas.schouten)]

Comment on attachment 619228 [details] [diff] [review]
Make sure we don't have an array buffer bound when drawing

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

::: mobile/android/base/gfx/SurfaceTextureLayer.java
@@ +277,5 @@
>  
>          GLES20.glUniformMatrix4fv(mTextureMatrixHandle, 1, false, mTextureTransform, 0);
>  
> +        // Unbind any the current array buffer so we can use client side buffers
> +        GLES20.glBindBuffer(GLES20.GL_ARRAY_BUFFER, 0);

Might want to make whitespacing here consistent with elsewhere.

Comment 39

[Jeff Muizelaar [:jrmuizel]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0d045e27fd99

Comment 41

[JP Rosevear [:jpr]]

Comment on attachment 619228 [details] [diff] [review]
Make sure we don't have an array buffer bound when drawing

Mobile only.

Comment 42

[Jeff Muizelaar [:jrmuizel]]

Attachment: Fix the same problem in plugin layer

Comment 43

[Jeff Muizelaar [:jrmuizel]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0d00e58830fe

Comment 44

[Jeff Muizelaar [:jrmuizel]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/686e78d4493b

Comment 45

[Mozilla RelEng Bot]

Try run for 190815168392 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=190815168392
Results (out of 15 total builds):
    success: 15
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-190815168392

Comment 46

[Chris Lord [:cwiiis]]

Comment on attachment 619228 [details] [diff] [review]
Make sure we don't have an array buffer bound when drawing

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

::: mobile/android/base/gfx/NinePatchTileLayer.java
@@ +139,5 @@
>          coordBuffer.position(0);
>          coordBuffer.put(coords);
>  
> +        // Unbind any the current array buffer so we can use client side buffers
> +        GLES20.glBindBuffer(GLES20.GL_ARRAY_BUFFER, 0);

Could we not just do this once at the start of LayerRenderer.drawBackground and LayerRenderer.drawForeground?

Comment 47

[JP Rosevear [:jpr]]

Comment on attachment 619251 [details] [diff] [review]
Fix the same problem in plugin layer

Mobile only, would like to confirm with snorp.

Comment 49

[Ryan VanderMeulen [:RyanVM]]

http://hg.mozilla.org/mozilla-central/rev/0d045e27fd99
http://hg.mozilla.org/mozilla-central/rev/0d00e58830fe

No tests?

Comment 50

[Cristian Nicolae (:xti)]

Verified fixed on:

Firefox 15.0a1 (2012-04-30)
Firefox 14.0a2 (2012-04-30)

Device: Samsung Captivate
OS: Android 2.2

Comment 52

[Brad Lassey [:blassey] (use needinfo?)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/a41f240334c8