Bug 70077 - Implement WEBGL_debug_shaders and WEBGL_debug_renderer_info extensions
Summary: Implement WEBGL_debug_shaders and WEBGL_debug_renderer_info extensions
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-10-13 17:12 PDT by Zhenyao Mo
Modified: 2011-10-19 17:00 PDT (History)
9 users (show)

See Also:


Attachments
Patch (37.60 KB, patch)
2011-10-14 12:44 PDT, Zhenyao Mo
no flags Details | Formatted Diff | Diff
Patch (37.81 KB, patch)
2011-10-14 13:23 PDT, Zhenyao Mo
no flags Details | Formatted Diff | Diff
Patch (22.36 KB, patch)
2011-10-14 14:30 PDT, Zhenyao Mo
no flags Details | Formatted Diff | Diff
Patch (23.96 KB, patch)
2011-10-14 14:39 PDT, Zhenyao Mo
no flags Details | Formatted Diff | Diff
Patch (24.00 KB, patch)
2011-10-14 14:47 PDT, Zhenyao Mo
no flags Details | Formatted Diff | Diff
Patch (38.17 KB, patch)
2011-10-14 15:24 PDT, Zhenyao Mo
no flags Details | Formatted Diff | Diff
Patch (41.59 KB, patch)
2011-10-14 17:06 PDT, Zhenyao Mo
no flags Details | Formatted Diff | Diff
Patch (58.22 KB, patch)
2011-10-16 13:10 PDT, Zhenyao Mo
no flags Details | Formatted Diff | Diff
Patch (60.98 KB, patch)
2011-10-17 08:11 PDT, Zhenyao Mo
no flags Details | Formatted Diff | Diff
Patch (61.07 KB, patch)
2011-10-17 08:28 PDT, Zhenyao Mo
no flags Details | Formatted Diff | Diff
Patch (46.44 KB, patch)
2011-10-19 15:25 PDT, 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.
Comment 1 Zhenyao Mo 2011-10-14 12:44:12 PDT
Created attachment 111055 [details]
Patch
Comment 2 WebKit Review Bot 2011-10-14 12:48:40 PDT
Please wait for approval from fishd@chromium.org before submitting because this patch contains changes to the Chromium public API.
Comment 3 Zhenyao Mo 2011-10-14 12:49:53 PDT
Whether the current WebGLRenderingContext is for privileged content or not is not implemented in this patch.

The current patch is tested with 

https://cvs.khronos.org/svn/repos/registry/trunk/public/webgl/sdk/tests/conformance/extensions/webgl_debug_renderer_info.html
https://cvs.khronos.org/svn/repos/registry/trunk/public/webgl/sdk/tests/conformance/extensions/webgl_debug_shaders.html

for both WebGLRenderingContext::allowPrivilegedExtensions() returns true and false.
Comment 4 Early Warning System Bot 2011-10-14 13:14:53 PDT
Comment on attachment 111055 [details]
Patch

Attachment 111055 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/10067607
Comment 5 Zhenyao Mo 2011-10-14 13:23:23 PDT
Created attachment 111066 [details]
Patch
Comment 6 Early Warning System Bot 2011-10-14 13:47:44 PDT
Comment on attachment 111066 [details]
Patch

Attachment 111066 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/10052674
Comment 7 Zhenyao Mo 2011-10-14 14:30:34 PDT
Created attachment 111079 [details]
Patch
Comment 8 Zhenyao Mo 2011-10-14 14:39:55 PDT
Created attachment 111080 [details]
Patch
Comment 9 Zhenyao Mo 2011-10-14 14:47:45 PDT
Created attachment 111084 [details]
Patch
Comment 10 Early Warning System Bot 2011-10-14 15:15:33 PDT
Comment on attachment 111084 [details]
Patch

Attachment 111084 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/10052699
Comment 11 Zhenyao Mo 2011-10-14 15:24:02 PDT
Created attachment 111095 [details]
Patch
Comment 12 Early Warning System Bot 2011-10-14 16:04:14 PDT
Comment on attachment 111095 [details]
Patch

