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

Issue 14456004: GPU client side changes for GpuMemoryBuffers (Closed)

Created:
7 years, 8 months ago by kaanb
Modified:
7 years, 7 months ago
CC:
chromium-reviews, darin-cc_chromium.org, apatrick_chromium, klobag.chromium, joth
Base URL:
https://chromium.googlesource.com/chromium/src.git@glapi
Visibility:
Public.

Description

GPU client side changes for GpuMemoryBuffers: - Introduces a new GL extension CHROMIUM_map_image that contains Create/Destroy/Map/Unmap/GetImageParameteriv methods. - A new data structure called GpuMemoryBufferTracker to track these buffers and images on the client side. BUG=175012 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=200505

Patch Set 1 #

Total comments: 28

Patch Set 2 : Incorporate code reviews the unit test doesn't compile #

Patch Set 3 : Forward declare GpuMemoryBufferTracker #

Patch Set 4 : Adding documentation for the extension #

Patch Set 5 : Move GLImage addition and removal from WGC3D to GpuMemoryBufferFactory and GLES2CmdDecoder respecti… #

Patch Set 6 : Fix android build #

Patch Set 7 : Uploading again #

Total comments: 18

Patch Set 8 : Incorporate code reviews #

Patch Set 9 : move GLImage operations back to WGC3D #

Patch Set 10 : Remove buffer id from buffer factory signature #

Patch Set 11 : Rebase #

Patch Set 12 : Cleanup after rebase #

Patch Set 13 : Added createImage/deleteImage commands and other overhaul #

Patch Set 14 : Implement DeleteImageBuffersHelper and remove unused GetNativeBufferForGpuMemoryBuffer #

Total comments: 4

Patch Set 15 : Incorporate code reviews #

Total comments: 6

Patch Set 16 : More code reviews #

Total comments: 22

Patch Set 17 : add methods to gl2extchromium.h #

Patch Set 18 : new GL API except glGetImageParameterivCHROMIUM #

Patch Set 19 : add GetImageParameterivCHROMIUM #

Patch Set 20 : Abstract out GpuMemoryTracker interface #

Patch Set 21 : Rollback decoder changes #

Total comments: 15

Patch Set 22 : Fix unittests #

Patch Set 23 : Updated the extension documentation #

Total comments: 85

Patch Set 24 : Incorporate code reviews #

Total comments: 24

Patch Set 25 : Address reveman's comments #

Patch Set 26 : Delete and add gl_image_mock again #

Total comments: 39

Patch Set 27 : Incorporate sievers' comments #

Total comments: 2

Patch Set 28 : Second set of feedback from sievers@ #

Total comments: 4

Patch Set 29 : Fix the unittest #

Patch Set 30 : Incorporate feedback #

Patch Set 31 : Add a comment for ppapi/ #

Patch Set 32 : s/gpu::/gpu::gles2::/ #

Patch Set 33 : Fix GLES2Implementation ctor call #

Patch Set 34 : Remove redundant GL_EXPORT from the header #

Patch Set 35 : Remove GLES2_IMPL_EXPORT from mocks #

