New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 644265 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Feature

Blocked on:
issue 781418

Blocking:
issue angleproject:1546



Sign in to add a comment

Add support for GL_EXT_window_rectangles to command buffer

Project Member Reported by bsalo...@google.com, Sep 6 2016

Issue description

This extension is used by Skia with difference clipping (where a rectangle or rounded rectangle is removed from the active clip). This is a fairly common clip operation used by blink. By using window rectangles in exclusive mode we enable the GPU to avoid performing any fragment work inside the window rectangles. This gives a large performance improvement when difference clips are used.

This is a small extension and shouldn't require a lot of work in the command buffer. There is one function and a small amount of state.
 
Cc: kbr@chromium.org vmi...@chromium.org
Owner: kainino@chromium.org
kainino@ could you take a look if you have some time?
Use of it will toggle global state that affects rendering, so all GL code needs to be aware of it if it uses Skia in its GL context. So in addition to the command buffer work, there's a bit of extra work in compositor to reset this state. Though, the compositor's use of Skia to render the filters may never trigger the EXT_wr codepath in Skia, but I'd imagine resetting the state would be the consistent thing to do..
Project Member

Comment 3 by sheriffbot@chromium.org, Sep 7 2016

Labels: Hotlist-Google
Project Member

Comment 4 by sheriffbot@chromium.org, Sep 7 2017

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available. If you change it back, also remove the "Hotlist-Recharge-Cold" label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Hotlist-Recharge-Cold
Status: Assigned (was: Untriaged)
Apologies I have yet to take a look at this. It's fallen near the bottom of my to-do list at this point. Let me know if it should be prioritized or if it's become obsolete.
This would definitely be great to have. Window rectangles give us a decent speedup when supported because difference clip rects are common in Chrome.

The API is quite simple, so I would hope the implementation isn't too time consuming.

Comment 7 by m...@nvidia.com, Oct 16 2017

kainino,

Please take a look at:

  https://www.slideshare.net/Mark_Kilgard/extwindowrectangles#13

This slide summarizes performance data from Chris Dalton @ Google (comment #6) measured with Skia's nanobench on real-world web pages recorded as Skia pictures.

The performance gains are quite substantial.  Power gains are comparable because obscure backdrop regions of web pages can be skipped at approximately infinite speed by the window rectangles test.

Review slides 11 to 19 for the how & why of this benefits of this functionality.

This mature functionality has been shipping for NVIDIA devices for about a year so ripe for use.  NVIDIA's Shield Tablet & Android TV products advertise the extension today as do all GeForce/Quadro drivers for both desktop OpenGL and ES 2.0/3.x.

Google is also likely to find window rectangles helpful in reducing the overdraw at corners for VR work (see the slides) so WebGL may benefit as well from having this extension exposed in ANGLE and the Command Buffer.

- Mark Kilgard, NVIDIA
Labels: -Pri-3 Pri-2
Summary: Add support for GL_EXT_window_rectangles to command buffer (was: Add support for GL_EXT_windows_rectangles to command buffer)
https://www.khronos.org/registry/OpenGL/extensions/EXT/EXT_window_rectangles.txt

I intend to finally get this done this quarter.
Status: Started (was: Assigned)

Comment 10 by m...@nvidia.com, Oct 26 2017

Will there need to be ANGLE support in addition to Command Buffer support?  Hopefully this bug applies to both.

While I'm not in a position to contribute to the implementation code, I will happily provide a code review.

FYI:  WebGL (and Chromium) generally don't allow Command Buffer or ANGLE to draw directly to a window (what OpenGL considers framebuffer object zero, the case where window rectangle testing is unavaialble) so window rectangle testing should always be available.  This also means shouldn't then be anything that would require an ANGLE prefixed/suffixed extension as the EXT specification should apply as written (similar to EXT_blend_minmax).  The EXT specification is written to apply *both* to desktop OpenGL contexts and ES contexts.

I'm also available for questions on the EXT_window_rectangles specification or driver questions (we have extensive driver regression tests for this functionality so the functionality should "just work").
mjk: Thanks for your support. Yes, we'll also need ANGLE support; that bug is issue angleproject:1546.

I don't think this is a question, but here's a bug report for the spec :)