Attachment 111095 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/10066667
Comment 13 Zhenyao Mo 2011-10-14 17:06:57 PDT
Created attachment 111111 [details]
Patch
Comment 14 Early Warning System Bot 2011-10-14 17:23:51 PDT
Comment on attachment 111111 [details]
Patch

Attachment 111111 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/10066747
Comment 15 Zhenyao Mo 2011-10-16 13:10:29 PDT
Created attachment 111185 [details]
Patch
Comment 16 Early Warning System Bot 2011-10-16 13:23:35 PDT
Comment on attachment 111185 [details]
Patch

Attachment 111185 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/10080502
Comment 17 Patrick R. Gansterer 2011-10-16 13:48:53 PDT
Please create _on_ clean enty to the changelog and add the new files to CMakeList, GNUMakeFile and vcproj too.
Comment 18 Daniel Bates 2011-10-16 21:06:56 PDT
Comment on attachment 111185 [details]
Patch

r- per comment 17.
Comment 19 Zhenyao Mo 2011-10-17 08:11:12 PDT
Created attachment 111265 [details]
Patch
Comment 20 WebKit Review Bot 2011-10-17 08:12:43 PDT
Attachment 111265 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/update-webkit', '--chromium']" exit_code: 2

Updating OpenSource
Current branch master is up to date.
Updating chromium port dependencies using gclient...
Error: Can't switch the checkout to http://v8.googlecode.com/svn/branches/3.6@9637; UUID don't match and there is local changes in /mnt/git/webkit-style-queue/Source/WebKit/chromium/v8. Delete the directory and try again.
Re-trying 'depot_tools/gclient sync'
Error: Can't switch the checkout to http://v8.googlecode.com/svn/branches/3.6@9637; UUID don't match and there is local changes in /mnt/git/webkit-style-queue/Source/WebKit/chromium/v8. Delete the directory and try again.
Re-trying 'depot_tools/gclient sync'
Error: Can't switch the checkout to http://v8.googlecode.com/svn/branches/3.6@9637; UUID don't match and there is local changes in /mnt/git/webkit-style-queue/Source/WebKit/chromium/v8. Delete the directory and try again.
Error: 'depot_tools/gclient sync' failed 3 tries and returned 256 at Tools/Scripts/update-webkit-chromium line 107.
Re-trying 'depot_tools/gclient sync'
No such file or directory at Tools/Scripts/update-webkit line 104.


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 21 Darin Fisher (:fishd, Google) 2011-10-17 08:17:17 PDT
Comment on attachment 111265 [details]
Patch

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

> Source/WebKit/chromium/public/WebGraphicsContext3D.h:370
> +    virtual WebString getTranslatedShaderSourceANGLE(WebGLId shader) { return WebString(); }

WebKit API change LGTM
Comment 22 Zhenyao Mo 2011-10-17 08:28:29 PDT
Created attachment 111268 [details]
Patch
Comment 23 WebKit Review Bot 2011-10-17 08:31:30 PDT
Attachment 111268 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/update-webkit', '--chromium']" exit_code: 2

Updating OpenSource
From git://git.webkit.org/WebKit
   a891ab4..3e87a7a  master     -> origin/master
	M	Tools/ChangeLog
	M	Tools/Scripts/webkitdirs.pm
r97622 = ed5ce88c4a887a8a1849fa2e1fe5f707e3a7c311 (refs/remotes/trunk)
	M	Tools/ChangeLog
	M	Tools/Scripts/run-qtwebkit-tests
