Closed Bug 615753 Opened 9 years ago Closed 9 years ago

Figure out buffering on android


(Core :: Graphics, defect)

Not set





(Reporter: jrmuizel, Assigned: jrmuizel)




(2 files, 2 obsolete files)

Currently things are sketchy. They need to be fixed.
Attached patch workaround (obsolete) — Splinter Review
The problem here is that we get two layer managers when we really should only have one.

Vlad, which nsWindow do you think should have the layer manager?

:  [ 0] 0x5065af20 [parent 0x00000000] [  0,  0  480x 762] vis 1 type 0
:   [ 0] 0x5065c620 [parent 0x5065af20] [  0,  0  480x 762] vis 1 type 3
:  [ 1] 0x5065aad0 [parent 0x00000000] [  0,  0   0x  0] vis 0 type 4
:   [ 0] 0x5065c790 [parent 0x5065aad0] [  0,  0   0x  0] vis 1 type 3

Right now both 0x5065af20 and 0x5065c620 get layer managers. It seems like non-android specific code calls GetLayerManager() on the child window.
Interesting.  In an ideal world, we wouldn't have that fullscreen child relationship at all.  But for us, that's the one that would have a layer manager.

However -- does anyone actually draw 0xf20?  I would think with that structure, the only draws would ever only go to 0x620 (the child).  It doesn't hurt if we have multiple layer managers, as the others will just be dormant.  It only hurts if we try to draw multiple ones at the same time...
DrawTo is called on the child window. OnDraw is called on the parent... I'm not sure what the difference between these is.
OnDraw calls DrawTo; OnDraw I believe will only be delivered to toplevel windows (since we always have to draw the entire visible screen-area, so entire top level window).  "DrawTo" takes a surface that says what buffer to draw to in the software case, but it's a bit badly named for the GL case.  But what you should see is something like:

      - actual drawing

the TOP->DrawTo should hit the "covering child" case where it'll see there's a child window that covers the entire area of the parent.
It might be possible for us to avoid the child window completely. Windows does this with:
Attached patch double bufferSplinter Review
It looks like the right thing to do here is just double buffer.

We don't seem to actually draw using both layer managers so we can keep the unused one around for now.
Attachment #495594 - Flags: review?(vladimir)
Attached patch double buffer v2 (obsolete) — Splinter Review
The above patch didn't work for me on a Samsung Galaxy S, it still flickered black. This fixes it for me.

Still seems like having two windows around is causing issues.
Attachment #495787 - Flags: review?(vladimir)
Comment on attachment 495787 [details] [diff] [review]
double buffer v2

ah yes, the clear needs to go away as well.  sorry I missed that!
Attachment #495787 - Flags: review?(vladimir) → review+
Attached patch Remove clear.Splinter Review
Just the part that wasn't included in the above push.

Carrying forward r=vlad
Attachment #494221 - Attachment is obsolete: true
Attachment #495787 - Attachment is obsolete: true
Attachment #495977 - Flags: review+
Here's the last part:
Closed: 9 years ago
Resolution: --- → FIXED
Assignee: nobody → jmuizelaar
You need to log in before you can comment on or make changes to this bug.