Patch Set 36 : Add missing parameter in GLES2Implementation ctor in GLES2Implementation unittest #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1059 lines, -30 lines) Patch
M android_webview/browser/gpu_memory_buffer_impl.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 1 chunk +1 line, -1 line 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 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 1 chunk +2 lines, -1 line 0 comments Download
A gpu/GLES2/extensions/CHROMIUM/CHROMIUM_map_image.txt View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 1 chunk +107 lines, -0 lines 0 comments Download
M gpu/GLES2/gl2chromium_autogen.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 2 chunks +6 lines, -0 lines 0 comments Download
M gpu/GLES2/gl2extchromium.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 1 chunk +33 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 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 5 chunks +38 lines, -2 lines 0 comments Download
M gpu/command_buffer/client/gles2_c_lib_autogen.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 4 chunks +28 lines, -0 lines 0 comments Download
M gpu/command_buffer/client/gles2_implementation.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 5 chunks +17 lines, -1 line 0 comments Download
M gpu/command_buffer/client/gles2_implementation.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 7 chunks +171 lines, -4 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 12 13 14 15 16 17 18 19 20 21 22 23 2 chunks +12 lines, -0 lines 0 comments Download
M gpu/command_buffer/client/gles2_implementation_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 1 chunk +2 lines, -1 line 0 comments Download
M gpu/command_buffer/client/gles2_interface_autogen.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 2 chunks +7 lines, -0 lines 0 comments Download
M gpu/command_buffer/client/gles2_interface_stub_autogen.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 2 chunks +7 lines, -0 lines 0 comments Download
M gpu/command_buffer/client/gles2_interface_stub_impl_autogen.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 2 chunks +15 lines, -0 lines 0 comments Download
M gpu/command_buffer/client/gles2_trace_implementation_autogen.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 2 chunks +7 lines, -0 lines 0 comments Download
M gpu/command_buffer/client/gles2_trace_implementation_impl_autogen.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 2 chunks +28 lines, -0 lines 0 comments Download
M gpu/command_buffer/client/gpu_memory_buffer.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 1 chunk +1 line, -1 line 0 comments Download
M gpu/command_buffer/client/gpu_memory_buffer_factory.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 2 chunks +2 lines, -0 lines 0 comments Download
M gpu/command_buffer/client/gpu_memory_buffer_factory.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 1 chunk +4 lines, -0 lines 0 comments Download
A gpu/command_buffer/client/gpu_memory_buffer_mock.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 1 chunk +32 lines, -0 lines 0 comments Download
A + gpu/command_buffer/client/gpu_memory_buffer_mock.cc View 1 chunk +6 lines, -7 lines 0 comments Download
A gpu/command_buffer/client/gpu_memory_buffer_tracker.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 1 chunk +43 lines, -0 lines 0 comments Download
A gpu/command_buffer/client/gpu_memory_buffer_tracker.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 1 chunk +57 lines, -0 lines 0 comments Download
A gpu/command_buffer/client/image_factory.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 1 chunk +33 lines, -0 lines 0 comments Download
A gpu/command_buffer/client/image_factory_mock.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 1 chunk +41 lines, -0 lines 0 comments Download
A + gpu/command_buffer/client/image_factory_mock.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 1 chunk +5 lines, -5 lines 0 comments Download
M gpu/command_buffer/cmd_buffer_functions.txt View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 2 chunks +5 lines, -0 lines 0 comments Download
M gpu/command_buffer/common/gles2_cmd_utils_implementation_autogen.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 1 chunk +1 line, -0 lines 0 comments Download
A gpu/command_buffer/tests/gl_gpu_memory_buffer_unittests.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 1 chunk +146 lines, -0 lines 0 comments Download
M gpu/command_buffer/tests/gl_manager.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 2 chunks +6 lines, -0 lines 0 comments Download
M gpu/command_buffer/tests/gl_manager.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 4 chunks +7 lines, -3 lines 0 comments Download
M gpu/gles2_conform_support/egl/display.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 1 chunk +2 lines, -1 line 0 comments Download
M gpu/gpu.gyp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 1 chunk +5 lines, -0 lines 0 comments Download
M gpu/gpu_common.gypi View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 1 chunk +3 lines, -0 lines 0 comments Download
M ppapi/shared_impl/ppb_graphics_3d_shared.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 1 chunk +3 lines, -1 line 0 comments Download
M ui/gl/gl.gyp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 1 chunk +2 lines, -0 lines 0 comments Download
A ui/gl/gl_image_mock.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 1 chunk +30 lines, -0 lines 0 comments Download
A ui/gl/gl_image_mock.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 1 chunk +15 lines, -0 lines 0 comments Download
M webkit/gpu/webgraphicscontext3d_in_process_command_buffer_impl.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 2 chunks +8 lines, -1 line 0 comments Download
M webkit/gpu/webgraphicscontext3d_in_process_command_buffer_impl.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 10 chunks +121 lines, -1 line 0 comments Download

Messages

