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

Issue 5105006: Resize synchronization for Linux. (Closed)

Created:
10 years, 1 month ago by jonathan.backer
Modified:
9 years, 7 months ago
CC:
chromium-reviews, brettw-cc_chromium.org, darin-cc_chromium.org, ben+cc_chromium.org, Nico, apatrick_chromium
Visibility:
Public.

Description

Resize synchronization for Linux. This patch makes synchronous calls from the GPU to the Browser process to resize windows. It must be synchronous because we must be sure when the resize happens, it must be initiated by the GPU because we have to time the resize with GL drawing, and the resize must be done by the Browser because of how GDK/GTK is structured. Specifically, when a window that a GL context is associated with is resized, the back buffer gets blanked. So it is important that we synchronize the resize with the drawing to the back buffer. On Linux, the X window that we are drawing to is wrapped in a GdkWindow inside the Browser process. GDK/GTK assumes that all changes to the window happen via GDK calls. In particular, the size of the window is cached inside the GdkWindow object so that it does not have to make a call to the X server in order to get window geometry. Unfortunately, this necessitates resizing the window inside of the Browser process. For more discussion of this approach and (some unsuccessfully attempted) alternatives see https://docs.google.com/a/google.com/document/d/1ZNouL-X_Ml1x8sqy-sofz63pDAeo36VWi_yQihaE2YI/edit?hl=en This patch set uncovered another bug: - open in two separate windows http://webkit.org/blog/386/3d-transforms/ and http://webkit.org/blog-files/3d-transforms/poster-circle.html - resize the former until it is smallish - watch the root layer of the former show up as the root layer of the later. To my knowledge, this is first trigger of this bug. If and when this patch is accepted, I will file the bug. BUG=http://code.google.com/p/chromium/issues/detail?id=54430 TEST=Go to http://peter.sh/2010/06/chromium-now-features-gpu-acceleration-and-css-3d-transforms/ . Rotate Z with the slider to trigger the compositor. Resize the window. The resize may be janky (we're uploading large textures), but it should display properly. Contributed by backer@chromium.org Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=67416

Patch Set 1 #

Total comments: 2

Patch Set 2 : Moved IPC call up to GpuCommandBufferStub via callback to satisfy checkdeps. #

Total comments: 17

Patch Set 3 : Handle resize in GpuCommandBufferStub instead of GLContext. #

Total comments: 1

Patch Set 4 : Fix nit. #

Total comments: 4

Patch Set 5 : Fix typo and add mock for unit tests. #

Patch Set 6 : Rebase off of head. #

Patch Set 7 : make -j17 all is my friend (fix the test build). #

Unified diffs Side-by-side diffs Delta from patch set Stats (+140 lines, -11 lines) Patch
M app/gfx/gl/gl_context.h View 2 1 chunk +0 lines, -5 lines 0 comments Download
M chrome/browser/gpu_process_host.h View 1 2 2 chunks +5 lines, -0 lines 0 comments Download
M chrome/browser/gpu_process_host.cc View 1 2 4 chunks +30 lines, -1 line 0 comments Download
M chrome/browser/renderer_host/render_widget_host.cc View 1 2 3 4 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/browser/renderer_host/render_widget_host_view.h View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/renderer_host/render_widget_host_view_gtk.h View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/renderer_host/render_widget_host_view_gtk.cc View 1 2 3 4 5 1 chunk +7 lines, -0 lines 0 comments Download
M chrome/browser/renderer_host/test/test_render_view_host.h View 1 chunk +1 line, -0 lines 0 comments Download
M chrome/common/gpu_messages_internal.h View 1 2 1 chunk +6 lines, -0 lines 0 comments Download
M chrome/gpu/gpu_command_buffer_stub.h View 1 chunk +4 lines, -0 lines 0 comments Download
M chrome/gpu/gpu_command_buffer_stub.cc View 2 3 2 chunks +17 lines, -0 lines 0 comments Download
M gfx/gtk_preserve_window.h View 1 chunk +6 lines, -0 lines 0 comments Download
M gfx/gtk_preserve_window.cc View 1 2 4 chunks +33 lines, -3 lines 0 comments Download
M gpu/command_buffer/service/gles2_cmd_decoder.h View 1 chunk +4 lines, -0 lines 0 comments Download
M gpu/command_buffer/service/gles2_cmd_decoder.cc View 3 4 4 chunks +14 lines, -2 lines 0 comments Download
M gpu/command_buffer/service/gles2_cmd_decoder_mock.h View 1 chunk +1 line, -0 lines 0 comments Download
M gpu/command_buffer/service/gpu_processor.h View 1 chunk +4 lines, -0 lines 0 comments Download
M gpu/command_buffer/service/gpu_processor.cc View 2 1 chunk +4 lines, -0 lines 0 comments Download

