The default bug view has changed. See this FAQ.

GL layers on Android: Most reftest failures are caused by pixels values being 'slightly' off

RESOLVED FIXED in mozilla10

Status

()

Core
Graphics
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: ajuma, Assigned: BenWa)

Tracking

Trunk
mozilla10
x86
Mac OS X
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(6 attachments, 2 obsolete attachments)

(Reporter)

Description

6 years ago
Created attachment 564201 [details] [diff] [review]
Always use nearest filtering

With GL layers enabled on Android, several reftests are failing (Bug 676831 flagged many such tests). It turns out that virtually all of these failures are caused by pixel values having one or more channels off by 4 or 8 (e.g., instead of (255,255,255), we get (247,255,247)).

Interestingly, if we require GL to always use nearest (rather than linear) filtering, all but one of the reftest failures are resolved. We wouldn't want to land such a change since, among other things, it makes zooming look ugly, but this might help point us in the direction of the real problem.
What happens if you turn off the MOZ_GFX_OPTIMIZE_MOBILE => nearest hacks in bug 688104?
(Reporter)

Comment 2

6 years ago
(In reply to Chris Jones [:cjones] [:warhammer] from comment #1)
> What happens if you turn off the MOZ_GFX_OPTIMIZE_MOBILE => nearest hacks in
> bug 688104?

Even with those hacks turned off, we get the same problem.
(Assignee)

Comment 3

6 years ago
Created attachment 564666 [details]
Log for reftest analyzer
(Assignee)

Updated

6 years ago
Depends on: 692194
(Assignee)

Comment 4

6 years ago
The problem seems to be caused by the use of LINEAR filter with TiledTextureImage with large textures. The following patch changes the leakage to a clear pattern on the right edge of the rectangle.

-    mTileSize = mGL->GetMaxTextureSize();
+    mTileSize = 512;//mGL->GetMaxTextureSize();
(Assignee)

Comment 5

6 years ago
Created attachment 565033 [details]
Log with 512 max TiledTextureImage
(Assignee)

Comment 6

6 years ago
And the test passes with a 256 tile size.
(Assignee)

Comment 7

6 years ago
Created attachment 565349 [details] [diff] [review]
ShaderHacking

Posting some shader hacking that I worked on with Jeff to get an idea of what is going on. Our general belief is that it is related to precision but we don't understand the problem.
(Assignee)

Comment 8

6 years ago
Created attachment 565391 [details] [diff] [review]
patch

More of a work around then a fix unfortunately.
Attachment #565391 - Flags: review?(jmuizelaar)
(Assignee)

Comment 9

6 years ago
Created attachment 566380 [details] [diff] [review]
Enable test fix by patch

With these two patch applied we fail 3 tests and pass 3 test that !layerOpenGL fails.

~/mozilla/mozilla-central/tree> find . -name "reftest.list" | xargs grep "Android&&"
./layout/reftests/text-overflow/reftest.list:fails-if(Android&&layersOpenGL) HTTP(..) == block-padding.html block-padding-ref.html
./layout/reftests/canvas/reftest.list:fails-if(Android&&layersOpenGL) == image-rendering-test.html image-rendering-ref.html
./layout/reftests/canvas/reftest.list:fails-if(Android&&layersOpenGL) == image-shadow.html image-shadow-ref.html
./layout/reftests/svg/reftest.list:fails-if(Android&&!layersOpenGL) == dynamic-conditions-01.svg pass.svg # bug 652050
./layout/reftests/svg/reftest.list:fails-if(Android&&!layersOpenGL) == dynamic-switch-01.svg pass.svg # bug 652050
./layout/reftests/svg/reftest.list:fails-if(Android&&!layersOpenGL) == switch-01.svg pass.svg # bug 652050
Attachment #566380 - Flags: review?(jmuizelaar)
Attachment #566380 - Flags: review?(jmuizelaar) → review+
Attachment #565391 - Flags: review?(jmuizelaar) → review+
(Assignee)

Comment 10

6 years ago
Created attachment 567165 [details] [diff] [review]
patch v2 (handle 3d transforms)
Attachment #565391 - Attachment is obsolete: true
Attachment #567165 - Flags: review?(jmuizelaar)
(Assignee)

Updated

6 years ago
Attachment #567165 - Attachment is obsolete: true
Attachment #567165 - Flags: review?(jmuizelaar)
(Assignee)

Comment 11

6 years ago
Created attachment 567168 [details] [diff] [review]
patch v3 (handle 3d transforms)
Assignee: nobody → bgirard
Status: NEW → ASSIGNED
(Assignee)

Updated

6 years ago
Attachment #567168 - Flags: review?(jmuizelaar)
Attachment #567168 - Flags: review?(jmuizelaar) → review+
(Assignee)

Comment 12

6 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/5061db810f1f
https://hg.mozilla.org/mozilla-central/rev/5061db810f1f
https://hg.mozilla.org/mozilla-central/rev/cd9fe4275983
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla10
(Assignee)

Comment 14

6 years ago
Still need to land test changes. I was waiting on TBPL results to compare my local results with tinderbox, currently the tree is closed.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 15

6 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/a456c129a2c3
https://hg.mozilla.org/mozilla-central/rev/a456c129a2c3
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago6 years ago
Resolution: --- → FIXED
merged follow-up as well https://hg.mozilla.org/mozilla-central/rev/6688e7c9cd8f
You need to log in before you can comment on or make changes to this bug.