The default bug view has changed. See this FAQ.

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

RESOLVED FIXED in mozilla10

Status

()

Core
Canvas: WebGL
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: drs, Assigned: drs)

Tracking

Trunk
mozilla10
All
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

6 years ago
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.
(Assignee)

Comment 1

6 years ago
Created attachment 570035 [details] [diff] [review]
Patch v1.0, fixed stencil alpha test failures.

Proposed patch. Ran mochitests locally on OSX 10.7.
Assignee: nobody → dsherk
Attachment #570035 - Flags: review?(jgilbert)
(Assignee)

Comment 2

6 years ago
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 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.
Attachment #570035 - Flags: review?(jgilbert) → review-
(Assignee)

Comment 4

6 years ago
Created attachment 570050 [details] [diff] [review]
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.
Attachment #570035 - Attachment is obsolete: true
Attachment #570050 - Flags: review?(jgilbert)
(Assignee)

Comment 5

6 years ago
Created attachment 570051 [details] [diff] [review]
Patch v1.0, fix for alpha being improperly set.

Whoops, forgot to remove this test from failing_tests_mac.txt.
Attachment #570050 - Attachment is obsolete: true
Attachment #570050 - Flags: review?(jgilbert)
Attachment #570051 - Flags: review?(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.
Attachment #570051 - Flags: review?(jgilbert) → review+
(Assignee)

Comment 7

6 years ago
Running on Try: https://tbpl.mozilla.org/?tree=Try&rev=20a50709f71c
(Assignee)

Comment 8

6 years ago
Created attachment 570382 [details] [diff] [review]
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
Attachment #570051 - Attachment is obsolete: true
Attachment #570382 - Flags: review+
More recent try: https://tbpl.mozilla.org/?tree=Try&rev=69bb43191112
Status: NEW → ASSIGNED
https://hg.mozilla.org/integration/mozilla-inbound/rev/4f67677970f2
Target Milestone: --- → mozilla10
https://hg.mozilla.org/mozilla-central/rev/4f67677970f2
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.