Messages

Total messages: 10 (0 generated)
jonathan.backer
10 years, 1 month ago (2010-11-19 22:46:07 UTC) #1
Ken Russell (switch to Gerrit)
http://codereview.chromium.org/5105006/diff/1/app/gfx/gl/gl_context_linux.cc File app/gfx/gl/gl_context_linux.cc (right): http://codereview.chromium.org/5105006/diff/1/app/gfx/gl/gl_context_linux.cc#newcode329 app/gfx/gl/gl_context_linux.cc:329: new GpuHostMsg_ResizeXID(window_, size.width(), size.height(), &result)); This violates a large ...
10 years, 1 month ago (2010-11-22 22:49:52 UTC) #2
jonathan.backer
http://codereview.chromium.org/5105006/diff/1/app/gfx/gl/gl_context_linux.cc File app/gfx/gl/gl_context_linux.cc (right): http://codereview.chromium.org/5105006/diff/1/app/gfx/gl/gl_context_linux.cc#newcode329 app/gfx/gl/gl_context_linux.cc:329: new GpuHostMsg_ResizeXID(window_, size.width(), size.height(), &result)); On 2010/11/22 22:49:52, kbr ...
10 years, 1 month ago (2010-11-23 16:41:12 UTC) #3
nduca
http://codereview.chromium.org/5105006/diff/5001/app/gfx/gl/gl_context.h File app/gfx/gl/gl_context.h (right): http://codereview.chromium.org/5105006/diff/5001/app/gfx/gl/gl_context.h#newcode42 app/gfx/gl/gl_context.h:42: // Set the size of the back buffer. Calls ...
10 years, 1 month ago (2010-11-23 16:59:48 UTC) #4
Ken Russell (switch to Gerrit)
Nat has a good point that the resize callback could be handled entirely on the ...
10 years, 1 month ago (2010-11-23 19:17:55 UTC) #5
jonathan.backer
http://codereview.chromium.org/5105006/diff/5001/app/gfx/gl/gl_context.h File app/gfx/gl/gl_context.h (right): http://codereview.chromium.org/5105006/diff/5001/app/gfx/gl/gl_context.h#newcode42 app/gfx/gl/gl_context.h:42: // Set the size of the back buffer. Calls ...
10 years, 1 month ago (2010-11-23 21:00:18 UTC) #6
nduca
lgtm, mod nits http://codereview.chromium.org/5105006/diff/18001/chrome/gpu/gpu_command_buffer_stub.cc File chrome/gpu/gpu_command_buffer_stub.cc (right): http://codereview.chromium.org/5105006/diff/18001/chrome/gpu/gpu_command_buffer_stub.cc#newcode117 chrome/gpu/gpu_command_buffer_stub.cc:117: #if defined(OS_LINUX) hmm, elif?
10 years, 1 month ago (2010-11-23 21:03:51 UTC) #7
Ken Russell (switch to Gerrit)
http://codereview.chromium.org/5105006/diff/36001/gpu/command_buffer/service/gles2_cmd_decoder.cc File gpu/command_buffer/service/gles2_cmd_decoder.cc (right): http://codereview.chromium.org/5105006/diff/36001/gpu/command_buffer/service/gles2_cmd_decoder.cc#newcode2561 gpu/command_buffer/service/gles2_cmd_decoder.cc:2561: #if defined(LINUX) #if defined(OS_LINUX) http://codereview.chromium.org/5105006/diff/36001/gpu/command_buffer/service/gles2_cmd_decoder.h File gpu/command_buffer/service/gles2_cmd_decoder.h (right): http://codereview.chromium.org/5105006/diff/36001/gpu/command_buffer/service/gles2_cmd_decoder.h#newcode94 ...
10 years, 1 month ago (2010-11-23 22:01:17 UTC) #8
jonathan.backer
http://codereview.chromium.org/5105006/diff/36001/gpu/command_buffer/service/gles2_cmd_decoder.cc File gpu/command_buffer/service/gles2_cmd_decoder.cc (right): http://codereview.chromium.org/5105006/diff/36001/gpu/command_buffer/service/gles2_cmd_decoder.cc#newcode2561 gpu/command_buffer/service/gles2_cmd_decoder.cc:2561: #if defined(LINUX) On 2010/11/23 22:01:17, kbr wrote: > #if ...
10 years, 1 month ago (2010-11-24 14:14:45 UTC) #9
Ken Russell (switch to Gerrit)
10 years, 1 month ago (2010-11-24 21:00:11 UTC) #10
LGTM

Powered by Google App Engine
This is Rietveld 408576698