On Sat, May 16, 2009 at 10:16 AM, Marc-Andre Decoste <m...@google.com> wrote:
> Salut, > > I found the cause of the problem, but didn't find a cure yet.. The > SkCanvas::ClipRect() call with the replace Op doesn't seem to work more than > once... The first rect gets drawn on the cancas properly, but all subsequent > rects drawn on the same canvas with different clip rects seem to be > ignored... If I don't make the call to clip the rect, then all the rects are > drawn, but some of the earlier rects can be over-painted with white when > other rects are drawn at the same height within the canvas... > > I tried to clear the clip rect after drawing each rectagle (using the > kDifference_Op), also tried unioning the clip rect before doing an intersect > op, still with no success... I'll keep digging to understand why the > subsequent clip rects don't work on the Mac when they work fine on Windows > and Linux... But I would appreciate some hints if you have some... :-) I haven't looked at the code, but this sounds a lot like a peculiarity of CoreGraphics I ran into a while back: setting the clip path with CGContextClipToRect() does the intersection of that rect with the existing clip path, rather than replacing it. Since the clip path can't be made larger, the only way to really restore it is to save the graphics state before setting the clip path, and restore the graphics state after drawing. So if we're not doing that, it's possible that the CoreGraphics path may not have the same semantics as the Skia path in this case. (It's my understanding that we use CoreGraphics on the Mac rather than Skia, but since you mentioned SkCanvas, I could be wrong.) Stephen > On Wed, May 13, 2009 at 1:25 PM, Marc-Andre Decoste <m...@google.com>wrote: > >> As I mentioned in the latest message, this code is OK after all :-)... >> The problem seems to be at the other end, when I split the drawing in >> sub-regions of the TranportDIB, on the rendering side... I wasn't expecting >> this, since this other code is not using any platform specific stuff >> directly... You can find it in here: >> http://codereview.chromium.org/108040 >> >> The problem I get is that I'm missing some refreshes... I have a little >> test HTML file that changes the content on of multiple parts of the page as >> I press on buttons, and it doesn't work until I change the code in >> chrome/renderer/render_widget.cc by calling PaintRect(bitmap_rect, >> canvas); instead of PaintRects(paint_rects, canvas);, which means we draw >> the whole bitmap, even though we ask the host to only paint a set of >> sub-rectangle from that bitmap. >> >> Do I explain it clearly enough? >> >> Thanks >> >> BYE >> MAD >> >> On Wed, May 13, 2009 at 1:16 PM, Amanda Walker <awal...@google.com>wrote: >> >>> Hmm. Marc, your change looks OK to me at first glance--how does it fail? >>> >>> --Amanda >>> >>> On Wed, May 13, 2009 at 12:10 PM, Avi Drissman <a...@google.com> wrote: >>> > I'm view meister, not render meister. Amanda is IIRC backing >>> store/canvas >>> > meister. >>> > >>> > Avi >>> > >>> > On Wed, May 13, 2009 at 9:04 AM, Marc-Andre Decoste <m...@google.com> >>> wrote: >>> >> >>> >> OK, so it seems this code is fine.... >>> >> I tried to make a temporary change in the other (platform independent) >>> >> part of the code, where we paint the sub-rects into a bigger bitmap... >>> I >>> >> thought it would work without any changes on the Mac side since it was >>> all >>> >> done with platform independent calls, but maybe a platform dependent >>> >> implementation on the other side of that call doesn't like it.. >>> >> So I'll start digging in this other direction... But only after >>> lunch... >>> >> Will keep you guys posted of my progress... >>> >> Thanks! >>> >> >>> >> BYE >>> >> MAD >>> >> >>> >> On Wed, May 13, 2009 at 9:35 AM, Mike Pinkerton <pinker...@google.com >>> > >>> >> wrote: >>> >>> >>> >>> Looping in Avi who is our render-meister. >>> >>> >>> >>> On Wed, May 13, 2009 at 9:24 AM, Marc-Andre Decoste <m...@google.com> >>> >>> wrote: >>> >>> > Salut Mac-Heros, >>> >>> > I'm trying to update the Mac platform specific code to some >>> changes >>> >>> > I did >>> >>> > to the Windows platform specific code in an attempt to improve our >>> >>> > paint >>> >>> > invalidation performance (as discussed on chromium-dev@). >>> >>> > Below a patch of my first (unsuccessful attempt). I must admit >>> that >>> >>> > even >>> >>> > on Windows it took me a few iterations to get it right, so I would >>> have >>> >>> > been >>> >>> > surprised to get it right on the first try on the Mac... >>> >>> > So the idea is that we now receive a bigger bitmap that contains >>> >>> > sub-rectangles that we want to use to redraw individually... So I >>> >>> > thought >>> >>> > that this was the goal of drawBitmapRect() but I guess I >>> misunderstood >>> >>> > it's >>> >>> > usage... Are you guys familiar with this? Anybody else I could ask >>> >>> > questions >>> >>> > to about this? >>> >>> > In the meant time, I'm also working with the linux version and >>> >>> > attempting >>> >>> > different shots in the dark on the Mac :-)... >>> >>> > Thanks! >>> >>> > BYE >>> >>> > MAD >>> >>> > Index: chrome/browser/renderer_host/backing_store_mac.cc >>> >>> > =================================================================== >>> >>> > --- chrome/browser/renderer_host/backing_store_mac.cc (revision >>> 15881) >>> >>> > +++ chrome/browser/renderer_host/backing_store_mac.cc (working >>> copy) >>> >>> > @@ -21,14 +21,21 @@ >>> >>> > >>> >>> > void BackingStore::PaintRect(base::ProcessHandle process, >>> >>> > TransportDIB* bitmap, >>> >>> > - const gfx::Rect& bitmap_rect) { >>> >>> > + const gfx::Rect& bitmap_rect, >>> >>> > + const gfx::Rect& paint_rect) { >>> >>> > 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, >>> >>> > @@ -117,6 +124,6 @@ >>> >>> > } >>> >>> > >>> >>> > // Now paint the new bitmap data. >>> >>> > - PaintRect(process, bitmap, bitmap_rect); >>> >>> > + PaintRect(process, bitmap, bitmap_rect, bitmap_rect); >>> >>> > return; >>> >>> > >>> >>> >>> >>> >>> >>> >>> >>> -- >>> >>> Mike Pinkerton >>> >>> Mac Weenie >>> >>> pinker...@google.com >>> >> >>> > >>> > >>> >> >> > > > > -- All truth passes through three stages. First, it is ridiculed. Second, it is violently opposed. Third, it is accepted as being self-evident. -- Schopenhauer --~--~---------~--~----~------------~-------~--~----~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~----------~----~----~----~------~----~------~--~---