Total messages: 74 (0 generated)
kaanb1
7 years, 8 months ago (2013-04-24 21:40:34 UTC) #1
kaanb1
Friendly ping!
7 years, 7 months ago (2013-04-25 22:46:12 UTC) #2
jamesr
https://codereview.chromium.org/14456004/diff/1/gpu/command_buffer/client/gles2_implementation.cc File gpu/command_buffer/client/gles2_implementation.cc (right): https://codereview.chromium.org/14456004/diff/1/gpu/command_buffer/client/gles2_implementation.cc#newcode2040 gpu/command_buffer/client/gles2_implementation.cc:2040: GetNativeHandleForBoundGpuMemoryBuffer())); this really seems like voodoo. are there no ...
7 years, 7 months ago (2013-04-25 23:24:14 UTC) #3
kaanb1
On 2013/04/25 23:24:14, jamesr wrote: > https://codereview.chromium.org/14456004/diff/1/gpu/command_buffer/client/gles2_implementation.cc > File gpu/command_buffer/client/gles2_implementation.cc (right): > > https://codereview.chromium.org/14456004/diff/1/gpu/command_buffer/client/gles2_implementation.cc#newcode2040 > ...
7 years, 7 months ago (2013-04-25 23:27:59 UTC) #4
apatrick_chromium
Can you add a spec to gpu\GLES2\extensions\CHROMIUM? https://codereview.chromium.org/14456004/diff/1/gpu/command_buffer/client/gles2_implementation.cc File gpu/command_buffer/client/gles2_implementation.cc (right): https://codereview.chromium.org/14456004/diff/1/gpu/command_buffer/client/gles2_implementation.cc#newcode2040 gpu/command_buffer/client/gles2_implementation.cc:2040: GetNativeHandleForBoundGpuMemoryBuffer())); On ...
7 years, 7 months ago (2013-04-25 23:39:03 UTC) #5
kaanb1
https://codereview.chromium.org/14456004/diff/1/gpu/command_buffer/client/gles2_implementation.cc File gpu/command_buffer/client/gles2_implementation.cc (right): https://codereview.chromium.org/14456004/diff/1/gpu/command_buffer/client/gles2_implementation.cc#newcode2040 gpu/command_buffer/client/gles2_implementation.cc:2040: GetNativeHandleForBoundGpuMemoryBuffer())); On 2013/04/25 23:39:03, apatrick_chromium wrote: > On 2013/04/25 ...
7 years, 7 months ago (2013-04-25 23:42:19 UTC) #6
greggman
https://codereview.chromium.org/14456004/diff/1/gpu/command_buffer/client/gles2_implementation.cc File gpu/command_buffer/client/gles2_implementation.cc (right): https://codereview.chromium.org/14456004/diff/1/gpu/command_buffer/client/gles2_implementation.cc#newcode637 gpu/command_buffer/client/gles2_implementation.cc:637: case GL_BOUND_GPU_MEMORY_BUFFER_CHROMIUM: Seems like following the other ENUMs this ...
7 years, 7 months ago (2013-04-25 23:45:33 UTC) #7
no sievers
https://codereview.chromium.org/14456004/diff/1/gpu/command_buffer/client/gles2_implementation.cc File gpu/command_buffer/client/gles2_implementation.cc (right): https://codereview.chromium.org/14456004/diff/1/gpu/command_buffer/client/gles2_implementation.cc#newcode2040 gpu/command_buffer/client/gles2_implementation.cc:2040: GetNativeHandleForBoundGpuMemoryBuffer())); On 2013/04/25 23:42:19, kaanb1 wrote: > On 2013/04/25 ...
7 years, 7 months ago (2013-04-26 00:17:43 UTC) #8
reveman
I think glImageBufferDataCHROMIUM should be separate from GL_IMAGE_BUFFER_CHROMIUM extension and have a different name. GLImage ...
7 years, 7 months ago (2013-04-26 02:47:56 UTC) #9
kaanb1
All done except unittests. https://codereview.chromium.org/14456004/diff/1/gpu/command_buffer/client/gles2_implementation.cc File gpu/command_buffer/client/gles2_implementation.cc (right): https://codereview.chromium.org/14456004/diff/1/gpu/command_buffer/client/gles2_implementation.cc#newcode637 gpu/command_buffer/client/gles2_implementation.cc:637: case GL_BOUND_GPU_MEMORY_BUFFER_CHROMIUM: On 2013/04/25 23:45:34, ...
7 years, 7 months ago (2013-04-26 03:01:20 UTC) #10
no sievers
imageBufferData() actually works as expected and the client does not need to know about GpuMemoryBuffer ...
7 years, 7 months ago (2013-04-26 10:03:23 UTC) #11
kaanb1
On 2013/04/26 10:03:23, Daniel Sievers wrote: > imageBufferData() actually works as expected and the client ...
7 years, 7 months ago (2013-04-26 16:08:26 UTC) #12
kaanb1
On 2013/04/26 16:08:26, kaanb1 wrote: > On 2013/04/26 10:03:23, Daniel Sievers wrote: > > imageBufferData() ...
7 years, 7 months ago (2013-04-26 20:21:35 UTC) #13
kaanb1
On 2013/04/26 20:21:35, kaanb1 wrote: > On 2013/04/26 16:08:26, kaanb1 wrote: > > On 2013/04/26 ...
7 years, 7 months ago (2013-04-27 01:26:37 UTC) #14
joth
https://codereview.chromium.org/14456004/diff/37002/android_webview/browser/gpu_memory_buffer_factory_impl.cc File android_webview/browser/gpu_memory_buffer_factory_impl.cc (right): https://codereview.chromium.org/14456004/diff/37002/android_webview/browser/gpu_memory_buffer_factory_impl.cc#newcode30 android_webview/browser/gpu_memory_buffer_factory_impl.cc:30: gpu::GetProcessDefaultImageManager()->AddImage(gl_image, buffer_id); Is there a RemoveImage() call? where does ...
7 years, 7 months ago (2013-04-29 17:21:21 UTC) #15
apatrick_chromium
https://codereview.chromium.org/14456004/diff/37002/gpu/GLES2/extensions/CHROMIUM/CHROMIUM_gpu_memory_buffer.txt File gpu/GLES2/extensions/CHROMIUM/CHROMIUM_gpu_memory_buffer.txt (right): https://codereview.chromium.org/14456004/diff/37002/gpu/GLES2/extensions/CHROMIUM/CHROMIUM_gpu_memory_buffer.txt#newcode52 gpu/GLES2/extensions/CHROMIUM/CHROMIUM_gpu_memory_buffer.txt:52: void ImageBufferDataCHROMIUM (GLenum target, GLsizei width, GLsizei height) I ...
7 years, 7 months ago (2013-04-29 19:36:10 UTC) #16
greggman
https://codereview.chromium.org/14456004/diff/37002/gpu/GLES2/extensions/CHROMIUM/CHROMIUM_gpu_memory_buffer.txt File gpu/GLES2/extensions/CHROMIUM/CHROMIUM_gpu_memory_buffer.txt (right): https://codereview.chromium.org/14456004/diff/37002/gpu/GLES2/extensions/CHROMIUM/CHROMIUM_gpu_memory_buffer.txt#newcode52 gpu/GLES2/extensions/CHROMIUM/CHROMIUM_gpu_memory_buffer.txt:52: void ImageBufferDataCHROMIUM (GLenum target, GLsizei width, GLsizei height) On ...
7 years, 7 months ago (2013-04-29 19:58:33 UTC) #17
kaanb
https://codereview.chromium.org/14456004/diff/37002/android_webview/browser/gpu_memory_buffer_factory_impl.cc File android_webview/browser/gpu_memory_buffer_factory_impl.cc (right): https://codereview.chromium.org/14456004/diff/37002/android_webview/browser/gpu_memory_buffer_factory_impl.cc#newcode30 android_webview/browser/gpu_memory_buffer_factory_impl.cc:30: gpu::GetProcessDefaultImageManager()->AddImage(gl_image, buffer_id); On 2013/04/29 17:21:21, joth wrote: > Is ...
7 years, 7 months ago (2013-04-30 03:40:10 UTC) #18
kaanb
Uploaded the new API please take another look.
7 years, 7 months ago (2013-05-02 22:23:00 UTC) #19
joth
https://codereview.chromium.org/14456004/diff/75001/gpu/command_buffer/client/gles2_implementation.cc File gpu/command_buffer/client/gles2_implementation.cc (right): https://codereview.chromium.org/14456004/diff/75001/gpu/command_buffer/client/gles2_implementation.cc#newcode3705 gpu/command_buffer/client/gles2_implementation.cc:3705: GLuint image_id, GLint width, GLint height) { image_id is ...
7 years, 7 months ago (2013-05-03 00:49:36 UTC) #20
joth
great, yep that's what I was thinking of.. I can see what's going on now ...
7 years, 7 months ago (2013-05-03 02:04:10 UTC) #21
kaanb
https://codereview.chromium.org/14456004/diff/82001/gpu/command_buffer/client/gpu_memory_buffer_tracker.h File gpu/command_buffer/client/gpu_memory_buffer_tracker.h (right): https://codereview.chromium.org/14456004/diff/82001/gpu/command_buffer/client/gpu_memory_buffer_tracker.h#newcode24 gpu/command_buffer/client/gpu_memory_buffer_tracker.h:24: explicit GpuMemoryBufferTracker(GpuMemoryBufferFactory* factory); On 2013/05/03 02:04:11, joth wrote: > ...
7 years, 7 months ago (2013-05-03 02:37:31 UTC) #22
kaanb
friendly ping!
7 years, 7 months ago (2013-05-03 21:58:47 UTC) #23
reveman
https://codereview.chromium.org/14456004/diff/85002/gpu/command_buffer/client/gles2_implementation.cc File gpu/command_buffer/client/gles2_implementation.cc (right): https://codereview.chromium.org/14456004/diff/85002/gpu/command_buffer/client/gles2_implementation.cc#newcode2334 gpu/command_buffer/client/gles2_implementation.cc:2334: bound_gpu_memory_buffer_id_ = buffer; The id returned from GenImageBuffers/GenImage is ...
7 years, 7 months ago (2013-05-03 22:45:00 UTC) #24
greggman
I think I'm still confused (sorry) Maybe this is a stupid question but can all ...
7 years, 7 months ago (2013-05-03 23:48:00 UTC) #25
greggman
Some clarification, I think what I'm suggesting is that gpu_memory_buffer_factory should not be exposed/connected/associated with ...
7 years, 7 months ago (2013-05-03 23:55:30 UTC) #26
kaanb
On 2013/05/03 23:55:30, greggman wrote: > Some clarification, I think what I'm suggesting is that ...
7 years, 7 months ago (2013-05-04 00:09:48 UTC) #27
greggman
On 2013/05/04 00:09:48, kaanb wrote: > On 2013/05/03 23:55:30, greggman wrote: > > Some clarification, ...
7 years, 7 months ago (2013-05-04 00:15:09 UTC) #28
reveman
On 2013/05/04 00:15:09, greggman wrote: > On 2013/05/04 00:09:48, kaanb wrote: > > On 2013/05/03 ...
7 years, 7 months ago (2013-05-04 00:54:25 UTC) #29
kaanb
https://codereview.chromium.org/14456004/diff/85002/gpu/GLES2/extensions/CHROMIUM/CHROMIUM_gpu_memory_buffer.txt File gpu/GLES2/extensions/CHROMIUM/CHROMIUM_gpu_memory_buffer.txt (right): https://codereview.chromium.org/14456004/diff/85002/gpu/GLES2/extensions/CHROMIUM/CHROMIUM_gpu_memory_buffer.txt#newcode33 gpu/GLES2/extensions/CHROMIUM/CHROMIUM_gpu_memory_buffer.txt:33: This extension helps avoid that extra copy from user ...
7 years, 7 months ago (2013-05-06 18:20:53 UTC) #30
reveman
https://codereview.chromium.org/14456004/diff/85002/gpu/command_buffer/client/gpu_memory_buffer_tracker.cc File gpu/command_buffer/client/gpu_memory_buffer_tracker.cc (right): https://codereview.chromium.org/14456004/diff/85002/gpu/command_buffer/client/gpu_memory_buffer_tracker.cc#newcode27 gpu/command_buffer/client/gpu_memory_buffer_tracker.cc:27: factory_->CreateGpuMemoryBuffer(width, height, image_id).release(); On 2013/05/06 18:20:53, kaanb wrote: > ...
7 years, 7 months ago (2013-05-07 02:25:03 UTC) #31
kaanb
I've uploaded the new API. Extension document and the unittests are still out of date, ...
7 years, 7 months ago (2013-05-08 00:32:35 UTC) #32
reveman
The GL API is looking good but the factory code needs more work. You have: ...
7 years, 7 months ago (2013-05-08 04:23:06 UTC) #33
kaanb1
On 2013/05/08 04:23:06, David Reveman wrote: > The GL API is looking good but the ...
7 years, 7 months ago (2013-05-08 16:13:25 UTC) #34
joth
Note that GpuMemoryBuffer::Creator was purely created to allow the android implementation to be injected, due ...
7 years, 7 months ago (2013-05-08 16:23:40 UTC) #35
joth
https://codereview.chromium.org/14456004/diff/95003/gpu/command_buffer/client/gpu_memory_buffer_factory.h File gpu/command_buffer/client/gpu_memory_buffer_factory.h (right): https://codereview.chromium.org/14456004/diff/95003/gpu/command_buffer/client/gpu_memory_buffer_factory.h#newcode20 gpu/command_buffer/client/gpu_memory_buffer_factory.h:20: GetProcessDefaultGpuMemoryBufferFactory(); On 2013/05/08 04:23:06, David Reveman wrote: > This ...
7 years, 7 months ago (2013-05-08 16:26:38 UTC) #36
kaanb
On 2013/05/08 16:26:38, joth wrote: > https://codereview.chromium.org/14456004/diff/95003/gpu/command_buffer/client/gpu_memory_buffer_factory.h > File gpu/command_buffer/client/gpu_memory_buffer_factory.h (right): > > https://codereview.chromium.org/14456004/diff/95003/gpu/command_buffer/client/gpu_memory_buffer_factory.h#newcode20 > ...
7 years, 7 months ago (2013-05-08 16:43:23 UTC) #37
joth
On 8 May 2013 17:43, <kaanb@chromium.org> wrote: > On 2013/05/08 16:26:38, joth wrote: > > ...
7 years, 7 months ago (2013-05-08 17:10:23 UTC) #38
jamesr1
My feedback was specifically to use callbacks for stateless calls. On Wed, May 8, 2013 ...
7 years, 7 months ago (2013-05-08 17:13:58 UTC) #39
kaanb1
Ok, I'll rename GpuMemoryBufferFactory interface to ImageFactory and leave global callback in place (at least ...
7 years, 7 months ago (2013-05-08 17:51:19 UTC) #40
reveman
Ok, sounds good. We can refactor this as needed when adding multi-process mode. https://codereview.chromium.org/14456004/diff/95003/gpu/command_buffer/client/gpu_memory_buffer_factory.h File ...
7 years, 7 months ago (2013-05-08 20:02:40 UTC) #41
kaanb
https://codereview.chromium.org/14456004/diff/95003/gpu/command_buffer/client/gpu_memory_buffer_factory.h File gpu/command_buffer/client/gpu_memory_buffer_factory.h (right): https://codereview.chromium.org/14456004/diff/95003/gpu/command_buffer/client/gpu_memory_buffer_factory.h#newcode20 gpu/command_buffer/client/gpu_memory_buffer_factory.h:20: GetProcessDefaultGpuMemoryBufferFactory(); On 2013/05/08 20:02:41, David Reveman wrote: > If ...
7 years, 7 months ago (2013-05-08 20:41:35 UTC) #42
no sievers
https://codereview.chromium.org/14456004/diff/95003/gpu/GLES2/gl2extchromium.h File gpu/GLES2/gl2extchromium.h (right): https://codereview.chromium.org/14456004/diff/95003/gpu/GLES2/gl2extchromium.h#newcode99 gpu/GLES2/gl2extchromium.h:99: #define GL_CHROMIUM_gpu_memory_buffer 1 Should this be renamed now that ...
7 years, 7 months ago (2013-05-08 21:54:20 UTC) #43
kaanb
On 2013/05/08 21:54:20, Daniel Sievers wrote: > https://codereview.chromium.org/14456004/diff/95003/gpu/GLES2/gl2extchromium.h > File gpu/GLES2/gl2extchromium.h (right): > > https://codereview.chromium.org/14456004/diff/95003/gpu/GLES2/gl2extchromium.h#newcode99 ...
7 years, 7 months ago (2013-05-08 21:55:32 UTC) #44
reveman
CHROMIUM_map_image sgtm https://codereview.chromium.org/14456004/diff/95003/gpu/command_buffer/client/gpu_memory_buffer_factory.h File gpu/command_buffer/client/gpu_memory_buffer_factory.h (right): https://codereview.chromium.org/14456004/diff/95003/gpu/command_buffer/client/gpu_memory_buffer_factory.h#newcode20 gpu/command_buffer/client/gpu_memory_buffer_factory.h:20: GetProcessDefaultGpuMemoryBufferFactory(); sgtm
7 years, 7 months ago (2013-05-08 23:33:25 UTC) #45
kaanb
It is ready for another look, I haven't updated the extension doc but everything else ...
7 years, 7 months ago (2013-05-09 05:13:13 UTC) #46
kaanb
Uploaded the extension documentation as well. All ready for another look.
7 years, 7 months ago (2013-05-09 18:31:06 UTC) #47
greggman
looks great. Just a few things https://codereview.chromium.org/14456004/diff/127001/gpu/GLES2/extensions/CHROMIUM/CHROMIUM_map_image.txt File gpu/GLES2/extensions/CHROMIUM/CHROMIUM_map_image.txt (right): https://codereview.chromium.org/14456004/diff/127001/gpu/GLES2/extensions/CHROMIUM/CHROMIUM_map_image.txt#newcode51 gpu/GLES2/extensions/CHROMIUM/CHROMIUM_map_image.txt:51: GLuint CreateImageCHROMIUM (GLsizei ...
7 years, 7 months ago (2013-05-10 00:12:22 UTC) #48
no sievers
API is looking good. A couple nits, mostly on the spec wording. I don't understand ...
7 years, 7 months ago (2013-05-10 01:21:51 UTC) #49
no sievers
Also, I'd find it nicer if the buffer tracker was not a virtual interface, which ...
7 years, 7 months ago (2013-05-10 01:47:00 UTC) #50
reveman
https://codereview.chromium.org/14456004/diff/95003/gpu/command_buffer/client/gpu_memory_buffer_factory.cc File gpu/command_buffer/client/gpu_memory_buffer_factory.cc (right): https://codereview.chromium.org/14456004/diff/95003/gpu/command_buffer/client/gpu_memory_buffer_factory.cc#newcode24 gpu/command_buffer/client/gpu_memory_buffer_factory.cc:24: g_gpu_memory_buffer_factory_ = new GpuMemoryBuffer::Creator(factory); Please move this leak into ...
7 years, 7 months ago (2013-05-10 02:06:16 UTC) #51
kaanb
+brettw as reviewer for ppapi/shared_impl/ppb_graphics_3d_shared.cc https://codereview.chromium.org/14456004/diff/127001/gpu/GLES2/extensions/CHROMIUM/CHROMIUM_map_image.txt File gpu/GLES2/extensions/CHROMIUM/CHROMIUM_map_image.txt (right): https://codereview.chromium.org/14456004/diff/127001/gpu/GLES2/extensions/CHROMIUM/CHROMIUM_map_image.txt#newcode19 gpu/GLES2/extensions/CHROMIUM/CHROMIUM_map_image.txt:19: This extension allows for ...
7 years, 7 months ago (2013-05-13 23:00:36 UTC) #52
reveman
lgtm with nits https://codereview.chromium.org/14456004/diff/142001/gpu/command_buffer/client/gles2_implementation.cc File gpu/command_buffer/client/gles2_implementation.cc (right): https://codereview.chromium.org/14456004/diff/142001/gpu/command_buffer/client/gles2_implementation.cc#newcode3667 gpu/command_buffer/client/gles2_implementation.cc:3667: mode = GpuMemoryBuffer::READ_OR_WRITE; Can we call ...
7 years, 7 months ago (2013-05-14 01:21:04 UTC) #53
kaanb
https://codereview.chromium.org/14456004/diff/142001/gpu/command_buffer/client/gles2_implementation.cc File gpu/command_buffer/client/gles2_implementation.cc (right): https://codereview.chromium.org/14456004/diff/142001/gpu/command_buffer/client/gles2_implementation.cc#newcode3667 gpu/command_buffer/client/gles2_implementation.cc:3667: mode = GpuMemoryBuffer::READ_OR_WRITE; On 2013/05/14 01:21:04, David Reveman wrote: ...
7 years, 7 months ago (2013-05-14 18:12:11 UTC) #54
no sievers
https://codereview.chromium.org/14456004/diff/127001/gpu/command_buffer/client/gles2_implementation.cc File gpu/command_buffer/client/gles2_implementation.cc (right): https://codereview.chromium.org/14456004/diff/127001/gpu/command_buffer/client/gles2_implementation.cc#newcode3677 gpu/command_buffer/client/gles2_implementation.cc:3677: gpu_buffer->Map(mode, &mapped_buffer); On 2013/05/13 23:00:36, kaanb wrote: > On ...
7 years, 7 months ago (2013-05-14 21:10:02 UTC) #55
no sievers
https://codereview.chromium.org/14456004/diff/156001/webkit/gpu/webgraphicscontext3d_in_process_command_buffer_impl.cc File webkit/gpu/webgraphicscontext3d_in_process_command_buffer_impl.cc (right): https://codereview.chromium.org/14456004/diff/156001/webkit/gpu/webgraphicscontext3d_in_process_command_buffer_impl.cc#newcode242 webkit/gpu/webgraphicscontext3d_in_process_command_buffer_impl.cc:242: base::AutoLock scoped_lock(lock_); On 2013/05/14 21:10:03, Daniel Sievers wrote: > ...
7 years, 7 months ago (2013-05-14 21:22:06 UTC) #56
no sievers
On 2013/05/14 21:22:06, Daniel Sievers wrote: > https://codereview.chromium.org/14456004/diff/156001/webkit/gpu/webgraphicscontext3d_in_process_command_buffer_impl.cc > File webkit/gpu/webgraphicscontext3d_in_process_command_buffer_impl.cc (right): > > https://codereview.chromium.org/14456004/diff/156001/webkit/gpu/webgraphicscontext3d_in_process_command_buffer_impl.cc#newcode242 ...
7 years, 7 months ago (2013-05-14 21:55:53 UTC) #57
no sievers
https://codereview.chromium.org/14456004/diff/156001/webkit/gpu/webgraphicscontext3d_in_process_command_buffer_impl.cc File webkit/gpu/webgraphicscontext3d_in_process_command_buffer_impl.cc (right): https://codereview.chromium.org/14456004/diff/156001/webkit/gpu/webgraphicscontext3d_in_process_command_buffer_impl.cc#newcode216 webkit/gpu/webgraphicscontext3d_in_process_command_buffer_impl.cc:216: scoped_refptr<ImageManager> image_manager_; This is a problem too, since ImageManager ...
7 years, 7 months ago (2013-05-14 22:18:08 UTC) #58
no sievers
For now, you could make it one ImageManager+ImageFactory per context. There does not seem to ...
7 years, 7 months ago (2013-05-14 22:26:31 UTC) #59
kaanb
https://codereview.chromium.org/14456004/diff/156001/gpu/command_buffer/client/gles2_implementation.cc File gpu/command_buffer/client/gles2_implementation.cc (right): https://codereview.chromium.org/14456004/diff/156001/gpu/command_buffer/client/gles2_implementation.cc#newcode3583 gpu/command_buffer/client/gles2_implementation.cc:3583: // that may refer to the image_id are executed ...
7 years, 7 months ago (2013-05-14 23:37:09 UTC) #60
greggman
lgtm https://codereview.chromium.org/14456004/diff/164001/gpu/command_buffer/client/gles2_implementation.h File gpu/command_buffer/client/gles2_implementation.h (right): https://codereview.chromium.org/14456004/diff/164001/gpu/command_buffer/client/gles2_implementation.h#newcode98 gpu/command_buffer/client/gles2_implementation.h:98: #define GL_READ_WRITE 0x88BA this should probably go in ...
7 years, 7 months ago (2013-05-15 00:26:52 UTC) #61
no sievers
lgtm. thanks! https://codereview.chromium.org/14456004/diff/118002/webkit/gpu/webgraphicscontext3d_in_process_command_buffer_impl.cc File webkit/gpu/webgraphicscontext3d_in_process_command_buffer_impl.cc (right): https://codereview.chromium.org/14456004/diff/118002/webkit/gpu/webgraphicscontext3d_in_process_command_buffer_impl.cc#newcode322 webkit/gpu/webgraphicscontext3d_in_process_command_buffer_impl.cc:322: ImageManager* image_manager_; nit: I'd say 'the ContextGroup ...
7 years, 7 months ago (2013-05-15 01:09:40 UTC) #62
kaanb
Friendly ping for brettw, viettrungluu and raymes. I only have a one-line diff in ppapi/ ...
7 years, 7 months ago (2013-05-15 01:44:26 UTC) #63
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/kaanb@chromium.org/14456004/175001
7 years, 7 months ago (2013-05-15 01:45:28 UTC) #64
commit-bot: I haz the power
Retried try job too often on chromium_presubmit for step(s) presubmit http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_presubmit&number=2975
7 years, 7 months ago (2013-05-15 01:55:39 UTC) #65
raymes
I assume you know what you're doing but I don't really know anything about the ...
7 years, 7 months ago (2013-05-15 16:05:01 UTC) #66
kaanb
On 2013/05/15 16:05:01, raymes wrote: > I assume you know what you're doing but I ...
7 years, 7 months ago (2013-05-15 17:17:43 UTC) #67
raymes
lgtm
7 years, 7 months ago (2013-05-15 17:26:29 UTC) #68
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/kaanb@chromium.org/14456004/178002
7 years, 7 months ago (2013-05-15 22:51:24 UTC) #69
joth
lgtm
7 years, 7 months ago (2013-05-15 23:10:21 UTC) #70
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/kaanb@chromium.org/14456004/214001
7 years, 7 months ago (2013-05-16 00:16:55 UTC) #71
commit-bot: I haz the power
Step "update" is always a major failure. Look at the try server FAQ for more ...
7 years, 7 months ago (2013-05-16 01:53:28 UTC) #72
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/kaanb@chromium.org/14456004/214001
7 years, 7 months ago (2013-05-16 05:30:55 UTC) #73
commit-bot: I haz the power
7 years, 7 months ago (2013-05-16 10:46:03 UTC) #74
Message was sent while issue was closed.
Change committed as 200505

Powered by Google App Engine
This is Rietveld 408576698