ARB_robustness extension provides a mechanism for OpenGL applications to to learn about graphics resets that affect the context. This will allow proper repopulating of textures in Chrome with active compositor.
Created attachment 69279 [details] Patch
Comment on attachment 69279 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=69279&action=review > WebCore/platform/graphics/opengl/GraphicsContext3DOpenGL.cpp:1456 > + I don't understand the point of this patch. It doesn't hook anything up. It simply adds placeholders. You should do at least one implementation to show that it actually works, along with a test case with a bad shader that will crash if ARB_robustness is not there. I understand if this is just a stepping stone to that goal. But this bug is for "hooking up" the extension, which this patch doesn't do.
(In reply to comment #2) > (From update of attachment 69279 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=69279&action=review > > > WebCore/platform/graphics/opengl/GraphicsContext3DOpenGL.cpp:1456 > > + > > I don't understand the point of this patch. It doesn't hook anything up. It simply adds placeholders. You should do at least one implementation to show that it actually works, along with a test case with a bad shader that will crash if ARB_robustness is not there. > > I understand if this is just a stepping stone to that goal. But this bug is for "hooking up" the extension, which this patch doesn't do. Several steps are needed toward the goal of handling "context lost" events in WebGL and other functionality such as CSS 3D support. Adding the needed entry points to GraphicsContext3D allows some of this work to occur in parallel. I have updated the synopsis of this bug to reflect the intent. Alexey, by way of review of your initial patch, I think you should leave the ARB suffix on the new enums and entry point. There is also an ongoing discussion about exactly how extensions should be added (see https://bugs.webkit.org/show_bug.cgi?id=45939 ), but to keep things consistent with the current style, I think you should add a query function indicating whether the extension is supported.
I did not see any enum values or entry points with ARB in them anywhere else related to GraphicsContext3D. Where can I find an example?
(In reply to comment #3) > (In reply to comment #2) > > (From update of attachment 69279 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=69279&action=review > > > > > WebCore/platform/graphics/opengl/GraphicsContext3DOpenGL.cpp:1456 > > > + > > > > I don't understand the point of this patch. It doesn't hook anything up. It simply adds placeholders. You should do at least one implementation to show that it actually works, along with a test case with a bad shader that will crash if ARB_robustness is not there. > > > > I understand if this is just a stepping stone to that goal. But this bug is for "hooking up" the extension, which this patch doesn't do. > > Several steps are needed toward the goal of handling "context lost" events in WebGL and other functionality such as CSS 3D support. Adding the needed entry points to GraphicsContext3D allows some of this work to occur in parallel. > > I have updated the synopsis of this bug to reflect the intent. Ok, that's better. Maybe you should create an umbrella bug so we can keep track of all the work? I will amend my review
Comment on attachment 69279 [details] Patch r+
Created attachment 69291 [details] added ARB suffix
Could you please fix the synopsis in the ChangeLogs to match the new synopsis of the bug?
Created attachment 69293 [details] fixed changelogs
This looks fine to me. Since Chris reviewed the original patch I'll defer to him to review this one.
Comment on attachment 69293 [details] fixed changelogs r=me. But we need to factor this call into what we ever do in https://bugs.webkit.org/show_bug.cgi?id=46894.
Comment on attachment 69293 [details] fixed changelogs Rejecting patch 69293 from commit-queue. Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=queues.webkit.org', 'apply-attachment', '--force-clean', '--non-interactive', '--quiet', 69293]" exit_code: 2 Cleaning working directory Updating working directory Logging in as commit-queue@webkit.org... Fetching: https://bugs.webkit.org/attachment.cgi?id=69293&action=edit Fetching: https://bugs.webkit.org/show_bug.cgi?id=46850&ctype=xml Processing 1 patch from 1 bug. Processing patch 69293 from bug 46850. Failed to run "[u'/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', u'--reviewer', u'Chris Marrin', u'--force']" exit_code: 1 Full output: http://queues.webkit.org/results/4226031
What happened with this commit queue failure? There doesn't seem to be any error detail. I'll try resubmitting the patch.
Comment on attachment 69293 [details] fixed changelogs Rejecting patch 69293 from commit-queue. Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=queues.webkit.org', 'apply-attachment', '--force-clean', '--non-interactive', '--quiet', 69293]" exit_code: 2 Cleaning working directory Updating working directory Logging in as commit-queue@webkit.org... Fetching: https://bugs.webkit.org/attachment.cgi?id=69293&action=edit Fetching: https://bugs.webkit.org/show_bug.cgi?id=46850&ctype=xml Processing 1 patch from 1 bug. Processing patch 69293 from bug 46850. Failed to run "[u'/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', u'--reviewer', u'Chris Marrin', u'--force']" exit_code: 1 Full output: http://queues.webkit.org/results/4149034
Ah, the issue is that part of the patch is against WebKit/chromium/src/GraphicsContext3D.cpp, which Alexey just renamed and which no longer exists.
Created attachment 69391 [details] renamed file fix
Comment on attachment 69391 [details] renamed file fix Clearing flags on attachment: 69391 Committed r68850: <http://trac.webkit.org/changeset/68850>
All reviewed patches have been landed. Closing bug.
Yeah, the cq logs are less useful atm. We're working on a fix.