Bug 45504 - [chromium] Don't call deleteTexture on a texture attached to an FBO that is not currently bound
Summary: [chromium] Don't call deleteTexture on a texture attached to an FBO that is n...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: James Robinson
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-09-09 17:28 PDT by James Robinson
Modified: 2010-09-09 20:25 PDT (History)
2 users (show)

See Also:


Attachments
Patch (2.60 KB, patch)
2010-09-09 17:32 PDT, James Robinson
no flags Details | Formatted Diff | Diff
Patch (2.70 KB, patch)
2010-09-09 19:08 PDT, James Robinson
kbr: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description James Robinson 2010-09-09 17:28:41 PDT
[chromium] Don't call deleteTexture on a texture attached to an FBO that is not currently bound
Comment 1 James Robinson 2010-09-09 17:32:01 PDT
Created attachment 67123 [details]
Patch
Comment 2 James Robinson 2010-09-09 17:33:12 PDT
Note that in addition to this not quite doing what we want, ANGLE currently crashes if you delete a texture attached to the non-current FBO.  The ANGLE crash will be fixed as well.
Comment 3 Kenneth Russell 2010-09-09 18:15:06 PDT
Comment on attachment 67123 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=67123&action=prettypatch

Basically looks fine. One comment.

> WebCore/platform/graphics/chromium/DrawingBufferChromium.cpp:121
> +    m_context->deleteTexture(m_internal->offscreenColorTexture);
You don't really need to delete the texture and create a new one in this case. All you need is to call texImage2D against the texture object again to reallocate its storage to the new size. You might consider doing that instead.
Comment 4 James Robinson 2010-09-09 19:05:11 PDT
(In reply to comment #3)
> (From update of attachment 67123 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=67123&action=prettypatch
> 
> Basically looks fine. One comment.
> 
> > WebCore/platform/graphics/chromium/DrawingBufferChromium.cpp:121
> > +    m_context->deleteTexture(m_internal->offscreenColorTexture);
> You don't really need to delete the texture and create a new one in this case. All you need is to call texImage2D against the texture object again to reallocate its storage to the new size. You might consider doing that instead.

That's a very good point.  I'll update the patch to do it that way instead.  Do I have to respecify the texParameters()s after doing that?
Comment 5 James Robinson 2010-09-09 19:08:44 PDT
Created attachment 67138 [details]
Patch
Comment 6 James Robinson 2010-09-09 19:10:49 PDT
Patch assumes I don't have to specify the texParameters again (which from basic testing appears to be the case).
Comment 7 Kenneth Russell 2010-09-09 19:42:25 PDT
Comment on attachment 67138 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=67138&action=prettypatch

Looks good to me. To answer your question above, no, you don't need to respecify the texture parameters -- they are associated with the texture object, and you are not deleting that, only reallocating its associated texel storage.
Comment 8 James Robinson 2010-09-09 20:25:36 PDT
Committed r67156: <http://trac.webkit.org/changeset/67156>