Closed Bug 546517 Opened 14 years ago Closed 14 years ago

OpenGL 2 Layers Backend

Categories

(Core :: Graphics, defect)

x86
Windows 7
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: bas.schouten, Assigned: bas.schouten)

References

Details

Attachments

(2 files, 6 obsolete files)

An OpenGL layers backend needs to be created for use on non-windows OSes and mobile operating systems.
Depends on: 550846
This is a preliminary OpenGL layers backend. It leaks though, something which I'm still trying to clear up. Additionally there's potential race conditions inside the Image classes which should be threadsafe but are not. And the code is still in a somewhat experimental state. It is not yet completely implementing layers, some features are missing, but most are there, it will enable hardware accelerated fullscreen video and such.

Posting this now though so people can have an early look and possibly play with it.
This is a simple patch which will allow widget to use OGL layers, and enable/disable it by commenting out the USE_OGL_LAYERS in nsWindow.h.
Attached patch same as first, but without glew (obsolete) — Splinter Review
Same as the first patch, just attaching here for ease of reviewing (>1MB of the original patch is the glew files)
Most of these comments are things that are ok for just getting something in, but for performance or other reasons we'll want to fix them as soon as we can.

Two general comments (I started mentioning this in a few places, but it's everywhere):

- glGet* is death.  Don't do it.  A little stack helper of the uniform values we need to keep/reset with push/pop/apply (see below) would be best.

- that applies to glGetUniformLocation, though I don't think that's as deadly as reading uniform values.  Still, that should really be cached -- for each program we ought to create a little program helper object that can cache uniform locations when they're looked up, etc.

+void
+ContainerLayerOGL::RenderLayer(int aPreviousFrameBuffer)
+{
+  /**
+   * Setup our temporary texture for rendering the contents of this container.
+   */
+  GLuint containerSurface;
+
+  glGenTextures(1, &containerSurface);
+  glBindTexture(GL_TEXTURE_2D, containerSurface);
+  glTexImage2D(GL_TEXTURE_2D,
+               0,
+               4,
+               mVisibleRect.width,
+               mVisibleRect.height,
+               0,
+               GL_BGRA,
+               GL_UNSIGNED_BYTE,
+               NULL);

Any reason why we're creating a texture each time?  I guess we're constantly going to be throwing away the texture contents, so we'll have to call TexImage2D no matter what; GPU memory fragmentation might be a concern if we have longer-lived textures.

Also note that for GL ES, NPOT textures are only universally supported if the clamp mode is CLAMP_TO_EDGE (which is not the default -- REPEAT is the default).  Also the min/mag filter needs to be NEAREST or LINEAR, but LINEAR is the default.

+  float destinationOffset[4];
+  glGetUniformfv(rgbProgram,
+                 glGetUniformLocation(rgbProgram, "RenderTargetOffset"),
+                 destinationOffset);

glGet* anything is performance death -- why do we need to get these things here?  We set it, we should be able to keep it elsewhere without needing to get it from the GL.  Now, there's a chance that the driver has this info cached, but I don't think we should rely on that.  Also, it's likely going to be much better to (for now) do all the glGets at once, and only then modify the uniform buffers.

+  // Restore old shader program variables.

Same issue as before -- these are really wasted uniform updates.  We should track this data and just update it as needed; if we want to use these as a stack, like it seems like they're being used, then a little stack object with push/pop/apply methods would be handy.  (Apply would actually call glUniform, to avoid setting them every time on pop even if another pop might be coming.)

+  GLint quadTransformLocation = 
+    glGetUniformLocation(rgbProgram, "LayerQuadTransform");

Uniform locations need to be cached; a little program helper object can be useful here.

+  glActiveTexture(GL_TEXTURE0);
+  glBindTexture(GL_TEXTURE_2D, containerSurface);

See above about GL ES 2.0 and NPOT textures; need to set CLAMP_TO_EDGE here.  Note that this will be an issue if you actually have an image that you -do- want to render with REPEAT, since it won't be possible to just go the simple route of using uv coords 0.0 ... num_repeats-1

+  glUniform1i(glGetUniformLocation(rgbProgram, "LayerTexture"), 0);
+  glUniform1f(glGetUniformLocation(rgbProgram, "LayerOpacity"), GetOpacity());
+
+  glUniformMatrix4fv(glGetUniformLocation(rgbProgram, "LayerTransform"),
+                     1,
+                     false,
+                     &mTransform._11);


Same as above re caching uniform locs.

+  glEnableClientState(GL_VERTEX_ARRAY);
+  glEnableClientState(GL_TEXTURE_COORD_ARRAY);
+  GLfloat vertices[] = { 0, 0, 1.0f, 0, 0, 1.0f, 1.0f, 1.0f };
+  GLfloat texcoords[] = { 0, 0, 1.0f, 0, 0, 1.0f, 1.0f, 1.0f };
+  glVertexPointer(2, GL_FLOAT, 0, vertices);
+  glTexCoordPointer(2, GL_FLOAT, 0, texcoords);

Um, hm.  If these are really going to be constant, they need to go into a VBO instead of using client side arrays; everything should really go into a VBO, but this especially.

+  glDrawArrays(GL_TRIANGLE_STRIP, 0, 4);
+
+  // Clean up resources.
+  glBindTexture(GL_TEXTURE_2D, 0);
+  glDeleteTextures(1, &containerSurface);

No need to rebind, when the texture is deleted the current texture binding is lost and will revert to 0.

+}
+}

