Figure out ImageFactory dependency |
||
Issue descriptionWe have a bit of a dependency problem as follows: 1. GpuMemoryBufferFactory is used by content/gpu, content/common/gpu/client, and content/browser/gpu. This seems to suggest it should live in gpu/ipc/common. 2. Suppose we move GpuMemoryBufferFactory gpu/ipc/common. It depends on ImageFactory for "AsImageFactory". ImageFactory lives in gpu/command_buffer/service/image_factory.h. 3. Depending on ImageFactory means we depend on gpu/command_buffer/service, and thus pulling in service code into common and client. Moving ImageFactory to gpu/command_buffer/common doesn't solve the problem because we need "ui/gl/gl_bindings.h" which pulls in a bunch other code. I think a possible solution is to get rid of AsImageFactory and make it a static_cast? Thoughts?
,
Mar 23 2016
I think GpuMemoryBufferFactory should be in service specific. That it happens to be used by client code (browser process) is a mistake. Ideally the part of GMBFactory that is used by the browser should be moved into a different interface that lives in common. GpuMemoryBufferSupport maybe.
,
Mar 23 2016
Sounds good. Thanks! I'll create a GpuMemoryBufferSupport and pull out: - isSupported - getNativeType As suggested offline. Thanks!
,
Mar 24 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c27742238e78f1e2f7f6fe37f101eb0d07483416 commit c27742238e78f1e2f7f6fe37f101eb0d07483416 Author: fsamuel <fsamuel@chromium.org> Date: Thu Mar 24 00:27:12 2016 Pull gpu service/client shared memory buffer code to GpuMemoryBufferSupport GpuMemoryBufferFactory is a gpu service concept and should not be accessed from the client. Client code accessed GpuMemoryBufferFactory to get the native GPU buffer type and to check whether a provided buffer is supported. Depending on GpuMemoryBufferFactory ends up pulling in other service-only bits. This CL addresses this issue. The two static methods accessed by clients are pulled into a new class: GpuMemoryBufferSupport placed in gpu/ipc/common. BUG= 597170 CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel Review URL: https://codereview.chromium.org/1831513003 Cr-Commit-Position: refs/heads/master@{#382987} [modify] https://crrev.com/c27742238e78f1e2f7f6fe37f101eb0d07483416/content/browser/gpu/browser_gpu_memory_buffer_manager.cc [modify] https://crrev.com/c27742238e78f1e2f7f6fe37f101eb0d07483416/content/common/gpu/client/gpu_memory_buffer_impl_io_surface.cc [modify] https://crrev.com/c27742238e78f1e2f7f6fe37f101eb0d07483416/content/common/gpu/client/gpu_memory_buffer_impl_ozone_native_pixmap.cc [modify] https://crrev.com/c27742238e78f1e2f7f6fe37f101eb0d07483416/content/common/gpu/client/gpu_memory_buffer_impl_surface_texture.cc [modify] https://crrev.com/c27742238e78f1e2f7f6fe37f101eb0d07483416/content/common/gpu/gpu_memory_buffer_factory.cc [modify] https://crrev.com/c27742238e78f1e2f7f6fe37f101eb0d07483416/content/common/gpu/gpu_memory_buffer_factory.h [modify] https://crrev.com/c27742238e78f1e2f7f6fe37f101eb0d07483416/content/common/gpu/gpu_memory_buffer_factory_io_surface.cc [modify] https://crrev.com/c27742238e78f1e2f7f6fe37f101eb0d07483416/content/common/gpu/gpu_memory_buffer_factory_io_surface.h [modify] https://crrev.com/c27742238e78f1e2f7f6fe37f101eb0d07483416/content/common/gpu/gpu_memory_buffer_factory_ozone_native_pixmap.cc [modify] https://crrev.com/c27742238e78f1e2f7f6fe37f101eb0d07483416/content/common/gpu/gpu_memory_buffer_factory_ozone_native_pixmap.h [modify] https://crrev.com/c27742238e78f1e2f7f6fe37f101eb0d07483416/content/common/gpu/gpu_memory_buffer_factory_surface_texture.cc [modify] https://crrev.com/c27742238e78f1e2f7f6fe37f101eb0d07483416/content/common/gpu/gpu_memory_buffer_factory_surface_texture.h [modify] https://crrev.com/c27742238e78f1e2f7f6fe37f101eb0d07483416/content/common/gpu/gpu_memory_buffer_factory_test_template.h [modify] https://crrev.com/c27742238e78f1e2f7f6fe37f101eb0d07483416/content/gpu/gpu_main.cc [modify] https://crrev.com/c27742238e78f1e2f7f6fe37f101eb0d07483416/content/gpu/in_process_gpu_thread.cc [modify] https://crrev.com/c27742238e78f1e2f7f6fe37f101eb0d07483416/gpu/gpu_ipc_common.gypi [modify] https://crrev.com/c27742238e78f1e2f7f6fe37f101eb0d07483416/gpu/ipc/common/BUILD.gn [add] https://crrev.com/c27742238e78f1e2f7f6fe37f101eb0d07483416/gpu/ipc/common/gpu_memory_buffer_support.cc [add] https://crrev.com/c27742238e78f1e2f7f6fe37f101eb0d07483416/gpu/ipc/common/gpu_memory_buffer_support.h
,
Mar 25 2016
This bug has been solved, and the fix has stuck around for a day and a half so it's unlikely to get reverted. I'm marking as FIXED. |
||
►
Sign in to add a comment |
||
Comment 1 by piman@chromium.org
, Mar 23 2016