Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(67)

Issue 10543125: gpu: Add support for GLX_EXT_texture_from_pixmap extension. (Closed)

Created:
8 years, 6 months ago by reveman
Modified:
8 years, 2 months ago
CC:
chromium-reviews, jochen+watch-content_chromium.org, dhollowa+watch_chromium.org, jam, joi+watch-content_chromium.org, darin-cc_chromium.org, tfarina
Visibility:
Public.

Description

gpu: Add support for GLX_EXT_texture_from_pixmap extension. Implement CHROMIUM_texture_from_image. This extension behaves just like EXT_texture_from_pixmap but uses chromium specific image identifiers rather than platform specific pixmap IDs. Add IPC message for creating an image identifier using a gfx::PluginWindowHandle. Each GPU channel maintains a different set of images and deleting an image will cause the internal image representation to be freed once it's no longer bound to a texture. BUG=132342 TEST=gpu_unittests --gtest_filter=TextureInfoTest.GetLevelImage:GLES2DecoderTest.BindTexImage2DCHROMIUM:GLES2DecoderTest.ReleaseTexImage2DCHROMIUM and manual Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=162654 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=162784

Patch Set 1 #

Total comments: 5

Patch Set 2 : Add GLImage class and the ability to associate an instance of this class to a texture level. #

Patch Set 3 : Remove .gitmodules change. #

Total comments: 4

Patch Set 4 : Add CreateViewImage/DeleteImage IPC and rename GLES extension to CHROMIUM_texture_from_image. #

Patch Set 5 : Fix gles2_conform_support build. #

Patch Set 6 : Fix gl_tests build and ui/gl link_settings. #

Patch Set 7 : Use gfx::PluginWindowHandle for image IPC. #

Patch Set 8 : s/imageId/image_id/ #

Total comments: 2

Patch Set 9 : Move CreateGLImage below virtual methods. #

Total comments: 26

Patch Set 10 : Add sync point to DeleteImage and address all review feedback. #

Total comments: 4

Patch Set 11 : Add image operation queue. #

Total comments: 11

Patch Set 12 : Style fixes and add ImageOperation struct. #

Patch Set 13 : Remove inappropriate use of scoped_refptr. #