nit: Some comments as to what namespaces these are closing would be handy.

+    glActiveTexture(GL_TEXTURE0);
+    glBindTexture(GL_TEXTURE_2D, yuvImage->mTextures[0]);
+    glActiveTexture(GL_TEXTURE1);
+    glBindTexture(GL_TEXTURE_2D, yuvImage->mTextures[1]);
+    glActiveTexture(GL_TEXTURE2);
+    glBindTexture(GL_TEXTURE_2D, yuvImage->mTextures[2]);

Note that there is an extension, APPLE_ycbcr_422, that provides for direct YUV textures in GL.  However, it wants non-planar layout of the data.  It's present primarily on Macs, but that's fine, since that's where we'll be using the GL backend the most... it might be worth being able to tell the decoder that we want interleaved instead of planar data if we have that extension.

+    glUniform1i(glGetUniformLocation(program, "YTexture"), 0);
+    glUniform1i(glGetUniformLocation(program, "UTexture"), 1);
+    glUniform1i(glGetUniformLocation(program, "VTexture"), 2);
+    glUniform1f(glGetUniformLocation(program, "LayerOpacity"), GetOpacity());
+    glUniformMatrix4fv(glGetUniformLocation(program, "LayerTransform"),
+                         1,
+                         false,
+                         &mTransform._11);
+    glEnableClientState(GL_VERTEX_ARRAY);
+    glEnableClientState(GL_TEXTURE_COORD_ARRAY);
+    GLfloat vertices[] = { 0, 0, 1.0f, 0, 0, 1.0f, 1.0f, 1.0f };
+    glVertexPointer(2, GL_FLOAT, 0, vertices);
+    glTexCoordPointer(2, GL_FLOAT, 0, vertices);

Same as above about this being in a VBO.

+    // Cleanup textures.
+    glBindTexture(GL_TEXTURE_2D, 0);
+    glActiveTexture(GL_TEXTURE1);
+    glBindTexture(GL_TEXTURE_2D, 0);
+    glActiveTexture(GL_TEXTURE0);
+    glBindTexture(GL_TEXTURE_2D, 0);

Hm, any reason to actually clear these?  I'd put ActiveTexture back to texture 0, but otherwise if a shader doesn't use a texture unit it doesn't matter if something is bound there... and if it does, it'll be rebound before usage anyway.

+    yuvImage->FreeTextures();

Wait what?  Well if we're going to blow them away, then yeah, just put the active texture to 0 and call FreeTextures.

+  } else if (image->GetFormat() == Image::CAIRO_SURFACE) {
+    CairoImageOGL *cairoImage =
+      static_cast<CairoImageOGL*>(image.get());
+
+    cairoImage->mTexture;

Er, you're missing some kind of verb here, I think :-)

+void
+PlanarYCbCrImageOGL::AllocateTextures()
+{
+  mManager->MakeCurrent();
+  glGenTextures(3, mTextures);
+
+  glPixelStorei(GL_UNPACK_ROW_LENGTH, mData.mYStride);

UNPACK_ROW_LENGTH isn't supported on ES 2.0.  Sorry.  (Like, not supported at all, don't think any extensions put it back either.)

+  glBindTexture(GL_TEXTURE_2D, mTextures[0]);
+  glTexImage2D(GL_TEXTURE_2D,
+               0,
+               1,

Eek, don't do this.  Pass GL_LUMINANCE.  ES at least requires that this be identical to the format (no format conversions are supported at teximage2d time).

+               mSize.width,
+               mSize.height,
+               0,
+               GL_LUMINANCE,
+               GL_UNSIGNED_BYTE,
+               mData.mYChannel);
+  glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
+  glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);

See earlier about setting CLAMP_TO_EDGE for S/T wrap modes for npot textures.

Ditto for the CbCr texture setup.

+void
+CairoImageOGL::SetData(const CairoImage::Data &aData)
+{
+  /**
+   * XXX - Should make sure this happens on the correct thread. Since this
+   * is supposed to be threadsafe.
+   */
+  mSize = aData.mSize;
+  mManager->MakeCurrent();
+
+  nsRefPtr<gfxImageSurface> imageSurface =
+    new gfxImageSurface(aData.mSize, gfxASurface::ImageFormatARGB32);
+
+  nsRefPtr<gfxContext> context = new gfxContext(imageSurface);
+
+  context->SetSource(aData.mSurface);
+  context->Paint();
+
+  glGenTextures(1, &mTexture);
+
+  glBindTexture(GL_TEXTURE_2D, mTexture);
+  glTexImage2D(GL_TEXTURE_2D,
+               0,
+               4,

pass GL_RGBA or whatever.

+               mSize.width,
+               mSize.height,
+               0,
+               GL_BGRA,

Sadly, you can't rely on BGRA textures being supported.  On ES 2.0 at least, it's an optional extension, and it's not supported on at least the Nexus One hardware.  However, you can do the channel swizzle in the fragment shader, so I'd suggest just having two shaders -- one for when the extension is present and one when it's not.  I *think* there's a perf win in using it if it's present, but I'm not sure; if there isn't, then we can just ignore it and always do the swizzle.

+               GL_UNSIGNED_BYTE,
+               imageSurface->Data());
+
+  glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
+  glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);

CLAMP_TO_EDGE etc.

