1048110 - Tiling doesn't handle padding correctly

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[Matt Woodrow (:mattwoodrow)]

Jeff fixed this for mobile with bug 1023473, but that fix doesn't help the path we use on desktop.

I want to fix this the same way we do for other backends, by padding the region we draw out to include all pixels that might potentially be sampled. This makes the extra pixels either contain real content (if some exists) or transparent if not. This isn't the same as clamping (which is what 1023473 implements) but it's much simpler and won't give us artifacts of undefined content.

Comment 1

[Matt Woodrow (:mattwoodrow)]

Attachment: Expand complex regions out if we're resampling

This matches our RotatedBuffer etc logic for avoiding resampling issues for complex regions.

We still have resampling issues for the 'outer' edges of buffers which previously were on the edge of a texture (and thus got CLAMP behaviour) and now likely exist in the middle of a tile.

Comment 2

[Matt Woodrow (:mattwoodrow)]

Attachment: Handle the edges of resampled regions

This should make sure we have a border of real content/transparent pixels around the visible region so we always sample from something sane.

It could be improved slightly by not expanding edges that already sit on a tile boundary. I'll have a look into doing that.

Comment 3

[Bas Schouten (:bas.schouten)]

Comment on attachment 8466882 [details] [diff] [review]
Expand complex regions out if we're resampling

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

This has me a little worried. This increases the potential for overfill significantly which is bad. There's better things we can do here by using slightly more complex vertex buffers. Longer term that's in my opinion what we should be looking into.

::: gfx/layers/composite/ContentHost.h
@@ +74,2 @@
>    {}
> +  

nit: whitespace

::: gfx/layers/composite/TiledContentHost.cpp
@@ +342,5 @@
>      if (!mLowPrecisionTiledBuffer.HasDoubleBufferedTiles()) {
>        mLowPrecisionTiledBuffer.ReadUnlock();
>      }
>    }
> +  

nit: whitespace

Comment 4

[Matt Woodrow (:mattwoodrow)]

Attachment: Handle the edges of resampled regions

Comment 5

[Matt Woodrow (:mattwoodrow)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b89f54e21c0b
https://hg.mozilla.org/integration/mozilla-inbound/rev/1cf6f4f27811

Comment 6

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

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/a06f75001b0f for various Android mochitest failures: https://tbpl.mozilla.org/?tree=Mozilla-Inbound&jobname=Android%204.0%20Panda%20mozilla-inbound%20debug%20test%20mochitest-&rev=0d43f74dad13

Comment 7

[Matt Woodrow (:mattwoodrow)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a287654390b1
https://hg.mozilla.org/integration/mozilla-inbound/rev/da90bd5e5145

Comment 8

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/a287654390b1
https://hg.mozilla.org/mozilla-central/rev/da90bd5e5145