Closed
Bug 737071
Opened 13 years ago
Closed 13 years ago
nexus s image layers are not drawn on B2G
Categories
(Core :: Graphics, defect)
Tracking
()
RESOLVED
FIXED
mozilla14
People
(Reporter: gal, Assigned: cjones)
References
Details
(Whiteboard: [whiteboard])
Attachments
(1 file, 1 obsolete file)
STR:
- install B2G trunk on nexus s
- go to gallery, pan images around
- images don't show until the layer is inserted back into the backgroun
Reporter | ||
Comment 1•13 years ago
|
||
This is a very urgent, top priority bug.
Comment 2•13 years ago
|
||
What do you mean by "Pan images around"? I just installed B2G trunk on a Nexus S, and I can pan the gallery images fine. Do I have to set things up differently?
Reporter | ||
Comment 3•13 years ago
|
||
Go to gallery. Move images left and right. On my phone nothing moves (black screen) until the image snaps into place. Also, no video plays (webm video). Can you try these things? If they work for you we have different hardware.
Comment 4•13 years ago
|
||
I can definitely reproduce it on video, so I'll work on that. Seems more straightforward :)
Assignee | ||
Comment 5•13 years ago
|
||
I poked at this for a few hours tonight, and found some interesting things
- video is usually b0rked. The framerate is extremely low.
- HOWEVER, if I pull down the notification bar, sometimes I effect a state change in which video starts to play smoothly and flicker-free. (There are transient artifacts, but I think they're due to a known PowerVR SGX bug.)
- when the video is b0rked / framerate is low, I see a disturbing trend for ImageLayerOGL::RenderLayer calls to take 0.1-300ms (that's right, 300ms). Texture upload itself consistently takes ~3ms, about right for memcpys.
- bisecting on RenderLayer, the culprit is the |gl->MakeCurrent()| below, annotated with an example timing info from a "bad" frame
502 void
503 ImageLayerOGL::AllocateTexturesYCbCr(PlanarYCbCrImage *aImage)
504 {
(start timer)
505 if (!aImage->mBufferSize)
506 return;
507
508 nsAutoPtr<PlanarYCbCrOGLBackendData> backendData(
509 new PlanarYCbCrOGLBackendData);
510
AllocateTexturesYCbCr - new pyccbackenddata: 0.0041 ms
511 PlanarYCbCrImage::Data &data = aImage->mData;
512
513 GLContext *gl = mOGLManager->glForResources();
514
AllocateTexturesYCbCr - glforresources: 0.0199 ms
515 gl->MakeCurrent();
AllocateTexturesYCbCr - makecurrent: 248.885 ms (!?!?!?!)
So the next things I would like to know are
- wtf is taking so long in the MakeCurrent call
- what changes when the notification bar is pulled down, sometimes, to effect the switch to smooth playback. Better STR for switching to smooth would help a lot.
Comment 6•13 years ago
|
||
MakeCurrent is Known Slow, but I've never seen it be *that* bad. Does the length of MakeCurrent change at all if you put a glFinish (on the previous context) before it? What about a glFlush?
Assignee | ||
Comment 7•13 years ago
|
||
AFAIK there's only one context involved here.
Updated•13 years ago
|
OS: Mac OS X → Android
Hardware: x86 → All
Updated•13 years ago
|
OS: Android → Gonk
Assignee | ||
Comment 9•13 years ago
|
||
... except for the global shared context. This fixes the video bug:
diff --git a/gfx/gl/GLContextProviderEGL.cpp b/gfx/gl/GLContextProviderEGL.cpp
index 446ca73..eedf0c5 100644
--- a/gfx/gl/GLContextProviderEGL.cpp
+++ b/gfx/gl/GLContextProviderEGL.cpp
@@ -1992,7 +1992,7 @@ static nsRefPtr<GLContext> gGlobalContext;
GLContext *
GLContextProviderEGL::GetGlobalContext()
{
- static bool triedToCreateContext = false;
+ static bool triedToCreateContext = true;//false;
if (!triedToCreateContext && !gGlobalContext) {
triedToCreateContext = true;
// Don't assign directly to gGlobalContext here, because
Assignee | ||
Comment 10•13 years ago
|
||
The only place this is really needed is in ImageLayerOGL for containers moving parent contexts, but
- that'll be incredibly uncommon
- the code already handles reparenting to another context
So this should be pure win.
Assignee: nobody → jones.chris.g
Attachment #608216 -
Flags: review?(bas.schouten)
Updated•13 years ago
|
Attachment #608216 -
Flags: review?(bas.schouten) → review+
Assignee | ||
Comment 11•13 years ago
|
||
Now with reupload that I mistakenly thought was there in v1.
Attachment #608216 -
Attachment is obsolete: true
Attachment #608221 -
Flags: review?(bas.schouten)
Assignee | ||
Comment 12•13 years ago
|
||
Comment on attachment 608221 [details] [diff] [review]
Remove LayerManagerOGL::glForResources() because it's not needed and causes performance degradtion sometimes, v2
+ cairoImage->SetBackendData(LayerManager::LAYERS_OPENGL, nsnull);
+ cairoImage = nsnull;
That's wrong. Fixed locally.
Comment 13•13 years ago
|
||
Comment on attachment 608221 [details] [diff] [review]
Remove LayerManagerOGL::glForResources() because it's not needed and causes performance degradtion sometimes, v2
Review of attachment 608221 [details] [diff] [review]:
-----------------------------------------------------------------
::: gfx/layers/opengl/ImageLayerOGL.cpp
@@ +307,5 @@
> + if (data && data->mTexture.GetGLContext() != gl()) {
> + // If this texture was allocated by another layer manager, clear
> + // it out and re-allocate below.
> + cairoImage->SetBackendData(LayerManager::LAYERS_OPENGL, nsnull);
> + cairoImage = nsnull;
data = nsnull;
@@ +525,5 @@
> new PlanarYCbCrOGLBackendData);
>
> PlanarYCbCrImage::Data &data = aImage->mData;
>
> + gl()->MakeCurrent();
Sometime we should have a good look at redundant make current calls in the layers system.
Attachment #608221 -
Flags: review?(bas.schouten) → review+
Assignee | ||
Comment 14•13 years ago
|
||
Whiteboard: [whiteboard]
Comment 15•13 years ago
|
||
I worry whether this will screw us on shutdown, since we rely on that extra GL context to allow us to delete textures when our original context has been deleted.
OS: Gonk → Mac OS X
Hardware: All → x86
Updated•13 years ago
|
OS: Mac OS X → Gonk
Hardware: x86 → ARM
Comment 16•13 years ago
|
||
(In reply to Joe Drew (:JOEDREW!) from comment #15)
> I worry whether this will screw us on shutdown, since we rely on that extra
> GL context to allow us to delete textures when our original context has been
> deleted.
We probably shouldn't bother deleting GL stuff on shutdown, really.
If contexts clean up after themselves when they are destroyed, we shouldn't bother formally cleaning up at all. However, since everything's shared, that might persist objects even though the original creating context has been destroyed. (But it's worth checking)
Comment 17•13 years ago
|
||
I didn't check the spec and didn't test it, but I found indications that if you do:
foo = new Context()
tex = foo.createTexture()
bar = new Context(share with foo)
foo.destroy
'tex' is still accessible from context bar, which is sensible.
However, if you then destroy bar, it'll clean everything up.
Comment 18•13 years ago
|
||
Er, when I said shutdown, I meant context shutdown. And, it turns out, I actually meant BasicTextureImage::~BasicTextureImage().
As long as that isn't affected, I'm ok with this.
Comment 19•13 years ago
|
||
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla14
You need to log in
before you can comment on or make changes to this bug.
Description
•