Use inner HWND for accelerated rendering on windows

chrome/gpu/gpu_command_buffer_stub.cc

Index: chrome/gpu/gpu_command_buffer_stub.cc
===================================================================
--- chrome/gpu/gpu_command_buffer_stub.cc	(revision 65702)
+++ chrome/gpu/gpu_command_buffer_stub.cc	(working copy)
@@ -38,10 +38,103 @@
       render_view_id_(render_view_id) {
 }
 
+#if defined(OS_WIN)
+static LRESULT CALLBACK CompositorWindowProc(HWND hWnd, UINT message,

[apatrick_chromium, 2010/11/11 22:32:15]

One parameter per line. hWnd, lParam and wParam should not be camel case.

[nduca, 2010/11/11 23:53:33]

Done.

+    WPARAM wParam, LPARAM lParam) {
+  switch (message) {
+  case WM_ERASEBKGND:
+    return 0;
+  case WM_DESTROY:
+    return 0;
+  case WM_PAINT:
+    {
+      PAINTSTRUCT paint;
+      BeginPaint(hWnd, &paint);
+      HANDLE h = GetProp(hWnd, L"CompositorWindowGpuCommandBufferStub");

[apatrick_chromium, 2010/11/11 22:32:15]

Can you make the property name a constant?

[nduca, 2010/11/11 23:53:33]

Done.

+      GpuCommandBufferStub* stub = reinterpret_cast<GpuCommandBufferStub*>(h);
+      stub->OnCompositorWindowPainted();
+      EndPaint(hWnd, &paint);
+    }
+    break;
+  case WM_CHAR:
+    return 0;

[apatrick_chromium, 2010/11/11 22:32:15]

Why handle WM_CHAR? Does setting WS_DISABLED not auomatically propagate input
events to the parent window?

[nduca, 2010/11/11 23:53:33]

This is largely a C&P of the code in render_widget_host_view_win.cc's
PluginHostWindow implementation. I'm not sure the motivation for it there,
either...

On 2010/11/11 22:32:15, apatrick_chromium wrote:

+  default:
+    return DefWindowProc(hWnd, message, wParam, lParam);
+  }
+  return 0;
+}
+
+void GpuCommandBufferStub::CreateCompositorWindow() {
+  DCHECK(handle_ != gfx::kNullPluginWindow);
+
+  // Ask the browser to create the the host window.
+  ChildThread* gpu_thread = ChildThread::current();
+  gfx::PluginWindowHandle host_window_id;
+  gpu_thread->Send(new GpuHostMsg_CreateCompositorHostWindow(renderer_id_,

[apatrick_chromium, 2010/11/11 22:32:15]

One argument per line.

[nduca, 2010/11/11 23:53:33]

Done.

+      render_view_id_, &host_window_id));
+  HWND host_window = static_cast<HWND>(host_window_id);
+
+  // Create the compositor window itself.
+  DCHECK(host_window);
+  static ATOM window_class = 0;
+  if (!window_class) {
+    WNDCLASSEX wcex;
+    wcex.cbSize         = sizeof(WNDCLASSEX);

[apatrick_chromium, 2010/11/11 22:32:15]

sizeof(wcex)

[nduca, 2010/11/11 23:53:33]

Done.

+    wcex.style          = 0;
+    wcex.lpfnWndProc    = CompositorWindowProc;
+    wcex.cbClsExtra     = 0;
+    wcex.cbWndExtra     = 0;
+    wcex.hInstance      = GetModuleHandle(NULL);
+    wcex.hIcon          = 0;
+    wcex.hCursor        = 0;
+    wcex.hbrBackground  = NULL;
+    wcex.lpszMenuName   = 0;
+    wcex.lpszClassName  = L"CompositorWindowClass";
+    wcex.hIconSm        = 0;
+    window_class = RegisterClassEx(&wcex);
+    DCHECK(window_class);
+  }
+
+  HWND compositor_window = CreateWindowEx(
+    WS_EX_LEFT | WS_EX_LTRREADING | WS_EX_RIGHTSCROLLBAR,

[apatrick_chromium, 2010/11/11 22:32:15]

One argument per line unless they are related, like width and height. You might
also comment the zeroes since it isn't obvious what they are.

[nduca, 2010/11/11 23:53:33]

Done.

+    MAKEINTATOM(window_class), 0,
+    WS_CHILD | WS_CLIPCHILDREN | WS_CLIPSIBLINGS | WS_DISABLED,
+    0, 0, 0, 0, host_window, 0, GetModuleHandle(NULL), 0);
+  DCHECK(compositor_window);
+  SetProp(compositor_window, L"CompositorWindowGpuCommandBufferStub",

[apatrick_chromium, 2010/11/11 22:32:15]

Use a string constant.

[nduca, 2010/11/11 23:53:33]

Done.

+      reinterpret_cast<HANDLE>(this));
+
+  RECT parent_rect;
+  GetWindowRect(host_window, &parent_rect);

[apatrick_chromium, 2010/11/11 22:32:15]

GetWindowRect or GetClientRect? Also, if this function fails, parent_rect will
be uninitialized and bad things might happen below.

[nduca, 2010/11/11 23:53:33]

Hahaha, I fail. Fixed.

I'm not convinced you can get in a situation where GetClientRect could fail here
--- short of the browser process itself exiting/crashing. We just made the
window about 15 lines above... I could put a guard on it, but we'd be hosed if
that happened anyway, no?

On 2010/11/11 22:32:15, apatrick_chromium wrote:

+
+  UINT flags = SWP_NOSENDCHANGING | SWP_NOCOPYBITS | SWP_NOZORDER |
+      SWP_NOACTIVATE | SWP_DEFERERASE | SWP_SHOWWINDOW;
+  SetWindowPos(compositor_window, NULL, 0, 0,

[apatrick_chromium, 2010/11/11 22:32:15]

One argument per line unless they are related, like width / height.

[nduca, 2010/11/11 23:53:33]

Done.

+      parent_rect.right - parent_rect.left,
+      parent_rect.bottom - parent_rect.top,
+      flags);
+  compositor_window_ = static_cast<gfx::PluginWindowHandle>(compositor_window);
+}
+
+void GpuCommandBufferStub::OnCompositorWindowPainted()
+{
+  ChildThread* gpu_thread = ChildThread::current();
+  gpu_thread->Send(new GpuHostMsg_ScheduleComposite(
+      renderer_id_, render_view_id_));
+}
+#endif // defined(OS_WIN)
+
+
 GpuCommandBufferStub::~GpuCommandBufferStub() {
   if (processor_.get()) {
     processor_->Destroy();
   }
+#if defined(OS_WIN)
+  if (compositor_window_) {
+    DestroyWindow(static_cast<HWND>(compositor_window_));

[apatrick_chromium, 2010/11/11 22:32:15]

Windows apparently lets properties leak when you destroy a window. Safer to call
RemoveProp before destroying.

[nduca, 2010/11/11 23:53:33]

Done.

+    compositor_window_ = NULL;
+  }
+#endif
 }
 
 void GpuCommandBufferStub::OnMessageReceived(const IPC::Message& message) {
@@ -79,6 +172,14 @@
 
   command_buffer_.reset(new gpu::CommandBufferService);
 
+  // Create the child window, if needed
+#if defined(OS_WIN)
+  CreateCompositorWindow();
+  gfx::PluginWindowHandle output_window_handle = compositor_window_;
+#else
+  gfx::PluginWindowHandle output_window_handle = handle_;
+#endif
+
   // Initialize the CommandBufferService and GPUProcessor.
   if (command_buffer_->Initialize(size)) {
     Buffer buffer = command_buffer_->GetRingBuffer();
@@ -87,7 +188,7 @@
           parent_ ? parent_->processor_.get() : NULL;
       processor_.reset(new gpu::GPUProcessor(command_buffer_.get(), NULL));
       if (processor_->Initialize(
-          handle_,
+          output_window_handle,
           initial_size_,
           allowed_extensions_.c_str(),
           requested_attribs_,