697757 - context-attributes-alpha-depth-stencil-antialias.html test failures

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Description

[Doug Sherk (:drs) (inactive)]

https://cvs.khronos.org/svn/repos/registry/trunk/public/webgl/sdk/tests/conformance/context/context-attributes-alpha-depth-stencil-antialias.html

This page fails because we're not giving the user back the info they inputted and instead getting an internal format.

Comment 1

[Doug Sherk (:drs) (inactive)]

Attachment: Patch v1.0, fixed stencil alpha test failures.

Proposed patch. Ran mochitests locally on OSX 10.7.

Comment 2

[Doug Sherk (:drs) (inactive)]

So this patch is wrong and we're going to hold off on fixing it for now. jgilbert tells me that it's probably due to this block of code in gfx/thebes/GLContext.cpp:

1104    // Allocate texture
1105     if (aUseReadFBO) {
1106         fBindTexture(LOCAL_GL_TEXTURE_2D, newOffscreenTexture);
1107         fTexParameteri(LOCAL_GL_TEXTURE_2D, LOCAL_GL_TEXTURE_MIN_FILTER, LOCAL_GL_LINEAR);
1108         fTexParameteri(LOCAL_GL_TEXTURE_2D, LOCAL_GL_TEXTURE_MAG_FILTER, LOCAL_GL_LINEAR);
1109 
1110         if (alpha) {
1111             fTexImage2D(LOCAL_GL_TEXTURE_2D,
1112                         0,
1113                         LOCAL_GL_RGBA,
1114                         aSize.width, aSize.height,
1115                         0,
1116                         LOCAL_GL_RGBA,
1117                         LOCAL_GL_UNSIGNED_BYTE,
1118                         NULL);
1119 
1120             cf.red = cf.green = cf.blue = cf.alpha = 8;
1121         } else {
1122             fTexImage2D(LOCAL_GL_TEXTURE_2D,
1123                         0,
1124                         LOCAL_GL_RGB,
1125                         aSize.width, aSize.height,
1126                         0,
1127                         LOCAL_GL_RGB,
1128 #ifdef XP_WIN
1129                         LOCAL_GL_UNSIGNED_BYTE,
1130 #else
1131                         mIsGLES2 ? LOCAL_GL_UNSIGNED_SHORT_5_6_5
1132                                  : LOCAL_GL_UNSIGNED_BYTE,
1133 #endif
1134                         NULL);
1135 
1136 #ifdef XP_WIN
1137             cf.red = cf.green = cf.blue = 8;
1138 #else
1139             cf.red = 5;
1140             cf.green = 6;
1141             cf.blue = 5;
1142 #endif
1143             cf.alpha = 0;
1144         }
1145     }

Comment 3

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 570035 [details] [diff] [review]
Patch v1.0, fixed stencil alpha test failures.

After offline discussion, the proper approach seems to be that we need to check whether our requests for the presence/absence of certain buffers is respected during offscreen context creation.

Comment 4

[Doug Sherk (:drs) (inactive)]

Attachment: Patch v1.0, fix for alpha being improperly set.

If we request alpha and it isn't enabled, or we request it off and it gets enabled, we now fail the pbuffer creation and fall back to FBOs. This is bad for performance but allows us to pass the conformance test in this case. Ran mochitests locally.

Comment 5

[Doug Sherk (:drs) (inactive)]

Attachment: Patch v1.0, fix for alpha being improperly set.

Whoops, forgot to remove this test from failing_tests_mac.txt.

Comment 6

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 570051 [details] [diff] [review]
Patch v1.0, fix for alpha being improperly set.

Review of attachment 570051 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good.

I'll just add that since we are able to fail out of PBuffer creation before we allocate the PBuffer, it should have a negligible effect on context start-up performance. Also it lets us provide a proper context for a given request.

Comment 7

[Doug Sherk (:drs) (inactive)]

Running on Try: https://tbpl.mozilla.org/?tree=Try&rev=20a50709f71c

Comment 8

[Doug Sherk (:drs) (inactive)]

Attachment: Patch v1.1, fixed stencil alpha test failures.

Didn't notice at first, but this also fixes tex-input-validation.html. I removed this from failing_tests_mac.txt.

+r carried

Comment 9

[Kelsey Gilbert [:jgilbert]]

More recent try: https://tbpl.mozilla.org/?tree=Try&rev=69bb43191112

Comment 10

[Kelsey Gilbert [:jgilbert]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4f67677970f2

Comment 11

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

https://hg.mozilla.org/mozilla-central/rev/4f67677970f2