>     WINDOW_RECTANGLE_EXT       8*x4xZ+  GetIntegeri_v  8*x(0,0,0,0)   Window rectangle box      17.3.X  scissor

I believe both of the "8*"s here should be "4*", per  issue #28 .
Blocking: angleproject:1546
#28 in the spec's issues list, not crbug #28.
Awesome!! Watch for perf improvements on platforms that support the extension :-)

Comment 16 by m...@nvidia.com, Nov 3 2017

kainino, thanks for the specification fix.  I've updated the specification and requested the Khronos registry be updated.  See:

  https://github.com/KhronosGroup/OpenGL-API/issues/21

The original rectangle limit was 8 (NVIDIA's implementation limit) but was reduced to 4 so other vendors could support the extension.  You identified that the state table wasn't updated.  Happy to integrate your update.

So is the Command Buffer support all that is needed for Chrome to begin benefiting from EXT_window_rectangles?  Specifically, is there ANGLE work needed too?
ANGLE support will be needed on Windows. Linux, Mac, and Android platforms which support this extension should get it right away.
Project Member

Comment 18 by bugdroid1@chromium.org, Nov 3 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bcbefdabffdb64447b040ee8b7d08c14e8e7e534

commit bcbefdabffdb64447b040ee8b7d08c14e8e7e534
Author: Kai Ninomiya <kainino@chromium.org>
Date: Fri Nov 03 19:57:27 2017

Implement EXT_window_rectangles in command buffer

Spec: https://www.khronos.org/registry/OpenGL/extensions/EXT/EXT_window_rectangles.txt

Bug:  644265 
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I3c4bae9cae8cf839fcf20103db1976e24e148000
Reviewed-on: https://chromium-review.googlesource.com/741866
Commit-Queue: Kai Ninomiya <kainino@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513884}
[modify] https://crrev.com/bcbefdabffdb64447b040ee8b7d08c14e8e7e534/gpu/BUILD.gn
[modify] https://crrev.com/bcbefdabffdb64447b040ee8b7d08c14e8e7e534/gpu/GLES2/gl2chromium_autogen.h
[modify] https://crrev.com/bcbefdabffdb64447b040ee8b7d08c14e8e7e534/gpu/command_buffer/build_gles2_cmd_buffer.py
[modify] https://crrev.com/bcbefdabffdb64447b040ee8b7d08c14e8e7e534/gpu/command_buffer/client/gles2_c_lib_autogen.h
[modify] https://crrev.com/bcbefdabffdb64447b040ee8b7d08c14e8e7e534/gpu/command_buffer/client/gles2_cmd_helper_autogen.h
[modify] https://crrev.com/bcbefdabffdb64447b040ee8b7d08c14e8e7e534/gpu/command_buffer/client/gles2_implementation_autogen.h
[modify] https://crrev.com/bcbefdabffdb64447b040ee8b7d08c14e8e7e534/gpu/command_buffer/client/gles2_implementation_impl_autogen.h
[modify] https://crrev.com/bcbefdabffdb64447b040ee8b7d08c14e8e7e534/gpu/command_buffer/client/gles2_implementation_unittest_autogen.h
[modify] https://crrev.com/bcbefdabffdb64447b040ee8b7d08c14e8e7e534/gpu/command_buffer/client/gles2_interface_autogen.h
[modify] https://crrev.com/bcbefdabffdb64447b040ee8b7d08c14e8e7e534/gpu/command_buffer/client/gles2_interface_stub_autogen.h
[modify] https://crrev.com/bcbefdabffdb64447b040ee8b7d08c14e8e7e534/gpu/command_buffer/client/gles2_interface_stub_impl_autogen.h
[modify] https://crrev.com/bcbefdabffdb64447b040ee8b7d08c14e8e7e534/gpu/command_buffer/client/gles2_trace_implementation_autogen.h
[modify] https://crrev.com/bcbefdabffdb64447b040ee8b7d08c14e8e7e534/gpu/command_buffer/client/gles2_trace_implementation_impl_autogen.h
[modify] https://crrev.com/bcbefdabffdb64447b040ee8b7d08c14e8e7e534/gpu/command_buffer/cmd_buffer_functions.txt
[modify] https://crrev.com/bcbefdabffdb64447b040ee8b7d08c14e8e7e534/gpu/command_buffer/common/gles2_cmd_format_autogen.h
[modify] https://crrev.com/bcbefdabffdb64447b040ee8b7d08c14e8e7e534/gpu/command_buffer/common/gles2_cmd_format_test_autogen.h
[modify] https://crrev.com/bcbefdabffdb64447b040ee8b7d08c14e8e7e534/gpu/command_buffer/common/gles2_cmd_ids_autogen.h
[modify] https://crrev.com/bcbefdabffdb64447b040ee8b7d08c14e8e7e534/gpu/command_buffer/common/gles2_cmd_utils.cc
[modify] https://crrev.com/bcbefdabffdb64447b040ee8b7d08c14e8e7e534/gpu/command_buffer/common/gles2_cmd_utils_autogen.h
[modify] https://crrev.com/bcbefdabffdb64447b040ee8b7d08c14e8e7e534/gpu/command_buffer/common/gles2_cmd_utils_implementation_autogen.h
[modify] https://crrev.com/bcbefdabffdb64447b040ee8b7d08c14e8e7e534/gpu/command_buffer/service/context_state.cc
[modify] https://crrev.com/bcbefdabffdb64447b040ee8b7d08c14e8e7e534/gpu/command_buffer/service/context_state.h
[modify] https://crrev.com/bcbefdabffdb64447b040ee8b7d08c14e8e7e534/gpu/command_buffer/service/context_state_autogen.h
[modify] https://crrev.com/bcbefdabffdb64447b040ee8b7d08c14e8e7e534/gpu/command_buffer/service/context_state_impl_autogen.h
[modify] https://crrev.com/bcbefdabffdb64447b040ee8b7d08c14e8e7e534/gpu/command_buffer/service/create_gr_gl_interface.cc
[modify] https://crrev.com/bcbefdabffdb64447b040ee8b7d08c14e8e7e534/gpu/command_buffer/service/feature_info.cc
[modify] https://crrev.com/bcbefdabffdb64447b040ee8b7d08c14e8e7e534/gpu/command_buffer/service/feature_info.h
[modify] https://crrev.com/bcbefdabffdb64447b040ee8b7d08c14e8e7e534/gpu/command_buffer/service/gles2_cmd_apply_framebuffer_attachment_cmaa_intel.cc
[modify] https://crrev.com/bcbefdabffdb64447b040ee8b7d08c14e8e7e534/gpu/command_buffer/service/gles2_cmd_copy_tex_image.cc
[modify] https://crrev.com/bcbefdabffdb64447b040ee8b7d08c14e8e7e534/gpu/command_buffer/service/gles2_cmd_copy_texture_chromium.cc
[modify] https://crrev.com/bcbefdabffdb64447b040ee8b7d08c14e8e7e534/gpu/command_buffer/service/gles2_cmd_decoder.cc
[modify] https://crrev.com/bcbefdabffdb64447b040ee8b7d08c14e8e7e534/gpu/command_buffer/service/gles2_cmd_decoder.h
[modify] https://crrev.com/bcbefdabffdb64447b040ee8b7d08c14e8e7e534/gpu/command_buffer/service/gles2_cmd_decoder_autogen.h
[modify] https://crrev.com/bcbefdabffdb64447b040ee8b7d08c14e8e7e534/gpu/command_buffer/service/gles2_cmd_decoder_mock.h
[modify] https://crrev.com/bcbefdabffdb64447b040ee8b7d08c14e8e7e534/gpu/command_buffer/service/gles2_cmd_decoder_passthrough.cc
[modify] https://crrev.com/bcbefdabffdb64447b040ee8b7d08c14e8e7e534/gpu/command_buffer/service/gles2_cmd_decoder_passthrough.h
[modify] https://crrev.com/bcbefdabffdb64447b040ee8b7d08c14e8e7e534/gpu/command_buffer/service/gles2_cmd_decoder_passthrough_doer_prototypes.h
[modify] https://crrev.com/bcbefdabffdb64447b040ee8b7d08c14e8e7e534/gpu/command_buffer/service/gles2_cmd_decoder_passthrough_doers.cc
[modify] https://crrev.com/bcbefdabffdb64447b040ee8b7d08c14e8e7e534/gpu/command_buffer/service/gles2_cmd_decoder_passthrough_handlers_autogen.cc
[modify] https://crrev.com/bcbefdabffdb64447b040ee8b7d08c14e8e7e534/gpu/command_buffer/service/gles2_cmd_decoder_unittest_base.cc
[modify] https://crrev.com/bcbefdabffdb64447b040ee8b7d08c14e8e7e534/gpu/command_buffer/service/gles2_cmd_decoder_unittest_base.h
[modify] https://crrev.com/bcbefdabffdb64447b040ee8b7d08c14e8e7e534/gpu/command_buffer/service/gles2_cmd_decoder_unittest_extensions.cc
[modify] https://crrev.com/bcbefdabffdb64447b040ee8b7d08c14e8e7e534/gpu/command_buffer/service/gles2_cmd_srgb_converter.cc
[modify] https://crrev.com/bcbefdabffdb64447b040ee8b7d08c14e8e7e534/gpu/command_buffer/service/gles2_cmd_validation_autogen.h
[modify] https://crrev.com/bcbefdabffdb64447b040ee8b7d08c14e8e7e534/gpu/command_buffer/service/gles2_cmd_validation_implementation_autogen.h
[add] https://crrev.com/bcbefdabffdb64447b040ee8b7d08c14e8e7e534/gpu/command_buffer/tests/gl_ext_window_rectangles_unittest.cc
[add] https://crrev.com/bcbefdabffdb64447b040ee8b7d08c14e8e7e534/gpu/command_buffer/tests/gl_virtual_contexts_ext_window_rectangles_unittest.cc
[modify] https://crrev.com/bcbefdabffdb64447b040ee8b7d08c14e8e7e534/ui/gl/generate_bindings.py
[modify] https://crrev.com/bcbefdabffdb64447b040ee8b7d08c14e8e7e534/ui/gl/gl_bindings.h
[modify] https://crrev.com/bcbefdabffdb64447b040ee8b7d08c14e8e7e534/ui/gl/gl_bindings_api_autogen_gl.h
[modify] https://crrev.com/bcbefdabffdb64447b040ee8b7d08c14e8e7e534/ui/gl/gl_bindings_autogen_gl.cc
[modify] https://crrev.com/bcbefdabffdb64447b040ee8b7d08c14e8e7e534/ui/gl/gl_bindings_autogen_gl.h
[modify] https://crrev.com/bcbefdabffdb64447b040ee8b7d08c14e8e7e534/ui/gl/gl_bindings_autogen_mock.cc
[modify] https://crrev.com/bcbefdabffdb64447b040ee8b7d08c14e8e7e534/ui/gl/gl_bindings_autogen_mock.h
[modify] https://crrev.com/bcbefdabffdb64447b040ee8b7d08c14e8e7e534/ui/gl/gl_mock_autogen_gl.h
[modify] https://crrev.com/bcbefdabffdb64447b040ee8b7d08c14e8e7e534/ui/gl/gl_stub_autogen_gl.h

