Closed Bug 822114 Opened 11 years ago Closed 11 years ago

Firefox WebGL: deleteTexture of a bound (but not active) texture makes the texture become the activeTexture after deletion


(Core :: Graphics: CanvasWebGL, defect)

11 Branch
Not set



Tracking Status
firefox18 --- wontfix
firefox19 --- fixed
firefox20 --- fixed


(Reporter: prouty, Assigned: jgilbert)



(Keywords: regression, testcase)


(2 files)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_8_2) AppleWebKit/537.11 (KHTML, like Gecko) Chrome/23.0.1271.97 Safari/537.11

Steps to reproduce:

var gl = canvas.getContext('moz-webgl');

var texture1 = gl.createTexture();
// Should be active by default, but for completeness:
gl.bindTexture(gl.TEXTURE_2D, texture1);

// Verify TEXTURE0 is indeed the current texture.
assert(gl.getParameter(gl.ACTIVE_TEXTURE) == gl.TEXTURE0);

var texture2 = gl.createTexture();
gl.bindTexture(gl.TEXTURE_2D, texture2);

//Revert to the active texture TEXTURE0 and verify.
assert(gl.getParameter(gl.ACTIVE_TEXTURE) == gl.TEXTURE0);

// Delete a bound texture which is not the currently active texture.

// Expect that the activeTexture is still TEXTURE0. This assertion fails; ACTIVE_TEXTURE becomes TEXTURE15.
assert(gl.getParameter(gl.ACTIVE_TEXTURE) == gl.TEXTURE0);

Actual results:

Create two or more textures. Bind both to different texture units. Delete a bound texture that is not bound to the active texture unit. The texture unit that was bound to that texture will now become the active texture.

In the example above, TEXTURE15 becomes the ACTIVE_TEXTURE immediately after the delete.
Create a new texture, bind the 2dTexture to something other than the
// previously active texture (by temporarily setting the active texture to
// something else and then reverting it back to the original value), and then
// delete the texture.
  // After the deleteTexture call, the activeTexture unexpectedly changes from
  // the active texture prior to the deleteTexture call to the texture index of
  // the bound texture. This is not specified in the WebGL spec and is a firefox
  // specific behavior.

Expected results:

The ACTIVE_TEXTURE should remain the active texture. In the example above, TEXTURE0 should have been the active texture immediately before and after the delete call.

The webgl standard does not specify any special behavior about a deleteTexture changing the active texture unit.

As a comparison, this same code example works correctly on all other browsers that support WebGL (Chrome, Safari, Opera). This behavior is a nuisance for developers and is not always easy to determine the source of error.
OS: Mac OS X → All
Component: Untriaged → Canvas: WebGL
Product: Firefox → Core
Could you attach a minimal testcase (.html), please.
Flags: needinfo?(prouty)
Thanks Loic for the quick reply. Cheers
Attachment #693031 - Attachment mime type: text/plain → text/html
Is it normal that the testcase loads a blank page? (in various versions of Firefox)
Is there something to see?
Flags: needinfo?(prouty)
Yes, that is normal. The bug is about webgl state (and in this case does not require uploading any textures or drawing anything). If you inspect the console log (or step through the javascript, you should see the state inconsistently very clearly.
Flags: needinfo?(prouty)
It looks like there is a regression because in FF10-, I got:

[01:27:03,112] At start, ACTIVE_TEXTURE is 0: true
[01:27:03,115] before deleteTexture, ACTIVE_TEXTURE is 0: true
[01:27:03,117] after deleteTexture, ACTIVE_TEXTURE is 0: true

Regression range:



Suspected bug:
Bug 704839 - Refactor mutual ownership of WebGL objects
Ever confirmed: true
Keywords: regression, testcase
Version: 17 Branch → 11 Branch
That sounds about right. We first noticed the error back in FF12 in April.

The fix is rather simple. Sorry I don't have a FF environment set up to create a patch, but it's a two line fix. If someone could make the patch for me, that would be much appreciated!

1053 WebGLContext::DeleteTexture(WebGLTexture *tex)
1054 {
1055     if (!IsContextStable())
1056         return;
1058     if (!ValidateObjectAllowDeletedOrNull("deleteTexture", tex))
1059         return;
1061     if (!tex || tex->IsDeleted())
1062         return;
1064     if (mBoundFramebuffer)
1065         mBoundFramebuffer->DetachTexture(tex);
++++     WebGLuint activeTexture = mActiveTexture;
1067     for (int32_t i = 0; i < mGLMaxTextureUnits; i++) {
1068         if ((tex->Target() == LOCAL_GL_TEXTURE_2D && mBound2DTextures[i] == tex) ||
1069             (tex->Target() == LOCAL_GL_TEXTURE_CUBE_MAP && mBoundCubeMapTextures[i] == tex))
1070         {
1071             ActiveTexture(LOCAL_GL_TEXTURE0 + i);
1072             BindTexture(tex->Target(), static_cast<WebGLTexture*>(nullptr));
1073         }
1074     }
++++     ActiveTexture(LOCAL_GL_TEXTURE0 + activeTexture);
1077     tex->RequestDelete();
1078 }
Indeed, this is a silly mistake. You'll probably notice that we actually leave the texture unit to the last texture unit which has a valid texture attached to it.

Hah, looks like you posted the same patch I had in mind.
Attached patch patchSplinter Review
Assignee: nobody → jgilbert
Attachment #693199 - Flags: review?(bjacob)
Comment on attachment 693199 [details] [diff] [review]

Great! Thanks for the quick fix. You rock!
Blocks: 704839
Comment on attachment 693199 [details] [diff] [review]

Review of attachment 693199 [details] [diff] [review]:

Two things:

oops, and thanks.
Attachment #693199 - Flags: review?(bjacob) → review+
Which milestone/release will this likely go out in (for Firefox)? Sorry for the noob questions!
It'll hit Aurora 19, and probably Beta 18. Beta depends on release drivers, Aurora is a pretty sure thing, though.

18 will hit Release on (or around) Jan 6. 19 will hit Release in mid February. (Around the 15th, I think)
Indeed, it's WebGLuint, not WebGLUint.
Try builds look like they're coming in fine:
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
Comment on attachment 693199 [details] [diff] [review]

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 704839
User impact if declined: Bad behavior for WebGL when deleting textures. (Fairly common use-case)
Testing completed (on m-c, etc.): On m-c.
Risk to taking this patch (and alternatives if risky): None.
String or UUID changes made by this patch: None.
Attachment #693199 - Flags: approval-mozilla-beta?
Attachment #693199 - Flags: approval-mozilla-b2g18?
Attachment #693199 - Flags: approval-mozilla-aurora?
Comment on attachment 693199 [details] [diff] [review]

Considering where we are close to release for FF18, a- this patch for beta.Low risk enough to approve on aurora 19.
Attachment #693199 - Flags: approval-mozilla-beta?
Attachment #693199 - Flags: approval-mozilla-beta-
Attachment #693199 - Flags: approval-mozilla-b2g18?
Attachment #693199 - Flags: approval-mozilla-b2g18-
Attachment #693199 - Flags: approval-mozilla-aurora?
Attachment #693199 - Flags: approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.