643793 - Accelerated layers: portrait mode causes viewport corruption

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Florian Hänel [:heeen]]

Attachment: screenshot

As far as I know you need to apply some patches to enable accelerated layers in general: bug 628566, bug 607687

After that, enable accelerated layers and view any page in portrait mode.
As a result, the content viewport is:
1) at the wrong position: it should be below the awesomebar, but the awesomebar is overlapped by it, although only the background tiles are visible
2) too short: it is as long as it it wide - it appears there is a mixup of width and height
3) clipped on the right side by the amount the awesomebar is visible from the top - again, it seems like this is a mixup of width and height.

The screenshot was made with the above patches, so the colored lines are texture seam debugging info added by me - disregard them.

Comment 1

[Florian Hänel [:heeen]]

Adding some people that might be interested

Comment 2

[Florian Hänel [:heeen]]

Attachment: screenshot of trunk without patches

Screenshot without my patches.
Also, this error might be of interest:

###!!! ASSERTION: root view / pres context visible size mismatch: 'bounds.Size() == mPresContext->GetVisibleArea().Size()', file mozilla-central/layout/base/nsPresShell.cpp, line 4936

Comment 3

[Florian Hänel [:heeen]]

I found if you comment out these Scissor calls it looks okay:
http://mxr.mozilla.org/mozilla-central/source/gfx/layers/opengl/ContainerLayerOGL.cpp#285

Comment 4

[Jeremias Bosch (:jbos)]

so does the cached version get called? Maybe cached should be updated on resize?

Comment 5

[Florian Hänel [:heeen]]

Attachment: fix clip rects being in layer space, not world space

The problem is that clip rects are in layer space, ignoring the world transform of the LayerManager, as set by the Qt mobility orientation code. This patch multiplies the world transform in to fix this.

Comment 6

[Florian Hänel [:heeen]]

Attachment: same as above, without hack to ignore tiling imagelayer

Comment 7

[Joe Drew (not getting mail)]

Comment on attachment 521488 [details] [diff] [review]
same as above, without hack to ignore tiling imagelayer

r=me provisionally, as long as you add more spaces in-between operators :-), but I wonder if we could automatically apply this on all scissor calls. Matt, what do you think?

Comment 8

[Matt Woodrow (:mattwoodrow)]

This patch needs to be updated since the shared scissoring code has landed.

I don't think we can easily abstract this conversion inside of GLContext because of cases like CreateFBOWithTexture where we are clearing the surface without taking the shader transform into account. Transforming this scissor would clear the wrong region.

Comment 9

[Florian Hänel [:heeen]]

Attachment: rebased

Comment 10

[Florian Hänel [:heeen]]

Comment on attachment 526718 [details] [diff] [review]
rebased

looks like a simple rebase

Comment 11

[Joe Drew (not getting mail)]

Comment on attachment 526718 [details] [diff] [review]
rebased

My only request is to insert whitespace between operators and operands, a = b and a * b.

Comment 12

[Florian Hänel [:heeen]]

Attachment: fixed missing spaces

inserted whitespaces

Comment 13

[Matt Woodrow (:mattwoodrow)]

http://hg.mozilla.org/mozilla-central/rev/c2bea9fde3b0

Comment 14

[Andreea Pod]

Is there a way for us to verify this?

Comment 15

[Andreea Pod]

Attachment: screenshot

This is how I see the page on Nightly and Aurora after I set layers.acceleration.disabled=false and layers.acceleration.force-enabled=true, is it ok? can be marked as verified fixed?

Comment 16

[Cristian Nicolae (:xti)]

Verified fixed on:
Mozilla/5.0 (Android;Linux armv7l;rv:9.0a1)Gecko/20110905
Firefox/9.0a1 Fennec/9.0a1
Device: Samsung Galaxy S
OS: Android 2.2