This test now fail : https://www.khronos.org/registry/webgl/conformance-suites/1.0.2/conformance/textures/copy-tex-image-2d-formats.html

Working revision : 138979:aeb5c295ead1 Failing revision : 138984:da575285ae04 History between those two revisions (hg log -r aeb5c295ead1::da575285ae04) : changeset: 138979:aeb5c295ead1 user: Matt Woodrow <mwoodrow@mozilla.com> date: Wed Jul 17 23:24:14 2013 -0400 summary: Bug 875232 - Make alpha channel optional for MacIOSurface. r=BenWa changeset: 138980:0db7008c7f9f user: Matt Woodrow <mwoodrow@mozilla.com> date: Wed Jul 17 23:24:15 2013 -0400 summary: Bug 875232 - Make most of the GLContext helper functions take a texture target parameter so that we can support GL_TEXTURE_RECTANGLE. r=jgilbert changeset: 138981:925fe6245160 user: Matt Woodrow <mwoodrow@mozilla.com> date: Wed Jul 17 23:24:15 2013 -0400 summary: Bug 875232 - Add SharedSurface_IOSurface for sharing textures on OSX. r=jgilbert changeset: 138982:6b8139c0dc74 user: Matt Woodrow <mwoodrow@mozilla.com> date: Wed Jul 17 23:24:15 2013 -0400 summary: Bug 875232 - Workaround glReadPixels being broken with framebuffers backed by an IOSurface by copying to a temporary texture first. r=jgilbert changeset: 138983:62d61e36d610 user: Matt Woodrow <mwoodrow@mozilla.com> date: Wed Jul 17 23:24:15 2013 -0400 summary: Bug 875232 - Add a BGRX shader program that reads from GL_TEXTURE_RECTANGLE. r=jrmuziel changeset: 138984:da575285ae04 user: Matt Woodrow <mwoodrow@mozilla.com> date: Wed Jul 17 23:24:15 2013 -0400 summary: Bug 888048 - Use CreateThebesSurfaceAliasForDrawTarget_hack to avoid having multiple cairo_surface_quartz objects for a single CGContext. r=nrc

Bug is still present in webgl 1.0.3 conformance test: http://www.khronos.org/registry/webgl/sdk/tests/conformance/textures/copy-tex-image-2d-formats.html

I have tracked the problem down to: http://dxr.mozilla.org/mozilla-central/source/gfx/2d/MacIOSurface.cpp#370 For reasons unknown to me when this is used as the data for the texture, calling glCopyTexImage2D with GL_ALPHA does not work. All other formats work fine (minus GL_LUMINANCE), and GL_RGBA even returns the alpha component. Replacing this with code like: > gl->fTexImage2D(LOCAL_GL_TEXTURE_RECTANGLE_ARB, 0, > ioSurf->HasAlpha() ? > LOCAL_GL_RGBA : > LOCAL_GL_RGB, > ioSurf->GetDevicePixelWidth(), > ioSurf->GetDevicePixelHeight(), > 0, > LOCAL_GL_BGRA, > LOCAL_GL_UNSIGNED_BYTE, > nullptr); works, but obviously causes other problems because mSurfaceIOPtr is not set. I did try a few of the suggestions from: http://uri-labs.com/macosx_headers/CGLIOSurface_h/index.html (couldn't find an official Apple document on it) and they didn't do anything.

This test is still failing with the D3D9, D3D11, and native OGL backends on Firefox Nightly38.0a1 (2015-01-30). However it passes on both Chrome 42.0.2291.1 canary and IE 11. Unfortunately this regression has already made it into the stable release. Should we consider reverting Matt Woodrow's commit?

Can a Mozillian please update the platform to include Windows? Verified on Windows 7.

Also the webgl-conformance flag? Thanks!

