Bug 188812 - WebGL 2 conformance: framebuffer-test
Summary: WebGL 2 conformance: framebuffer-test
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Justin Fan
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-08-21 13:55 PDT by Justin Fan
Modified: 2018-08-27 17:38 PDT (History)
14 users (show)

See Also:


Attachments
Patch (21.31 KB, patch)
2018-08-21 14:46 PDT, Justin Fan
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews107 for mac-sierra-wk2 (3.23 MB, application/zip)
2018-08-21 16:52 PDT, EWS Watchlist
no flags Details
Patch (21.27 KB, patch)
2018-08-21 17:04 PDT, Justin Fan
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews122 for ios-simulator-wk2 (2.40 MB, application/zip)
2018-08-21 20:45 PDT, EWS Watchlist
no flags Details
Archive of layout-test-results from ews202 for win-future (12.95 MB, application/zip)
2018-08-22 05:56 PDT, EWS Watchlist
no flags Details
Patch (21.25 KB, patch)
2018-08-22 16:28 PDT, Justin Fan
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews103 for mac-sierra (2.68 MB, application/zip)
2018-08-22 17:27 PDT, EWS Watchlist
no flags Details
Archive of layout-test-results from ews117 for mac-sierra (3.17 MB, application/zip)
2018-08-22 18:00 PDT, EWS Watchlist
no flags Details
Patch (27.87 KB, patch)
2018-08-22 18:05 PDT, Justin Fan
no flags Details | Formatted Diff | Diff
Patch (27.66 KB, patch)
2018-08-23 16:46 PDT, Justin Fan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Justin Fan 2018-08-21 13:55:38 PDT
WebGL 2 conformance: framebuffer-test
Comment 1 Justin Fan 2018-08-21 14:00:56 PDT
<rdar://problem/43452040>
Comment 2 Radar WebKit Bug Importer 2018-08-21 14:02:13 PDT
<rdar://problem/43576071>
Comment 3 Justin Fan 2018-08-21 14:05:23 PDT
<rdar://problem/43452040>
Comment 4 Justin Fan 2018-08-21 14:46:02 PDT
Created attachment 347703 [details]
Patch
Comment 5 EWS Watchlist 2018-08-21 15:50:17 PDT
Attachment 347703 [details] did not pass style-queue:


ERROR: Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp:1272:  Multi line control clauses should use braces.  [whitespace/braces] [4]
Total errors found: 1 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 6 Justin Fan 2018-08-21 16:48:13 PDT
iOS isn't 100% there yet, but having trouble building it locally.

Will fix WebGL 1 versions of framebuffer-test that are failing on mac-wk2.
Comment 7 EWS Watchlist 2018-08-21 16:52:35 PDT
Comment on attachment 347703 [details]
Patch

Attachment 347703 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/8936441

New failing tests:
webgl/1.0.2/conformance/renderbuffers/framebuffer-test.html
fast/canvas/webgl/framebuffer-test.html
Comment 8 EWS Watchlist 2018-08-21 16:52:37 PDT
Created attachment 347727 [details]
Archive of layout-test-results from ews107 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6
Comment 9 Justin Fan 2018-08-21 17:04:21 PDT
Created attachment 347734 [details]
Patch
Comment 10 EWS Watchlist 2018-08-21 17:07:17 PDT
Attachment 347734 [details] did not pass style-queue:


ERROR: Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp:1272:  Multi line control clauses should use braces.  [whitespace/braces] [4]
Total errors found: 1 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 11 EWS Watchlist 2018-08-21 20:45:06 PDT
Comment on attachment 347734 [details]
Patch

Attachment 347734 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/8939314

New failing tests:
webgl/2.0.0/conformance2/renderbuffers/framebuffer-test.html
Comment 12 EWS Watchlist 2018-08-21 20:45:08 PDT
Created attachment 347761 [details]
Archive of layout-test-results from ews122 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews122  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.4
Comment 13 EWS Watchlist 2018-08-22 05:56:39 PDT
Comment on attachment 347734 [details]
Patch

Attachment 347734 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/8942928

New failing tests:
http/tests/security/canvas-remote-read-remote-video-redirect.html
http/tests/security/canvas-remote-read-remote-video-localhost.html
Comment 14 EWS Watchlist 2018-08-22 05:56:51 PDT
Created attachment 347784 [details]
Archive of layout-test-results from ews202 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews202  Port: win-future  Platform: CYGWIN_NT-6.1-2.10.0-0.325-5-3-x86_64-64bit
Comment 15 Justin Fan 2018-08-22 16:28:30 PDT
Created attachment 347871 [details]
Patch
Comment 16 EWS Watchlist 2018-08-22 16:31:02 PDT
Attachment 347871 [details] did not pass style-queue:


ERROR: Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp:1272:  Multi line control clauses should use braces.  [whitespace/braces] [4]
Total errors found: 1 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 17 EWS Watchlist 2018-08-22 17:27:17 PDT
Comment on attachment 347871 [details]
Patch

