Closed Bug 666385 Opened 8 years ago Closed 8 years ago

While panning/scrolling in Firefox Mobile 5 page content is rendered in the wrong position


(Firefox for Android Graveyard :: Panning/Zooming, defect)

Firefox 5
Not set


(Not tracked)



(Reporter: kael, Assigned: BenWa)




(3 files, 3 obsolete files)

I've been seeing this bug frequently since installing the Beta channel on my phone; I don't remember ever seeing it in the release version of 4.x. When scrolling, content from other parts of the page is randomly overlayed on the current page content. It's hard to describe beyond the screenshot (and I only just today managed to finally capture a screenshot - the artifacts vanish instantly as soon as I stop scrolling, and the android screen capture combo tended to make them vanish as well.)

The content tends to be textual content from elsewhere on the page. I've seen it almost every time I visit Reddit, but it seems to affect lots of other websites too.
Adding a screenshot of what things look like normally.
What device are you using and what Android OS version?
Samsung Galaxy S2 i9100, Android 2.3.3
Kevin Gadd, are you using a stock ROM? Also, what's the URL of the pages seen on your screenshots?
Correct, it's a stock ROM.

The page in the screenshots is .

I also can reproduce this on basically any reddit page's comments section.
I can reproduce this with a nightly build on a Nexus S, but only when OGL accelerated layers are enabled. 

Kevin, could you please post the output of about:support (specifically the Graphics section)?
Adapter Description: Android GT-I9100 samsung smdkc210
WebGL Renderer: ARM - Mali-400 MP - OpenGL ES 2.0
GPU Accelerated Windows: 0/1. Blocked for your graphics card because of unresolved driver issues.

I think the update to 6.0 on the beta channel (Market installed FF6 for me just a few days ago) may have blacklisted my GPU; I haven't seen this bug since then.
Are these being blacklisted because of a driver bug that may be causing this bug or because we have layers bugs?

I had to force layers with prefs to reproduce the bug.
It's because we have layers bugs. In, all devices except Galaxy S devices are blacklisted, and even for Galaxy S devices, layers are disabled by default (since layers.acceleration.disabled is true by default for Android devices).
I'm not seeing this problem in the Nightly builds. In the Beta build I do see this on the Galaxy S II.
Taking a look at this issue.

Cwiis thinks this may have regressed when we started using tiling.
Assignee: nobody → bgirard
Just a quick update on my progress:

I'm quite certain the problem is with TiledTextureImage, with the following change the rendering problem become significantly worse. As well as some areas such as the title bar truncate after the width of one tile.

diff --git a/gfx/thebes/GLContext.cpp b/gfx/thebes/GLContext.cpp
--- a/gfx/thebes/GLContext.cpp
+++ b/gfx/thebes/GLContext.cpp
@@ -709,42 +709,47 @@ TiledTextureImage::TiledTextureImage(GLC
                                      PRBool aUseNearestFilter)
     : TextureImage(aSize, LOCAL_GL_CLAMP_TO_EDGE, aContentType, aUseNearestFilter)
     , mCurrentImage(0)
     , mInUpdate(PR_FALSE)
     , mGL(aGL)
     , mUseNearestFilter(aUseNearestFilter)
     , mTextureState(Created)
