Bug 74228 – Implement GLES2 CheckFramebufferStatus() behavior

• Status: RESOLVED
• Resolution: FIXED
• Component: WebGL

[Zhenyao Mo]

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.

[Zhenyao Mo]

Created attachment 118671 [details]
Patch

[Zhenyao Mo]

This is an effort to make framebuffer-object-attachment.html pass in webkit/chromium.

Please review.

[WebKit Review Bot]

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

[Kenneth Russell]

Comment on attachment 118671 [details]
Patch

Looks like this test is still failing with these changes. Marking r- until this is cleared up.

[Zhenyao Mo]

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.

[Zhenyao Mo]

Created attachment 118832 [details]
Patch

[Kenneth Russell]

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

[Zhenyao Mo]

Created attachment 119029 [details]
Patch

[Zhenyao Mo]

(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.

[Kenneth Russell]

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.

[Zhenyao Mo]

Committed r102697: <http://trac.webkit.org/changeset/102697>