Patch Set 14 : Add kGLImplementationMockGL case to gl_image_android.cc. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1523 lines, -82 lines) Patch
M content/browser/gpu/browser_gpu_channel_host_factory.h View 1 2 3 4 5 6 7 8 9 2 chunks +14 lines, -0 lines 0 comments Download
M content/browser/gpu/browser_gpu_channel_host_factory.cc View 1 2 3 4 5 6 7 8 9 1 chunk +66 lines, -0 lines 0 comments Download
M content/browser/gpu/gpu_process_host.h View 1 2 3 4 5 6 7 8 9 6 chunks +19 lines, -0 lines 0 comments Download
M content/browser/gpu/gpu_process_host.cc View 1 2 3 4 5 6 7 8 9 4 chunks +43 lines, -0 lines 0 comments Download
M content/common/gpu/client/gpu_channel_host.h View 1 2 3 4 5 6 7 8 9 2 chunks +7 lines, -0 lines 0 comments Download
M content/common/gpu/client/webgraphicscontext3d_command_buffer_impl.h View 1 2 3 4 5 6 7 1 chunk +3 lines, -0 lines 0 comments Download
M content/common/gpu/client/webgraphicscontext3d_command_buffer_impl.cc View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +5 lines, -0 lines 0 comments Download
M content/common/gpu/gpu_channel.h View 1 2 3 4 5 6 3 chunks +10 lines, -0 lines 0 comments Download
M content/common/gpu/gpu_channel.cc View 1 2 3 4 5 6 7 8 9 10 11 12 5 chunks +38 lines, -0 lines 0 comments Download
M content/common/gpu/gpu_channel_manager.h View 1 2 3 4 5 6 7 8 9 10 11 5 chunks +19 lines, -1 line 0 comments Download
M content/common/gpu/gpu_channel_manager.cc View 1 2 3 4 5 6 7 8 9 10 11 4 chunks +87 lines, -0 lines 0 comments Download
M content/common/gpu/gpu_command_buffer_stub.h View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +2 lines, -0 lines 0 comments Download
M content/common/gpu/gpu_command_buffer_stub.cc View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +2 lines, -0 lines 0 comments Download
M content/common/gpu/gpu_messages.h View 1 2 3 4 5 6 7 8 9 2 chunks +17 lines, -0 lines 0 comments Download
M content/renderer/render_thread_impl.h View 1 2 3 4 5 6 7 8 9 1 chunk +5 lines, -0 lines 0 comments Download
M content/renderer/render_thread_impl.cc View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +11 lines, -0 lines 0 comments Download
M gpu/command_buffer/build_gles2_cmd_buffer.py View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +12 lines, -0 lines 0 comments Download
M gpu/command_buffer/client/gles2_c_lib_autogen.h View 1 2 3 1 chunk +6 lines, -0 lines 0 comments Download
M gpu/command_buffer/client/gles2_cmd_helper_autogen.h View 1 2 3 1 chunk +16 lines, -0 lines 0 comments Download
M gpu/command_buffer/client/gles2_implementation_autogen.h View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +12 lines, -0 lines 0 comments Download
M gpu/command_buffer/client/gles2_implementation_unittest_autogen.h View 1 2 3 1 chunk +22 lines, -0 lines 0 comments Download
M gpu/command_buffer/cmd_buffer_functions.txt View 1 2 3 1 chunk +2 lines, -1 line 0 comments Download
M gpu/command_buffer/common/gles2_cmd_format_autogen.h View 1 2 3 1 chunk +76 lines, -0 lines 0 comments Download
M gpu/command_buffer/common/gles2_cmd_format_test_autogen.h View 1 2 3 1 chunk +30 lines, -0 lines 0 comments Download
M gpu/command_buffer/common/gles2_cmd_ids_autogen.h View 1 2 3 1 chunk +2 lines, -0 lines 0 comments Download
M gpu/command_buffer/service/context_group.h View 1 2 3 4 chunks +7 lines, -0 lines 0 comments Download
M gpu/command_buffer/service/context_group.cc View 1 2 3 2 chunks +3 lines, -0 lines 0 comments Download
M gpu/command_buffer/service/context_group_unittest.cc View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M gpu/command_buffer/service/gles2_cmd_decoder.cc View 1 2 3 4 5 6 7 8 9 10 11 12 7 chunks +176 lines, -74 lines 0 comments Download
M gpu/command_buffer/service/gles2_cmd_decoder_autogen.h View 1 2 3 1 chunk +24 lines, -0 lines 0 comments Download
M gpu/command_buffer/service/gles2_cmd_decoder_unittest.cc View 1 2 3 4 5 6 7 8 9 2 chunks +82 lines, -0 lines 0 comments Download
M gpu/command_buffer/service/gles2_cmd_decoder_unittest_3_autogen.h View 1 2 3 1 chunk +2 lines, -0 lines 0 comments Download
M gpu/command_buffer/service/gles2_cmd_decoder_unittest_base.cc View 1 2 3 4 5 6 7 8 9 1 chunk +1 line, -0 lines 0 comments Download
A gpu/command_buffer/service/image_manager.h View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +43 lines, -0 lines 0 comments Download
A gpu/command_buffer/service/image_manager.cc View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +35 lines, -0 lines 0 comments Download
M gpu/command_buffer/service/texture_manager.h View 1 2 3 4 5 6 7 8 9 10 11 12 6 chunks +20 lines, -0 lines 0 comments Download
M gpu/command_buffer/service/texture_manager.cc View 1 2 3 4 5 6 7 8 9 10 11 12 5 chunks +60 lines, -1 line 0 comments Download
M gpu/command_buffer/service/texture_manager_unittest.cc View 1 2 3 4 5 6 1 chunk +20 lines, -0 lines 0 comments Download
M gpu/command_buffer/tests/gl_manager.cc View 1 2 3 4 5 1 chunk +1 line, -0 lines 0 comments Download
M gpu/command_buffer_service.gypi View 1 2 3 4 5 6 7 8 9 10 1 chunk +2 lines, -0 lines 0 comments Download
M gpu/gles2_conform_support/egl/display.cc View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
M third_party/khronos/GLES2/gl2ext.h View 1 2 3 1 chunk +16 lines, -0 lines 0 comments Download
M third_party/khronos/README.chromium View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M ui/gl/gl.gyp View 1 2 3 4 5 3 chunks +16 lines, -0 lines 0 comments Download
A ui/gl/gl_image.h View 1 2 3 4 5 6 7 8 9 1 chunk +50 lines, -0 lines 0 comments Download
A + ui/gl/gl_image.cc View 1 2 3 1 chunk +12 lines, -3 lines 0 comments Download
A ui/gl/gl_image_android.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +27 lines, -0 lines 0 comments Download
A ui/gl/gl_image_glx.h View 1 2 3 4 5 6 7 8 9 1 chunk +42 lines, -0 lines 0 comments Download
A ui/gl/gl_image_glx.cc View 1 2 3 4 5 6 7 8 9 1 chunk +175 lines, -0 lines 0 comments Download
A ui/gl/gl_image_linux.cc View 1 2 3 4 5 6 1 chunk +38 lines, -0 lines 0 comments Download
A ui/gl/gl_image_mac.cc View 1 2 3 4 5 6 1 chunk +29 lines, -0 lines 0 comments Download
A ui/gl/gl_image_stub.h View 1 2 3 1 chunk +27 lines, -0 lines 0 comments Download
A ui/gl/gl_image_stub.cc View 1 2 3 1 chunk +25 lines, -0 lines 0 comments Download
A ui/gl/gl_image_win.cc View 1 2 3 4 5 6 1 chunk +29 lines, -0 lines 0 comments Download
M ui/gl/gl_surface_glx.h View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M ui/gl/gl_surface_glx.cc View 1 2 3 3 chunks +8 lines, -0 lines 0 comments Download
M webkit/gpu/webgraphicscontext3d_in_process_command_buffer_impl.h View 1 2 3 1 chunk +3 lines, -0 lines 0 comments Download
M webkit/gpu/webgraphicscontext3d_in_process_command_buffer_impl.cc View 1 2 3 2 chunks +7 lines, -1 line 0 comments Download
M webkit/gpu/webgraphicscontext3d_in_process_impl.h View 1 2 3 1 chunk +3 lines, -0 lines 0 comments Download
M webkit/gpu/webgraphicscontext3d_in_process_impl.cc View 1 2 3 4 5 6 7 8 9 1 chunk +10 lines, -0 lines 0 comments Download

