Closed Bug 1484101 Opened 2 years ago Closed 2 years ago

black or white line across the screen on Firefox for Android

Categories

(Core :: Graphics: Layers, defect)

ARM
Android
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla63
Tracking Status
firefox-esr52 --- unaffected
firefox-esr60 --- unaffected
firefox61 --- unaffected
firefox62 --- unaffected
firefox63 + fixed

People

(Reporter: kbrosnan, Assigned: jnicol)

References

Details

(Keywords: correctness, regression)

Attachments

(3 files)

Attached image black line example
I and Haycam (on Aug-10) have seen lines running across the screen on Firefox for Android Nightly. Most of the time I am seeing black lines. Though I have seen white once or twice. This happens a few times a day. It is difficult to reproduce. I've seen it on news.ycombinator.com, facebook mobile and reddit.
Attached image White line example
Duplicate of this bug: 1484452
I would not want to ship this regression to beta if possible. Is there bandwdith to look at this?
Flags: needinfo?(dbolter)
Jamie, any thoughts?
Flags: needinfo?(dbolter) → needinfo?(jnicol)
Looking in to it
Flags: needinfo?(jnicol)
This was fun.

There seems to be 2 problems at play here - 1) is that the critical display port is being snapped to 0.5*tile_size increments, rather than tile_size. And 2) is an edge-padding regression from bug 1478815, part 4.

Edge-padding used to be done separately for each tile, in ClientMultiTiledLayerBuffer::ValidateTile, using the tileVisibleRegion which was in the correct coordinate space for the tile. 

After this change, padding is done in ClientMultiTiledLayerBuffer::Update, using DrawTargetTiled::PadEdges. PadEdges tries to translate the supplied region to each tile's coordinate space. However - we have set up the DrawTargetTiled so that its top-leftmost tile has an origin of (0,0) rather than the actual coordinate in the layer, then set a transform on the ctx to translate to that from layer-space. The region passed to PadEdges, newValidRegion, is in layer-space rather than "draw target space".

So we are scrolling down the page and a new row of tiles scrolls in to the display port. Actually, *half* a new row of tiles, because of our display port bug. Let's say tilesize=512, dp=(y=256,h=2560). We paint these new tiles, and no others. So their actual origins are y=2560, but the DrawTargetTiled thinks y=0. We then call PadEdges with a region (y=256,h=2560). The bottom edge of this should be intersecting with the new tiles, but because of the origin thing it is actually the top edge that intersects. So we pad up instead of down, i.e. we copy a row of uninitialized garbage in to our valid region.

The solution is pretty simple - Move newValidRegion by -mTilingOrigin before passing to PadEdges. (Alternatively PadEdges could use the current matrix to adjust for that itself.)

I'd also like to figure out why our critical display port is snapping to half-tiles, but that should probably be a separate bug.
Assignee: nobody → jnicol
Component: Graphics → Graphics: Layers
Keywords: correctness
In MultiTiledContentClient we can create a DrawTargetTiled with a
different origin than the layer we are painting. We must therefore
ensure when edge-padding that we provide the valid region in the draw
target's device-space rather than layer-space. Not doing so was
causing us to pad out in incorrect directions, causing visible seams.
Comment on attachment 9003188 [details]
Bug 1484101 - Ensure DrawTargetTiled::PadEdges is called with region in device space. r=rhunt

Ryan Hunt [:rhunt] has approved the revision.
Attachment #9003188 - Flags: review+
Pushed by jnicol@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e65592761f4c
Ensure DrawTargetTiled::PadEdges is called with region in device space. r=rhunt
https://hg.mozilla.org/mozilla-central/rev/e65592761f4c
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Hi Jamie, assuming this affects beta62, could you please nominate for beta uplift? We will gtb final Fennec beta build on Monday. NI Ryan, lizzard.
Flags: needinfo?(ryanvm)
Flags: needinfo?(lhenry)
Flags: needinfo?(jnicol)
Jamie found the regressing bug in comment 6. It only affects Nightly 63.
Flags: needinfo?(jnicol)
Blocks: 1478815
Flags: needinfo?(ryanvm)
Flags: needinfo?(lhenry)
You need to log in before you can comment on or make changes to this bug.