diff --git a/gfx/layers/opengl/LayerManagerOGL.cpp b/gfx/layers/opengl/LayerManagerOGL.cpp
new file mode 100644
--- /dev/null
+++ b/gfx/layers/opengl/LayerManagerOGL.cpp

+#pragma comment(lib, "Opengl32.lib")

Cheater.  On a related note, when I checked in code that always linked us to opengl32.lib on windows for webgl, our startup time went up by like 500ms.  So that might be a blocker to getting this checked in, even preffed off -- might have to ditch glew and do runtime linking if it's enabled, like what webgl does.



+LayerManagerOGL::~LayerManagerOGL()
+{
+  MakeCurrent();
+  glUseProgram(0);
+  glDeleteProgram(mRGBLayerProgram);
+  glDeleteProgram(mYUVLayerProgram);
+  glDeleteShader(mVertexShader);
+  glDeleteShader(mRGBShader);
+  glDeleteShader(mYUVShader);
+  wglDeleteContext(mContext);
+  ::ReleaseDC((HWND)mWidget->GetNativeData(NS_NATIVE_WINDOW), mDC);

No need to delete programs/shaders/all that stuff.  When the context goes away, if the resources aren't shared with anything else, they'll get released.

+}
+
+bool
+LayerManagerOGL::Initialize()
+{
+#ifdef XP_WIN
+  mDC = (HDC)mWidget->GetNativeData(NS_NATIVE_GRAPHIC);
+  PIXELFORMATDESCRIPTOR pfd;
+  ZeroMemory(&pfd, sizeof(pfd));
+
+  pfd.nSize = sizeof(pfd);
+  pfd.nVersion = 1;
+  pfd.dwFlags = PFD_DRAW_TO_WINDOW | PFD_SUPPORT_OPENGL |
+    PFD_DOUBLEBUFFER;
+  pfd.iPixelType = PFD_TYPE_RGBA;
+  pfd.cColorBits = 32;
+  pfd.cDepthBits = 0;
+  pfd.iLayerType = PFD_MAIN_PLANE;
+  int iFormat = ChoosePixelFormat(mDC, &pfd);
+
+  SetPixelFormat(mDC, iFormat, &pfd);
+  mContext = wglCreateContext(mDC);
+#endif
+
+  MakeCurrent();
+
+  if (!sGlewInit) {
+    GLenum init = glewInit();
+    /**
+     * XXX - Check for all required extensions and return false if unsuccesful
+     */
+    if (init != GLEW_OK) {
+      return false;
+    }
+    sGlewInit = PR_TRUE;
+  }
+
+  glBlendFuncSeparate(GL_ONE, GL_ONE_MINUS_SRC_ALPHA, GL_ONE, GL_ONE);
+  glEnable(GL_BLEND);
+  glEnable(GL_TEXTURE_2D);
+  glEnable(GL_SCISSOR_TEST);
+
+  mRGBLayerProgram = glCreateProgram();
+  mYUVLayerProgram = glCreateProgram();
+  mVertexShader = glCreateShader(GL_VERTEX_SHADER);
+  mRGBShader = glCreateShader(GL_FRAGMENT_SHADER);
+  mYUVShader = glCreateShader(GL_FRAGMENT_SHADER);
+
+  glShaderSource(mVertexShader, 1, (const GLchar**)&sVertexShader, NULL);
+  glShaderSource(mRGBShader, 1, (const GLchar**)&sRGBLayerPS, NULL);
+  glShaderSource(mYUVShader, 1, (const GLchar**)&sYUVLayerPS, NULL);
+
+  glCompileShader(mVertexShader);
+  glCompileShader(mRGBShader);
+  glCompileShader(mYUVShader);
+
+  GLint status;
+  glGetShaderiv(mVertexShader, GL_COMPILE_STATUS, &status);
+
+  if (!status) {
+    return false;
+  }
+
+  glAttachShader(mRGBLayerProgram, mVertexShader);
+  glAttachShader(mYUVLayerProgram, mVertexShader);
+  glAttachShader(mRGBLayerProgram, mRGBShader);
+  glAttachShader(mYUVLayerProgram, mYUVShader);
+
+  glLinkProgram(mRGBLayerProgram);
+  glLinkProgram(mYUVLayerProgram);
+
+  glGetProgramiv(mRGBLayerProgram, GL_LINK_STATUS, &status);

Er, do something with this status :)  The little program/shader wrapper would help here.

+
+  return true;
+}