-    mTileSize = mGL->GetMaxTextureSize();
+    mTileSize = 128;//mGL->GetMaxTextureSize();
     if (aSize != nsIntSize(0,0)) {
Attached patch patch (obsolete) — Splinter Review
We seem to be counting the offset incorrectly here. Removing this lines fixes the problem for me. Ajuma did you want to try it on your build? The bug is easy to reproduce on long comment page by scrolling to the bottom.
Attachment #547826 - Flags: feedback?(ajuma)
Comment on attachment 547826 [details] [diff] [review]

Review of attachment 547826 [details] [diff] [review]:

This does indeed fix the problem for me.
Attachment #547826 - Flags: feedback?(ajuma) → feedback+
Comment on attachment 547826 [details] [diff] [review]

This line is causing the problem. I follow the calculation that are happening here but not this specific line. Is this line perhaps not needed? Me and ajuma have confirm that it's removal fixes this bug.
Attachment #547826 - Flags: review?(florian.haenel)
Whiteboard: review-needed
Attached patch Fix Scrolling v1 (obsolete) — Splinter Review
This patch fixes the android issues without breaking Mac.
Attachment #547826 - Attachment is obsolete: true
Attachment #552175 - Flags: review?(florian.haenel)
Attachment #547826 - Flags: review?(florian.haenel)
The scrolling problem was because we weren't handling buffer rotation correctly.
Attachment #552175 - Flags: review?(florian.haenel) → review?(roc)
Comment on attachment 552175 [details] [diff] [review]
Fix Scrolling v1

Review of attachment 552175 [details] [diff] [review]:

::: gfx/layers/opengl/ThebesLayerOGL.cpp
@@ +205,4 @@
>        nsIntRegion subregion(region);
> +      textureRect.MoveBy(origin.x, origin.y);
> +      textureRect.MoveBy(GetBufferRotation());
> +      subregion.And(subregion, textureRect); // region this texture is visible in

origin + rotation = mBufferRect, so why not just use that? But then you're not really handling the rotation at all. So I don't understand how this works. Do you?

I haven't looked at this tiled-texture code before. Is there a guarantee that tiles don't cross a rotation boundary? If not, how do you handle tiles that cross a rotation boundary?

I wonder why we even need to mix tiles with rotation. It seems to me that if we have tiles, we don't need to use rotation.
Attachment #552175 - Flags: review?(roc) → review+
You're right, mBufferRect should be equivalent.

Tiled textures don't use rotation as far as I have seen on Android. I will confirm this before landing this patch. The reason I did add rotation to the calculations was because the changes I made worked on Android but regressed on Mac where we do use rotations.
I replaced GetBufferRotation with GetBufferRect.
Attachment #552175 - Attachment is obsolete: true
Ready for checkin with patch summary, waiting on mozilla-inbound to reopen to land this.
Attachment #553533 - Attachment is obsolete: true
Whiteboard: review-needed
backed out to investigate a regression on Tdhtml that seems to affect only Mac. If it won't be this bug, may be bug 674251 that is in the same range but less likely to affect only one platform.
Whiteboard: [inbound]
Backing out the patch didn't fix the problem. I will re-land this patch tomorrow unless there is any objections.
I actually think we should split out tiled buffers into an entirely new kind of buffer that doesn't do rotation at all. I think that would simplify this code a lot. Right now we don't seem to have a guarantee that tiles and rotation won't be used together, and if they are used together things will break.
(In reply to Benoit Girard (:BenWa) from comment #25)
> Backing out the patch didn't fix the problem. I will re-land this patch
> tomorrow unless there is any objections.

confirmed, it was not due to this patch, sorry.
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #26)

This is a major blocker for Android, can we address this refactoring in a follow up bug?
Yes, but I think we should do that refactoring right away after this bug, not indefinitely far in the future.
There's been significant changes to ThebesLayerOGL.cpp and this patch will need a non trivial bit rot fix. I will work on this early next week.
I haven't been able to reproduce this bug. There's been significant code changes in ThebesLayerOGL. I think it's reasonable to close this bug. Feel free to re-open if someone can reproduce this.
Closed: 8 years ago
Resolution: --- → FIXED
Retested bug with :
Devices: Samsung Galaxy S, Motorola DROID 2

on both:
Firefox Beta : Mozilla/5.0 (Android; Linux armv7l; rv:7.0) Gecko/20110922 Firefox/7.0 Fennec/7.0

Nightly Build: Mozilla/5.0 (Android; Linux armv7l; rv:9.0a1) Gecko/20110927 Firefox/9.0a1 Fennec/9.0a1

I wasn't able to reproduce this bug either.
Page from screenshots ( ) and some other random webpages are properly displayed and panning/scrolling is working as expected. 

Verifying bug.
You need to log in before you can comment on or make changes to this bug.