Bug 126448

Summary:

[WebGL2] Framebuffer and renderbuffer updates

Product:

WebKit

Reporter:

Dean Jackson <dino>

Component:

WebGL

Assignee:

Kenneth Russell <kbr>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

cdumez, changseok, esprehn+autocc, ews-watchlist, graouts, gyuyoung.kim, jdarpinian, kbr, kondapallykalyan, noam, tsavell, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

See Also:

https://bugs.webkit.org/show_bug.cgi?id=214763

Bug Depends on:

209098

Bug Blocks:

126404, 214660, 214763

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none

Dean Jackson

Reported 2014-01-03 12:05:32 PST

There are a bunch of new framebuffer methods: void blitFramebuffer(GLint srcX0, GLint srcY0, GLint srcX1, GLint srcY1, GLint dstX0, GLint dstY0, GLint dstX1, GLint dstY1, GLbitfield mask, GLenum filter); void framebufferTextureLayer(GLenum target, GLenum attachment, GLuint texture, GLint level, GLint layer); any getInternalformatParameter(GLenum target, GLenum internalformat, GLenum pname); void invalidateFramebuffer(GLenum target, sequence<GLenum> attachments); void invalidateSubFramebuffer (GLenum target, sequence<GLenum> attachments, GLint x, GLint y, GLsizei width, GLsizei height); void readBuffer(GLenum mode);

Attachments
Patch (177.45 KB, patch) 2020-07-23 17:34 PDT, Kenneth Russell	no flags	Details Formatted Diff Diff
Patch (177.35 KB, patch) 2020-07-23 17:43 PDT, Kenneth Russell	no flags	Details Formatted Diff Diff
Patch (177.17 KB, patch) 2020-07-23 18:03 PDT, Kenneth Russell	no flags	Details Formatted Diff Diff
Patch (177.12 KB, patch) 2020-07-23 18:11 PDT, Kenneth Russell	no flags	Details Formatted Diff Diff
Patch (177.18 KB, patch) 2020-07-23 18:21 PDT, Kenneth Russell	no flags	Details Formatted Diff Diff
Patch (178.53 KB, patch) 2020-07-23 18:34 PDT, Kenneth Russell	no flags	Details Formatted Diff Diff
Patch (178.65 KB, patch) 2020-07-23 18:43 PDT, Kenneth Russell	no flags	Details Formatted Diff Diff
Patch (178.66 KB, patch) 2020-07-24 09:27 PDT, Kenneth Russell	no flags	Details Formatted Diff Diff
Show Obsolete (7) View All Add attachment proposed patch, testcase, etc.

Dean Jackson

Comment 1 2014-01-03 12:05:45 PST

<rdar://problem/15002334>

Kenneth Russell

Comment 2 2020-07-21 17:09:06 PDT

Taking this bug. James implemented at least some of this functionality in Bug 209098.

Kenneth Russell

Comment 3 2020-07-23 08:58:18 PDT

*** Bug 126447 has been marked as a duplicate of this bug. ***

Kenneth Russell

Comment 4 2020-07-23 09:00:14 PDT

Expanding this bug to cover adding the following entrypoints: getInternalformatParameter getRenderbufferParameter renderbufferStorage renderbufferStorageMultisample jdarpinian@ hooked most of these up in Bug 209098, but it's necessary to scan through and make sure all of the necessary validation is in place.

Kenneth Russell

Comment 5 2020-07-23 17:34:58 PDT

Created attachment 405103 [details] Patch

Kenneth Russell

Comment 6 2020-07-23 17:37:11 PDT

The attached patch is large, but the code was ported over from Chromium's WebGL 2.0 implementation, and has been well tested. The new code makes several more WebGL 2.0 conformance tests pass 100%.

Kenneth Russell

Comment 7 2020-07-23 17:43:29 PDT

Created attachment 405104 [details] Patch

Kenneth Russell

Comment 8 2020-07-23 17:43:58 PDT

Removed a parameter that was unused in release builds.

Dean Jackson

Comment 9 2020-07-23 17:52:59 PDT

Comment on attachment 405104 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=405104&action=review > Source/WebCore/html/canvas/WebGL2RenderingContext.cpp:760 > + if (!validateTexFuncLayer("framebufferTextureLayer", texTarget, layer)) > + return; > + if (!validateTexFuncLevel("framebufferTextureLayer", texTarget, level)) > + return; Took me a few reads to work out what was different :) > Source/WebCore/html/canvas/WebGL2RenderingContext.cpp:887 > + Vector<GCGLenum> translatedAttachments = attachments; > + if (!checkAndTranslateAttachments("invalidateFramebuffer", target, translatedAttachments)) Since you're taking a copy anyway, why not have checkAndTranslateAttachments return a translated vector?

Kenneth Russell

Comment 10 2020-07-23 17:56:04 PDT

Comment on attachment 405104 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=405104&action=review Thanks for your super fast review Dean! Some more unused parameters to be fixed up; I'll upload revised patches. >> Source/WebCore/html/canvas/WebGL2RenderingContext.cpp:887 >> + if (!checkAndTranslateAttachments("invalidateFramebuffer", target, translatedAttachments)) > > Since you're taking a copy anyway, why not have checkAndTranslateAttachments return a translated vector? It's not clear to me how to express errors in this scenario. Do you have a suggestion?

Kenneth Russell

Comment 11 2020-07-23 18:03:01 PDT

Created attachment 405108 [details] Patch

Kenneth Russell

Comment 12 2020-07-23 18:11:12 PDT

Created attachment 405110 [details] Patch

Kenneth Russell

Comment 13 2020-07-23 18:11:52 PDT

Marked Dean as the reviewer in the latest patch. I'll CQ if the EWS bots are all green.

Kenneth Russell

Comment 14 2020-07-23 18:21:30 PDT

Created attachment 405112 [details] Patch