+void
+LayerManagerOGL::Render()
+{
+  RECT rc;
+  ::GetClientRect((HWND)mWidget->GetNativeData(NS_NATIVE_WINDOW), &rc);
+  GLint width = rc.right - rc.left;
+  GLint height = rc.bottom - rc.top;
+
+  MakeCurrent();
+
+  glDrawBuffer(GL_BACK);
+  glClearColor(0.0, 0.0, 0.0, 0.0);
+  glClear(GL_COLOR_BUFFER_BIT);

Clear depth as well, even though you might not have a depth buffer -- for some hardware, a config with a depth buffer is faster than one without, and not clearing the depth buffer there hurts a ton.

+  glReadBuffer(GL_BACK);
+  glDrawBuffer(GL_FRONT);

Wait, what?

+  if (mTarget) {
+    CopyToTarget();
+  } else {
+    const nsIntRect *r;
+    for (nsIntRegionRectIterator iter(mClippingRegion);
+         (r = iter.Next()) != nsnull;) {
+      glWindowPos2i(r->x, height - (r->y + r->height));
+      glCopyPixels(r->x, height - (r->y + r->height), r->width, r->height, GL_COLOR);
+    }
+  }
+  glFinish();

Hmm, so this is the stuff that needs to be changed based on the irc conversation a few days ago -- don't use BACK/FRONT (in fact, on GL ES, there is no ReadBuffer/DrawBuffer, no WindowPos, no CopyPixels, etc.).  If we can't repaint the entire window each render, then we'll need to use a FBO and keep around our own permanent back buffer that we'll texture from to render to the main window.

+void
+LayerManagerOGL::CopyToTarget()
+{
+  RECT rc;
+  ::GetClientRect((HWND)mWidget->GetNativeData(NS_NATIVE_WINDOW), &rc);
+  GLint width = rc.right - rc.left;
+  GLint height = rc.bottom - rc.top;
+  char *tmpData = new char[width * height * 4];
+
+  glReadPixels(0,
+               0,
+               width,
+               height,
+               GL_BGRA,
+               GL_UNSIGNED_BYTE,
+               tmpData);
+  glFinish();
+
+  nsRefPtr<gfxImageSurface> imageSurface =
+    new gfxImageSurface(gfxIntSize(width, height),
+                        gfxASurface::ImageFormatARGB32);
+
+  for (int y = 0; y < height; y++) {
+    memcpy(imageSurface->Data() + imageSurface->Stride() * (height - y - 1),
+           tmpData + width * 4 * y,
+           width * 4);
+  }
+  delete [] tmpData;
+
+  mTarget->SetOperator(gfxContext::OPERATOR_OVER);
+  mTarget->SetSource(imageSurface);
+  mTarget->Paint();
+}

Hm, I'm missing something.  What does CopyToTarget do?  What's the target?  Is this just a helper function to get the contents, and not something that's used for the actual rendering path?

+LayerOGL*
+LayerOGL::GetNextSibbling()
+{
+  return mNextSibbling;
+}

typo: "Sibling" has one "b"

+#endif /* GFX_LAYERMANAGEROGL_H */
diff --git a/gfx/layers/opengl/LayerManagerOGLShaders.h b/gfx/layers/opengl/LayerManagerOGLShaders.h

Youch.  This is pretty ugly.  We should preprocess this from something sane into a .h.  Can be done later.

new file mode 100644
--- /dev/null
+++ b/gfx/layers/opengl/LayerManagerOGLShaders.h
@@ -0,0 +1,43 @@
+#define SHADER_GLOBAL_VARS "uniform mat4 MatrixProj; \
+uniform mat4 LayerQuadTransform; \
+uniform mat4 LayerTransform; \
+uniform vec4 RenderTargetOffset; \
+uniform float LayerOpacity; \
+uniform float FlipYAxis; \
+\
+uniform sampler2D LayerTexture; \
+uniform sampler2D YTexture; \
+uniform sampler2D UTexture; \
+uniform sampler2D VTexture; \
+varying vec2 textureCoordinate;"

nit: A useful convention is prefixing uniforms with 'u', attributes with 'a', and varying values with 'v' to keep them clear.

+static const GLchar *sVertexShader = SHADER_GLOBAL_VARS "void main() \
+{ \
+    vec4 finalPosition = gl_Vertex; \

This is bad -- don't rely on gl_* anything, except for a few output things; basically gl_Position and gl_FragColor.  GL ES 2 doesn't have them, and neither does the "forward context" of GL 3.x.  Instead, declare all these (= gl_Vertex, gl_MultiTexCoord0, etc. should be aVertex, aTexCoord0, etc.) as attributes and bind the attributes directly instead of using VERTEX_POINTER etc. in the code.

diff --git a/gfx/layers/opengl/ThebesLayerOGL.cpp b/gfx/layers/opengl/ThebesLayerOGL.cpp
new file mode 100644
--- /dev/null
+++ b/gfx/layers/opengl/ThebesLayerOGL.cpp

+void
+ThebesLayerOGL::SetVisibleRegion(const nsIntRegion &aRegion)
+{
+  if (aRegion.GetBounds() == mVisibleRect) {
+    return;
+  }
+  mVisibleRect = aRegion.GetBounds();
+
+  static_cast<LayerManagerOGL*>(mManager)->MakeCurrent();
+
+  if (mTexture) {
+    glDeleteTextures(1, &mTexture);
+  }
+
+  glGenTextures(1, &mTexture);

No need to delete if you're just going to call TexImage2D -- just call Gen if mTexture == 0.

+  GLint format;
+  if (mIsOpaqueContent) {
+    format = GL_RGB;
+  } else {
+    format = GL_RGBA;
+  }

Hm, what is this format used for?  Also, GL_RGB is a 3-byte format; with some pixelstore stuff I believe you can get it to ignore the 4th pixel, but I don't think that pixelstore stuff is supported on GL ES.  You probably want to load all images as RGBA, and then use different fragment shaders for compositing based on whether the alpha is to be ignored or not.

+  mInvalidatedRect = mVisibleRect;
+
+  glBindTexture(GL_TEXTURE_2D, mTexture);
+  glTexImage2D(GL_TEXTURE_2D,
+               0,
+               4,

use correct format again

+               mVisibleRect.width,
+               mVisibleRect.height,
+               0,
+               GL_BGRA,

same thing as before about BGRA

+               GL_UNSIGNED_BYTE,
+               NULL);
+
+  glTexEnvf(GL_TEXTURE_ENV, GL_TEXTURE_ENV_MODE, GL_MODULATE);
+
+  glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
+  glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);

CLAMP_S/CLAMP_T npot etc.
I had some questions! Other than that I've processed all comments.

(In reply to comment #4)
> Note that there is an extension, APPLE_ycbcr_422, that provides for direct YUV
> textures in GL.  However, it wants non-planar layout of the data.  It's present
> primarily on Macs, but that's fine, since that's where we'll be using the GL
> backend the most... it might be worth being able to tell the decoder that we
> want interleaved instead of planar data if we have that extension.

We can add that as an optimization later I think, I'd rather keep this like it is for now. Most of the videos we get are most likely 4:2:0 anyway, and not 4:2:2. This works universally for  4:4:4, 4:2:2 and 4:2:0

> 
> UNPACK_ROW_LENGTH isn't supported on ES 2.0.  Sorry.  (Like, not supported at
> all, don't think any extensions put it back either.)

Yeah, I figured we'd do something else there in the ES version, but I kind of figured we could do this for now? It's not hard to change.

> 
> Sadly, you can't rely on BGRA textures being supported.  On ES 2.0 at least,
> it's an optional extension, and it's not supported on at least the Nexus One
> hardware.  However, you can do the channel swizzle in the fragment shader, so
> I'd suggest just having two shaders -- one for when the extension is present
> and one when it's not.  I *think* there's a perf win in using it if it's
> present, but I'm not sure; if there isn't, then we can just ignore it and
> always do the swizzle.

I'll look into that, is there a way we can share uniforms between programs? Having to keep setting the uniform values for each program will become -really- annoying when we have lots of programs, and may negatively impact performance.
> +#pragma comment(lib, "Opengl32.lib")
> 
> Cheater.  On a related note, when I checked in code that always linked us to
> opengl32.lib on windows for webgl, our startup time went up by like 500ms.  So
> that might be a blocker to getting this checked in, even preffed off -- might
> have to ditch glew and do runtime linking if it's enabled, like what webgl
> does.

I'll write a runtime linking thing if need be once everything works, it should be trivial. Are you okay with keeping the function names the same (perhaps kept inside the mozilla::layers namespace), or would you really like them as class members of like what the glWrap class does?

> Hm, I'm missing something.  What does CopyToTarget do?  What's the target?  Is
> this just a helper function to get the contents, and not something that's used
> for the actual rendering path?

It copies to a target thebes context as per BeginTransactionWithTarget(). It's used only by reftests at the moment I believe.
> 
> Hm, what is this format used for?  Also, GL_RGB is a 3-byte format; with some
> pixelstore stuff I believe you can get it to ignore the 4th pixel, but I don't
> think that pixelstore stuff is supported on GL ES.  You probably want to load
> all images as RGBA, and then use different fragment shaders for compositing
> based on whether the alpha is to be ignored or not.

Ugh! More shader programs, see above comments!
Getting the video codec to output packed YUV is hard. Where the benefit of packed YUV is really compelling, like the N900 texture streaming stuff, we convert from planar to packed in the DSP after the regular codec processing (as I understand it). Also, we will primarily be dealing with 4:2:0. So I don't think the Apple YUV format can help us.
An updated layers backend processing most of Vlad's comments so far. I've included reftest run output at: http://people.mozilla.org/~bschouten/reftest-full.out
Attachment #433687 - Attachment is obsolete: true
Attachment #433780 - Attachment is obsolete: true
(In reply to comment #7)
> Created an attachment (id=434459) [details]
> Experimental preliminary OGL Layers Backend v2
> 
> An updated layers backend processing most of Vlad's comments so far. I've
> included reftest run output at:
> http://people.mozilla.org/~bschouten/reftest-full.out

glsl supports multiplying vectors by constants.
Attached patch OGL Layers Backend v3 (obsolete) — Splinter Review
This is the latest incarnation, most comments have been processed but some small issues remain:

- Still using BGRA, the swizzling Shader can be done with the OGL ES adaption I believe.
- Need to support CopyFrom properly.

There's more work to be done ofcourse, but I believe we can address many things separately going forward. I've noticed the OGL backend cannot be used on transparent windows on windows. I believe this may not even be possible. Ofcourse we can still use it for doing fullscreen video, and we will want to do windows using D3D going forward anyway.
Attachment #434459 - Attachment is obsolete: true
Attachment #435526 - Flags: review?(vladimir)
(In reply to comment #9)
> - Need to support CopyFrom properly.

No you don't, that's going away.
Blocks: 555839
Comment on attachment 435526 [details] [diff] [review]
OGL Layers Backend v3

Looks fine, a few quick things to fix:

+void
+LayerManagerOGL::Render()
..
+  sglWrapper.Clear(GL_COLOR_BUFFER_BIT);
+  sglWrapper.Clear(GL_DEPTH_BUFFER_BIT);

Clear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT);


+void
+LayerManagerOGL::CopyToTarget()
+{
+  RECT rc;
+  ::GetClientRect((HWND)mWidget->GetNativeData(NS_NATIVE_WINDOW), &rc);
+  GLint width = rc.right - rc.left;
+  GLint height = rc.bottom - rc.top;
+  char *tmpData = new char[width * height * 4];

Should probably check for int overflow here.

+  sglWrapper.ReadBuffer(GL_COLOR_ATTACHMENT0_EXT);
+
+  sglWrapper.ReadPixels(0,
+               0,
+               width,
+               height,
+               GL_BGRA,
+               GL_UNSIGNED_BYTE,
+               tmpData);
+  sglWrapper.Finish();
+
+  nsRefPtr<gfxImageSurface> imageSurface =
+    new gfxImageSurface(gfxIntSize(width, height),
+                        gfxASurface::ImageFormatARGB32);
+
+  for (int y = 0; y < height; y++) {
+    memcpy(imageSurface->Data() + imageSurface->Stride() * y,
+           tmpData + width * 4 * y,
+           width * 4);
+  }
+  delete [] tmpData;
+
+  mTarget->SetOperator(gfxContext::OPERATOR_OVER);
+  mTarget->SetSource(imageSurface);
+  mTarget->Paint();
+}

If this is at all performance sensitive, check whether
imageSurface->Stride() == width * 4, and if so, just readPixels
directly into the right buffer?


+#ifndef GFX_LAYERMANAGEROGL_H
+#define GFX_LAYERMANAGEROGL_H
+
+#include "Layers.h"
+
+#include <windows.h>
+typedef unsigned int GLuint;
+typedef int GLint;
+typedef float GLfloat;
+typedef char GLchar;
+
+#define BUFFER_OFFSET(i) ((char *)NULL + (i))

Last two lines have some bogus ^M's 

+/**
+ * Helper class for Layer Programs.
+ */

This could keep track of the previously-set values and avoid calling
Uniform* if they're identical.  File a followup bug to optimize uniform usage?




diff --git a/gfx/layers/opengl/LayerManagerOGLShaders.h b/gfx/layers/opengl/LayerManagerOGLShaders.h
new file mode 100644
--- /dev/null
+++ b/gfx/layers/opengl/LayerManagerOGLShaders.h

File has ^M line endings -- fix/kill please?




diff --git a/gfx/layers/opengl/glWrapper.cpp b/gfx/layers/opengl/glWrapper.cpp
new file mode 100644
--- /dev/null
+++ b/gfx/layers/opengl/glWrapper.cpp
@@ -0,0 +1,205 @@
+/* -*- Mode: C++; tab-width: 20; indent-tabs-mode: nil; c-basic-offset: 4 -*-
+ * ***** BEGIN LICENSE BLOCK *****
+ * Version: MPL 1.1/GPL 2.0/LGPL 2.1
+ *
+ * The contents of this file are subject to the Mozilla Public License Version
+ * 1.1 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ * http://www.mozilla.org/MPL/
+ *
+ * Software distributed under the License is distributed on an "AS IS" basis,
+ * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
+ * for the specific language governing rights and limitations under the
+ * License.
+ *
+ * The Original Code is Mozilla Corporation code.
+ *
+ * The Initial Developer of the Original Code is Mozilla Foundation.
+ * Portions created by the Initial Developer are Copyright (C) 2009
+ * the Initial Developer. All Rights Reserved.
+ *
+ * Contributor(s):
+ *   Bas Schouten <bschouten@mozilla.org>
+ *
+ * Alternatively, the contents of this file may be used under the terms of
+ * either the GNU General Public License Version 2 or later (the "GPL"), or
+ * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
+ * in which case the provisions of the GPL or the LGPL are applicable instead
+ * of those above. If you wish to allow use of your version of this file only
+ * under the terms of either the GPL or the LGPL, and not to allow others to
+ * use your version of this file under the terms of the MPL, indicate your
+ * decision by deleting the provisions above and replace them with the notice
+ * and other provisions required by the GPL or the LGPL. If you do not delete
+ * the provisions above, a recipient may use your version of this file under
+ * the terms of any one of the MPL, the GPL or the LGPL.
+ *
+ * ***** END LICENSE BLOCK ***** */
+
+#include "glWrapper.h"
+#include "nsDebug.h"
+
+#ifdef XP_WIN
+#define glGetProcAddress wGetProcAddress
+#endif
+
+glWrapper sglWrapper;
+
+struct OGLFunction {
+  PRFuncPtr *functor;
+  const char *name;
+};
+
+glWrapper::glWrapper()
+  : mIsInitialized(PR_FALSE)
+  , mOGLLibrary(NULL)
+{
+}
+
+#ifdef XP_WIN
+HGLRC
+glWrapper::wCreateContext(HDC aDC)
+{
+  if (!wCreateContextInternal) {
+    if (!mOGLLibrary) {
+      mOGLLibrary = PR_LoadLibrary("Opengl32.dll");
+      if (!mOGLLibrary) {
+        NS_WARNING("Couldn't load OpenGL DLL.");
+        return NULL;
+      }
+    }
+    
+    wCreateContextInternal = (PFNWGLCREATECONTEXTPROC)
+      PR_FindSymbol(mOGLLibrary, "wglCreateContext");
+    wDeleteContext = (PFNWGLDELETECONTEXTPROC)
+      PR_FindSymbol(mOGLLibrary, "wglDeleteContext");
+    wMakeCurrent = (PFNWGLMAKECURRENTPROC)
+      PR_FindSymbol(mOGLLibrary, "wglMakeCurrent");
+    wGetProcAddress = (PFNWGLGETPROCADDRESSPROC)
+      PR_FindSymbol(mOGLLibrary, "wglGetProcAddress");
+  }
+  HGLRC retval = wCreateContextInternal(aDC);
+
+  if (!retval) {
+    return retval;
+  }
+  wMakeCurrent(aDC, retval);
+
+  if (!EnsureInitialized()) {
+    NS_WARNING("Failed to initialize OpenGL wrapper.");
+    return NULL;
+  }
+
+  return retval;
+}
+#endif
+
+PRBool
+glWrapper::LoadSymbols(OGLFunction *functions)
+{
+  for (int i = 0; functions[i].functor; i++) {
+    *functions[i].functor =
+      PR_FindFunctionSymbol(mOGLLibrary, functions[i].name);
+
+    if (*functions[i].functor) {
+      continue;
+    }
+
+    if (!glGetProcAddress) {
+      return PR_FALSE;
+    }
+    *functions[i].functor = (PRFuncPtr)glGetProcAddress(functions[i].name);
+    if (!*functions[i].functor) {
+      return PR_FALSE;
+    }
+  }
+
+  return PR_TRUE;
+}
+
+PRBool
+glWrapper::EnsureInitialized()
+{
+  if (mIsInitialized) {
+    return PR_TRUE;
+  }
+
+  OGLFunction functions[] = {
+
+    { (PRFuncPtr*) &BlendFuncSeparate, "glBlendFuncSeparate" },
+    
+    { (PRFuncPtr*) &Enable, "glEnable" },
+    { (PRFuncPtr*) &EnableClientState, "glEnableClientState" },
+    { (PRFuncPtr*) &EnableVertexAttribArray, "glEnableVertexAttribArray" },
+    { (PRFuncPtr*) &Disable, "glDisable" },
+    { (PRFuncPtr*) &DisableClientState, "glDisableClientState" },
+    { (PRFuncPtr*) &DisableVertexAttribArray, "glDisableVertexAttribArray" },
+
+    { (PRFuncPtr*) &DrawArrays, "glDrawArrays" },
+
+    { (PRFuncPtr*) &Finish, "glFinish" },
+    { (PRFuncPtr*) &Clear, "glClear" },
+    { (PRFuncPtr*) &Scissor, "glScissor" },
+    { (PRFuncPtr*) &Viewport, "glViewport" },
+    { (PRFuncPtr*) &ClearColor, "glClearColor" },
+    { (PRFuncPtr*) &ReadBuffer, "glReadBuffer" },
+    { (PRFuncPtr*) &ReadPixels, "glReadPixels" },
+
+    { (PRFuncPtr*) &TexEnvf, "glTexEnvf" },
+    { (PRFuncPtr*) &TexParameteri, "glTexParameteri" },
+    { (PRFuncPtr*) &ActiveTexture, "glActiveTexture" },
+    { (PRFuncPtr*) &PixelStorei, "glPixelStorei" },
+
+    { (PRFuncPtr*) &GenTextures, "glGenTextures" },
+    { (PRFuncPtr*) &GenBuffers, "glGenBuffers" },
+    { (PRFuncPtr*) &GenFramebuffersEXT, "glGenFramebuffersEXT" },
+    { (PRFuncPtr*) &DeleteTextures, "glDeleteTextures" },
+    { (PRFuncPtr*) &DeleteFramebuffersEXT, "glDeleteFramebuffersEXT" },
+    
+    { (PRFuncPtr*) &BindTexture, "glBindTexture" },
+    { (PRFuncPtr*) &BindBuffer, "glBindBuffer" },
+    { (PRFuncPtr*) &BindFramebufferEXT, "glBindFramebufferEXT" },
+
+    { (PRFuncPtr*) &FramebufferTexture2DEXT, "glFramebufferTexture2DEXT" }, 
+
+    { (PRFuncPtr*) &BufferData, "glBufferData" },
+
+    { (PRFuncPtr*) &VertexPointer, "glVertexPointer" },
+    { (PRFuncPtr*) &TexCoordPointer, "glTexCoordPointer" },
+
+    { (PRFuncPtr*) &TexImage2D, "glTexImage2D" },
+    { (PRFuncPtr*) &TexSubImage2D, "glTexSubImage2D" },
+
+    { (PRFuncPtr*) &CreateShader, "glCreateShader" },
+    { (PRFuncPtr*) &CreateProgram, "glCreateProgram" },
+    { (PRFuncPtr*) &DeleteProgram, "glDeleteProgram" },
+    { (PRFuncPtr*) &UseProgram, "glUseProgram" },
+    { (PRFuncPtr*) &ShaderSource, "glShaderSource" },
+    { (PRFuncPtr*) &CompileShader, "glCompileShader" },
+    { (PRFuncPtr*) &AttachShader, "glAttachShader" },
+    { (PRFuncPtr*) &LinkProgram, "glLinkProgram" },
+    { (PRFuncPtr*) &GetProgramiv, "glGetProgramiv" },
+    { (PRFuncPtr*) &GetShaderiv, "glGetShaderiv" },
+
+    { (PRFuncPtr*) &BindAttribLocation, "glBindAttribLocation" },
+    { (PRFuncPtr*) &VertexAttribPointer, "glVertexAttribPointer" },
+
+    { (PRFuncPtr*) &Uniform1i, "glUniform1i" },
+    { (PRFuncPtr*) &Uniform1f, "glUniform1f" },
+    { (PRFuncPtr*) &Uniform4f, "glUniform4f" },
+    { (PRFuncPtr*) &UniformMatrix4fv, "glUniformMatrix4fv" },
+    { (PRFuncPtr*) &GetUniformLocation, "glGetUniformLocation" },
+
+    { (PRFuncPtr*) &GetString, "glGetString" },
+
+    { NULL, NULL }


Lots more ^M line endings; fix please?

+#define GL_FRAGMENT_SHADER 0x8B30
+#define GL_VERTEX_SHADER 0x8B31
+#define GL_COMPILE_STATUS 0x8B81
+#define GL_LINK_STATUS 0x8B82
+#define GL_STATIC_DRAW 0x88E4
+#define GL_FRAMEBUFFER_EXT 0x8D40
+#define GL_COLOR_ATTACHMENT0_EXT 0x8CE0
+#define GL_BGRA 0x80E1
+#define GL_CLAMP_TO_EDGE 0x812F
+#define GL_TEXTURE0 0x84C0
+#define GL_TEXTURE1 0x84C1

^M etc.

+
+  typedef GLuint (APIENTRY * PFNGLCREATESHADERPROC) (GLenum);
+  typedef GLuint (APIENTRY * PFNGLCREATEPROGRAMPROC) (void);
+  typedef void (APIENTRY * PFNGLDELETEPROGRAMPROC) (GLuint);
+  typedef void (APIENTRY * PFNGLUSEPROGRAMPROC) (GLuint);
+  typedef void (APIENTRY * PFNGLSHADERSOURCEPROC) (GLuint,
+                                                   GLsizei,
+                                                   const GLchar **,
+                                                   const GLint *);
+  typedef void (APIENTRY * PFNGLCOMPILESHADERPROC) (GLuint);
+  typedef void (APIENTRY * PFNGLATTACHSHADERPROC) (GLuint, GLuint);
+  typedef void (APIENTRY * PFNGLLINKPROGRAMPROC) (GLuint);
+  typedef void (APIENTRY * PFNGLGETPROGRAMIVPROC) (GLuint, GLenum, GLint *);
+  typedef void (APIENTRY * PFNGLGETSHADERIVPROC) (GLuint, GLenum, GLint *);
+
+  typedef void (APIENTRY * PFNGLBINDATTRIBLOCATIONPROC) (GLuint,
+                                                         GLuint,
+                                                         const GLchar *);
+  typedef void (APIENTRY * PFNGLVERTEXATTRIBPOINTERPROC) (GLuint,
+                                                          GLint,
+                                                          GLenum,
+                                                          GLboolean,
+                                                          GLsizei,
+                                                          const GLvoid *);
+
+  typedef void (APIENTRY * PFNGLUNIFORM1IPROC) (GLint, GLint);
+  typedef void (APIENTRY * PFNGLUNIFORM1FPROC) (GLint, GLfloat);
+  typedef void (APIENTRY * PFNGLUNIFORM4FPROC) (GLint,
+                                                GLfloat,
+                                                GLfloat,
+                                                GLfloat,
+                                                GLfloat);
+  typedef void (APIENTRY * PFNGLUNIFORMMATRIX4FVPROC) (GLint,
+                                                       GLsizei,
+                                                       GLboolean,
+                                                       const GLfloat *);
+  typedef GLint (APIENTRY * PFNGLGETUNIFORMLOCATIONPROC) (GLuint,
+                                                          const GLchar *);
+
+  typedef GLubyte* (APIENTRY * PFNGLGETSTRINGPROC) (GLenum);
+
+#ifdef XP_WIN


and more ^M

+
+  PFNGLCREATESHADERPROC CreateShader;
+  PFNGLCREATEPROGRAMPROC CreateProgram;
+  PFNGLDELETEPROGRAMPROC DeleteProgram;
+  PFNGLUSEPROGRAMPROC UseProgram;
+  PFNGLSHADERSOURCEPROC ShaderSource;
+  PFNGLCOMPILESHADERPROC CompileShader;
+  PFNGLATTACHSHADERPROC AttachShader;
+  PFNGLLINKPROGRAMPROC LinkProgram;
+  PFNGLGETPROGRAMIVPROC GetProgramiv;
+  PFNGLGETSHADERIVPROC GetShaderiv;
+
+  PFNGLBINDATTRIBLOCATIONPROC BindAttribLocation;
+  PFNGLVERTEXATTRIBPOINTERPROC VertexAttribPointer;
+
+  PFNGLUNIFORM1IPROC Uniform1i;
+  PFNGLUNIFORM1FPROC Uniform1f;
+  PFNGLUNIFORM4FPROC Uniform4f;
+  PFNGLUNIFORMMATRIX4FVPROC UniformMatrix4fv;
+  PFNGLGETUNIFORMLOCATIONPROC GetUniformLocation;

And more ^M still
Attachment #435526 - Flags: review?(vladimir) → review+
Attached patch OGL Layers Backend v4 (obsolete) — Splinter Review
Updated with the last comments, and moving OGL definitions to local, has been fired at the try servers to verify building on all platforms.
Attachment #435526 - Attachment is obsolete: true
Attachment #435756 - Flags: review?(vladimir)
Attached patch OGL Layers Backend v4 (obsolete) — Splinter Review
This time with the glDefs.h file which was missing.
Attachment #435756 - Attachment is obsolete: true
Attachment #435757 - Flags: review?(vladimir)
Attachment #435756 - Flags: review?(vladimir)
Hopefully this -will- compile on all platforms.
Attachment #435757 - Attachment is obsolete: true
Attachment #435809 - Flags: review?(vladimir)
Attachment #435757 - Flags: review?(vladimir)
Pushed http://hg.mozilla.org/mozilla-central/rev/4981947357e5.
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Depends on: 565326
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: