Bug 74228 - Implement GLES2 CheckFramebufferStatus() behavior
Summary: Implement GLES2 CheckFramebufferStatus() behavior
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Zhenyao Mo
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-12-09 16:59 PST by Zhenyao Mo
Modified: 2011-12-13 12:53 PST (History)
4 users (show)

See Also:


Attachments
Patch (74.17 KB, patch)
2011-12-09 17:30 PST, Zhenyao Mo
no flags Details | Formatted Diff | Diff
Patch (63.76 KB, patch)
2011-12-12 13:32 PST, Zhenyao Mo
no flags Details | Formatted Diff | Diff
Patch (75.05 KB, patch)
2011-12-13 09:49 PST, Zhenyao Mo
kbr: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Zhenyao Mo 2011-12-09 16:59:49 PST
At the moment, we do some partial check, then pass down to the driver's glCheckFramebufferStatus().  However, desktop gl and gles2's behaviors are not the same.  So we should do the software check in WebGLRenderingContext.
Comment 1 Zhenyao Mo 2011-12-09 17:30:58 PST
Created attachment 118671 [details]
Patch
Comment 2 Zhenyao Mo 2011-12-09 17:32:54 PST
This is an effort to make framebuffer-object-attachment.html pass in webkit/chromium.

Please review.
Comment 3 WebKit Review Bot 2011-12-09 20:25:35 PST
Comment on attachment 118671 [details]
Patch

Attachment 118671 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/10833495

New failing tests:
fast/canvas/webgl/framebuffer-object-attachment.html
Comment 4 Kenneth Russell 2011-12-12 12:45:51 PST
Comment on attachment 118671 [details]
Patch

Looks like this test is still failing with these changes. Marking r- until this is cleared up.
Comment 5 Zhenyao Mo 2011-12-12 13:10:48 PST
This failure is due to a bug in mesa library.

I tested chromium with hardware GPU, the test passed fine; then tested with --use-gl=osmesa, the test failed.

I'll add the test to test_expectations.txt.
Comment 6 Zhenyao Mo 2011-12-12 13:32:01 PST
Created attachment 118832 [details]
Patch
Comment 7 Kenneth Russell 2011-12-12 19:06:08 PST
Comment on attachment 118832 [details]
Patch

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

This patch is missing the actual code changes. A couple of minor naming convention issues as well.

> LayoutTests/fast/canvas/webgl/framebuffer-object-attachment.html:28
> +function CheckFramebufferForAllowedStatuses(allowedStatuses)

Naming convention: Check -> check

> LayoutTests/fast/canvas/webgl/framebuffer-object-attachment.html:235
> +function CheckFramebuffer(expected) {

Check -> check
Comment 8 Zhenyao Mo 2011-12-13 09:49:44 PST
Created attachment 119029 [details]
Patch
Comment 9 Zhenyao Mo 2011-12-13 09:51:18 PST
(In reply to comment #7)
> (From update of attachment 118832 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=118832&action=review
> 
> This patch is missing the actual code changes. A couple of minor naming convention issues as well.

Done.  I didn''t realize webkit-patch upload only deals with the current folder and its sub-folders.  I thought where you run it doesn't matter.

> 
> > LayoutTests/fast/canvas/webgl/framebuffer-object-attachment.html:28
> > +function CheckFramebufferForAllowedStatuses(allowedStatuses)
> 
> Naming convention: Check -> check

Done in khronos and synced here.

> 
> > LayoutTests/fast/canvas/webgl/framebuffer-object-attachment.html:235
> > +function CheckFramebuffer(expected) {
> 
> Check -> check

Same.
Comment 10 Kenneth Russell 2011-12-13 10:36:54 PST
Comment on attachment 119029 [details]
Patch

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

Looks good overall. Couple of minor issues with the ChangeLog that can be fixed upon landing. Please make sure it passes the EWS bots before committing.

> Source/WebCore/ChangeLog:6
> +        * html/canvas/WebGLFramebuffer.cpp:

Missing the "Reviewed by" line.

> LayoutTests/ChangeLog:4
> +        https://bugs.webkit.org/show_bug.cgi?id=74228

Missing "Reviewed by" line.
Comment 11 Zhenyao Mo 2011-12-13 12:53:43 PST
Committed r102697: <http://trac.webkit.org/changeset/102697>