diff options
| -rw-r--r-- | chrome/browser/renderer_host/backing_store.h | 14 | ||||
| -rw-r--r-- | chrome/browser/renderer_host/backing_store_mac.cc | 18 | ||||
| -rw-r--r-- | chrome/browser/renderer_host/backing_store_manager.cc | 12 | ||||
| -rw-r--r-- | chrome/browser/renderer_host/backing_store_manager.h | 1 | ||||
| -rw-r--r-- | chrome/browser/renderer_host/backing_store_win.cc | 43 | ||||
| -rw-r--r-- | chrome/browser/renderer_host/backing_store_x.cc | 121 | ||||
| -rw-r--r-- | chrome/browser/renderer_host/render_widget_host.cc | 92 | ||||
| -rw-r--r-- | chrome/browser/renderer_host/render_widget_host.h | 7 | ||||
| -rw-r--r-- | chrome/browser/views/frame/browser_view.cc | 10 | ||||
| -rw-r--r-- | chrome/common/render_messages.h | 13 | ||||
| -rw-r--r-- | chrome/renderer/render_widget.cc | 170 | ||||
| -rw-r--r-- | chrome/renderer/render_widget.h | 5 |
12 files changed, 384 insertions, 122 deletions
diff --git a/chrome/browser/renderer_host/backing_store.h b/chrome/browser/renderer_host/backing_store.h index 5d817d0..5bb3945 100644 --- a/chrome/browser/renderer_host/backing_store.h +++ b/chrome/browser/renderer_host/backing_store.h @@ -63,9 +63,12 @@ class BackingStore { #endif // Paints the bitmap from the renderer onto the backing store. + // bitmap_rect is the rect of the whole bitmap, and paint_rect + // is a sub-rect of bitmap that we want to draw. void PaintRect(base::ProcessHandle process, TransportDIB* bitmap, - const gfx::Rect& bitmap_rect); + const gfx::Rect& bitmap_rect, + const gfx::Rect& paint_rect); // Scrolls the given rect in the backing store, replacing the given region // identified by |bitmap_rect| by the bitmap in the file identified by the @@ -95,8 +98,11 @@ class BackingStore { #elif defined(OS_LINUX) // Paints the bitmap from the renderer onto the backing store without // using Xrender to composite the pixmaps. + // bitmap_rect is the rect of the whole bitmap, and paint_rect + // is a sub-rect of bitmap that we want to draw. void PaintRectWithoutXrender(TransportDIB* bitmap, - const gfx::Rect& bitmap_rect); + const gfx::Rect& bitmap_rect, + const gfx::Rect& paint_rect); // This is the connection to the X server where this backing store will be // displayed. @@ -119,6 +125,10 @@ class BackingStore { void* pixmap_gc_; #endif + // Sanity checks on the size of the rects to draw so that we don't allocate + // enourmous pixmaps. + static int kMaxBitmapLengthAllowed; + DISALLOW_COPY_AND_ASSIGN(BackingStore); }; diff --git a/chrome/browser/renderer_host/backing_store_mac.cc b/chrome/browser/renderer_host/backing_store_mac.cc index 569e3f6..8983208 100644 --- a/chrome/browser/renderer_host/backing_store_mac.cc +++ b/chrome/browser/renderer_host/backing_store_mac.cc @@ -21,14 +21,24 @@ BackingStore::~BackingStore() { void BackingStore::PaintRect(base::ProcessHandle process, TransportDIB* bitmap, - const gfx::Rect& bitmap_rect) { + const gfx::Rect& bitmap_rect, + const gfx::Rect& paint_rect) { + DCHECK(bitmap_rect.Contains(paint_rect) && + paint_rect.x() < kMaxBitmapLengthAllowed && + paint_rect.y() < kMaxBitmapLengthAllowed); SkBitmap skbitmap; skbitmap.setConfig(SkBitmap::kARGB_8888_Config, bitmap_rect.width(), bitmap_rect.height(), 4 * bitmap_rect.width()); skbitmap.setPixels(bitmap->memory()); - - canvas_.drawBitmap(skbitmap, bitmap_rect.x(), bitmap_rect.y()); + SkIRect src_rect; + src_rect.set(paint_rect.x(), paint_rect.y(), + paint_rect.right(), paint_rect.bottom()); + src_rect.offset(-bitmap_rect.x(), -bitmap_rect.y()); + SkRect dst_rect; + dst_rect.iset(paint_rect.x(), paint_rect.y(), + paint_rect.right(), paint_rect.bottom()); + canvas_.drawBitmapRect(skbitmap, &src_rect, dst_rect); } void BackingStore::ScrollRect(base::ProcessHandle process, @@ -118,6 +128,6 @@ void BackingStore::ScrollRect(base::ProcessHandle process, } // Now paint the new bitmap data. - PaintRect(process, bitmap, bitmap_rect); + PaintRect(process, bitmap, bitmap_rect, bitmap_rect); return; } diff --git a/chrome/browser/renderer_host/backing_store_manager.cc b/chrome/browser/renderer_host/backing_store_manager.cc index 3ff2e76..eb3600e 100644 --- a/chrome/browser/renderer_host/backing_store_manager.cc +++ b/chrome/browser/renderer_host/backing_store_manager.cc @@ -49,6 +49,11 @@ BackingStore* CreateBackingStore(RenderWidgetHost* host, } // namespace +// Assume that somewhere along the line, someone will do width * height * 4 +// with signed numbers. If the maximum value is 2**31, then 2**31 / 4 = +// 2**29 and floor(sqrt(2**29)) = 23170. +int BackingStore::kMaxBitmapLengthAllowed = 23170; + // BackingStoreManager --------------------------------------------------------- // static @@ -74,13 +79,14 @@ BackingStore* BackingStoreManager::PrepareBackingStore( base::ProcessHandle process_handle, TransportDIB* bitmap, const gfx::Rect& bitmap_rect, + const gfx::Rect& paint_rect, bool* needs_full_paint) { BackingStore* backing_store = GetBackingStore(host, backing_store_size); if (!backing_store) { // We need to get Webkit to generate a new paint here, as we // don't have a previous snapshot. - if (bitmap_rect.size() != backing_store_size || - bitmap_rect.x() != 0 || bitmap_rect.y() != 0) { + if (paint_rect.size() != backing_store_size || + paint_rect.x() != 0 || paint_rect.y() != 0) { DCHECK(needs_full_paint != NULL); *needs_full_paint = true; } @@ -88,7 +94,7 @@ BackingStore* BackingStoreManager::PrepareBackingStore( } DCHECK(backing_store != NULL); - backing_store->PaintRect(process_handle, bitmap, bitmap_rect); + backing_store->PaintRect(process_handle, bitmap, bitmap_rect, paint_rect); return backing_store; } diff --git a/chrome/browser/renderer_host/backing_store_manager.h b/chrome/browser/renderer_host/backing_store_manager.h index 5b929f6..d273c17 100644 --- a/chrome/browser/renderer_host/backing_store_manager.h +++ b/chrome/browser/renderer_host/backing_store_manager.h @@ -49,6 +49,7 @@ class BackingStoreManager { base::ProcessHandle process_handle, TransportDIB* bitmap, const gfx::Rect& bitmap_rect, + const gfx::Rect& paint_rect, bool* needs_full_paint); // Returns a matching backing store for the host. diff --git a/chrome/browser/renderer_host/backing_store_win.cc b/chrome/browser/renderer_host/backing_store_win.cc index 1241e42..edb861d 100644 --- a/chrome/browser/renderer_host/backing_store_win.cc +++ b/chrome/browser/renderer_host/backing_store_win.cc @@ -54,7 +54,11 @@ BackingStore::~BackingStore() { void BackingStore::PaintRect(base::ProcessHandle process, TransportDIB* bitmap, - const gfx::Rect& bitmap_rect) { + const gfx::Rect& bitmap_rect, + const gfx::Rect& paint_rect) { + DCHECK(bitmap_rect.Contains(paint_rect) && + paint_rect.x() < kMaxBitmapLengthAllowed && + paint_rect.y() < kMaxBitmapLengthAllowed); if (!backing_store_dib_) { backing_store_dib_ = CreateDIB(hdc_, size_.width(), size_.height(), color_depth_); @@ -67,16 +71,33 @@ void BackingStore::PaintRect(base::ProcessHandle process, BITMAPINFOHEADER hdr; gfx::CreateBitmapHeader(bitmap_rect.width(), bitmap_rect.height(), &hdr); - // Account for a bitmap_rect that exceeds the bounds of our view + // Account for a paint_rect that exceeds the bounds of our view. gfx::Rect view_rect(0, 0, size_.width(), size_.height()); - gfx::Rect paint_rect = view_rect.Intersect(bitmap_rect); - - int rv = StretchDIBits(hdc_, - paint_rect.x(), paint_rect.y(), - paint_rect.width(), paint_rect.height(), - 0, 0, // source x,y. - paint_rect.width(), paint_rect.height(), - bitmap->memory(), + gfx::Rect paint_view_rect = view_rect.Intersect(paint_rect); + + // CreateBitmapHeader specifies a negative height, + // so the y destination is from the bottom. + int source_x = paint_view_rect.x() - bitmap_rect.x(); + int source_y = bitmap_rect.bottom() - paint_view_rect.bottom(); + int source_height = paint_view_rect.height(); + int destination_y = paint_view_rect.y(); + int destination_height = paint_view_rect.height(); + if (source_y == 0 && source_x == 0 && + paint_view_rect.height() != bitmap_rect.height()) { + // StretchDIBits has a bug where it won't take the proper source + // rect if it starts at (0, 0) in the source but not in the destination, + // so we must use a mirror blit trick as proposed here: + // http://wiki.allegro.cc/index.php?title=StretchDIBits + destination_y += destination_height - 1; + destination_height = -destination_height; + source_y = bitmap_rect.height() - paint_view_rect.y() + 1; + source_height = -source_height; + } + + int rv = StretchDIBits(hdc_, paint_view_rect.x(), destination_y, + paint_view_rect.width(), destination_height, + source_x, source_y, paint_view_rect.width(), + source_height, bitmap->memory(), reinterpret_cast<BITMAPINFO*>(&hdr), DIB_RGB_COLORS, SRCCOPY); DCHECK(rv != GDI_ERROR); @@ -97,5 +118,5 @@ void BackingStore::ScrollRect(base::ProcessHandle process, // We expect that damaged_rect should equal bitmap_rect. DCHECK(gfx::Rect(damaged_rect) == bitmap_rect); - PaintRect(process, bitmap, bitmap_rect); + PaintRect(process, bitmap, bitmap_rect, bitmap_rect); } diff --git a/chrome/browser/renderer_host/backing_store_x.cc b/chrome/browser/renderer_host/backing_store_x.cc index 6cb7c91..22187ee 100644 --- a/chrome/browser/renderer_host/backing_store_x.cc +++ b/chrome/browser/renderer_host/backing_store_x.cc @@ -77,17 +77,20 @@ BackingStore::~BackingStore() { } void BackingStore::PaintRectWithoutXrender(TransportDIB* bitmap, - const gfx::Rect &bitmap_rect) { - const int width = bitmap_rect.width(); - const int height = bitmap_rect.height(); - Pixmap pixmap = XCreatePixmap(display_, root_window_, width, height, - visual_depth_); - + const gfx::Rect& bitmap_rect, + const gfx::Rect& paint_rect) { + const int paint_width = paint_rect.width(); + const int paint_height = paint_rect.height(); + Pixmap pixmap = XCreatePixmap(display_, root_window_, paint_width, + paint_height, visual_depth_); + + const int bitmap_width = bitmap_rect.width(); + const int bitmap_height = bitmap_rect.height(); XImage image; memset(&image, 0, sizeof(image)); - image.width = width; - image.height = height; + image.width = bitmap_width; + image.height = bitmap_height; image.format = ZPixmap; image.byte_order = LSBFirst; image.bitmap_unit = 8; @@ -96,16 +99,18 @@ void BackingStore::PaintRectWithoutXrender(TransportDIB* bitmap, image.green_mask = 0xff00; image.blue_mask = 0xff0000; + const int x_offset = paint_rect.x() - bitmap_rect.x(); + const int y_offset = paint_rect.y() - bitmap_rect.y(); if (pixmap_bpp_ == 32) { // If the X server depth is already 32-bits, then our job is easy. image.depth = visual_depth_; image.bits_per_pixel = 32; - image.bytes_per_line = width * 4; + image.bytes_per_line = bitmap_width * 4; image.data = static_cast<char*>(bitmap->memory()); XPutImage(display_, pixmap, static_cast<GC>(pixmap_gc_), &image, - 0, 0 /* source x, y */, 0, 0 /* dest x, y */, - width, height); + x_offset, y_offset /* source x, y */, 0, 0 /* dest x, y */, + paint_width, paint_height); } else if (pixmap_bpp_ == 24) { // In this case we just need to strip the alpha channel out of each // pixel. This is the case which covers VNC servers since they don't @@ -114,52 +119,68 @@ void BackingStore::PaintRectWithoutXrender(TransportDIB* bitmap, // It's possible to use some fancy SSE tricks here, but since this is the // slow path anyway, we do it slowly. - uint8_t* bitmap24 = static_cast<uint8_t*>(malloc(3 * width * height)); + uint8_t* bitmap24 = static_cast<uint8_t*>(malloc(3 * paint_width * + paint_height)); if (!bitmap24) return; const uint32_t* bitmap_in = static_cast<const uint32_t*>(bitmap->memory()); - for (int y = 0; y < height; ++y) { - for (int x = 0; x < width; ++x) { + const int x_limit = paint_rect.right() - bitmap_rect.x(); + const int y_limit = paint_rect.bottom() - bitmap_rect.y(); + const int row_length = bitmap_width; + bitmap_in += row_length * y_offset; + for (int y = y_offset; y < y_limit; ++y) { + bitmap_in += x_offset; + for (int x = x_offset; x < x_limit; ++x) { const uint32_t pixel = *(bitmap_in++); bitmap24[0] = (pixel >> 16) & 0xff; bitmap24[1] = (pixel >> 8) & 0xff; bitmap24[2] = pixel & 0xff; bitmap24 += 3; } + bitmap_in += row_length - x_limit; } + image.width = paint_width; + image.height = paint_height; image.depth = visual_depth_; image.bits_per_pixel = 24; - image.bytes_per_line = width * 3; + image.bytes_per_line = paint_width * 3; image.data = reinterpret_cast<char*>(bitmap24); XPutImage(display_, pixmap, static_cast<GC>(pixmap_gc_), &image, 0, 0 /* source x, y */, 0, 0 /* dest x, y */, - width, height); + paint_width, paint_height); free(bitmap24); } else if (pixmap_bpp_ == 16) { // Some folks have VNC setups which still use 16-bit visuals and VNC // doesn't include Xrender. - uint16_t* bitmap16 = static_cast<uint16_t*>(malloc(2 * width * height)); + uint16_t* bitmap16 = static_cast<uint16_t*>(malloc(2 * paint_width * + paint_height)); if (!bitmap16) return; uint16_t* const orig_bitmap16 = bitmap16; const uint32_t* bitmap_in = static_cast<const uint32_t*>(bitmap->memory()); - for (int y = 0; y < height; ++y) { - for (int x = 0; x < width; ++x) { + const int x_limit = paint_rect.right() - bitmap_rect.x(); + const int y_limit = paint_rect.bottom() - bitmap_rect.y(); + const int row_length = bitmap_width; + bitmap_in += row_length * y_offset; + for (int y = y_offset; y < y_limit; ++y) { + bitmap_in += x_offset; + for (int x = x_offset; x < x_limit; ++x) { const uint32_t pixel = *(bitmap_in++); uint16_t out_pixel = ((pixel >> 8) & 0xf800) | ((pixel >> 5) & 0x07e0) | ((pixel >> 3) & 0x001f); *(bitmap16++) = out_pixel; } + bitmap_in += row_length - x_limit; } image.depth = visual_depth_; image.bits_per_pixel = 16; - image.bytes_per_line = width * 2; + image.bytes_per_line = paint_width * 2; image.data = reinterpret_cast<char*>(orig_bitmap16); image.red_mask = 0xf800; @@ -168,7 +189,7 @@ void BackingStore::PaintRectWithoutXrender(TransportDIB* bitmap, XPutImage(display_, pixmap, static_cast<GC>(pixmap_gc_), &image, 0, 0 /* source x, y */, 0, 0 /* dest x, y */, - width, height); + paint_width, paint_height); free(orig_bitmap16); } else { CHECK(false) << "Sorry, we don't support your visual depth without " @@ -178,31 +199,37 @@ void BackingStore::PaintRectWithoutXrender(TransportDIB* bitmap, XCopyArea(display_, pixmap /* source */, pixmap_ /* target */, static_cast<GC>(pixmap_gc_), - 0, 0 /* source x, y */, bitmap_rect.width(), bitmap_rect.height(), - bitmap_rect.x(), bitmap_rect.y() /* dest x, y */); + 0, 0 /* source x, y */, paint_width, paint_height, + paint_rect.x(), paint_rect.y() /* dest x, y */); XFreePixmap(display_, pixmap); } void BackingStore::PaintRect(base::ProcessHandle process, TransportDIB* bitmap, - const gfx::Rect& bitmap_rect) { + const gfx::Rect& bitmap_rect, + const gfx::Rect& paint_rect) { + DCHECK(bitmap_rect.Contains(paint_rect)); if (!display_) return; if (bitmap_rect.IsEmpty()) return; - const int width = bitmap_rect.width(); - const int height = bitmap_rect.height(); - // Assume that somewhere along the line, someone will do width * height * 4 - // with signed numbers. If the maximum value is 2**31, then 2**31 / 4 = - // 2**29 and floor(sqrt(2**29)) = 23170. - if (width > 23170 || height > 23170) + const int paint_width = paint_rect.width(); + const int paint_height = paint_rect.height(); + + // For the cases where we only use the size of the paint_rect for the bitmap + // we start at (0,0) but we use a local var to allow setting elsewhere + // if we get to use the whole bitmap_rect size for the bitmap. + int src_x = 0; + int src_y = 0; + if (paint_width > kMaxBitmapLengthAllowed || + paint_width > kMaxBitmapLengthAllowed) return; if (!use_render_) - return PaintRectWithoutXrender(bitmap, bitmap_rect); + return PaintRectWithoutXrender(bitmap, bitmap_rect, paint_rect); Picture picture; Pixmap pixmap; @@ -221,8 +248,12 @@ void BackingStore::PaintRect(base::ProcessHandle process, // difference between the |data| pointer and the address of the mapping in // |shminfo|. Since both are NULL, the offset will be calculated to be 0, // which is correct for us. - pixmap = XShmCreatePixmap(display_, root_window_, NULL, &shminfo, width, - height, 32); + pixmap = XShmCreatePixmap(display_, root_window_, NULL, &shminfo, + bitmap_rect.width(), bitmap_rect.height(), 32); + + // Since we use the whole source bitmap, we must offset the source. + src_x = paint_rect.x() - bitmap_rect.x(); + src_y = paint_rect.y() - bitmap_rect.y(); } else { // No shared memory support, we have to copy the bitmap contents to the X // server. Xlib wraps the underlying PutImage call behind several layers of @@ -231,34 +262,36 @@ void BackingStore::PaintRect(base::ProcessHandle process, XImage image; memset(&image, 0, sizeof(image)); - image.width = width; - image.height = height; + image.width = paint_width; + image.height = paint_height; image.depth = 32; image.bits_per_pixel = 32; image.format = ZPixmap; image.byte_order = LSBFirst; image.bitmap_unit = 8; image.bitmap_bit_order = LSBFirst; - image.bytes_per_line = width * 4; + image.bytes_per_line = paint_width * 4; image.red_mask = 0xff; image.green_mask = 0xff00; image.blue_mask = 0xff0000; + // TODO(agl): check if we can make this more efficient. image.data = static_cast<char*>(bitmap->memory()); - pixmap = XCreatePixmap(display_, root_window_, width, height, 32); + pixmap = XCreatePixmap(display_, root_window_, paint_width, paint_height, + 32); GC gc = XCreateGC(display_, pixmap, 0, NULL); - XPutImage(display_, pixmap, gc, &image, - 0, 0 /* source x, y */, 0, 0 /* dest x, y */, - width, height); + XPutImage(display_, pixmap, gc, &image, paint_rect.x() - bitmap_rect.x(), + paint_rect.y() - bitmap_rect.y() /* source x, y */, + 0, 0 /* dest x, y */, paint_width, paint_height); XFreeGC(display_, gc); } picture = x11_util::CreatePictureFromSkiaPixmap(display_, pixmap); XRenderComposite(display_, PictOpSrc, picture /* source */, 0 /* mask */, - picture_ /* dest */, 0, 0 /* source x, y */, + picture_ /* dest */, src_x, src_y /* source x, y */, 0, 0 /* mask x, y */, - bitmap_rect.x(), bitmap_rect.y() /* target x, y */, - width, height); + paint_rect.x(), paint_rect.y() /* target x, y */, + paint_width, paint_height); // In the case of shared memory, we wait for the composite to complete so that // we are sure that the X server has finished reading. @@ -305,7 +338,7 @@ void BackingStore::ScrollRect(base::ProcessHandle process, } } - PaintRect(process, bitmap, bitmap_rect); + PaintRect(process, bitmap, bitmap_rect, bitmap_rect); } void BackingStore::ShowRect(const gfx::Rect& rect, XID target) { diff --git a/chrome/browser/renderer_host/render_widget_host.cc b/chrome/browser/renderer_host/render_widget_host.cc index fb6303b..95e7cf3 100644 --- a/chrome/browser/renderer_host/render_widget_host.cc +++ b/chrome/browser/renderer_host/render_widget_host.cc @@ -14,6 +14,7 @@ #include "chrome/browser/renderer_host/render_widget_host_view.h" #include "chrome/common/notification_service.h" #include "chrome/common/render_messages.h" +#include "skia/ext/platform_canvas.h" #include "views/view.h" #include "webkit/glue/webcursor.h" #include "webkit/glue/webtextdirection.h" @@ -41,6 +42,32 @@ static const int kPaintMsgTimeoutMS = 40; // How long to wait before we consider a renderer hung. static const int kHungRendererDelayMs = 20000; +// Helper function to confirm if a set of rects completely cover a given area. +// The caller already know that all the rects are contained within the area, +// and they shold not overlap either, so all we need to do is compare the areas. +static bool CoversArea(const std::vector<gfx::Rect>& rects, + const gfx::Size& size) { +#ifndef NDEBUG + // Make sure there are no overlapping rects. That would make the + // test below irrelevant. We only do it in the debug build since + // this case "should" be covered in the renderer. + gfx::Rect view_rect(size.width(), size.height()); + size_t last_index = rects.size() - 1; + for (size_t i = 0; i < last_index; ++i) { + DCHECK(view_rect.Contains(rects[i])); + for (size_t j = i + 1; i < rects.size(); ++i) + DCHECK(!rects[i].Intersects(rects[j])); + } + DCHECK(view_rect.Contains(rects[last_index])); +#endif + int target_area = size.height() * size.width(); + int covered_area = 0; + for (size_t i = 0; i < rects.size(); ++i) { + covered_area += rects[i].height() * rects[i].width(); + } + return covered_area < target_area; +} + /////////////////////////////////////////////////////////////////////////////// // RenderWidgetHost @@ -498,18 +525,19 @@ void RenderWidgetHost::OnMsgPaintRect( DCHECK(!params.bitmap_rect.IsEmpty()); DCHECK(!params.view_size.IsEmpty()); - const size_t size = params.bitmap_rect.height() * - params.bitmap_rect.width() * 4; TransportDIB* dib = process_->GetTransportDIB(params.bitmap); if (dib) { + const size_t size = params.bitmap_rect.height() * + skia::PlatformCanvas::StrideForWidth(params.bitmap_rect.width()); if (dib->size() < size) { DLOG(WARNING) << "Transport DIB too small for given rectangle"; process()->ReceivedBadMessage(ViewHostMsg_PaintRect__ID); } else { - // Paint the backing store. This will update it with the renderer-supplied - // bits. The view will read out of the backing store later to actually - // draw to the screen. - PaintBackingStoreRect(dib, params.bitmap_rect, params.view_size); + // Paint the backing store. This will update it with the + // renderer-supplied bits. The view will read out of the backing + // store later to actually draw to the screen. + PaintBackingStoreRects(dib, params.bitmap_rect, params.paint_rects, + params.view_size); } } @@ -529,7 +557,12 @@ void RenderWidgetHost::OnMsgPaintRect( if (view_) { view_->MovePluginWindows(params.plugin_window_moves); view_being_painted_ = true; - view_->DidPaintRect(params.bitmap_rect); + if (params.paint_rects.empty()) { + view_->DidPaintRect(params.bitmap_rect); + } else { + for (size_t i = 0; i < params.paint_rects.size(); ++i) + view_->DidPaintRect(params.paint_rects[i]); + } view_being_painted_ = false; } @@ -620,7 +653,7 @@ void RenderWidgetHost::OnMsgInputEventAck(const IPC::Message& message) { } if (WebInputEvent::isKeyboardEventType(type)) { - if (key_queue_.size() == 0) { + if (key_queue_.empty()) { LOG(ERROR) << "Got a KeyEvent back from the renderer but we " << "don't seem to have sent it to the renderer!"; } else if (key_queue_.front().type != type) { @@ -688,9 +721,10 @@ void RenderWidgetHost::OnMsgShowPopup(const IPC::Message& message) { #endif } -void RenderWidgetHost::PaintBackingStoreRect(TransportDIB* bitmap, - const gfx::Rect& bitmap_rect, - const gfx::Size& view_size) { +void RenderWidgetHost::PaintBackingStoreRects( + TransportDIB* bitmap, const gfx::Rect& bitmap_rect, + const std::vector<gfx::Rect>& paint_rects, + const gfx::Size& view_size) { // The view may be destroyed already. if (!view_) return; @@ -704,12 +738,36 @@ void RenderWidgetHost::PaintBackingStoreRect(TransportDIB* bitmap, } bool needs_full_paint = false; - BackingStore* backing_store = - BackingStoreManager::PrepareBackingStore(this, view_size, - process_->process().handle(), - bitmap, bitmap_rect, - &needs_full_paint); - DCHECK(backing_store != NULL); + base::ProcessHandle process_handle = process_->process().handle(); + if (paint_rects.empty()) { + BackingStore* backing_store = + BackingStoreManager::PrepareBackingStore(this, view_size, + process_handle, bitmap, bitmap_rect, bitmap_rect, + &needs_full_paint); + DCHECK(backing_store != NULL); + } else { + bool checked_coverage = false; + // TODO(agl): Reduce the number of X server round trips on Linux. + for (size_t i = 0; i < paint_rects.size(); ++i) { + BackingStore* backing_store = + BackingStoreManager::PrepareBackingStore(this, view_size, + process_handle, bitmap, bitmap_rect, paint_rects[i], + &needs_full_paint); + DCHECK(backing_store != NULL); + if (needs_full_paint) { + // We should not need a full paint more than once for a given view size + DCHECK(!checked_coverage); + checked_coverage = true; + + // Before we ask for a full repaint, we check if we already have a + // full coverage of the view size in out list of paint_rects + if (CoversArea(paint_rects, view_size)) + break; + needs_full_paint = false; + } + } + } + if (needs_full_paint) { repaint_start_time_ = TimeTicks::Now(); repaint_ack_pending_ = true; diff --git a/chrome/browser/renderer_host/render_widget_host.h b/chrome/browser/renderer_host/render_widget_host.h index 8ac2376..886bd2c 100644 --- a/chrome/browser/renderer_host/render_widget_host.h +++ b/chrome/browser/renderer_host/render_widget_host.h @@ -328,9 +328,10 @@ class RenderWidgetHost : public IPC::Channel::Listener { void OnMsgShowPopup(const IPC::Message& message); // Paints the given bitmap to the current backing store at the given location. - void PaintBackingStoreRect(TransportDIB* dib, - const gfx::Rect& bitmap_rect, - const gfx::Size& view_size); + void PaintBackingStoreRects(TransportDIB* bitmap, + const gfx::Rect& bitmap_rect, + const std::vector<gfx::Rect>& paint_rects, + const gfx::Size& view_size); // Scrolls the given |clip_rect| in the backing by the given dx/dy amount. The // |dib| and its corresponding location |bitmap_rect| in the backing store diff --git a/chrome/browser/views/frame/browser_view.cc b/chrome/browser/views/frame/browser_view.cc index 2665610..2a6ea40 100644 --- a/chrome/browser/views/frame/browser_view.cc +++ b/chrome/browser/views/frame/browser_view.cc @@ -134,11 +134,8 @@ class ResizeCorner : public views::View { } static gfx::Size GetSize() { - // This is disabled until we find what makes us slower when we let - // WebKit know that we have a resizer rect... - // return gfx::Size(views::NativeScrollBar::GetVerticalScrollBarWidth(), - // views::NativeScrollBar::GetHorizontalScrollBarHeight()); - return gfx::Size(); + return gfx::Size(views::NativeScrollBar::GetVerticalScrollBarWidth(), + views::NativeScrollBar::GetHorizontalScrollBarHeight()); } virtual gfx::Size GetPreferredSize() { @@ -1329,7 +1326,8 @@ void BrowserView::Init() { #if defined(OS_WIN) SetProp(GetWidget()->GetNativeView(), kBrowserViewKey, this); #else - g_object_set_data(G_OBJECT(GetWidget()->GetNativeView()), kBrowserViewKey, this); + g_object_set_data(G_OBJECT(GetWidget()->GetNativeView()), kBrowserViewKey, + this); #endif // Start a hung plugin window detector for this browser object (as long as diff --git a/chrome/common/render_messages.h b/chrome/common/render_messages.h index 57cdcc8..96ffd631 100644 --- a/chrome/common/render_messages.h +++ b/chrome/common/render_messages.h @@ -159,12 +159,17 @@ struct ViewHostMsg_PaintRect_Flags { }; struct ViewHostMsg_PaintRect_Params { - // The bitmap to be painted into the rect given by bitmap_rect. + // The bitmap to be painted into the rects given by paint_rects. TransportDIB::Id bitmap; - // The position and size of the bitmap. + // The position and size of the the bitmap relative to the view port. gfx::Rect bitmap_rect; + // The position and size of the rects where the bitmap has data to be + // painted to the view. An empty vector means that the whole bitmap has + // data to be painted to the view. + std::vector<gfx::Rect> paint_rects; + // The size of the RenderView when this message was generated. This is // included so the host knows how large the view is from the perspective of // the renderer process. This is necessary in case a resize operation is in @@ -860,6 +865,7 @@ struct ParamTraits<ViewHostMsg_PaintRect_Params> { static void Write(Message* m, const param_type& p) { WriteParam(m, p.bitmap); WriteParam(m, p.bitmap_rect); + WriteParam(m, p.paint_rects); WriteParam(m, p.view_size); WriteParam(m, p.plugin_window_moves); WriteParam(m, p.flags); @@ -868,6 +874,7 @@ struct ParamTraits<ViewHostMsg_PaintRect_Params> { return ReadParam(m, iter, &p->bitmap) && ReadParam(m, iter, &p->bitmap_rect) && + ReadParam(m, iter, &p->paint_rects) && ReadParam(m, iter, &p->view_size) && ReadParam(m, iter, &p->plugin_window_moves) && ReadParam(m, iter, &p->flags); @@ -878,6 +885,8 @@ struct ParamTraits<ViewHostMsg_PaintRect_Params> { l->append(L", "); LogParam(p.bitmap_rect, l); l->append(L", "); + LogParam(p.paint_rects, l); + l->append(L", "); LogParam(p.view_size, l); l->append(L", "); LogParam(p.plugin_window_moves, l); diff --git a/chrome/renderer/render_widget.cc b/chrome/renderer/render_widget.cc index 5332d74..fd414ce 100644 --- a/chrome/renderer/render_widget.cc +++ b/chrome/renderer/render_widget.cc @@ -32,6 +32,92 @@ using WebKit::WebRect; using WebKit::WebScreenInfo; using WebKit::WebSize; +namespace { + +// A helper class to manage access to the RenderWidget::paint_rects vector. +class PaintRegion { + public: + explicit PaintRegion(std::vector<gfx::Rect>* rects) + : rects_(*rects), + coalesce_(false) { + } + + gfx::Rect GetBoundingRect() const { + gfx::Rect bounding_rect; + for (size_t i = 0; i < rects_.size(); ++i) + bounding_rect = bounding_rect.Union(rects_[i]); + return bounding_rect; + } + + void Add(gfx::Rect rect) { + if (rect.IsEmpty()) + return; + + if (rects_.size() >= kNumRectsThreshold) { + coalesce_ = true; + for (size_t i = rects_.size() - 1; i > 0; --i) { + rects_[0] = rects_[0].Union(rects_[i]); + rects_.pop_back(); + } + } + + if (coalesce_) { + DCHECK(rects_.size() == 1); + rects_[0] = rects_[0].Union(rect); + } else { + // Look for all rects that would intersect with this new + // rect, union them and remove them from the list so that we + // have only one rect for any pixel in the invalidation. + std::vector<gfx::Rect>::iterator it = rects_.begin(); + while (it != rects_.end()) { + if (rect.Intersects(*it)) { + rect = rect.Union(*it); + rects_.erase(it); + // We must go back to the beginning since some rects we previously + // checked might now intersects with the union of the two rects, + // even though they didn't intersect with them individually. + it = rects_.begin(); + } else { + ++it; + } + } + rects_.push_back(rect); + } + } + + void Clip(const gfx::Rect& clip_rect) { + std::vector<gfx::Rect>::iterator it = rects_.begin(); + while (it != rects_.end()) { + if (!clip_rect.Contains(*it)) + *it = clip_rect.Intersect(*it); + ++it; + } + } + + bool Intersects(const gfx::Rect& rect) const { + for (size_t i = 0; i < rects_.size(); ++i) { + if (rects_[i].Intersects(rect)) + return true; + } + return false; + } + + private: + std::vector<gfx::Rect>& rects_; + bool coalesce_; + static const size_t kNumRectsThreshold; +}; +// We use a threshold for the number of rects because in many cases as +// described here: +// http://my.opera.com/desktopteam/blog/2008/03/28/painting-performance-fixes +// we get diminishing returns from using more sub-rectangles, it becomes more +// efficient to Union the rects. The nubmer 25 is also used in WebKit for +// deferred paints in the FrameView. It was chosen here based on some +// manual experiments using the benchmark from the article sited above. +const size_t PaintRegion::kNumRectsThreshold = 25; + +} // namespace + RenderWidget::RenderWidget(RenderThreadBase* render_thread, bool activatable) : routing_id_(MSG_ROUTING_NONE), webwidget_(NULL), @@ -200,13 +286,14 @@ void RenderWidget::OnResize(const gfx::Size& new_size, // an ACK if we are resized to a non-empty rect. webwidget_->Resize(new_size); if (!new_size.IsEmpty()) { - DCHECK(!paint_rect_.IsEmpty()); + DCHECK(!paint_rects_.empty()); // This should have caused an invalidation of the entire view. The damaged // rect could be larger than new_size if we are being made smaller. - DCHECK_GE(paint_rect_.width(), new_size.width()); - DCHECK_GE(paint_rect_.height(), new_size.height()); - + DCHECK_GE(PaintRegion(&paint_rects_).GetBoundingRect().width(), + new_size.width()); + DCHECK_GE(PaintRegion(&paint_rects_).GetBoundingRect().height(), + new_size.height()); // We will send the Resize_ACK flag once we paint again. set_next_paint_is_resize_ack(); } @@ -314,12 +401,12 @@ void RenderWidget::ClearFocus() { webwidget_->SetFocus(false); } -void RenderWidget::PaintRect(const gfx::Rect& rect, - skia::PlatformCanvas* canvas) { - - // Bring the canvas into the coordinate system of the paint rect. - canvas->translate(static_cast<SkScalar>(-rect.x()), - static_cast<SkScalar>(-rect.y())); +void RenderWidget::PaintThisRect(const gfx::Rect& rect, + skia::PlatformCanvas* canvas) { + // Make sure we don't erase previous rects in the canvas + SkRect clip_rect; + clip_rect.iset(rect.x(), rect.y(), rect.right(), rect.bottom()); + canvas->clipRect(clip_rect, SkRegion::kReplace_Op); // If there is a custom background, tile it. if (!background_.empty()) { @@ -333,19 +420,36 @@ void RenderWidget::PaintRect(const gfx::Rect& rect, } webwidget_->Paint(canvas, rect); +} + +void RenderWidget::PaintRect(const gfx::Rect& rect, + skia::PlatformCanvas* canvas) { + // Bring the canvas into the coordinate system of the paint rect. + canvas->translate(static_cast<SkScalar>(-rect.x()), + static_cast<SkScalar>(-rect.y())); + PaintThisRect(rect, canvas); + + // Flush to underlying bitmap. TODO(darin): is this needed? + canvas->getTopPlatformDevice().accessBitmap(false); +} + +void RenderWidget::PaintRects(const std::vector<gfx::Rect>& rects, + skia::PlatformCanvas* canvas) { + for (size_t i = 0; i < rects.size(); ++i) + PaintThisRect(rects[i], canvas); // Flush to underlying bitmap. TODO(darin): is this needed? canvas->getTopPlatformDevice().accessBitmap(false); } void RenderWidget::DoDeferredPaint() { - if (!webwidget_ || paint_reply_pending() || paint_rect_.IsEmpty()) + if (!webwidget_ || paint_reply_pending() || paint_rects_.empty()) return; // When we are hidden, we want to suppress painting, but we still need to // mark this DoDeferredPaint as complete. if (is_hidden_ || size_.IsEmpty()) { - paint_rect_ = gfx::Rect(); + paint_rects_.clear(); needs_repainting_on_restore_ = true; return; } @@ -355,26 +459,36 @@ void RenderWidget::DoDeferredPaint() { // OK, save the current paint_rect to a local since painting may cause more // invalidation. Some WebCore rendering objects only layout when painted. - gfx::Rect damaged_rect = paint_rect_; - paint_rect_ = gfx::Rect(); + std::vector<gfx::Rect> paint_rects = paint_rects_; + paint_rects_.clear(); // Compute a buffer for painting and cache it. + DCHECK(!current_paint_buf_); + + // we use the whole view size as opposed to damaged rect size we have a pool + // of paint buffers anyway, and this size has surely been used at least once + // to paint the whole view... And it also prevents, somehow, an intermittent + // bug we get when painting the sub-rectangles with StretchDIBits on Windows. + gfx::Rect bitmap_rect = gfx::Rect(gfx::Point(0, 0), size_); skia::PlatformCanvas* canvas = RenderProcess::current()->GetDrawingCanvas(¤t_paint_buf_, - damaged_rect); + bitmap_rect); if (!canvas) { NOTREACHED(); return; } - PaintRect(damaged_rect, canvas); + // We must make sure all our paint rects are within the bitmap bounds. + PaintRegion(&paint_rects).Clip(bitmap_rect); + PaintRects(paint_rects, canvas); ViewHostMsg_PaintRect_Params params; - params.bitmap_rect = damaged_rect; + params.bitmap = current_paint_buf_->id(); + params.bitmap_rect = bitmap_rect; + params.paint_rects = paint_rects; params.view_size = size_; params.plugin_window_moves = plugin_window_moves_; params.flags = next_paint_flags_; - params.bitmap = current_paint_buf_->id(); delete canvas; @@ -477,24 +591,21 @@ gfx::NativeViewId RenderWidget::GetContainingView(WebWidget* webwidget) { void RenderWidget::DidInvalidateRect(WebWidget* webwidget, const WebRect& rect) { // We only want one pending DoDeferredPaint call at any time... - bool paint_pending = !paint_rect_.IsEmpty(); + bool paint_pending = !paint_rects_.empty(); // If this invalidate overlaps with a pending scroll, then we have to // downgrade to invalidating the scroll rect. + PaintRegion rect_vector(&paint_rects_); if (gfx::Rect(rect).Intersects(scroll_rect_)) { - paint_rect_ = paint_rect_.Union(scroll_rect_); + rect_vector.Add(scroll_rect_); scroll_rect_ = gfx::Rect(); } - gfx::Rect view_rect(0, 0, size_.width(), size_.height()); - // TODO(iyengar) Investigate why we have painting issues when - // we ignore invalid regions outside the view. - // Ignore invalidates that occur outside the bounds of the view - // TODO(darin): maybe this should move into the paint code? - // paint_rect_ = view_rect.Intersect(paint_rect_.Union(rect)); - paint_rect_ = paint_rect_.Union(view_rect.Intersect(rect)); + // We don't need to intersect with the view rect here since we have to + // do it later on in case the view rect changes between invalidations. + rect_vector.Add(rect); - if (paint_rect_.IsEmpty() || paint_reply_pending() || paint_pending) + if (paint_rects_.empty() || paint_reply_pending() || paint_pending) return; // Perform painting asynchronously. This serves two purposes: @@ -514,7 +625,8 @@ void RenderWidget::DidScrollRect(WebWidget* webwidget, int dx, int dy, dy = 0; } - bool intersects_with_painting = paint_rect_.Intersects(clip_rect); + bool intersects_with_painting = + PaintRegion(&paint_rects_).Intersects(clip_rect); // If we already have a pending scroll operation or if this scroll operation // intersects the existing paint region, then just failover to invalidating. diff --git a/chrome/renderer/render_widget.h b/chrome/renderer/render_widget.h index 440a7e2..8a501ba 100644 --- a/chrome/renderer/render_widget.h +++ b/chrome/renderer/render_widget.h @@ -116,7 +116,10 @@ class RenderWidget : public IPC::Channel::Listener, // Paints the given rectangular region of the WebWidget into canvas (a // shared memory segment returned by AllocPaintBuf on Windows). The caller // must ensure that the given rect fits within the bounds of the WebWidget. + void PaintThisRect(const gfx::Rect& rect, skia::PlatformCanvas* canvas); void PaintRect(const gfx::Rect& rect, skia::PlatformCanvas* canvas); + void PaintRects(const std::vector<gfx::Rect>& rect, + skia::PlatformCanvas* canvas); void DoDeferredPaint(); void DoDeferredScroll(); @@ -221,7 +224,7 @@ class RenderWidget : public IPC::Channel::Listener, // The smallest bounding rectangle that needs to be re-painted. This is non- // empty if a paint event is pending. - gfx::Rect paint_rect_; + std::vector<gfx::Rect> paint_rects_; // The clip rect for the pending scroll event. This is non-empty if a // scroll event is pending. |