Attachment 347871 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/8951486

New failing tests:
webgl/2.0.0/conformance2/renderbuffers/framebuffer-test.html
Comment 18 EWS Watchlist 2018-08-22 17:27:18 PDT
Created attachment 347882 [details]
Archive of layout-test-results from ews103 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 19 EWS Watchlist 2018-08-22 18:00:14 PDT
Comment on attachment 347871 [details]
Patch

Attachment 347871 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/8951520

New failing tests:
webgl/2.0.0/conformance2/renderbuffers/framebuffer-test.html
Comment 20 EWS Watchlist 2018-08-22 18:00:16 PDT
Created attachment 347887 [details]
Archive of layout-test-results from ews117 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 21 Justin Fan 2018-08-22 18:05:54 PDT
Created attachment 347889 [details]
Patch
Comment 22 Justin Fan 2018-08-22 18:06:44 PDT
Oops, expected test output for the newly enabled test was not checked in. Added now.
Comment 23 Myles C. Maxfield 2018-08-23 16:09:50 PDT
Comment on attachment 347889 [details]
Patch

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

> Source/WebCore/ChangeLog:9
> +        chance to fix memory leak in PlatformScreenMac/gpuIDForDisplayMask().

Do any of our memory tests/bots have webgl content?

> Source/WebCore/html/canvas/WebGL2RenderingContext.cpp:1189
> +    case GraphicsContext3D::DEPTH:
> +    case GraphicsContext3D::STENCIL:

.... but we don't need to simulate these?

> Source/WebCore/html/canvas/WebGL2RenderingContext.cpp:1221
> -        // OpenGL ES 2.0 specifies INVALID_ENUM in this case, while desktop GL specifies INVALID_OPERATION.
> -        synthesizeGLError(GraphicsContext3D::INVALID_ENUM, "getFramebufferAttachmentParameter", "invalid parameter name");
> +        synthesizeGLError(GraphicsContext3D::INVALID_OPERATION, functionName, "invalid parameter name");

Why?

> Source/WebCore/html/canvas/WebGL2RenderingContext.cpp:-1207
> -        case GraphicsContext3D::FRAMEBUFFER_ATTACHMENT_TEXTURE_LEVEL:
> -        case GraphicsContext3D::FRAMEBUFFER_ATTACHMENT_TEXTURE_CUBE_MAP_FACE:

Why?

> Source/WebCore/html/canvas/WebGL2RenderingContext.cpp:1828
>      case GraphicsContext3D::TEXTURE_BINDING_2D_ARRAY:
>      case GraphicsContext3D::TEXTURE_BINDING_3D:
>      case GraphicsContext3D::READ_FRAMEBUFFER_BINDING:

Did you intend for all of these to fall through?
Comment 24 Justin Fan 2018-08-23 16:45:41 PDT
Comment on attachment 347889 [details]
Patch

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

>> Source/WebCore/html/canvas/WebGL2RenderingContext.cpp:1189
>> +    case GraphicsContext3D::STENCIL:
> 
> .... but we don't need to simulate these?

Nope, because they are valid enums in desktop GL and ES

>> Source/WebCore/html/canvas/WebGL2RenderingContext.cpp:1221
>> +        synthesizeGLError(GraphicsContext3D::INVALID_OPERATION, functionName, "invalid parameter name");
> 
> Why?

OpenGL ES 3 specification matches desktop GL in this case.

>> Source/WebCore/html/canvas/WebGL2RenderingContext.cpp:-1207
>> -        case GraphicsContext3D::FRAMEBUFFER_ATTACHMENT_TEXTURE_CUBE_MAP_FACE:
> 
> Why?

Oops, these should not have been moved out of the isTexture case. Fixed.

>> Source/WebCore/html/canvas/WebGL2RenderingContext.cpp:1828
>>      case GraphicsContext3D::READ_FRAMEBUFFER_BINDING:
> 
> Did you intend for all of these to fall through?

I did naht!
Comment 25 Justin Fan 2018-08-23 16:46:48 PDT
Created attachment 347971 [details]
Patch
Comment 26 Jon Lee 2018-08-23 17:13:24 PDT
Comment on attachment 347971 [details]
Patch

