Last Comment Bug 694140 - With GL layers, panning/zooming causes corruption and wrong colours on Fennec
: With GL layers, panning/zooming causes corruption and wrong colours on Fennec
Status: RESOLVED FIXED
: regression
Product: Core
Classification: Components
Component: Graphics (show other bugs)
: Trunk
: x86 Mac OS X
: -- normal (vote)
: mozilla10
Assigned To: Oleg Romashin (:romaxa)
:
:
Mentors:
Depends on:
Blocks: opengl-mobile 690469
  Show dependency treegraph
 
Reported: 2011-10-12 14:12 PDT by Ali Juma [:ajuma]
Modified: 2011-10-16 10:42 PDT (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Screenshot: wrong colours (130.18 KB, image/png)
2011-10-12 14:13 PDT, Ali Juma [:ajuma]
no flags Details
Screenshot: Mix of content at different zoom levels (91.98 KB, image/png)
2011-10-12 14:14 PDT, Ali Juma [:ajuma]
no flags Details
Disable lazy backBuffer creation until rootcause fixed. (1.90 KB, patch)
2011-10-13 13:30 PDT, Oleg Romashin (:romaxa)
no flags Details | Diff | Splinter Review
Disable lazy backBuffer sync for SingleBuffer mode (1.23 KB, patch)
2011-10-13 13:46 PDT, Oleg Romashin (:romaxa)
ajuma.bugzilla: review+
Details | Diff | Splinter Review
Fixed color problem (2.16 KB, patch)
2011-10-13 16:03 PDT, Oleg Romashin (:romaxa)
cjones.bugs: review+
ajuma.bugzilla: review+
Details | Diff | Splinter Review

Description Ali Juma [:ajuma] 2011-10-12 14:12:35 PDT
When zoomed in and then repeatedly zooming in and out while panning at the same time, I'm seeing a mix of content at different zoom levels, and incorrect colours (e.g. red becoming blue). This is happening both on a Nexus S and on a Galaxy Tab.

The amount of zooming and panning needed to see these problems varies, even on the same page -- sometimes it's right away, and other times it requires a bit longer (no more than a minute though).

This is a regression. Range: http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=58c9046d8485&tochange=a5b88a4500aa
Comment 1 Ali Juma [:ajuma] 2011-10-12 14:13:21 PDT
Created attachment 566628 [details]
Screenshot: wrong colours
Comment 2 Ali Juma [:ajuma] 2011-10-12 14:14:14 PDT
Created attachment 566629 [details]
Screenshot: Mix of content at different zoom levels
Comment 3 Ali Juma [:ajuma] 2011-10-13 07:37:31 PDT
This turns out to be a regression caused by the landing of Bug 690469.
Comment 4 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-10-13 12:18:50 PDT
This looks like readback from front to back buffers is being messed up.
Comment 5 Oleg Romashin (:romaxa) 2011-10-13 13:30:42 PDT
Created attachment 566922 [details] [diff] [review]
Disable lazy backBuffer creation until rootcause fixed.

Ok, problem here is that PaintThebes, BasicThebesBuffer::BeginPaint assume that all buffer operations are finished and buffer is the all the time same (no double-buffering)... and calling SyncBackAndFront just in the beginning of PaintThebes not enough..

I need to think more about fixing that problem with ThebesLayerBuffer, will try to find better place for SyncBackAndFront...
Here is minor fix which does not revert whole Thebes Swap rework, but just making Buffers in sync right after Swap.
Comment 6 Oleg Romashin (:romaxa) 2011-10-13 13:46:36 PDT
Created attachment 566927 [details] [diff] [review]
Disable lazy backBuffer sync for SingleBuffer mode
Comment 7 Ali Juma [:ajuma] 2011-10-13 14:12:58 PDT
Comment on attachment 566927 [details] [diff] [review]
Disable lazy backBuffer sync for SingleBuffer mode

This fixes the corruption caused by panning/zooming, but not the colour problem.

Please change the commit message to say what the patch does. 

Also, when this lands, we should keep the bug open while we find the real cause (and deal with the colour issue).
Comment 8 Oleg Romashin (:romaxa) 2011-10-13 16:01:21 PDT
Ok, I found what is the problem with thebes layers...
with static front we had front destroyed when backBuffer content type changed... but now we only do that when size is changed...
BackingSurface with locking handle it because we handle properly composition form surface to texture with different format types... but 
GLContext::UploadSurfaceToTexture cannot do that... (another bug?)
Comment 9 Oleg Romashin (:romaxa) 2011-10-13 16:03:59 PDT
Created attachment 566965 [details] [diff] [review]
Fixed color problem

Ok, ehre is fix for color problem, we should check if new surface and current texture have equal format
Comment 10 Ali Juma [:ajuma] 2011-10-14 07:32:56 PDT
(In reply to Oleg Romashin (:romaxa) from comment #9)
> Created attachment 566965 [details] [diff] [review] [diff] [details] [review]
> Fixed color problem

This patch does indeed fix both problems for me.
Comment 11 Ed Morley [:emorley] 2011-10-15 07:20:24 PDT
In my queue with a few other bits that are being sent to try first and then onto inbound :-)

https://tbpl.mozilla.org/?tree=Try&rev=488d4a5f274a

Note You need to log in before you can comment on or make changes to this bug.