css animations aren't clipped correctly

RESOLVED FIXED in Firefox 26

Status

()

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

People

(Reporter: alexander.jautz, Assigned: nrc)

Tracking

Trunk
mozilla27
ARM
Android
Points:
---

Firefox Tracking Flags

(firefox26+ fixed, firefox27 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:19.0) Gecko/20100101 Firefox/19.0
Build ID: 20130307023931

Steps to reproduce:

Animating a filmstrip image with css animation in a div container.

example: http://gt.letzfetz.com/


Actual results:

The animation with the smaller image (2280px x 152px) doesn't get clipped correctly by the surrounding div and the whole filmstrip is visible.
The animation with the larger image (4097px x 152px) works.

This happens with a Galaxy S3 
Android 4.1.1
Firefox 19.0.2 

With a Galaxy Nexus the minimum size for a working animation is different (>2048). So I think this has something to do with the max texture size of the mobile device?

example: http://gt.letzfetz.com/


Expected results:

The whole filmstrip shouldn't be visible.
(Reporter)

Updated

5 years ago
OS: Windows 7 → Android

Updated

5 years ago
Status: UNCONFIRMED → NEW
Component: General → Graphics, Panning and Zooming
Ever confirmed: true
Hardware: x86_64 → ARM

Comment 1

5 years ago
Moving to gfx and cc'ing BenWa and nrc - sounds like a layers clipping issue, possibly involving tiled layers.
Component: Graphics, Panning and Zooming → Graphics: Layers
Product: Firefox for Android → Core
Version: Firefox 19 → Trunk
(Assignee)

Comment 2

5 years ago
We think this is a regression since 18. I can't reproduce on my Nexus 7 or HTC One, but kentuckyfriedtakahe did repro on an S3. I'll see if I can mess with texture sizes or something to repro.
Blocks: 890199
Duplicate of this bug: 922097
Any update here, nrc?
Flags: needinfo?(ncameron)
(Assignee)

Comment 5

4 years ago
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #4)
> Any update here, nrc?

Afraid not, I couldn't repro it on any of my devices and been busy with other things so haven't got to it. Do you have a device I can repro on? If so, then I could have a look while I'm in TO.
Flags: needinfo?(ncameron)
I gave my Nexus 4 to nrc since it can repro the problem. He said he will look into it.
Blocks: 866585
(Assignee)

Comment 7

4 years ago
So this is bug is a barrel of fun. Turns out that clipping doesn't work because we never actually enable GL_SCISSOR_TEST. Lucky we never really use clipping :-p I'm amazed we didn't notice this. On the bright side, we ought to make a performance gain from this. Note that we would only notice this when an _active_ layer would otherwise over-write something else (I noticed this on Mac last year because an animated transform caused painted over the browser chrome). This bug did not appear on my devices because the page is layerised differently - the animated numbers are Thebes layers, whereas on Nexus 4 (where I can repro) the top number is an image layer and the bottom is a Thebes layer, on desktop both are image layers - I assume in the Thebes layer case, clipping is done by layout. A hypothesis for why this happens is that it is due to the maximum texture size on each device - that fits with the bug in this test case only manifesting for the larger image (hypothesis courtesy of roc; I'll test it tomorrow). I'm also assuming that scissor test is working on desktop (we would notice if it were not, and we don't see the bug even though we have image layers). I have not tested b2g.

Enabling scissor test doesn't actually fix this - the transform is wrong so we need to fix that as well as making sure scissor test is enabled. Again, I'll look into it tomorrow.
(Assignee)

Comment 8

4 years ago
Looks like we enable the scissor test here:

http://dxr.mozilla.org/mozilla-central/source/gfx/layers/opengl/CompositorOGL.cpp#l829

I have no idea why it doesn't actually work (I guess it should happen before the calls to fScissor just above, but maybe it doesn't matter as long as we call before presenting).
Not sure if this is relevant at all, but in our Java code (invoked from DrawWindowUnderlay) we do some stuff with scissoring too: http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/gfx/LayerRenderer.java#594

We've had problems before where the GL state left by Java wasn't what was expected by C++ (and vice-versa) so it might be another instance of that.
(Assignee)

Comment 10

4 years ago
Created attachment 813624 [details] [diff] [review]
patch
Assignee: nobody → ncameron
Attachment #813624 - Flags: review?(bugmail.mozilla)
Comment on attachment 813624 [details] [diff] [review]
patch

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

This looks sane to me, but I don't think I'm qualified to really r+ this change. Looking at some file histories suggests maybe mattwoodrow or nsilva could also review? Not really sure.
Attachment #813624 - Flags: review?(bugmail.mozilla)
Attachment #813624 - Flags: review?
Attachment #813624 - Flags: review+
(Assignee)

Updated

4 years ago
Attachment #813624 - Flags: review? → review?(nical.bugzilla)
(Assignee)

Comment 12

4 years ago
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #11)
> Comment on attachment 813624 [details] [diff] [review]
> patch
> 
> Review of attachment 813624 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> This looks sane to me, but I don't think I'm qualified to really r+ this
> change. Looking at some file histories suggests maybe mattwoodrow or nsilva
> could also review? Not really sure.

Thanks, going for nical.

Updated

4 years ago
Attachment #813624 - Flags: review?(nical.bugzilla) → review+
(Assignee)

Comment 13

4 years ago
Try push: https://tbpl.mozilla.org/?tree=Try&rev=72e18be07a2e
(Assignee)

Comment 14

4 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/23b807108524
https://hg.mozilla.org/mozilla-central/rev/23b807108524
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
No longer blocks: 890199
Duplicate of this bug: 890199
No longer blocks: 866585
Duplicate of this bug: 866585
Duplicate of this bug: 920548
(Assignee)

Updated

4 years ago
tracking-firefox26: --- → ?
Target Milestone: mozilla27 → ---
(Assignee)

Updated

4 years ago
status-firefox26: --- → affected
(Assignee)

Comment 19

4 years ago
Comment on attachment 813624 [details] [diff] [review]
patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): unknown
User impact if declined: incorrect clipping in some cases
Testing completed (on m-c, etc.): been on trunk for a few days
Risk to taking this patch (and alternatives if risky): low to medium - not many changes and nothing outrageous, but not trivial either and we obviously don't have great testing in this area or we would have caught this bug earlier.
String or IDL/UUID changes made by this patch: none
Attachment #813624 - Flags: approval-mozilla-aurora?
tracking-firefox26: ? → +
Attachment #813624 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/f80fd27d367f
status-firefox26: affected → fixed
Target milestone is used to track when it landed on m-c. Please don't un-set it unless you're backing out.
status-firefox27: --- → fixed
Target Milestone: --- → mozilla27

Updated

4 years ago
Duplicate of this bug: 927595
Duplicate of this bug: 857282

Updated

4 years ago
Duplicate of this bug: 929402
You need to log in before you can comment on or make changes to this bug.