Closed
Bug 604533
Opened 14 years ago
Closed 14 years ago
"Tearing"/"Shearing" while panning on local pages/error console in Fennec
Categories
(Core :: Graphics, defect)
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
fennec | 2.0b3+ | --- |
People
(Reporter: mwu, Assigned: blassey)
References
Details
Attachments
(2 files, 1 obsolete file)
While panning, sometimes things go crazy and draw wrong. Example attached. It fixes itself a few seconds later.
This is reminiscent of bug 579736 and bug 583115, but obviously the underlying cause would be different. It's also strange that this doesn't appear on remote pages.
I was able to reproduce this on desktop and device using steps similar to those in bug 599876. Still not sure what's up though.
Assignee | ||
Updated•14 years ago
|
tracking-fennec: --- → 2.0b3+
Comment 4•14 years ago
|
||
I made a movie: http://www.youtube.com/watch?v=7fklbzqDmzY
Assignee | ||
Updated•14 years ago
|
Assignee: nobody → jmuizelaar
Comment 6•14 years ago
|
||
This is 100% reproducible with the following steps:
1) Load fennec
2) Kill the plugin-container process via adb shell
3) Reload content process, load about:crashes
4) Click on the crash report in the list to submit it
5) While the throbber is spinning (it spins forever currently), pan the page
Comment 7•14 years ago
|
||
I'm guessing the animation of the throbber is what makes this so easy to reproduce in this case.
I suspect the problem here is that cairo image surface don't really support self-copies. I suspect the origin of the rendering artifacts is here http://mxr.mozilla.org/mozilla-central/source/gfx/layers/ThebesLayerBuffer.cpp#250, so we would want to not attempt that optimization. There are various levels of cleanliness in a disabling patch; dirtiest hack would be skipping this #if !mobile, cleanest would probably be adding gfxASurface::CanCopyFromSelf(), false for gfxImageSurface, and adding a fallback path in thebeslayerbuffer. I can look at this late tonight if no one else has in the meantime.
(Should add that comment 8 refers to a temporary workaround for b3. The real fix would obviously be to make cairo image surfaces support self-copies to the extent that the impl can be performant.)
Assignee | ||
Comment 10•14 years ago
|
||
Assignee: jmuizelaar → blassey.bugs
Attachment #492799 -
Flags: review?(jmuizelaar)
Updated•14 years ago
|
Attachment #492799 -
Flags: review?(roc)
Comment 11•14 years ago
|
||
Comment on attachment 492799 [details] [diff] [review]
patch
I'm worried about the negative performance impact that this will have but I'm also not a fan of self-copy and would rather we fix the performance problem without using self-copy. Still, correctness is better than performance here, so r+.
I've asked roc for review here because he likely has an opinion, but I think it should be ok to land without waiting for him.
Attachment #492799 -
Flags: review?(jmuizelaar) → review+
Comment on attachment 492799 [details] [diff] [review]
patch
No, you can't force this workaround on desktop too, where it's a valid and correct optimization.
Attachment #492799 -
Flags: review-
(Rather, on the gfx surfaces we use on desktop, i.e. not gfxImageSurface.)
(In reply to comment #11)
> I'm worried about the negative performance impact that this will have but I'm
> also not a fan of self-copy and would rather we fix the performance problem
> without using self-copy. Still, correctness is better than performance here, so
> r+.
We can't avoid the self-copy without allocating a buffer. It seems unlikely to me that we can avoid self-copy without some kind of performance hit.
The cairo approach of saying that self-copies are undefined is a bad decision. Maybe we should hack around it by adding a method to gfxASurface that tells us whether self-copies are supported?
Assignee | ||
Comment 15•14 years ago
|
||
Attachment #492799 -
Attachment is obsolete: true
Attachment #492888 -
Flags: review?(roc)
Attachment #492888 -
Flags: review?(jones.chris.g)
Attachment #492799 -
Flags: review?(roc)
Attachment #492888 -
Flags: review?(roc) → review+
Comment on attachment 492888 [details] [diff] [review]
patch
(superfluous r+ from me, though you I guess could r=cjones sr=roc for gfx API change.)
Attachment #492888 -
Flags: review?(jones.chris.g) → review+
Assignee | ||
Comment 17•14 years ago
|
||
pushed http://hg.mozilla.org/mozilla-central/rev/9c3babe383e1 with s/bool/PRBool
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Jeff/Rob: the correct followup to this bug would be, "make cairo image surfaces support self-copies to the extent that the impl can be performant", right?
Yes. Actually Matt Woodrow did some work on that.
Turns out we had bug 563488 sitting in reserve all along. Ah well.
You need to log in
before you can comment on or make changes to this bug.
Description
•