Can we confirm manually that this does not regress WebGL1?
Comment 27 Jon Lee 2018-08-23 17:15:25 PDT
(In reply to Jon Lee from comment #26)
> Comment on attachment 347971 [details]
> Patch
> 
> Can we confirm manually that this does not regress WebGL1?

Also, please see my comment in 188825.
Comment 28 Justin Fan 2018-08-24 12:29:07 PDT
(In reply to Jon Lee from comment #27)
> (In reply to Jon Lee from comment #26)
> > Comment on attachment 347971 [details]
> > Patch
> > 
> > Can we confirm manually that this does not regress WebGL1?
> 
> Also, please see my comment in 188825.

It passes the WebGL 1 version of framebuffer-test.html and the other WebGL 1 tests, but I can run the conformance suite in its entirety to sanity check.

Woops, I'd responded in the Radar but not Bugzilla. I'll make the changes you requested.
Comment 29 EWS 2018-08-24 16:36:12 PDT
Comment on attachment 347971 [details]
Patch

Rejecting attachment 347971 [details] from commit-queue.

justin_fan@apple.com does not have committer permissions according to https://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/contributors.json.

- If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags.

- If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/contributors.json by adding yourself to the file (no review needed).  The commit-queue restarts itself every 2 hours.  After restart the commit-queue will correctly respect your committer rights.
Comment 30 WebKit Commit Bot 2018-08-24 17:10:35 PDT
Comment on attachment 347971 [details]
Patch

Rejecting attachment 347971 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-02', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 347971, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Logging in as commit-queue@webkit.org...
Fetching: https://bugs.webkit.org/attachment.cgi?id=347971&action=edit
Fetching: https://bugs.webkit.org/show_bug.cgi?id=188812&ctype=xml&excludefield=attachmentdata
Processing 1 patch from 1 bug.
Updating working directory
Processing patch 347971 from bug 188812.
Fetching: https://bugs.webkit.org/attachment.cgi?id=347971
Failed to run "['git', 'svn', 'dcommit', '--rmdir']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	LayoutTests/ChangeLog
	M	LayoutTests/TestExpectations
	M	LayoutTests/webgl/2.0.0/conformance2/renderbuffers/framebuffer-test-expected.txt
	M	Source/WebCore/ChangeLog
	M	Source/WebCore/html/canvas/WebGL2RenderingContext.cpp
	M	Source/WebCore/html/canvas/WebGL2RenderingContext.h
	M	Source/WebCore/html/canvas/WebGLFramebuffer.cpp
	M	Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp
	M	Source/WebCore/html/canvas/WebGLRenderingContextBase.h

ERROR from SVN:
A repository hook failed: Commit blocked by pre-commit hook (exit code 1) with output:
Commits are currently disabled while we update infrastructure.
W: 8e506d042a2979d8f71b1af22b185b268b1fa391 and refs/remotes/origin/master differ, using rebase:
:040000 040000 32204f33c15fff8d0c795c246f6277246c953411 9631be1100237ff43f4ef7c0005cc5185c77722d M	LayoutTests
:040000 040000 358c3aacb2325223e22bf509fe0206f0e58e55ce 162abbfb909fd78b90c6722de4a4368f8d472b8a M	Source
Current branch master is up to date.
ERROR: Not all changes have been committed into SVN, however the committed
ones (if any) seem to be successfully integrated into the working tree.
Please see the above messages for details.


Failed to run "['git', 'svn', 'dcommit', '--rmdir']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	LayoutTests/ChangeLog
	M	LayoutTests/TestExpectations
	M	LayoutTests/webgl/2.0.0/conformance2/renderbuffers/framebuffer-test-expected.txt
	M	Source/WebCore/ChangeLog
	M	Source/WebCore/html/canvas/WebGL2RenderingContext.cpp
	M	Source/WebCore/html/canvas/WebGL2RenderingContext.h
	M	Source/WebCore/html/canvas/WebGLFramebuffer.cpp
	M	Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp
	M	Source/WebCore/html/canvas/WebGLRenderingContextBase.h

ERROR from SVN:
A repository hook failed: Commit blocked by pre-commit hook (exit code 1) with output:
Commits are currently disabled while we update infrastructure.
W: 8e506d042a2979d8f71b1af22b185b268b1fa391 and refs/remotes/origin/master differ, using rebase:
:040000 040000 32204f33c15fff8d0c795c246f6277246c953411 9631be1100237ff43f4ef7c0005cc5185c77722d M	LayoutTests
:040000 040000 358c3aacb2325223e22bf509fe0206f0e58e55ce 162abbfb909fd78b90c6722de4a4368f8d472b8a M	Source
Current branch master is up to date.
ERROR: Not all changes have been committed into SVN, however the committed
ones (if any) seem to be successfully integrated into the working tree.
Please see the above messages for details.


Failed to run "['git', 'svn', 'dcommit', '--rmdir']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit
Updating OpenSource
Current branch master is up to date.
Total errors found: 0 in 1 files

Full output: https://webkit-queues.webkit.org/results/8977174
Comment 31 EWS 2018-08-27 14:39:35 PDT
Comment on attachment 347971 [details]
Patch

Rejecting attachment 347971 [details] from commit-queue.

justin_fan@apple.com does not have committer permissions according to https://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/contributors.json.

- If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags.

- If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/contributors.json by adding yourself to the file (no review needed).  The commit-queue restarts itself every 2 hours.  After restart the commit-queue will correctly respect your committer rights.
Comment 32 WebKit Commit Bot 2018-08-27 17:37:57 PDT
Comment on attachment 347971 [details]
Patch

Clearing flags on attachment: 347971

Committed r235417: <https://trac.webkit.org/changeset/235417>
Comment 33 WebKit Commit Bot 2018-08-27 17:38:00 PDT
All reviewed patches have been landed.  Closing bug.