RESOLVED FIXED 126448
[WebGL2] Framebuffer and renderbuffer updates 
https://bugs.webkit.org/show_bug.cgi?id=126448
Summary [WebGL2] Framebuffer and renderbuffer updates
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
DetailsFormatted DiffDiff
Patch (177.35 KB, patch)
2020-07-23 17:43 PDT, Kenneth Russell 		no flags
DetailsFormatted DiffDiff
Patch (177.17 KB, patch)
2020-07-23 18:03 PDT, Kenneth Russell 		no flags
DetailsFormatted DiffDiff
Patch (177.12 KB, patch)
2020-07-23 18:11 PDT, Kenneth Russell 		no flags
DetailsFormatted DiffDiff
Patch (177.18 KB, patch)
2020-07-23 18:21 PDT, Kenneth Russell 		no flags
DetailsFormatted DiffDiff
Patch (178.53 KB, patch)
2020-07-23 18:34 PDT, Kenneth Russell 		no flags
DetailsFormatted DiffDiff
Patch (178.65 KB, patch)
2020-07-23 18:43 PDT, Kenneth Russell 		no flags
DetailsFormatted DiffDiff
Patch (178.66 KB, patch)
2020-07-24 09:27 PDT, Kenneth Russell 		no flags
DetailsFormatted DiffDiff
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
Kenneth Russell
Comment 15 2020-07-23 18:34:21 PDT
Created attachment 405113 [details] Patch
Kenneth Russell
Comment 16 2020-07-23 18:43:33 PDT
Created attachment 405116 [details] Patch
Kenneth Russell
Comment 17 2020-07-24 09:27:34 PDT
Created attachment 405155 [details] Patch
Kenneth Russell
Comment 18 2020-07-24 11:27:15 PDT
Finally all green! CQ'ing.
EWS
Comment 19 2020-07-24 11:33:20 PDT
Committed r264845: <https://trac.webkit.org/changeset/264845> All reviewed patches have been landed. Closing bug and clearing flags on attachment 405155 [details].
Radar WebKit Bug Importer
Comment 20 2020-07-24 11:34:23 PDT
<rdar://problem/66065236>
Truitt Savell
Comment 21 2020-07-27 09:50:02 PDT
It looks like the changes in https://trac.webkit.org/changeset/264845/webkit has broken webgl/2.0.0/conformance2/textures/misc/tex-unpack-params.html for all Mac platforms. History: https://results.webkit.org/?suite=layout-tests&test=webgl%2F2.0.0%2Fconformance2%2Ftextures%2Fmisc%2Ftex-unpack-params.html Diff: https://build.webkit.org/results/Apple-Catalina-Debug-WK2-Tests/r264919%20(5729)/webgl/2.0.0/conformance2/textures/misc/tex-unpack-params-diff.txt It looks like a lot of the individual subtests are getting reported in the diff. It looks like this was not caught by EWS because of some preexisting expectation causing EWS to skip running this test. Can this get looked at today?
Kenneth Russell
Comment 22 2020-07-31 17:46:10 PDT
For the record, Bug 214763 was filed about the new failure of webgl/2.0.0/conformance2/textures/misc/tex-unpack-params.html after this change.
Note You need to log in before you can comment on or make changes to this bug.