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 | 114 | ||||
| -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, 122 insertions, 377 deletions
diff --git a/chrome/browser/renderer_host/backing_store.h b/chrome/browser/renderer_host/backing_store.h index 5bb3945..5d817d0 100644 --- a/chrome/browser/renderer_host/backing_store.h +++ b/chrome/browser/renderer_host/backing_store.h @@ -63,12 +63,9 @@ 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& paint_rect); + const gfx::Rect& bitmap_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 @@ -98,11 +95,8 @@ 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& paint_rect); + const gfx::Rect& bitmap_rect); // This is the connection to the X server where this backing store will be // displayed. @@ -125,10 +119,6 @@ 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 8983208..569e3f6 100644 --- a/chrome/browser/renderer_host/backing_store_mac.cc +++ b/chrome/browser/renderer_host/backing_store_mac.cc @@ -21,24 +21,14 @@ BackingStore::~BackingStore() { void BackingStore::PaintRect(base::ProcessHandle process, TransportDIB* bitmap, - const gfx::Rect& bitmap_rect, - const gfx::Rect& paint_rect) { - DCHECK(bitmap_rect.Contains(paint_rect) && - paint_rect.x() < kMaxBitmapLengthAllowed && - paint_rect.y() < kMaxBitmapLengthAllowed); + const gfx::Rect& bitmap_rect) { SkBitmap skbitmap; skbitmap.setConfig(SkBitmap::kARGB_8888_Config, bitmap_rect.width(), bitmap_rect.height(), 4 * bitmap_rect.width()); skbitmap.setPixels(bitmap->memory()); - 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); + + canvas_.drawBitmap(skbitmap, bitmap_rect.x(), bitmap_rect.y()); } void BackingStore::ScrollRect(base::ProcessHandle process, @@ -128,6 +118,6 @@ void BackingStore::ScrollRect(base::ProcessHandle process, } // Now paint the new bitmap data. - PaintRect(process, bitmap, bitmap_rect, bitmap_rect); + PaintRect(process, bitmap, bitmap_rect); return; } diff --git a/chrome/browser/renderer_host/backing_store_manager.cc b/chrome/browser/renderer_host/backing_store_manager.cc index eb3600e..3ff2e76 100644 --- a/chrome/browser/renderer_host/backing_store_manager.cc +++ b/chrome/browser/renderer_host/backing_store_manager.cc @@ -49,11 +49,6 @@ 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 @@ -79,14 +74,13 @@ 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 (paint_rect.size() != backing_store_size || - paint_rect.x() != 0 || paint_rect.y() != 0) { + if (bitmap_rect.size() != backing_store_size || + bitmap_rect.x() != 0 || bitmap_rect.y() != 0) { DCHECK(needs_full_paint != NULL); *needs_full_paint = true; } @@ -94,7 +88,7 @@ BackingStore* BackingStoreManager::PrepareBackingStore( } DCHECK(backing_store != NULL); - backing_store->PaintRect(process_handle, bitmap, bitmap_rect, paint_rect); + backing_store->PaintRect(process_handle, bitmap, bitmap_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 d273c17..5b929f6 100644 --- a/chrome/browser/renderer_host/backing_store_manager.h +++ b/chrome/browser/renderer_host/backing_store_manager.h @@ -49,7 +49,6 @@ 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 edb861d..1241e42 100644 --- a/chrome/browser/renderer_host/backing_store_win.cc +++ b/chrome/browser/renderer_host/backing_store_win.cc @@ -54,11 +54,7 @@ BackingStore::~BackingStore() { void BackingStore::PaintRect(base::ProcessHandle process, TransportDIB* bitmap, - const gfx::Rect& bitmap_rect, - const gfx::Rect& paint_rect) { - DCHECK(bitmap_rect.Contains(paint_rect) && - paint_rect.x() < kMaxBitmapLengthAllowed && - paint_rect.y() < kMaxBitmapLengthAllowed); + const gfx::Rect& bitmap_rect) { if (!backing_store_dib_) { backing_store_dib_ = CreateDIB(hdc_, size_.width(), size_.height(), color_depth_); @@ -71,33 +67,16 @@ void BackingStore::PaintRect(base::ProcessHandle process, BITMAPINFOHEADER hdr; gfx::CreateBitmapHeader(bitmap_rect.width(), bitmap_rect.height(), &hdr); - // Account for a paint_rect that exceeds the bounds of our view. + // Account for a bitmap_rect that exceeds the bounds of our view gfx::Rect view_rect(0, 0, size_.width(), size_.height()); - 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(), + 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(), reinterpret_cast<BITMAPINFO*>(&hdr), DIB_RGB_COLORS, SRCCOPY); DCHECK(rv != GDI_ERROR); @@ -118,5 +97,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, bitmap_rect); + PaintRect(process, bitmap, bitmap_rect); } diff --git a/chrome/browser/renderer_host/backing_store_x.cc b/chrome/browser/renderer_host/backing_store_x.cc index ff7a489..6cb7c91 100644 --- a/chrome/browser/renderer_host/backing_store_x.cc +++ b/chrome/browser/renderer_host/backing_store_x.cc @@ -77,20 +77,17 @@ BackingStore::~BackingStore() { } void BackingStore::PaintRectWithoutXrender(TransportDIB* bitmap, - 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(); + 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_); + XImage image; memset(&image, 0, sizeof(image)); - image.width = bitmap_width; - image.height = bitmap_height; + image.width = width; + image.height = height; image.format = ZPixmap; image.byte_order = LSBFirst; image.bitmap_unit = 8; @@ -99,18 +96,16 @@ 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 = bitmap_width * 4; + image.bytes_per_line = width * 4; image.data = static_cast<char*>(bitmap->memory()); XPutImage(display_, pixmap, static_cast<GC>(pixmap_gc_), &image, - x_offset, y_offset /* source x, y */, 0, 0 /* dest x, y */, - paint_width, paint_height); + 0, 0 /* source x, y */, 0, 0 /* dest x, y */, + width, 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 @@ -119,68 +114,52 @@ 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 * paint_width * - paint_height)); + uint8_t* bitmap24 = static_cast<uint8_t*>(malloc(3 * width * height)); if (!bitmap24) return; const uint32_t* bitmap_in = static_cast<const uint32_t*>(bitmap->memory()); - 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) { + for (int y = 0; y < height; ++y) { + for (int x = 0; x < width; ++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 = paint_width * 3; + image.bytes_per_line = 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 */, - paint_width, paint_height); + width, 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 * paint_width * - paint_height)); + uint16_t* bitmap16 = static_cast<uint16_t*>(malloc(2 * width * height)); if (!bitmap16) return; uint16_t* const orig_bitmap16 = bitmap16; const uint32_t* bitmap_in = static_cast<const uint32_t*>(bitmap->memory()); - 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) { + for (int y = 0; y < height; ++y) { + for (int x = 0; x < width; ++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 = paint_width * 2; + image.bytes_per_line = width * 2; image.data = reinterpret_cast<char*>(orig_bitmap16); image.red_mask = 0xf800; @@ -189,7 +168,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 */, - paint_width, paint_height); + width, height); free(orig_bitmap16); } else { CHECK(false) << "Sorry, we don't support your visual depth without " @@ -199,31 +178,31 @@ void BackingStore::PaintRectWithoutXrender(TransportDIB* bitmap, XCopyArea(display_, pixmap /* source */, pixmap_ /* target */, static_cast<GC>(pixmap_gc_), - 0, 0 /* source x, y */, paint_width, paint_height, - paint_rect.x(), paint_rect.y() /* dest x, y */); + 0, 0 /* source x, y */, bitmap_rect.width(), bitmap_rect.height(), + bitmap_rect.x(), bitmap_rect.y() /* dest x, y */); XFreePixmap(display_, pixmap); } void BackingStore::PaintRect(base::ProcessHandle process, TransportDIB* bitmap, - const gfx::Rect& bitmap_rect, - const gfx::Rect& paint_rect) { - DCHECK(bitmap_rect.Contains(paint_rect)); + const gfx::Rect& bitmap_rect) { if (!display_) return; if (bitmap_rect.IsEmpty()) return; - const int paint_width = paint_rect.width(); - const int paint_height = paint_rect.height(); - if (paint_width > kMaxBitmapLengthAllowed || - paint_width > kMaxBitmapLengthAllowed) + 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) return; if (!use_render_) - return PaintRectWithoutXrender(bitmap, bitmap_rect, paint_rect); + return PaintRectWithoutXrender(bitmap, bitmap_rect); Picture picture; Pixmap pixmap; @@ -242,11 +221,8 @@ 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, - 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(); + pixmap = XShmCreatePixmap(display_, root_window_, NULL, &shminfo, width, + height, 32); } 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 @@ -255,36 +231,34 @@ void BackingStore::PaintRect(base::ProcessHandle process, XImage image; memset(&image, 0, sizeof(image)); - image.width = paint_width; - image.height = paint_height; + image.width = width; + image.height = 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 = paint_width * 4; + image.bytes_per_line = 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_, paint_width, paint_height, - 32); + pixmap = XCreatePixmap(display_, root_window_, width, height, 32); GC gc = XCreateGC(display_, pixmap, 0, NULL); - 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); + XPutImage(display_, pixmap, gc, &image, + 0, 0 /* source x, y */, 0, 0 /* dest x, y */, + width, height); XFreeGC(display_, gc); } picture = x11_util::CreatePictureFromSkiaPixmap(display_, pixmap); XRenderComposite(display_, PictOpSrc, picture /* source */, 0 /* mask */, - picture_ /* dest */, src_x, src_y /* source x, y */, + picture_ /* dest */, 0, 0 /* source x, y */, 0, 0 /* mask x, y */, - paint_rect.x(), paint_rect.y() /* target x, y */, - paint_width, paint_height); + bitmap_rect.x(), bitmap_rect.y() /* target x, y */, + width, 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. @@ -331,7 +305,7 @@ void BackingStore::ScrollRect(base::ProcessHandle process, } } - PaintRect(process, bitmap, bitmap_rect, bitmap_rect); + PaintRect(process, bitmap, 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 95e7cf3..fb6303b 100644 --- a/chrome/browser/renderer_host/render_widget_host.cc +++ b/chrome/browser/renderer_host/render_widget_host.cc @@ -14,7 +14,6 @@ #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" @@ -42,32 +41,6 @@ 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 @@ -525,19 +498,18 @@ 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. - PaintBackingStoreRects(dib, params.bitmap_rect, params.paint_rects, - 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. + PaintBackingStoreRect(dib, params.bitmap_rect, params.view_size); } } @@ -557,12 +529,7 @@ void RenderWidgetHost::OnMsgPaintRect( if (view_) { view_->MovePluginWindows(params.plugin_window_moves); view_being_painted_ = true; - 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_->DidPaintRect(params.bitmap_rect); view_being_painted_ = false; } @@ -653,7 +620,7 @@ void RenderWidgetHost::OnMsgInputEventAck(const IPC::Message& message) { } if (WebInputEvent::isKeyboardEventType(type)) { - if (key_queue_.empty()) { + if (key_queue_.size() == 0) { 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) { @@ -721,10 +688,9 @@ void RenderWidgetHost::OnMsgShowPopup(const IPC::Message& message) { #endif } -void RenderWidgetHost::PaintBackingStoreRects( - TransportDIB* bitmap, const gfx::Rect& bitmap_rect, - const std::vector<gfx::Rect>& paint_rects, - const gfx::Size& view_size) { +void RenderWidgetHost::PaintBackingStoreRect(TransportDIB* bitmap, + const gfx::Rect& bitmap_rect, + const gfx::Size& view_size) { // The view may be destroyed already. if (!view_) return; @@ -738,36 +704,12 @@ void RenderWidgetHost::PaintBackingStoreRects( } bool needs_full_paint = false; - 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; - } - } - } - + BackingStore* backing_store = + BackingStoreManager::PrepareBackingStore(this, view_size, + process_->process().handle(), + bitmap, bitmap_rect, + &needs_full_paint); + DCHECK(backing_store != NULL); 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 886bd2c..8ac2376 100644 --- a/chrome/browser/renderer_host/render_widget_host.h +++ b/chrome/browser/renderer_host/render_widget_host.h @@ -328,10 +328,9 @@ 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 PaintBackingStoreRects(TransportDIB* bitmap, - const gfx::Rect& bitmap_rect, - const std::vector<gfx::Rect>& paint_rects, - const gfx::Size& view_size); + void PaintBackingStoreRect(TransportDIB* dib, + const gfx::Rect& bitmap_rect, + 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 2a6ea40..2665610 100644 --- a/chrome/browser/views/frame/browser_view.cc +++ b/chrome/browser/views/frame/browser_view.cc @@ -134,8 +134,11 @@ class ResizeCorner : public views::View { } static gfx::Size GetSize() { - return gfx::Size(views::NativeScrollBar::GetVerticalScrollBarWidth(), - views::NativeScrollBar::GetHorizontalScrollBarHeight()); + // 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(); } virtual gfx::Size GetPreferredSize() { @@ -1326,8 +1329,7 @@ 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 9bfe710..50749cb 100644 --- a/chrome/common/render_messages.h +++ b/chrome/common/render_messages.h @@ -158,17 +158,12 @@ struct ViewHostMsg_PaintRect_Flags { }; struct ViewHostMsg_PaintRect_Params { - // The bitmap to be painted into the rects given by paint_rects. + // The bitmap to be painted into the rect given by bitmap_rect. TransportDIB::Id bitmap; - // The position and size of the the bitmap relative to the view port. + // The position and size of the bitmap. 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 @@ -864,7 +859,6 @@ 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); @@ -873,7 +867,6 @@ 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); @@ -884,8 +877,6 @@ 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 fd414ce..5332d74 100644 --- a/chrome/renderer/render_widget.cc +++ b/chrome/renderer/render_widget.cc @@ -32,92 +32,6 @@ 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), @@ -286,14 +200,13 @@ 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_rects_.empty()); + DCHECK(!paint_rect_.IsEmpty()); // 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(PaintRegion(&paint_rects_).GetBoundingRect().width(), - new_size.width()); - DCHECK_GE(PaintRegion(&paint_rects_).GetBoundingRect().height(), - new_size.height()); + DCHECK_GE(paint_rect_.width(), new_size.width()); + DCHECK_GE(paint_rect_.height(), new_size.height()); + // We will send the Resize_ACK flag once we paint again. set_next_paint_is_resize_ack(); } @@ -401,12 +314,12 @@ void RenderWidget::ClearFocus() { webwidget_->SetFocus(false); } -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); +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())); // If there is a custom background, tile it. if (!background_.empty()) { @@ -420,36 +333,19 @@ void RenderWidget::PaintThisRect(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_rects_.empty()) + if (!webwidget_ || paint_reply_pending() || paint_rect_.IsEmpty()) 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_rects_.clear(); + paint_rect_ = gfx::Rect(); needs_repainting_on_restore_ = true; return; } @@ -459,36 +355,26 @@ 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. - std::vector<gfx::Rect> paint_rects = paint_rects_; - paint_rects_.clear(); + gfx::Rect damaged_rect = paint_rect_; + paint_rect_ = gfx::Rect(); // 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_, - bitmap_rect); + damaged_rect); if (!canvas) { NOTREACHED(); return; } - // We must make sure all our paint rects are within the bitmap bounds. - PaintRegion(&paint_rects).Clip(bitmap_rect); - PaintRects(paint_rects, canvas); + PaintRect(damaged_rect, canvas); ViewHostMsg_PaintRect_Params params; - params.bitmap = current_paint_buf_->id(); - params.bitmap_rect = bitmap_rect; - params.paint_rects = paint_rects; + params.bitmap_rect = damaged_rect; params.view_size = size_; params.plugin_window_moves = plugin_window_moves_; params.flags = next_paint_flags_; + params.bitmap = current_paint_buf_->id(); delete canvas; @@ -591,21 +477,24 @@ 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_rects_.empty(); + bool paint_pending = !paint_rect_.IsEmpty(); // 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_)) { - rect_vector.Add(scroll_rect_); + paint_rect_ = paint_rect_.Union(scroll_rect_); scroll_rect_ = gfx::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); + 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)); - if (paint_rects_.empty() || paint_reply_pending() || paint_pending) + if (paint_rect_.IsEmpty() || paint_reply_pending() || paint_pending) return; // Perform painting asynchronously. This serves two purposes: @@ -625,8 +514,7 @@ void RenderWidget::DidScrollRect(WebWidget* webwidget, int dx, int dy, dy = 0; } - bool intersects_with_painting = - PaintRegion(&paint_rects_).Intersects(clip_rect); + bool intersects_with_painting = paint_rect_.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 8a501ba..440a7e2 100644 --- a/chrome/renderer/render_widget.h +++ b/chrome/renderer/render_widget.h @@ -116,10 +116,7 @@ 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(); @@ -224,7 +221,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. - std::vector<gfx::Rect> paint_rects_; + gfx::Rect paint_rect_; // The clip rect for the pending scroll event. This is non-empty if a // scroll event is pending. |