Comment 19 by kbr@chromium.org, Nov 4 2017

Blockedon: 781418
>> The original rectangle limit was 8 (NVIDIA's implementation limit) but was reduced to 4 so other vendors could support the extension.

Curious: which other vendors support the extension?

Also: why not just make the limit 1? A single exclusive window is still very useful as chrome clips by a difference rectangle very frequently.
Project Member

Comment 21 by bugdroid1@chromium.org, Nov 9 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/308174ca7345d19763e9a97181423fabf416bef2

commit 308174ca7345d19763e9a97181423fabf416bef2
Author: Kai Ninomiya <kainino@chromium.org>
Date: Thu Nov 09 04:11:31 2017

Add missing setup for GL_EXT_window_rectangles

Followup to http://crrev.com/c/741866

Bug:  781418 ,  644265 
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I7bca652054bc7af2fd876b03ec43f4ef08f50405
Reviewed-on: https://chromium-review.googlesource.com/758969
Reviewed-by: Zhenyao Mo <zmo@chromium.org>
Commit-Queue: Kai Ninomiya <kainino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515093}
[modify] https://crrev.com/308174ca7345d19763e9a97181423fabf416bef2/content/test/gpu/gpu_tests/webgl2_conformance_expectations.py
[modify] https://crrev.com/308174ca7345d19763e9a97181423fabf416bef2/gpu/skia_bindings/gl_bindings_skia_cmd_buffer.cc

