Closed Bug 811157 Opened 9 years ago Closed 9 years ago

Redraw issues in 3rd party app CrabJump

Categories

(Core :: Graphics: Layers, defect, P2)

ARM
Gonk (Firefox OS)
defect

Tracking

()

RESOLVED FIXED
B2G C3 (12dec-1jan)
blocking-basecamp +
Tracking Status
firefox19 --- fixed
firefox20 --- fixed
b2g18 --- fixed

People

(Reporter: adora, Assigned: jrmuizel)

Details

(Whiteboard: [b2g-gfx])

Attachments

(1 file, 2 obsolete files)

Cars flicker every few seconds, and the turtle and seal(?) are occasionally displayed enlarged and in the wrong location.

Easy to duplicate on Firefox OS, I'm not able to duplicate on Android (Galaxy Nexus).

Screenshot:  http://adora.io/screens/crabjump-20121112-172115.png

At the bottom of the screen, you can see a transparent artifact of the purple seal thing.
It's showing the same flickering behavior on nightly for desktop, too.

Marketplace link:  https://marketplace.mozilla.org/app/crabjump/
Component: General → Graphics: Layers
Product: Boot2Gecko → Core
Confirmed. And...another gfx bug.
blocking-basecamp: --- → ?
Version: unspecified → Trunk
It is only occurred in "Animated Sprite". And when they are in viewport, the screen tends to flash...
After some debug with log and gdb, I found following logs before a strange frame presentation:

11-13 15:34:53.920   106   242 E libgenlock: perform_lock_unlock_operation: GENLOCK_IOC_LOCK failed (lockType0x1, err=Invalid argument fd=189)
11-13 15:34:53.920   106   242 E msm7627a.gralloc: gralloc_lock: genlock_lock_buffer (lockType=0x2) failed
11-13 15:34:53.920   106   242 W GraphicBufferMapper: lock(...) failed -22 (Invalid argument)
11-13 15:34:53.920   106   242 I Gonk    : Could not lock GraphicBuffer
11-13 15:34:53.920   106   242 E libgenlock: perform_lock_unlock_operation: GENLOCK_IOC_LOCK failed (lockType0x1, err=Invalid argument fd=192)
11-13 15:34:53.920   106   242 E msm7627a.gralloc: gralloc_lock: genlock_lock_buffer (lockType=0x2) failed
11-13 15:34:53.920   106   242 W GraphicBufferMapper: lock(...) failed -22 (Invalid argument)
11-13 15:34:53.920   106   242 I Gonk    : Could not lock GraphicBuffer

This means the bug maybe some kind of gralloc lock related.
Assignee: nobody → chung
Status: NEW → RESOLVED
blocking-basecamp: ? → +
Closed: 9 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 811131
Reopening as Jonas thinks this is a different issue.
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
I have not investigated bug 811131 much but according to the discussion there, I think they are different.

