Small resampling errors in Android GL layer compositing

RESOLVED FIXED in mozilla28

Status

()

Core
Graphics: Layers
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: roc, Assigned: roc)

Tracking

Trunk
mozilla28
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

We have various small resampling errors on Android. Basically, some layers that should be composited with a simple pixel-aligned copy get slightly resampled, so at internal color boundaries, some pixels are rendered slightly off from the desired channel values.

I have a patch that works around the problem by detecting when we have integer-only translation transforms in CompositorOGL::DrawQuad, and in that case switching sampling to GL_NEAREST. When I apply this patch, a number of tests currently marked as failing on Android start to pass, namely
REFTEST TEST-UNEXPECTED-PASS | http://10.250.49.170:30266/tests/layout/reftests/bugs/243519-4b.html | image comparison (==)
REFTEST TEST-UNEXPECTED-PASS | http://10.250.49.170:30266/tests/layout/reftests/bugs/243519-4d.html | image comparison (==)
REFTEST TEST-UNEXPECTED-PASS | http://10.250.49.170:30266/tests/layout/reftests/bugs/243519-4f.html | image comparison (==)
REFTEST TEST-UNEXPECTED-PASS | http://10.250.49.170:30266/tests/layout/reftests/bugs/243519-5c.html | image comparison (==)
REFTEST TEST-UNEXPECTED-PASS | http://10.250.49.170:30266/tests/layout/reftests/bugs/243519-5d.html | image comparison (==)
REFTEST TEST-UNEXPECTED-PASS | http://10.250.49.170:30266/tests/layout/reftests/bugs/243519-9a.html | image comparison (==)
REFTEST TEST-UNEXPECTED-PASS | http://10.250.49.170:30266/tests/layout/reftests/bugs/243519-9b.html | image comparison (==)
REFTEST TEST-UNEXPECTED-PASS | http://10.250.49.170:30266/tests/layout/reftests/bugs/243519-9c.html | image comparison (==)
REFTEST TEST-UNEXPECTED-PASS | http://10.250.49.170:30266/tests/layout/reftests/bugs/243519-9d.html | image comparison (==)
REFTEST TEST-UNEXPECTED-PASS | http://10.250.49.170:30266/tests/layout/reftests/bugs/243519-9e.html | image comparison (==)
REFTEST TEST-UNEXPECTED-PASS | http://10.250.49.170:30266/tests/layout/reftests/bugs/243519-9f.html | image comparison (==)
REFTEST TEST-UNEXPECTED-PASS | http://10.250.49.170:30266/tests/layout/reftests/bugs/402807-1.html | image comparison (==)
REFTEST TEST-UNEXPECTED-PASS | http://10.250.49.163:30144/tests/layout/reftests/bugs/433700.html | image comparison (==)
REFTEST TEST-UNEXPECTED-PASS | http://10.250.49.163:30144/tests/layout/reftests/bugs/458487-4a.html | image comparison (==)
REFTEST TEST-UNEXPECTED-PASS | http://10.250.49.163:30144/tests/layout/reftests/bugs/458487-4b.html | image comparison (==)
REFTEST TEST-UNEXPECTED-PASS | http://10.250.49.163:30144/tests/layout/reftests/bugs/458487-4c.html | image comparison (==)
REFTEST TEST-UNEXPECTED-PASS | http://10.250.49.163:30144/tests/layout/reftests/bugs/586683-1.html | image comparison (==)
REFTEST TEST-UNEXPECTED-PASS | http://10.250.49.163:30144/tests/layout/reftests/bugs/602200-2.html | image comparison (==)
REFTEST TEST-UNEXPECTED-PASS | http://10.250.49.163:30144/tests/layout/reftests/bugs/633344-1.html | image comparison (==)
REFTEST TEST-UNEXPECTED-PASS | http://10.250.49.163:30144/tests/layout/reftests/bugs/836844-1.html | image comparison (==)
REFTEST TEST-UNEXPECTED-PASS | http://10.250.49.168:30238/tests/layout/reftests/transform-3d/scale3d-all.html | image comparison (==)
REFTEST TEST-UNEXPECTED-PASS | http://10.250.49.168:30238/tests/layout/reftests/transform-3d/scale3d-all-separate.html | image comparison (==)
So whatever this problem is, it's been around for a while.

This bug quite severely affects bug 876321, where we start to layerize moving absolutely-positioned elements, and a lot of existing reftests start to fail with small errors. In that bug, I did quite a lot of investigation to try to figure out what the problem is. I failed. The data we pass into GLContext looks good. Maybe I missed something or maybe the problem is in drivers. (I see different but similar failures on try vs on my Nexus S locally.)

So I propose we land my workaround to green up these tests. If someone wants to pursue the underlying problem into the guts of our GL compositor and/or Android itself, they're welcome to do so, and it will be easy enough to back out this patch for that investigation.
Created attachment 831765 [details] [diff] [review]
fix

In addition to this, I will need a patch that updates reftest manifests for passing tests. I'll attaach that separately once a clean try run is done.
Attachment #831765 - Flags: review?(jmuizelaar)
Note that bug 937987 does not fix these issues.
Comment on attachment 831765 [details] [diff] [review]
fix

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

::: gfx/layers/opengl/CompositorOGL.cpp
@@ +1107,5 @@
> +          aTransform.Is2DIntegerTranslation() &&
> +          textureTransform.Is2D(&textureTransform2D) &&
> +          textureTransform2D.HasOnlyIntegerTranslation()) {
> +        filter = GraphicsFilter::FILTER_NEAREST;
> +      }

I'm not really thrilled with this. It shouldn't be needed to get correct sampling. That being said I understand the motivation. Can you add a comment describing why it's there and expressing some sadness?
Attachment #831765 - Flags: review?(jmuizelaar) → review+
Sure.

Also, I decided to make this code Android-only to ensure that the same issues don't creep into other platforms without us noticing.
I like that as well.

Comment 7

4 years ago
https://hg.mozilla.org/mozilla-central/rev/ba6e69527aca
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
You need to log in before you can comment on or make changes to this bug.