Comment 22 by m...@nvidia.com, Nov 9 2017

> Curious: which other vendors support the extension?

Mesa3D and Nouveau have patches:

    https://patchwork.freedesktop.org/patch/92671/
    https://patchwork.freedesktop.org/patch/92672/
    https://patchwork.freedesktop.org/patch/92673/

I'm no expert on other vendors support...  Try this:

  http://opengl.gpuinfo.org/gl_listreports.php?listreportsbyextension=GL_EXT_window_rectangles

> Also: why not just make the limit 1? A single exclusive window is still very useful as chrome clips by a difference rectangle very frequently.

Mesa & AMD engineers I consulted with thought ATI hardware could support the extension with 4 window rectangles.

I don't think there was a vendor that just supported 1.
Status: Fixed (was: Started)
Project Member

Comment 24 by bugdroid1@chromium.org, Nov 28 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ab63f022f5f0bfbe516f0728d03e55fcca851bd3

commit ab63f022f5f0bfbe516f0728d03e55fcca851bd3
Author: Gordana Cmiljanovic <gordana.cmiljanovic@mips.com>
Date: Tue Nov 28 18:43:22 2017

Disable GLVirtualContextsEXTWindowRectanglesTest for some devices

The test is causing crash on devices which do not suport OpenGL ES3.