In my experience in Android, this issue may related to streaming texture usage problem after ICS version. I will try the solution there to see whether they are the same.
I found that I was wrong. This issue is not the same with Android one. The buffer just got freed...
Hmm...The gralloc failed issue seems not the true cause of this one. I have to investigate it deeply to find out what happened. I guess it may be OMTC related...
Chiajung, how's this going?
Flags: needinfo?(chung)
(In reply to Andrew Overholt [:overholt] from comment #10)
> Chiajung, how's this going?

Sorry for late update.

I debugged this in more detail and found it should be layer/composition related but I can not tell where the root cause is in current stage.

I found the issue is somehow related to scale transformation applied to the image layer. So, I tried to log some information when painting, and the size/coordinate is normal. But if I log scale factor of the matrix applied to each visible layer, sometimes the scale is strange. On Otoro platform, normal frames have scale factor 0.42, but sometimes it is 1.00 and cause flashing frame or huge sprites. I am not sure whether it is related, but this symptom is observed only when more than 5 moving sprites(cars, turtles and seals) are visible.

However, the matrix computation staff is really complex which I know nothing about now. And since I have other works to do currently, I think it may be fixed faster if some other familiar with it to take it.
Flags: needinfo?(chung)
Jeff, can you take a look at this?
Assignee: chung → jmuizelaar
Setting priority based on triage discussions.  Feel free to decrease priority if you disagree.
Priority: -- → P2
Forcing off omta doesn't make the artifacts go away.
ChooseScaleAndSetTransform is clamping an alternating scale of sane and 2.4 to 1.0 when things are flashing.
(In reply to Jeff Muizelaar [:jrmuizel] from comment #15)
> ChooseScaleAndSetTransform is clamping an alternating scale of sane and 2.4
> to 1.0 when things are flashing.

I was confused we're actually clamping 0.41666 to 1.0
Mass Modify: All un-milestoned, unresolved blocking-basecamp+ bugs are being moved into the C3 milestone. Note that the target milestone does not mean that these bugs can't be resolved prior to 12/10, rather C2 bugs should be prioritized ahead of C3 bugs.
Target Milestone: --- → B2G C3 (12dec-1jan)
So it looks like this has to do with the prescale that we put on the layer and I expect it's a regression from off-main thread animations. Basically it seems like we alternate between having a prescale on the layer of 1.0 and 1/2.4. The bug seems to be in ChooseScaleAndSetTransform but I'm not exactly sure how it's supposed to work. Certainly the behaviour of clamping only if the scale is not the old scale is a bit suspicious.

Roc, any thoughts?
So the other thing that's going on is that nsIFrame::TryUpdateTransformOnly()
is setting a completely different base transform on the layer.
(In reply to Jeff Muizelaar [:jrmuizel] from comment #19)
> So the other thing that's going on is that nsIFrame::TryUpdateTransformOnly()
> is setting a completely different base transform on the layer.

Yes, the scale stuff in ChooseScaleAndSetTransform is not being applied here.

Perhaps in ChooseScaleAndSetTransform, if the layer's BaseTransform ends up being different from *aTransform, we should set a flag or userdata on the layer and nsIFrame::TryUpdateTransformOnly should refuse to update such layers.
Something like this?
Attachment #690480 - Flags: feedback?(roc)
 
> Yes, the scale stuff in ChooseScaleAndSetTransform is not being applied here.
> 
> Perhaps in ChooseScaleAndSetTransform, if the layer's BaseTransform ends up
> being different from *aTransform, we should set a flag or userdata on the
> layer and nsIFrame::TryUpdateTransformOnly should refuse to update such
> layers.

That shouldn't matter should it?

ChooseScaleAndSetTransform shouldn't modify the actual transform, only the scale factors, and we only let TryUpdateTransformOnly succeed if the matrix has only the translate values changed.
(In reply to Matt Woodrow (:mattwoodrow) from comment #23)
> ChooseScaleAndSetTransform shouldn't modify the actual transform, only the
> scale factors, and we only let TryUpdateTransformOnly succeed if the matrix
> has only the translate values changed.

Yes, that's true.
So this patch isn't correct and I don't know what's going on here :-). But patch part 1 in bug 663776 should help ... once I get hold of a Mac to fix the assertion failure. Jeff, maybe you could help with that :-).
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #25)
> So this patch isn't correct and I don't know what's going on here :-). But
> patch part 1 in bug 663776 should help ... once I get hold of a Mac to fix
> the assertion failure. Jeff, maybe you could help with that :-).

I did try patch part 1 and it was insufficient to fix this. Still I can look at the mac problem.
(In reply to Jeff Muizelaar [:jrmuizel] from comment #26)
> I did try patch part 1 and it was insufficient to fix this. Still I can look
> at the mac problem.

Don't bother, I'll work on that. I think you should take part 1 and then dig further into the rest of this bug --- understanding more about what's going on in comment #19.
Attached patch Use GetBaseTransform instead (obsolete) — Splinter Review
This seems more correct.
Attachment #690480 - Attachment is obsolete: true
Attachment #690480 - Flags: feedback?(roc)
Attachment #691502 - Flags: review?(roc)
Attachment #691502 - Attachment is obsolete: true
Attachment #691502 - Flags: review?(roc)
Attachment #691504 - Flags: review?(roc)
Comment on attachment 691504 [details] [diff] [review]
Use GetBaseTransform instead

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

::: layout/base/FrameLayerBuilder.cpp
@@ +2694,4 @@
>        // or it was previously unscaled.
>        bool clamp = true;
>        gfxMatrix oldFrameTransform2d;
> +      if (aLayer->GetBaseTransform().Is2D(&oldFrameTransform2d)) {

I landed this on inbound last night.

::: layout/generic/nsFrame.cpp
@@ +4930,4 @@
>    // changes.)
>   static const gfx::Float kError = 0.0001;
>    if (!transform3d.Is2D(&transform) ||
> +      !layer->GetBaseTransform().Is2D(&previousTransform) ||

This looks good :-)
Attachment #691504 - Flags: review?(roc) → review+
https://hg.mozilla.org/mozilla-central/rev/991647fcdc27
Status: REOPENED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.