Messages

Total messages: 37 (0 generated)
reveman
Hi, this is required to land host window management support for aura. It's similar to ...
8 years, 6 months ago (2012-06-12 22:27:09 UTC) #1
piman
couple of high-level comments. It would be kinda nice to avoid #ifdef USE_X11 in common ...
8 years, 6 months ago (2012-06-12 23:15:38 UTC) #2
google-reveman
On Tue, Jun 12, 2012 at 7:15 PM, <piman@chromium.org> wrote: > couple of high-level comments. ...
8 years, 6 months ago (2012-06-12 23:46:18 UTC) #3
piman
On Tue, Jun 12, 2012 at 4:46 PM, David Reveman <reveman@google.com> wrote: > On Tue, ...
8 years, 6 months ago (2012-06-12 23:58:50 UTC) #4
greggman
How do you get a pixmap id? Can a rouge program just start guessing ids? ...
8 years, 6 months ago (2012-06-13 00:15:51 UTC) #5
google-reveman
On Tue, Jun 12, 2012 at 8:15 PM, <gman@chromium.org> wrote: > How do you get ...
8 years, 6 months ago (2012-06-13 00:55:32 UTC) #6
apatrick_chromium
The IOSurface support is broken so best not to copy it. I think the pixmap ...
8 years, 6 months ago (2012-06-13 01:07:32 UTC) #7
reveman
On 2012/06/13 01:07:32, apatrick_chromium wrote: > The IOSurface support is broken so best not to ...
8 years, 5 months ago (2012-07-12 17:08:56 UTC) #8
greggman
On 2012/07/12 17:08:56, David Reveman wrote: > On 2012/06/13 01:07:32, apatrick_chromium wrote: > > The ...
8 years, 5 months ago (2012-07-17 23:59:31 UTC) #9
piman
On 2012/07/17 23:59:31, greggman wrote: > On 2012/07/12 17:08:56, David Reveman wrote: > > On ...
8 years, 5 months ago (2012-07-18 00:08:40 UTC) #10
greggman
On 2012/07/18 00:08:40, piman wrote: > On 2012/07/17 23:59:31, greggman wrote: > > On 2012/07/12 ...
8 years, 5 months ago (2012-07-18 01:22:11 UTC) #11
piman
On Tue, Jul 17, 2012 at 6:22 PM, <gman@chromium.org> wrote: > On 2012/07/18 00:08:40, piman ...
8 years, 5 months ago (2012-07-18 07:15:25 UTC) #12
reveman
I'm about to pick up this work again, but rather than using a flag passed ...
8 years, 2 months ago (2012-10-08 18:12:35 UTC) #13
piman
On Mon, Oct 8, 2012 at 11:12 AM, <reveman@chromium.org> wrote: > I'm about to pick ...
8 years, 2 months ago (2012-10-08 19:20:26 UTC) #14
reveman
On 2012/10/08 19:20:26, piman wrote: > On Mon, Oct 8, 2012 at 11:12 AM, <mailto:reveman@chromium.org> ...
8 years, 2 months ago (2012-10-08 21:11:01 UTC) #15
apatrick_chromium
I don't know if this is necessarily the way to go but, just to make ...
8 years, 2 months ago (2012-10-09 19:24:35 UTC) #16
reveman
PTAL
8 years, 2 months ago (2012-10-11 22:42:13 UTC) #17
apatrick_chromium
I'll be out for a week so I defer to others. Don't feel you need ...
8 years, 2 months ago (2012-10-11 23:09:57 UTC) #18
tfarina
http://codereview.chromium.org/10543125/diff/47001/ui/gl/gl_image.h File ui/gl/gl_image.h (right): http://codereview.chromium.org/10543125/diff/47001/ui/gl/gl_image.h#newcode31 ui/gl/gl_image.h:31: static scoped_refptr<GLImage> CreateGLImage(gfx::PluginWindowHandle window); nit: place this before the ...
8 years, 2 months ago (2012-10-12 01:40:48 UTC) #19
reveman
https://codereview.chromium.org/10543125/diff/18001/content/common/gpu/client/webgraphicscontext3d_command_buffer_impl.h File content/common/gpu/client/webgraphicscontext3d_command_buffer_impl.h (right): https://codereview.chromium.org/10543125/diff/18001/content/common/gpu/client/webgraphicscontext3d_command_buffer_impl.h#newcode576 content/common/gpu/client/webgraphicscontext3d_command_buffer_impl.h:576: virtual void texImagePixmap2DCHROMIUM(WGC3Denum target, WGC3Duint pixmapId); On 2012/10/11 23:09:57, ...
8 years, 2 months ago (2012-10-12 03:29:11 UTC) #20
reveman
On 2012/10/11 23:09:57, apatrick_chromium wrote: > I'll be out for a week so I defer ...
8 years, 2 months ago (2012-10-12 03:45:58 UTC) #21
piman
https://codereview.chromium.org/10543125/diff/29052/content/browser/gpu/browser_gpu_channel_host_factory.cc File content/browser/gpu/browser_gpu_channel_host_factory.cc (right): https://codereview.chromium.org/10543125/diff/29052/content/browser/gpu/browser_gpu_channel_host_factory.cc#newcode197 content/browser/gpu/browser_gpu_channel_host_factory.cc:197: image_id)); Note: there's still a race on destruction. Take ...
8 years, 2 months ago (2012-10-12 22:05:11 UTC) #22
reveman
https://codereview.chromium.org/10543125/diff/29052/content/browser/gpu/browser_gpu_channel_host_factory.cc File content/browser/gpu/browser_gpu_channel_host_factory.cc (right): https://codereview.chromium.org/10543125/diff/29052/content/browser/gpu/browser_gpu_channel_host_factory.cc#newcode197 content/browser/gpu/browser_gpu_channel_host_factory.cc:197: image_id)); On 2012/10/12 22:05:11, piman wrote: > Note: there's ...
8 years, 2 months ago (2012-10-13 05:25:53 UTC) #23
piman
Ok, one last thing... https://codereview.chromium.org/10543125/diff/60001/content/common/gpu/gpu_channel_manager.cc File content/common/gpu/gpu_channel_manager.cc (right): https://codereview.chromium.org/10543125/diff/60001/content/common/gpu/gpu_channel_manager.cc#newcode195 content/common/gpu/gpu_channel_manager.cc:195: if (image_sync_point_) I'm not convinced ...
8 years, 2 months ago (2012-10-13 18:00:37 UTC) #24
reveman
PTAL https://codereview.chromium.org/10543125/diff/60001/content/common/gpu/gpu_channel_manager.cc File content/common/gpu/gpu_channel_manager.cc (right): https://codereview.chromium.org/10543125/diff/60001/content/common/gpu/gpu_channel_manager.cc#newcode195 content/common/gpu/gpu_channel_manager.cc:195: if (image_sync_point_) On 2012/10/13 18:00:38, piman wrote: > ...
8 years, 2 months ago (2012-10-17 07:37:11 UTC) #25
piman
https://codereview.chromium.org/10543125/diff/55007/content/common/gpu/gpu_channel_manager.cc File content/common/gpu/gpu_channel_manager.cc (right): https://codereview.chromium.org/10543125/diff/55007/content/common/gpu/gpu_channel_manager.cc#newcode193 content/common/gpu/gpu_channel_manager.cc:193: else nit: style requires braces here because the statement ...
8 years, 2 months ago (2012-10-17 17:46:20 UTC) #26
greggman
Looks good. I agree with Al though, there's a security issue in that any context ...
8 years, 2 months ago (2012-10-17 18:32:16 UTC) #27
piman
On Wed, Oct 17, 2012 at 11:32 AM, <gman@chromium.org> wrote: > Looks good. > > ...
8 years, 2 months ago (2012-10-17 18:35:27 UTC) #28
greggman
On 2012/10/17 18:35:27, piman wrote: > On Wed, Oct 17, 2012 at 11:32 AM, <mailto:gman@chromium.org> ...
8 years, 2 months ago (2012-10-17 20:33:55 UTC) #29
reveman
https://codereview.chromium.org/10543125/diff/55007/content/common/gpu/gpu_channel_manager.cc File content/common/gpu/gpu_channel_manager.cc (right): https://codereview.chromium.org/10543125/diff/55007/content/common/gpu/gpu_channel_manager.cc#newcode193 content/common/gpu/gpu_channel_manager.cc:193: else On 2012/10/17 17:46:20, piman wrote: > nit: style ...
8 years, 2 months ago (2012-10-17 21:22:54 UTC) #30
greggman
On 2012/10/17 21:22:54, David Reveman wrote: > https://codereview.chromium.org/10543125/diff/55007/content/common/gpu/gpu_channel_manager.cc > File content/common/gpu/gpu_channel_manager.cc (right): > > https://codereview.chromium.org/10543125/diff/55007/content/common/gpu/gpu_channel_manager.cc#newcode193 ...
8 years, 2 months ago (2012-10-17 21:40:15 UTC) #31
reveman
On 2012/10/17 21:40:15, greggman wrote: > On 2012/10/17 21:22:54, David Reveman wrote: > > > ...
8 years, 2 months ago (2012-10-17 21:57:48 UTC) #32
reveman
On 2012/10/17 21:40:15, greggman wrote: > On 2012/10/17 21:22:54, David Reveman wrote: > > > ...
8 years, 2 months ago (2012-10-17 22:31:18 UTC) #33
piman
lgtm
8 years, 2 months ago (2012-10-18 03:13:53 UTC) #34
reveman
Gregg, sorry for the scoped_refptr confusion. This latest patch should be getting it right. Glad ...
8 years, 2 months ago (2012-10-18 03:21:03 UTC) #35
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/reveman@chromium.org/10543125/79002
8 years, 2 months ago (2012-10-18 03:21:24 UTC) #36
commit-bot: I haz the power
8 years, 2 months ago (2012-10-18 05:15:57 UTC) #37
Change committed as 162654

Powered by Google App Engine
This is Rietveld 408576698