The default bug view has changed. See this FAQ.

nexus s image layers are not drawn on B2G

RESOLVED FIXED in mozilla14

Status

()

Core
Graphics
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: gal, Assigned: cjones)

Tracking

Trunk
mozilla14
ARM
Gonk (Firefox OS)
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [whiteboard])

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

5 years ago
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

5 years ago
This is a very urgent, top priority bug.
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

5 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.
I can definitely reproduce it on video, so I'll work on that. Seems more straightforward :)
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.
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?
AFAIK there's only one context involved here.
OS: Mac OS X → Android
Hardware: x86 → All
OS: Android → Gonk
Duplicate of this bug: 738082
... 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
Created attachment 608216 [details] [diff] [review]
Remove LayerManagerOGL::glForResources() because it's not needed and causes performance degradtion sometimes

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)
Attachment #608216 - Flags: review?(bas.schouten) → review+
Created attachment 608221 [details] [diff] [review]
Remove LayerManagerOGL::glForResources() because it's not needed and causes performance degradtion sometimes, v2

Now with reupload that I mistakenly thought was there in v1.
Attachment #608216 - Attachment is obsolete: true
Attachment #608221 - Flags: review?(bas.schouten)
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 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+
https://hg.mozilla.org/integration/mozilla-inbound/rev/1a0a9c190dd9
Whiteboard: [whiteboard]
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
OS: Mac OS X → Gonk
Hardware: x86 → ARM
(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)
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.
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.
https://hg.mozilla.org/mozilla-central/rev/1a0a9c190dd9
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla14
You need to log in before you can comment on or make changes to this bug.