Errors in the web console: "FAIL at (0, 0) expected: 0,0,0,127 was 0,0,0,0" js-test-pre.js:112 "FAIL at (0, 0) expected: 64,64,64,255 was 0,0,0,255" js-test-pre.js:96 "FAIL at (0, 0) expected: 64,64,64,127 was 0,0,0,0" js-test-pre.js:96 Error: WebGL: copyTexImage2D: format ALPHA is not a subset of the current framebuffer format, which is [Unknown enum name]. copy-tex-image-2d-formats.html:161 Error: WebGL: copyTexImage2D: format LUMINANCE_ALPHA is not a subset of the current framebuffer format, which is [Unknown enum name]. copy-tex-image-2d-formats.html:161 Error: WebGL: copyTexImage2D: format RGBA is not a subset of the current framebuffer format, which is [Unknown enum name]. copy-tex-image-2d-formats.html:161 Error: WebGL: copyTexImage2D: format ALPHA is not a subset of the current framebuffer format, which is RGB. copy-tex-image-2d-formats.html:161 Error: WebGL: copyTexImage2D: format LUMINANCE_ALPHA is not a subset of the current framebuffer format, which is RGB. copy-tex-image-2d-formats.html:161 "FAIL at (0, 0) expected: 64,64,64,255 was 0,0,0,255" js-test-pre.js:96 Error: WebGL: copyTexImage2D: format RGBA is not a subset of the current framebuffer format, which is RGB.

This works around the errors with glCopyTexImage2D from framebuffers backed by textures allocated via IOSurface.

Comment on attachment 8563955 [details] [diff] [review] Work around glCopyTexImage2D errors on OSX Review of attachment 8563955 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/gl/GLContext.h @@ -1068,5 @@ > > - BeforeGLReadCall(); > - raw_fCopyTexImage2D(target, level, internalformat, > - x, y, width, height, border); > - AfterGLReadCall(); Where did my Before/AfterReadCalls go?

(In reply to Jeff Gilbert [:jgilbert] from comment #9) > Comment on attachment 8563955 [details] [diff] [review] > Work around glCopyTexImage2D errors on OSX > > Review of attachment 8563955 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: gfx/gl/GLContext.h > @@ -1068,5 @@ > > > > - BeforeGLReadCall(); > > - raw_fCopyTexImage2D(target, level, internalformat, > > - x, y, width, height, border); > > - AfterGLReadCall(); > > Where did my Before/AfterReadCalls go? Cut'n'paste error. I'll add them back.

Put back the missing pre/post copy calls.

I suppose this will be an issue for sub variant too.

remote: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8f6836b9c122

Comment on attachment 8565827 [details] [diff] [review] Work around glCopyTexImage2D errors on framebuffers backed by IOSurface Review of attachment 8565827 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/gl/GLContext.h @@ -1929,5 @@ > mSymbols.fCompileShader(shader); > AFTER_GL_CALL; > } > > -private: We still want this private around. We really should friend the calling class, instead of removing the private marker. People should really not use this by itself.

Apparently, according to the manpage, this should create a "NULL" texture. According to Mesa source code, it doesn't allocate any storage for the image. Maybe this should result in TexImage2D(..., 0, 0, NULL)???!

Comment on attachment 8578425 [details] [diff] [review] Skip processing of CopyTexImage2D with 0 width or height Review of attachment 8578425 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/canvas/WebGLContextGL.cpp @@ +545,5 @@ > + // Bug 896693: https://www.opengl.org/sdk/docs/man3/xhtml/glCopyTexImage2D.xml > + // says that width or height set to 0 results in a NULL texture. Lets not > + // do any work then. > + if (width == 0 || height == 0) > + return; This isn't really correct though. We need to replace the specified texImage with a 0x0 texture, just as if TexImage2D(width=0, height=0) was called on it. (which is valid as well) We should just do the work to make CopyTexSubImage2D_base handle 0 for width and/or height.

> We should just do the work to make CopyTexSubImage2D_base handle 0 for width > and/or height. So we should call TexImage2D() in that case? With the current patch I get errors because we end up with a "NULL" texture which fails the completeness check when attached to an FBO. Maybe I should just return false if width or height is 0 and let raw_fCopyTexImage2D handle it at GLContext::fCopyTexImage2D()?

(In reply to Dan Glastonbury :djg :kamidphish from comment #18) > > We should just do the work to make CopyTexSubImage2D_base handle 0 for width > > and/or height. > > So we should call TexImage2D() in that case? With the current patch I get > errors because we end up with a "NULL" texture which fails the completeness > check when attached to an FBO. As it should. Is the test wrong? > > Maybe I should just return false if width or height is 0 and let > raw_fCopyTexImage2D handle it at GLContext::fCopyTexImage2D()? We *should* be able to pass down a zero width/height call to glCopyTexImage2D. In the absence of driver bugs, let's just pass it through to the driver.