Bug 51668

Summary:

cleanupAfterGraphicsCall is never called in WebGLRenderingContext::checkFramebufferStatus

Product:

WebKit

Reporter:

Takashi Toyoshima <toyoshim>

Component:

WebGL

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Trivial

CC:

cmarrin, eric, kbr, simon.fraser, webkit.review.bot, zmo

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
a simple patch which doesn't change the program meaning	none
revised patch (previous one contained TAB character in ChangeLog and it broke style check)	none
Patch	none

Takashi Toyoshima

Reported 2010-12-27 22:43:05 PST

cleanupAfterGraphicsCall() is called at the end of functions for marking graphic context as dirty. In WebGLRenderingContext::checkFramebufferStatus(), cleanupAfterGraphicsCall() is called after return. It mean this cleanAfterGraphicsCall() is never called. Actually, argument of this function is 'false', and it mean this function do nothing. But I think this is a trivial oversight, and it cause compile error with some gcc option (for example, -finstrument-functions). This problem appears in a debug configuration of chromium. (http://codereview.chromium.org/5991005/)

Attachments
a simple patch which doesn't change the program meaning (1.35 KB, patch) 2010-12-27 22:55 PST, Takashi Toyoshima	no flags	Details Formatted Diff Diff
revised patch (previous one contained TAB character in ChangeLog and it broke style check) (1.36 KB, patch) 2010-12-27 23:03 PST, Takashi Toyoshima	no flags	Details Formatted Diff Diff
Patch (1.39 KB, patch) 2010-12-28 21:02 PST, Takashi Toyoshima	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Takashi Toyoshima

Comment 1 2010-12-27 22:55:49 PST

Created attachment 77538 [details] a simple patch which doesn't change the program meaning

WebKit Review Bot

Comment 2 2010-12-27 22:56:42 PST

Attachment 77538 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'WebCore/ChangeLog', u'WebCore/html/canvas/WebGLRenderingContext.cpp']" exit_code: 1 WebCore/ChangeLog:6: Line contains tab character. [whitespace/tab] [5] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.

Takashi Toyoshima

Comment 3 2010-12-27 23:03:11 PST

Created attachment 77539 [details] revised patch (previous one contained TAB character in ChangeLog and it broke style check)

Eric Seidel (no email)

Comment 4 2010-12-28 13:52:38 PST

Are you sure this is the right call order?

Zhenyao Mo

Comment 5 2010-12-28 13:57:54 PST

Comment on attachment 77539 [details] revised patch (previous one contained TAB character in ChangeLog and it broke style check) View in context: https://bugs.webkit.org/attachment.cgi?id=77539&action=review > WebCore/html/canvas/WebGLRenderingContext.cpp:565 > } You should call m_context->checkFramebufferStatus(target), cache the returned value, cleanupAfterGraphicsCall, then return the cached value.

Takashi Toyoshima

Comment 6 2010-12-28 20:46:55 PST

Thank you for reviewing the code. I'll post revised one.

Takashi Toyoshima

Comment 7 2010-12-28 21:02:16 PST

Created attachment 77600 [details] Patch

Kenneth Russell

Comment 8 2010-12-29 18:25:41 PST

Comment on attachment 77600 [details] Patch Looks fine per Mo's earlier review.

Zhenyao Mo

Comment 9 2011-01-07 09:51:49 PST

Comment on attachment 77600 [details] Patch Clearing flags on attachment: 77600 Committed r75251: <http://trac.webkit.org/changeset/75251>

Zhenyao Mo

Comment 10 2011-01-07 09:51:54 PST

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.