Tune helper context buffer sizes

Tune helper context buffer sizes

BUG=243703
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=207241

[no sievers, 2013-06-17 19:06:35]

https://codereview.chromium.org/17294003/diff/1/content/browser/renderer_host...
File content/browser/renderer_host/image_transport_factory_android.cc (right):

https://codereview.chromium.org/17294003/diff/1/content/browser/renderer_host...
content/browser/renderer_host/image_transport_factory_android.cc:100: static
size_t kBitsPerByte = 8;
nit: const

https://codereview.chromium.org/17294003/diff/1/content/browser/renderer_host...
content/browser/renderer_host/image_transport_factory_android.cc:105:
display_info.GetBitsPerPixel() / kBitsPerByte;
I don't think we need to look at the display bpp here, since the transfer buffer
is used for texture uploads and readbacks where we choose the format. So just
hardcode to 4?

https://codereview.chromium.org/17294003/diff/1/content/browser/renderer_host...
content/browser/renderer_host/image_transport_factory_android.cc:115:
kDefaultMaxTransferBufferSize));
These values seem to not cap anything if you assume 480*800 as the lowest res we
want to optimize for (start 1.5m, max. 4.6m).

[kaanb, 2013-06-18 11:20:03]

https://codereview.chromium.org/17294003/diff/1/content/browser/renderer_host...
File content/browser/renderer_host/image_transport_factory_android.cc (right):

https://codereview.chromium.org/17294003/diff/1/content/browser/renderer_host...
content/browser/renderer_host/image_transport_factory_android.cc:100: static
size_t kBitsPerByte = 8;
On 2013/06/17 19:06:35, Daniel Sievers wrote:
> nit: const

Done.

https://codereview.chromium.org/17294003/diff/1/content/browser/renderer_host...
content/browser/renderer_host/image_transport_factory_android.cc:105:
display_info.GetBitsPerPixel() / kBitsPerByte;
On 2013/06/17 19:06:35, Daniel Sievers wrote:
> I don't think we need to look at the display bpp here, since the transfer
buffer
> is used for texture uploads and readbacks where we choose the format. So just
> hardcode to 4?

Done.

https://codereview.chromium.org/17294003/diff/1/content/browser/renderer_host...
content/browser/renderer_host/image_transport_factory_android.cc:115:
kDefaultMaxTransferBufferSize));
Yes, we're not capping the starting size but we're capping the max size. The
default is 16MB.

[no sievers, 2013-06-18 17:15:02]

lgtm

[greggman, 2013-06-19 06:36:48]

lgtm

[commit-bot: I haz the power, 2013-06-19 06:37:30]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kaanb@chromium.org/17294003/5001

[commit-bot: I haz the power, 2013-06-19 06:37:33]

Failed to apply patch for
content/browser/renderer_host/image_transport_factory_android.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file content/browser/renderer_host/image_transport_factory_android.cc
  Hunk #1 succeeded at 13 (offset 1 line).
  Hunk #2 FAILED at 97.
  1 out of 2 hunks FAILED -- saving rejects to file
content/browser/renderer_host/image_transport_factory_android.cc.rej

Patch:       content/browser/renderer_host/image_transport_factory_android.cc
Index: content/browser/renderer_host/image_transport_factory_android.cc
diff --git a/content/browser/renderer_host/image_transport_factory_android.cc
b/content/browser/renderer_host/image_transport_factory_android.cc
index
9703f85f4d0690c28541ac050a0a910bf1b020a1..98a650eabf9f7965d1bf82f841b31a7e0997ef03
100644
--- a/content/browser/renderer_host/image_transport_factory_android.cc
+++ b/content/browser/renderer_host/image_transport_factory_android.cc
@@ -12,6 +12,7 @@
 #include "content/common/gpu/gpu_process_launch_causes.h"
 #include "third_party/WebKit/public/platform/WebGraphicsContext3D.h"
 #include "third_party/khronos/GLES2/gl2.h"
+#include "ui/gfx/android/device_display_info.h"
 #include
"webkit/common/gpu/webgraphicscontext3d_in_process_command_buffer_impl.h"
 
 namespace content {
@@ -96,10 +97,22 @@
CmdBufferImageTransportFactory::CmdBufferImageTransportFactory() {
                                                            url,
                                                            factory,
                                                            swap_client));
-  context_->InitializeWithDefaultBufferSizes(
+  static const size_t kBytesPerPixel = 4;
+  gfx::DeviceDisplayInfo display_info;
+  size_t full_screen_texture_size_in_bytes =
+      display_info.GetDisplayHeight() *
+      display_info.GetDisplayWidth() *
+      kBytesPerPixel;
+  context_->Initialize(
       attrs,
       false,
-      CAUSE_FOR_GPU_LAUNCH_WEBGRAPHICSCONTEXT3DCOMMANDBUFFERIMPL_INITIALIZE);
+      CAUSE_FOR_GPU_LAUNCH_WEBGRAPHICSCONTEXT3DCOMMANDBUFFERIMPL_INITIALIZE,
+      64 * 1024,  // command buffer size
+      std::min(full_screen_texture_size_in_bytes,
+               kDefaultStartTransferBufferSize),
+      kDefaultMinTransferBufferSize,
+      std::min(3 * full_screen_texture_size_in_bytes,
+               kDefaultMaxTransferBufferSize));
 }
 
 CmdBufferImageTransportFactory::~CmdBufferImageTransportFactory() {

[commit-bot: I haz the power, 2013-06-19 08:40:39]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kaanb@chromium.org/17294003/10001

[commit-bot: I haz the power, 2013-06-19 14:46:14]

Change committed as 207241