Bug:  644265 
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: Id98475c27897e1ca4cd207af4d145ab0c4733a85
Reviewed-on: https://chromium-review.googlesource.com/788112
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Reviewed-by: Zhenyao Mo <zmo@chromium.org>
Commit-Queue: Kai Ninomiya <kainino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519765}
[modify] https://crrev.com/ab63f022f5f0bfbe516f0728d03e55fcca851bd3/gpu/command_buffer/tests/gl_virtual_contexts_ext_window_rectangles_unittest.cc

Project Member

Comment 25 by bugdroid1@chromium.org, Dec 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9f5e5274f4e24a6a5daadef078a71eaa8a1a4655

commit 9f5e5274f4e24a6a5daadef078a71eaa8a1a4655
Author: Milko Leporis <milko.leporis@mips.com>
Date: Tue Dec 19 17:39:42 2017

Disable GLEXTWindowRectanglesTest for some devices

The test is causing crash on devices which do not suport OpenGL ES3.
This is the same issue as fixed in:
https://chromium-review.googlesource.com/c/chromium/src/+/788112

BUG= 644265 

Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: Iafc52435d2e32282b4a53baba5edf80ea6dbfed6
Reviewed-on: https://chromium-review.googlesource.com/832746
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Reviewed-by: Zhenyao Mo <zmo@chromium.org>
Commit-Queue: Zhenyao Mo <zmo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#525074}
[modify] https://crrev.com/9f5e5274f4e24a6a5daadef078a71eaa8a1a4655/gpu/command_buffer/tests/gl_ext_window_rectangles_unittest.cc

Sign in to add a comment