Closed
Bug 567499
Opened 14 years ago
Closed 14 years ago
webgl gl-object-get-call test fails
Categories
(Core :: Graphics: CanvasWebGL, defect)
Core
Graphics: CanvasWebGL
Tracking
()
RESOLVED
FIXED
People
(Reporter: bjacob, Assigned: bjacob)
Details
Attachments
(3 files, 1 obsolete file)
6.00 KB,
patch
|
vlad
:
review+
|
Details | Diff | Splinter Review |
12.66 KB,
patch
|
vlad
:
review+
|
Details | Diff | Splinter Review |
5.74 KB,
patch
|
vlad
:
review+
|
Details | Diff | Splinter Review |
Speaking about this test: https://cvs.khronos.org/svn/repos/registry/trunk/public/webgl/sdk/tests/fast/gl-object-get-calls.html The attached patch fixes half of the failures in that test, but I am stuch with the GetTexParameter failures: FAIL gl.getTexParameter(gl.TEXTURE_2D, gl.TEXTURE_MAG_FILTER) should be 9728. Threw exception [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsICanvasRenderingContextWebGL.getTexParameter]" nsresult: "0x80004005 (NS_ERROR_FAILURE)" Here's what happens: in WebGLContext::GetTexParameter, at the first line we have NativeJSContext js; Which calls the default constructor, which does: error = nsContentUtils::XPConnect()->GetCurrentNativeCallContext(&ncc); if (NS_FAILED(error)) return; if (!ncc) { error = NS_ERROR_FAILURE; return; } What's happening is that ncc is null, so we return there, with error = NS_ERROR_FAILURE. Any idea what's happening?
Assignee | ||
Comment 1•14 years ago
|
||
Not yet asking for review, let's fix the rest first...
Summary: Getting Khrono's gl-object-get-call test to pass → webgl gl-object-get-call test fails
Assignee | ||
Comment 2•14 years ago
|
||
So, the line NativeJSContext.h:20, error = nsContentUtils::XPConnect()->GetCurrentNativeCallContext(&ncc); if where things go wrong, inside of the GetCurrentNativeCallContext() call. Here's the code of that function: NS_IMETHODIMP nsXPConnect::GetCurrentNativeCallContext( nsAXPCNativeCallContext * *aCurrentNativeCallContext) { NS_ASSERTION(aCurrentNativeCallContext, "bad param"); so far, so good, aCurrentNativeCallContext is non-null. XPCPerThreadData* data = XPCPerThreadData::GetData(nsnull); here, XPCPerThreadData::GetData returns sMainThreadData which is non-null. if(data) { data is non-null, so we enter that branch. *aCurrentNativeCallContext = data->GetCallContext(); here we enter GetCallContext(), which returns mCallContext, which is null! return NS_OK; So we return here, and on the caller side, ncc is null. What is going wrong?
Assignee | ||
Comment 3•14 years ago
|
||
OK, here's a patch ready for review: it makes us pass all of that test, except the following: 1) the GetTexParameter bug discussed above: I'm really clueless about it. 2) the following failure: FAIL gl.getUniform(boolProgram, bvalLoc) should be true (of type boolean). Was 1 (of type number). Should we add full support for boolean uniforms? I have a hard time interpreting the spec: in 5.14.10 there's a table mentioning boolean uniforms, but there is no boolean counterpart to the uniform[if][1234] functions.
Attachment #446860 -
Attachment is obsolete: true
Attachment #448414 -
Flags: review?(vladimir)
Assignee | ||
Comment 4•14 years ago
|
||
Note: patch is meant to be applied on top of the patch from bug 567565.
Attachment #448414 -
Flags: review?(vladimir) → review+
Assignee: nobody → bjacob
Flags: in-testsuite?
Assignee | ||
Updated•14 years ago
|
Keywords: checkin-needed
(In reply to comment #4) > Note: patch is meant to be applied on top of the patch from bug 567565. (You mean bug 565404, I believe :-)
http://hg.mozilla.org/mozilla-central/rev/fd3e5ddb8dda
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Updated•14 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 7•14 years ago
|
||
Wait! As mentioned in comment 3, this is still not fully fixed, there still are a few errors! And as I explained above, I need help with those :) Here is the corresponding text in the output of the test: FAIL gl.getTexParameter(gl.TEXTURE_2D, gl.TEXTURE_MAG_FILTER) should be 9728. Threw exception [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsICanvasRenderingContextWebGL.getTexParameter]" nsresult: "0x80004005 (NS_ERROR_FAILURE)" location: "JS frame :: https://cvs.khronos.org/svn/repos/registry/trunk/public/webgl/sdk/tests/resources/js-test-pre.js :: shouldBe :: line 137" data: no] FAIL gl.getTexParameter(gl.TEXTURE_2D, gl.TEXTURE_MIN_FILTER) should be 9728. Threw exception [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsICanvasRenderingContextWebGL.getTexParameter]" nsresult: "0x80004005 (NS_ERROR_FAILURE)" location: "JS frame :: https://cvs.khronos.org/svn/repos/registry/trunk/public/webgl/sdk/tests/resources/js-test-pre.js :: shouldBe :: line 137" data: no] FAIL gl.getTexParameter(gl.TEXTURE_2D, gl.TEXTURE_WRAP_S) should be 33071. Threw exception [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsICanvasRenderingContextWebGL.getTexParameter]" nsresult: "0x80004005 (NS_ERROR_FAILURE)" location: "JS frame :: https://cvs.khronos.org/svn/repos/registry/trunk/public/webgl/sdk/tests/resources/js-test-pre.js :: shouldBe :: line 137" data: no] FAIL gl.getTexParameter(gl.TEXTURE_2D, gl.TEXTURE_WRAP_T) should be 33071. Threw exception [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsICanvasRenderingContextWebGL.getTexParameter]" nsresult: "0x80004005 (NS_ERROR_FAILURE)" location: "JS frame :: https://cvs.khronos.org/svn/repos/registry/trunk/public/webgl/sdk/tests/resources/js-test-pre.js :: shouldBe :: line 137" data: no] FAIL gl.getUniform(boolProgram, bvalLoc) should be true (of type boolean). Was 1 (of type number).
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
boolean uniforms are set via Uniform1i, but I guess there's a problem there with webgl, since we got rid of gl.TRUE and gl.FALSE -- send mail to the webgl list about it?
Assignee | ||
Comment 9•14 years ago
|
||
The very strange GetTexParameter bug is still present today, and I am still clueless about it, Vlad :)
Huh, no idea for gettexparameter -- check and see if it's even getting called in that case? Is the NativeCallContext creation failing? Also, that function call needs to check 'target' to make sure it's a valid texture target.
Assignee | ||
Comment 11•14 years ago
|
||
Have you looked at my comments above? ;-) yes GetTexParameter is called, the error happens inside of the NativeJSContext default constructor.
Oh! Sorry. So the problem is that this is getting quickstubbed when it shouldn't be -- anything that uses the native xpconnect call context can't be. Add it to the list of functions not quickstubbed in dom_quickstubs.qsconf
from looking at this again -- can you change this to use nsIVariant for the return? That way we can skip the NativeCallContext, it can get quickstubbed, and we can return booleans/floats/whatever is needed easily.
Assignee | ||
Comment 14•14 years ago
|
||
Thanks Vlad for the advice. This patch fixes this issue in all getXxxParameter functions.
Attachment #451688 -
Flags: review?(vladimir)
Assignee | ||
Comment 15•14 years ago
|
||
This fixes the last failure in this test: we now pass it completely! The GLSL program used in this test is declaring boolean uniforms, so we must support that; at the same time, it tests the subtlety that boolean _array_ uniforms must be handled as uint8 arrays, according to the spec.
Attachment #451689 -
Flags: review?(vladimir)
Comment on attachment 451688 [details] [diff] [review] Let getXxxParameter functions return nsIVariant Great! Looks good, but one fix: > NS_IMETHODIMP >-WebGLContext::GetTexParameter(WebGLenum target, WebGLenum pname) >+WebGLContext::GetTexParameter(WebGLenum target, WebGLenum pname, nsIVariant **retval) ^ This still needs to check that |target| is either 2D or CUBE_MAP :)
Attachment #451688 -
Flags: review?(vladimir) → review+
Comment on attachment 451689 [details] [diff] [review] Fix getUniform in the bool case and return nsIVariant Just a few style quirks: > if (unitSize == 1) >- js.SetRetVal(fv[0]); >+ wrval->SetAsFloat(fv[0]); > else >- js.SetRetVal(fv, unitSize); >+ wrval->SetAsArray(nsIDataType::VTYPE_FLOAT, nsnull, >+ unitSize, static_cast<void*>(fv)); Non-trivial (multiline) else statement, can you put braces around this stuff? Also if any part of an if clause has braces, all should (see next comment). > } else if (baseType == LOCAL_GL_INT) { > GLint iv[16]; > gl->fGetUniformiv(progname, location->Location(), iv); > if (unitSize == 1) >- js.SetRetVal(iv[0]); >+ wrval->SetAsInt32(iv[0]); > else >- js.SetRetVal((PRInt32*)iv, unitSize); >- >+ wrval->SetAsArray(nsIDataType::VTYPE_INT32, nsnull, >+ unitSize, static_cast<void*>(iv)); Same thing >+ } else if (baseType == LOCAL_GL_BOOL) { >+ GLint iv[16]; >+ gl->fGetUniformiv(progname, location->Location(), iv); >+ if (unitSize == 1) >+ wrval->SetAsBool(PRBool(iv[0])); >+ else { { } around the if clause as well, since the else has one. >+ PRUint8 uv[16]; >+ for (int k = 0; k < unitSize; k++) >+ uv[k] = PRUint8(iv[k]); >+ wrval->SetAsArray(nsIDataType::VTYPE_UINT8, nsnull, >+ unitSize, static_cast<void*>(uv)); >+ }
Attachment #451689 -
Flags: review?(vladimir) → review+
Assignee | ||
Comment 18•14 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/212b80c33b5b http://hg.mozilla.org/mozilla-central/rev/3fa3d8483e20
Status: REOPENED → RESOLVED
Closed: 14 years ago → 14 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•