r97623 = 3e87a7a4343696b900ed25b42741b4e88dcaee01 (refs/remotes/trunk)
First, rewinding head to replay your work on top of it...
Fast-forwarded master to refs/remotes/trunk.
Updating chromium port dependencies using gclient...
Error: Can't switch the checkout to http://v8.googlecode.com/svn/branches/3.6@9637; UUID don't match and there is local changes in /mnt/git/webkit-style-queue/Source/WebKit/chromium/v8. Delete the directory and try again.
Re-trying 'depot_tools/gclient sync'
Error: Can't switch the checkout to http://v8.googlecode.com/svn/branches/3.6@9637; UUID don't match and there is local changes in /mnt/git/webkit-style-queue/Source/WebKit/chromium/v8. Delete the directory and try again.
Re-trying 'depot_tools/gclient sync'
Error: Can't switch the checkout to http://v8.googlecode.com/svn/branches/3.6@9637; UUID don't match and there is local changes in /mnt/git/webkit-style-queue/Source/WebKit/chromium/v8. Delete the directory and try again.
Error: 'depot_tools/gclient sync' failed 3 tries and returned 256 at Tools/Scripts/update-webkit-chromium line 107.
Re-trying 'depot_tools/gclient sync'
No such file or directory at Tools/Scripts/update-webkit line 104.


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 24 Zhenyao Mo 2011-10-18 09:32:28 PDT
QT bot is finally green.  Please review.
Comment 25 Kenneth Russell 2011-10-19 14:43:38 PDT
Comment on attachment 111268 [details]
Patch

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

The code looks good overall. Couple of issues that need to be cleaned up.

> Source/WebCore/ChangeLog:49
> +2011-10-14  Mark Hahnenberg  <mhahnenberg@apple.com>

There's a merge problem in the ChangeLog. Please unapply, sync and reapply your changes.

> Source/WebCore/html/canvas/WebGLRenderingContext.cpp:2017
> +    }

This block is duplicated from above and needs to be removed.

> Source/WebCore/html/canvas/WebGLRenderingContext.h:630
> +    // Determine if we are running the privileged code in the browser.

"the privileged" -> "privileged". Also, for clarity, I think you should add ", for example, a Safari or Chrome extension".

> Source/WebKit/chromium/ChangeLog:15
> +2011-10-14  Dmitry Lomov  <dslomov@google.com>

Another merge problem in the ChangeLog.
Comment 26 Zhenyao Mo 2011-10-19 15:25:04 PDT
Created attachment 111682 [details]
Patch
Comment 27 Zhenyao Mo 2011-10-19 15:27:19 PDT
(In reply to comment #25)
> (From update of attachment 111268 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=111268&action=review
> 
> The code looks good overall. Couple of issues that need to be cleaned up.
> 
> > Source/WebCore/ChangeLog:49
> > +2011-10-14  Mark Hahnenberg  <mhahnenberg@apple.com>
> 
> There's a merge problem in the ChangeLog. Please unapply, sync and reapply your changes.

Done.

> 
> > Source/WebCore/html/canvas/WebGLRenderingContext.cpp:2017
> > +    }
> 
> This block is duplicated from above and needs to be removed.

Done.

> 
> > Source/WebCore/html/canvas/WebGLRenderingContext.h:630
> > +    // Determine if we are running the privileged code in the browser.
> 
> "the privileged" -> "privileged". Also, for clarity, I think you should add ", for example, a Safari or Chrome extension".

Done.

> 
> > Source/WebKit/chromium/ChangeLog:15
> > +2011-10-14  Dmitry Lomov  <dslomov@google.com>
> 
> Another merge problem in the ChangeLog.

Done.
Comment 28 Kenneth Russell 2011-10-19 16:34:10 PDT
Comment on attachment 111682 [details]
Patch

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

Looks good modulo two tiny nits. r=me

> Source/WebCore/ChangeLog:1
> +2011-10-14  Zhenyao Mo  <zmo@google.com>

Please make sure the date gets fixed upon landing.

> Source/WebKit/chromium/ChangeLog:1
> +2011-10-14  Zhenyao Mo  <zmo@google.com>

Same here.
Comment 29 Zhenyao Mo 2011-10-19 17:00:03 PDT
Committed r97896: <http://trac.webkit.org/changeset/97896>