[chromium-dev] Re: Linux build totally busted.

[Marc-Andre Decoste]

Yes, it was my patch, it is completely my fault, I take all the blame.. I'm
very sorry about that...
The patch had been reviewed but through another revision that was mentioned
in the comments... I should have put a link to it as opposed to only the
revision ID (I'm used to internal tools that inserts the link automagically
so I only put the rev number).

The problem is that after a merge conflict, I completely forgot to rebuild
on Mac and Linux (which I have been doing regularly for over a month for
this specific patch that has been opened forever and it wasn't trivial to do
so) and so I broke the build last night (well, night in EDT). I quickly
found the problem and fixed is and tried to build it on Linux but V8 didn't
want to build after many re-sync and clobbers, I had not clue what was
happening there, so I sent it to the try server and it built find, so I
committed the re-landing... I know... I should not rely on the try server,
but I have tried this patch many many times on all platforms and never had a
problem... I tried again this morning and I had to create a whole new view
of the repos to be able to properly build.

The current problem is easy to fix, it is a last minute check that I added
in platform independent code in reply to a code review suggestion and it
didn't trigger on Windows (so I didn't see the error when I tested the
platform independent code) and for some reason it started triggering on
Linux after the last time I tried to run on Linux... It was a simply typo of
an embedded for loop index that was causing a protective _DEBUG only DCHECK
to fire... I could fix it easily this morning... But I'm not going to try
this patch again soon, cause I noticed that it didn't completely solve the
performance problem I was trying to solve... So I don't feel too good about
it... (anybody has a *Harakiri* sword to spare? :-)...

I'm very sorry that I caused so much trouble, and I will be much more
careful next time...

I'm hope you will all have a very good day anyway... :-)

BYE
SAD (the-developer-formerly-known-as-MAD :-)

On Thu, Jun 11, 2009 at 7:30 AM, Dean McNamee de...@chromium.org wrote:


 Reoccuring lesson, when you're looking for a shotty change, look for
 the ones with the worst commit messages:

 Of course I overlooked the change 50 times because the message tells
 you absolutely nothing, doesn't have a review URL, etc.

 I don't feel like figuring out what is wrong with this patch, I will
 TBR a revert right now.  Insert typical complaints about writing
 Linux specific code, clearly not testing it at all, and wasting a
 bunch of peoples time.

 commit 7c4e9d282fb3af3ea5c732d37908a133662e02e2
 Author: m...@google.com m...@google.com
 @0039d316-1c4b-4281-b951-d872f2087c98
 Date:   Thu Jun 11 01:08:27 2009 +

Relanding reverted patch 18090.


git-svn-id: svn://chrome-svn/chrome/trunk/s...@18130
 0039d316-1c4b-4281-b951-d872f2087c98



 On Thu, Jun 11, 2009 at 1:17 PM, Dean McNameede...@chromium.org wrote:
  One of the ideas would be to bisect the archived builds
  (http://build.chromium.org/buildbot/continuous/linux/LATEST/).
 
  I am also working on hunting this down.  I'll let you know when/if I
  find anything.
 
  Thanks for the great help
  -- dean
 
  On Thu, Jun 11, 2009 at 1:15 PM, Craig
  Schlentercraig.schlen...@gmail.com wrote:
 
  It might not be r18131 ... I compiled head without 18131 and even
  though it looked like that helped earlier, it's broken again now.
  Interestingly when opening new tabs, only some of them are broken
  which is probably why I thought it was ok earlier ... will leave
  something bisecting in the backround but it's a rather long cycle time
  on my laptop to do each build.
 
  --Craig
 
  On Thu, Jun 11, 2009 at 12:42 PM, Craig
  Schlentercraig.schlen...@gmail.com wrote:
  Hi Dean
 
  I am seeing the same thing ... it seems to be r18131 that is at fault.
  I have not tracked it down further than that ...
 
  Shared build is also bust. See attached patch ... dunno if that's the
 right fix.
 
  --Craig
 
  On Thu, Jun 11, 2009 at 11:58 AM, Dean McNameede...@chromium.org
 wrote:
  Something related to images, or resources, or decoders, or painting,
  or something.  Multiple reports of everything being complete broken.
 
  Screenshot of my recent build attached.
 
  -- dean
 
  
 
 
 
  
 
 

 


--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---

[chromium-dev] Re: Drawing code on Mac

[Marc-Andre Decoste]

Thanks Amanda (and Stephen :-)...

I finally got to look at this code in the train this morning (after a very
relaxing extended long weekend :-)... And I have an idea of how we could do
this more easily and hopefully safe enough...

I don't think I can do the save/restore trick in
PlatformDeviceMac::LoadClippingRegionToCGContext(), because this a static
method with no context... But I noticed that it is only called at one place,
within BitmapPlatformDeviceMac::BitmapPlatformDeviceMacData::LoadConfig()
which is also the only place where LoadTransformToCGContext() is called, so
we could simply rely on restore/save there... But is it safe enough?

The other alternative, would be to create an image mask in
PlatformDeviceMac::LoadClippingRegionToCGContext(), and use
CGContextClipToMask() instead of CGContextClipToRect()... But note that
ClipToMask was only introduced in 10.4, I know that our development
environment only supports 10.5 and up, but do we expect Chromium to run on
versions earlier than 10.4?

Thanks again!

BYE
MAD

On Sat, May 16, 2009 at 4:55 PM, Amanda Walker ama...@chromium.org wrote:

 The code to look at is in skia/ext/platform_device_mac.cc:123 and
 following (PlatformDeviceMac::LoadClippingRegionToCGContext).  It
 calls CGContextClipToRect, but does not reset the clip path first, so
 we we end up with the behavior Stephen described.

 The best solution would be to do a save when the device is created and
 a restore / save pair before loading a new clip path.  We'll have to
 reload the rest of the Skia drawing state back into the CGContext
 after the restore, of course, but that shouldn't be too bad.  We might
 also be able to define a mask that we draw through (and that we can
 reset at will), rather than manipulating the CGContext's clip region
 at all.

 It is possible to reset a CGContext's clip region (which matches the
 Skia drawing model a bit better) using ClipCGContextToRegion, but that
 was deprecated in 10.4 and won't be around for 64-bit applications, so
 I'd rather not use it unless the other approaches turns out to be
 impractical for some reason.

 --Amanda

 On Sat, May 16, 2009 at 3:44 PM, Marc-Andre Decoste m...@google.com
 wrote:
  Salut Stephen,
 
 thanks for the hint... Note that I have tried calling save/restore on
 the
  canvas around each sub-rect drawing... With no luck... I also tried to
 make
  the call to  canvas-getTopPlatformDevice().accessBitmap(false); after
 every
  sub-rect... same thing...
 
 I guess I'll wait for Amanda to take a look at it tomorrow or
 Monday... I
  quickly looked at the skia/ext/platform_canvas_mac.cc file content but
  didn't see anything related to clip rects in there...
 
  Thanks!
 
  BYE
  MAD
 
  On Sat, May 16, 2009 at 2:13 PM, Stephen White senorbla...@chromium.org
 
  wrote:
 
  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

[chromium-dev] Re: Drawing code on Mac

[Marc-Andre Decoste]

The quick and easy restore/save trick in LoadConfig() seems to work fine (at
least in all the tests I did)...

I'll update the patch with these change and publish it for review...

Let me know ASAP if you think this is a bad idea so that I can try the other
(more complex, though probably safer) solution using the mask..

Thanks!

BYE
MAD

On Wed, May 20, 2009 at 9:22 AM, Marc-Andre Decoste m...@google.com wrote:

 Thanks Amanda (and Stephen :-)...

 I finally got to look at this code in the train this morning (after a very
 relaxing extended long weekend :-)... And I have an idea of how we could do
 this more easily and hopefully safe enough...

 I don't think I can do the save/restore trick in
 PlatformDeviceMac::LoadClippingRegionToCGContext(), because this a static
 method with no context... But I noticed that it is only called at one place,
 within BitmapPlatformDeviceMac::BitmapPlatformDeviceMacData::LoadConfig()
 which is also the only place where LoadTransformToCGContext() is called, so
 we could simply rely on restore/save there... But is it safe enough?

 The other alternative, would be to create an image mask in
 PlatformDeviceMac::LoadClippingRegionToCGContext(), and use
 CGContextClipToMask() instead of CGContextClipToRect()... But note that
 ClipToMask was only introduced in 10.4, I know that our development
 environment only supports 10.5 and up, but do we expect Chromium to run on
 versions earlier than 10.4?

 Thanks again!

 BYE
 MAD


 On Sat, May 16, 2009 at 4:55 PM, Amanda Walker ama...@chromium.orgwrote:

 The code to look at is in skia/ext/platform_device_mac.cc:123 and
 following (PlatformDeviceMac::LoadClippingRegionToCGContext).  It
 calls CGContextClipToRect, but does not reset the clip path first, so
 we we end up with the behavior Stephen described.

 The best solution would be to do a save when the device is created and
 a restore / save pair before loading a new clip path.  We'll have to
 reload the rest of the Skia drawing state back into the CGContext
 after the restore, of course, but that shouldn't be too bad.  We might
 also be able to define a mask that we draw through (and that we can
 reset at will), rather than manipulating the CGContext's clip region
 at all.

 It is possible to reset a CGContext's clip region (which matches the
 Skia drawing model a bit better) using ClipCGContextToRegion, but that
 was deprecated in 10.4 and won't be around for 64-bit applications, so
 I'd rather not use it unless the other approaches turns out to be
 impractical for some reason.

 --Amanda

 On Sat, May 16, 2009 at 3:44 PM, Marc-Andre Decoste m...@google.com
 wrote:
  Salut Stephen,
 
 thanks for the hint... Note that I have tried calling save/restore on
 the
  canvas around each sub-rect drawing... With no luck... I also tried to
 make
  the call to  canvas-getTopPlatformDevice().accessBitmap(false); after
 every
  sub-rect... same thing...
 
 I guess I'll wait for Amanda to take a look at it tomorrow or
 Monday... I
  quickly looked at the skia/ext/platform_canvas_mac.cc file content but
  didn't see anything related to clip rects in there...
 
  Thanks!
 
  BYE
  MAD
 
  On Sat, May 16, 2009 at 2:13 PM, Stephen White 
 senorbla...@chromium.org
  wrote:
 
  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

[chromium-dev] Re: Drawing code on Mac

[Marc-Andre Decoste]

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... :-)

Thanks!

BYE
MAD

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__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

[chromium-dev] Re: Drawing code on Mac

[Marc-Andre Decoste]

Salut Stephen,

   thanks for the hint... Note that I have tried calling save/restore on the
canvas around each sub-rect drawing... With no luck... I also tried to make
the call to  canvas-getTopPlatformDevice().accessBitmap(false); after every
sub-rect... same thing...

   I guess I'll wait for Amanda to take a look at it tomorrow or Monday... I
quickly looked at the skia/ext/platform_canvas_mac.cc file content but
didn't see anything related to clip rects in there...

Thanks!

BYE
MAD

On Sat, May 16, 2009 at 2:13 PM, Stephen White senorbla...@chromium.orgwrote:

 On Sat, May 16, 2009 at 10:16 AM, Marc-Andre Decoste m...@google.comwrote:

 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.comwrote:

 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.comwrote:

 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

[chromium-dev] Re: Help needed for Chromium on the Mac refresh problem...

[Marc-Andre Decoste]

Here's what we're now doing on the renderer side:

   - In the render widget, we now keep all invalidation as an independent
   sub-rect in a vector as opposed to union them as we used to.
   - Once, we are ready to paint:
  - We create a bitmap that is the size of the whole view and wrap it in
  a canvas.
  - For each invalidation sub-rec
 - We set a clip rect (on the canvas) to the invalidation sub-rect.
 - And then we call WebWidget::Paint() with the canvas and the
 sub-rect.

   Anything smells fishy in there?

BYE
MAD

On Thu, May 14, 2009 at 3:14 PM, Avi Drissman a...@chromium.org wrote:

 We haven't drawn one big rectangle for a long while. It took me a while to
 fiddle with the transforms to get it all right, but we correctly blit small
 rectangles for small updates (e.g. cursor blinks).

 OTOH, Mike has a point, that getting the coordinate transforms correct is
 important, tricky, and a pain in the rear. That still might be the issue.
 But if you're just calling into the RWHV repeatedly, it should handle it
 just fine.

 Avi

 On Thu, May 14, 2009 at 12:02 PM, Mike Pinkerton 
 pinker...@chromium.orgwrote:


 Just off the top of my head, it could be a coordinate problem. Cocoa
 uses a bottom-left coordinate system (the bottom left of the view is
 0,0 as opposed to the top left). Since before we were just drawing a
 single big rectangle, it might not have mattered if we forgot to do
 the coordinate flipping. Again, just a wild guess based on things I've
 seen in the past and differences in the drawing paths.

 On Thu, May 14, 2009 at 2:33 PM, Marc-Andre Decoste m...@google.com
 wrote:
  Salut Adam,
 The problem is that I'm missing some refreshes... I see them more
 clearly
  when interacting with Google maps, or a little HTML page I created that
 has
  buttons that change the content of scattered table cells to force
 multiple
  individual rect invalidations in one shot... And those don't display if
 I
  try to paint individual rects in the bitmap...
 But it all works fine if I paint the whole bitmap, and then only draw
 the
  sub-rects from the bitmap... This is why I think the new Mac specific
 code
  is correct... Otherwise, I don't see how I would see the correct
 refreshes
  in that latter case... Unless I missed something...
 
  Thanks!
  BYE
  MAD
  On Thu, May 14, 2009 at 2:27 PM, Adam Langley a...@chromium.org wrote:
 
  On Thu, May 14, 2009 at 11:21 AM, Marc-Andre Decoste m...@google.com
  wrote:
  Since there is a change in the Windows specific code of the
 renderer
   host
   backing store (to draw the sub-rects of the passed bitmap), I thought
 my
   goof could have been in my adaptation of the Mac version of that code
   (http://codereview.chromium.org/108040/diff/2038/3040). But it seems
 to
   be
   working fine if I disable the other part of that change (which is to
   paint
   only sub-rectangles in the bitmap). So if I paint a complete bitmap,
 and
   pass it to the host with a list of sub-rectagles to draw... It
 works...
   So I
   guess the goof is in the code that paint sub-rectangles in the
 bitmap...
  But this code works fine on Windows and is not done in a Windows
   specific
   way... So I'm kind of lost and don't know where to look... Anybody
 has a
   clue here?
 
  What's the problem that you see?
 
  Surely it could still be in the Mac specific code if you're painting
  from the wrong regions in the bitmap?
 
 
  AGL
 
 
  
 



 --
 Mike Pinkerton
 Mac Weenie
 pinker...@google.com

 



--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---

[chromium-dev] Re: Need help in finding a performance problem...

[Marc-Andre Decoste]

Salut again,
   I finally got it all working well on Windows (at least as far as I can
see), and the numbers seem to say that this optimization would compensate
for the slow down that was introduced when the resizer corner was enabled...

   I still have some doubts about the validity of the numbers I get on my
machine, so please try it out if you have ways to consistently get similar
profiling numbers on your machine...

   Also, please look at the code to make sure I don't do anything foolish in
there... You can find it here: http://codereview.chromium.org/108040. Note
that this still only works on Windows... I will start doing the changes to
make it also work on Mac and Linux this afternoon... Once I get my
post-lunch latté :-)

Enjoy!

BYE
MAD :-)

On Mon, May 11, 2009 at 8:44 PM, Marc-Andre Decoste m...@chromium.orgwrote:

 Feel free to ping delme/del agl if you need help on Linux.  ;)


 Will do... thanks for the offer :-)

 BYE
 MAD

 On Mon, May 11, 2009 at 8:42 PM, Evan Martin e...@chromium.org wrote:


 On Mon, May 11, 2009 at 5:40 PM, Marc-Andre Decoste m...@chromium.org
 wrote:
  However, I think that means if you leave the TransportDIB as the union
 
  of all the dirty rects, everything should Just Work.  So your change
 
  becomes sending one unioned dirty rect image of pixel data, some of
 
  which that may not have actually been drawn to, along with an array of
 
  actually dirty subrectangles within the image.
 
  This is exactly what I'm doing on Windows... The receiving side loops on
 the
  dirty sub-rectangles and get their pixels from the single btimap being
  sent... And this bitmap is only valid in the areas of the
 sub-rectangles
  passed in a vector via IPC.
  Are we saying the same thing here?

 Yes.

  P.S.: I'm still validating that it is all OK on the Windows side, just
 fixed
  what I hope was the last refresh bug with Google maps, and then, I'll
 update
  the GTK and Coco version of the painting code...

 Feel free to ping delme/del agl if you need help on Linux.  ;)

 



--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---

[chromium-dev] Re: Need help in finding a performance problem...

[Marc-Andre Decoste]

 However, I think that means if you leave the TransportDIB as the union

of all the dirty rects, everything should Just Work.  So your change

becomes sending one unioned dirty rect image of pixel data, some of

which that may not have actually been drawn to, along with an array of

actually dirty subrectangles within the image.


This is exactly what I'm doing on Windows... The receiving side loops on the
dirty sub-rectangles and get their pixels from the single btimap being
sent... And this bitmap is only valid in the areas of the sub-rectangles
passed in a vector via IPC.

Are we saying the same thing here?
BYE
MAD
P.S.: I'm still validating that it is all OK on the Windows side, just fixed
what I hope was the last refresh bug with Google maps, and then, I'll update
the GTK and Coco version of the painting code...

On Mon, May 11, 2009 at 6:16 PM, Evan Martin e...@chromium.org wrote:

 On Sat, May 9, 2009 at 10:41 AM, Darin Fisher da...@chromium.org wrote:
  At a high level, you're using one TransportDIB per rectangle, but it
  should be one per message (with multiple rects worth of image data
  inside). You can't really use any benchmarking results while this is
  the case.
 
  I agree.  We should only require a single TransportDIB.  It is
 conceptually
  just an array of pixels, so you should be able to append to that array
 with
  all of the new pixels and keep track of the offsets for each sub-bitmap.

 On X, the TransportDIB becomes a single Pixmap which we then bitblt
 from via X API calls.  So we can't treat it *exactly* as a big array
 of pixels with offsets -- if the Pixmap is 400px wide and we have a
 tiny 20px wide dirty resize corner, we still need to write those 20px
 lines with a 400px stride.

 However, I think that means if you leave the TransportDIB as the union
 of all the dirty rects, everything should Just Work.  So your change
 becomes sending one unioned dirty rect image of pixel data, some of
 which that may not have actually been drawn to, along with an array of
 actually dirty subrectangles within the image.


--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---

[chromium-dev] Re: Need help in finding a performance problem...

[Marc-Andre Decoste]

Salut Darin,
   this why I changed my initial approach (which was to send an array of
smaller bitmaps alongside the array of dirty rects), to use a single bitmap
with only the dirty rects drawn on it...

   To overcome a weird intermittent bug I was getting with our current usage
of StretchDIBits, I now use a TransportDIB that is as big as the whole view
rect (as opposed to the union rect)... I think it is OK, since we use a
memory pool for those and we will have done a whole view rect invalidation
at least once before getting into smaller invalidation, so that should use
even less memory, even though we use a bigger bitmap then needed, right?

BYE
MAD

On Mon, May 11, 2009 at 6:26 PM, Darin Fisher da...@chromium.org wrote:

 On Mon, May 11, 2009 at 3:16 PM, Evan Martin e...@chromium.org wrote:
  On Sat, May 9, 2009 at 10:41 AM, Darin Fisher da...@chromium.org
 wrote:
  At a high level, you're using one TransportDIB per rectangle, but it
  should be one per message (with multiple rects worth of image data
  inside). You can't really use any benchmarking results while this is
  the case.
 
  I agree.  We should only require a single TransportDIB.  It is
 conceptually
  just an array of pixels, so you should be able to append to that array
 with
  all of the new pixels and keep track of the offsets for each sub-bitmap.
 
  On X, the TransportDIB becomes a single Pixmap which we then bitblt
  from via X API calls.  So we can't treat it *exactly* as a big array
  of pixels with offsets -- if the Pixmap is 400px wide and we have a
  tiny 20px wide dirty resize corner, we still need to write those 20px
  lines with a 400px stride.
 
  However, I think that means if you leave the TransportDIB as the union
  of all the dirty rects, everything should Just Work.  So your change
  becomes sending one unioned dirty rect image of pixel data, some of
  which that may not have actually been drawn to, along with an array of
  actually dirty subrectangles within the image.
 

 I see my main concern was about wasting memory since each shared
 memory allocation has significant overhead, and the minimize
 allocation size is quite large, so even if you don't need much memory,
 you end up using a lot of memory.

 -Darin


--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---

[chromium-dev] Re: Need help in finding a performance problem...

[Marc-Andre Decoste]

 Feel free to ping delme/del agl if you need help on Linux.  ;)


Will do... thanks for the offer :-)

BYE
MAD

On Mon, May 11, 2009 at 8:42 PM, Evan Martin e...@chromium.org wrote:


 On Mon, May 11, 2009 at 5:40 PM, Marc-Andre Decoste m...@chromium.org
 wrote:
  However, I think that means if you leave the TransportDIB as the union
 
  of all the dirty rects, everything should Just Work.  So your change
 
  becomes sending one unioned dirty rect image of pixel data, some of
 
  which that may not have actually been drawn to, along with an array of
 
  actually dirty subrectangles within the image.
 
  This is exactly what I'm doing on Windows... The receiving side loops on
 the
  dirty sub-rectangles and get their pixels from the single btimap being
  sent... And this bitmap is only valid in the areas of the
 sub-rectangles
  passed in a vector via IPC.
  Are we saying the same thing here?

 Yes.

  P.S.: I'm still validating that it is all OK on the Windows side, just
 fixed
  what I hope was the last refresh bug with Google maps, and then, I'll
 update
  the GTK and Coco version of the painting code...

 Feel free to ping delme/del agl if you need help on Linux.  ;)

 


--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---

[chromium-dev] Re: Need help in finding a performance problem...

[Marc-Andre Decoste]

Salut,
   I've been trying to use a single bitmap, but it is much more work than I
expected... And I couldn't get it to work flawlessly yet either... I get
some refresh problems once in a while that I can't figure out... I solved
many little bumps on the way, but now it works in most cases, except more
complex pages like Google maps, which are giving me a lot of trouble...
   Here's what I'm doing:

   - In RenderWidget::DoDeferredPaint(), I create a drawing canvas the size
   of the union of all invalidated rects (I also tried creating one the size of
   the whole view rect, just to see... and it didn't help).

   - I translate the canvas based on the origin of the union rect, and then
   call webwidget_-Paint() for each invalidated rect individually, properly
   setting the clip rect on the canvas.
   (When I try with a bitmap the size of the whole view rect, I skip the
   translation, of course.)

   - I send that single bitmap with its rect and the list of sub-rects to
   paint.

   - In RenderWidgethost::PaintBackingStoreRects(), I now pass a vector of
   rects to paint, and the rect of the whole bitmap to paint from.
   I added a loop to call BackingStoreManager::PrepareBackingStore() for
   each rect to paint.

   - The BackingStore::PaintRect() method now receives the rect for the
   bitmap (as well as the rect to paint, since they are not necessarily the
   same anymore).
   I then use this to specify the source origin point in the bitmap where to
   point from.

   I still want to investigate a few things before I clean up the current
version I have of this code and upload it to rietveld so that people can
look at it and give me hints as to where I may have goofed...

   But if you already have hints as to what could not be working in the
approach I described above, please send them my way... If you want more
details about the approach and/or the problems that I'm having, let me know,
and I will send them to you...

Thanks!

BYE
MAD
P.S.: I'm actually wondering if I'm wasting my time here... I tried to to
some profiling anyway, even though the code doesn't work flawlessly... And I
have not seen any dramatic improvements... Though as I said before, I don't
think I can trust the numbers I get on my machine... So I keep digging...

On Tue, May 5, 2009 at 6:26 PM, Marc-Andre Decoste m...@chromium.org wrote:

 OK, I'll make the change... Interestingly, this was my original idea, but I
 misunderstood your initial suggestions thinking you preferred an array of
 bitmaps too...
 I'll send a notice once I uploaded a new patch...

 Thanks!

 BYE
 MAD


 On Tue, May 5, 2009 at 6:08 PM, Adam Langley a...@chromium.org wrote:

 On Tue, May 5, 2009 at 3:00 PM, Marc-Andre Decoste m...@chromium.org
 wrote:
  That would be awsome...
  I just uploaded the patch here:
  http://codereview.chromium.org/108040

 At a high level, you're using one TransportDIB per rectangle, but it
 should be one per message (with multiple rects worth of image data
 inside). You can't really use any benchmarking results while this is
 the case.


 AGL




--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---

[chromium-dev] Re: Need help in finding a performance problem...

[Marc-Andre Decoste]

Salut,
   I've been trying to locally collect performance data to confirm whether
this was a good enough improvement or not and I can't seem to get consistent
enough results on my machine to draw a conclusion... Some things look
faster, and others look slower, but I sometimes get faster results with the
resize corner enabled, compare to disabled (which doesn't really make
sense)... So I don't think I can rely on these numbers...

   So I decided that I will go through with the code review of the changes,
and if Adam and Darin are happy with it, I will commit and monitor the
performance dashboard to see how it goes there... If it goes bad, I'll
revert... And if it goes well... I'll scream my happiness as loud as I can
(and those who know me a bit, know that I CAN be pretty LOUD!!! ;-)...

BYE
MAD

On Fri, May 1, 2009 at 1:00 PM, Adam Langley a...@chromium.org wrote:

 On Fri, May 1, 2009 at 9:06 AM, Marc-Andre Decoste m...@chromium.org
 wrote:
  Salut Evan,
thanks, I will do that... And the results seems better than I initially
  thought...

 If you get performance improvements, please do commit :)

 Evan is correct that Darin needs to check this over, but I'll happy
 code review everything where I can.


 AGL


--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---

[chromium-dev] Re: Need help in finding a performance problem...

[Marc-Andre Decoste]

That would be awsome...
I just uploaded the patch here:
http://codereview.chromium.org/108040

BYE
MAD

On Tue, May 5, 2009 at 5:14 PM, Mike Pinkerton pinker...@chromium.orgwrote:

 I was able to get very consistent results before with just TestShell
 on Mac running the mozilla page-cycler test locally. Would using
 TestShell instead of Chromium help, or do you want me to try your
 patch on Mac?

 Let me know.

 On Tue, May 5, 2009 at 2:08 PM, Marc-Andre Decoste m...@chromium.org
 wrote:
  Salut,
 I've been trying to locally collect performance data to confirm
 whether
  this was a good enough improvement or not and I can't seem to get
 consistent
  enough results on my machine to draw a conclusion... Some things look
  faster, and others look slower, but I sometimes get faster results with
 the
  resize corner enabled, compare to disabled (which doesn't really make
  sense)... So I don't think I can rely on these numbers...
 So I decided that I will go through with the code review of the
 changes,
  and if Adam and Darin are happy with it, I will commit and monitor the
  performance dashboard to see how it goes there... If it goes bad, I'll
  revert... And if it goes well... I'll scream my happiness as loud as I
 can
  (and those who know me a bit, know that I CAN be pretty LOUD!!! ;-)...
  BYE
  MAD
 
  On Fri, May 1, 2009 at 1:00 PM, Adam Langley a...@chromium.org wrote:
 
  On Fri, May 1, 2009 at 9:06 AM, Marc-Andre Decoste m...@chromium.org
  wrote:
   Salut Evan,
 thanks, I will do that... And the results seems better than I
   initially
   thought...
 
  If you get performance improvements, please do commit :)
 
  Evan is correct that Darin needs to check this over, but I'll happy
  code review everything where I can.
 
 
  AGL
 
 



 --
 Mike Pinkerton
 Mac Weenie
 pinker...@google.com


--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---

[chromium-dev] Re: Need help in finding a performance problem...

[Marc-Andre Decoste]

OK, I'll make the change... Interestingly, this was my original idea, but I
misunderstood your initial suggestions thinking you preferred an array of
bitmaps too...
I'll send a notice once I uploaded a new patch...

Thanks!

BYE
MAD

On Tue, May 5, 2009 at 6:08 PM, Adam Langley a...@chromium.org wrote:

 On Tue, May 5, 2009 at 3:00 PM, Marc-Andre Decoste m...@chromium.org
 wrote:
  That would be awsome...
  I just uploaded the patch here:
  http://codereview.chromium.org/108040

 At a high level, you're using one TransportDIB per rectangle, but it
 should be one per message (with multiple rects worth of image data
 inside). You can't really use any benchmarking results while this is
 the case.


 AGL


--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---

[chromium-dev] Re: Need help in finding a performance problem...

[Marc-Andre Decoste]

Salut again,
   I got a bit further, but not quite there yet...

   The (0, 0, 17, 17) invalidation request came from the fact that WebKit
invalidates the scroll bars when their enabled state is set, and it was done
before the frame rect of the scroll bar is set... So I fixed that (by
simply reversing the call order) and so we save a few extraneous
invalidations less... But that didn't make much of a difference in the
timing unfortunately.

   Then I tried to pass an array of bitmaps and rects (as opposed to a union
of rects) to the paint rect IPC message, and it cut off 100ms of my 3.2
seconds scenario (so down to 3.1s), but it also cuts off some time off of
the scenario without the resizer rect (which went from about 2.72s to
roughly 2.68s)...

   So these don't seem to be THE reason for this slow down... So I keep
digging... But I wonder if it would be worth it or not to commit these
changes anyway, since they do provide a small performance improvement (at
least in the scenario I'm working with, would be interesting to compare to
other scenarios, I may try that later today)...

Again, thanks for any hints if you have some... ;-)

BYE
MAD

On Thu, Apr 30, 2009 at 2:50 PM, Marc-Andre Decoste m...@chromium.orgwrote:

 As an intermediate point of reference, looking at the calls
 to RenderWidget::DidInvalidateRect() and tracing the cases where the new
 rect doesn't intersect with the current paint_rect, I get the following
 results:
 Without the resizer rect:
 (0, 0, 743, 633) not in (0, 0, 0, 0)
 (362, 8, 1, 1) not in (0, 0, 0, 0)
 (726, 0, 17, 609) not in (0, 0, 0, 0)
 (0, 0, 17, 17) not in (0, 0, 0, 0)

 With the resizer rect:
 (0, 0, 743, 633) not in (0, 0, 0, 0)
 (362, 8, 1, 1) not in (0, 0, 0, 0)
 (726, 0, 17, 592) not in (0, 0, 0, 0)
 (726, 592, 17, 17) not in (0, 0, 0, 0)
 (0, 0, 17, 17) not in (726, 592, 17, 17)
 (21, 149, 12, 25) not in (0, 0, 0, 0)

So we do get a few more, and the scariest is the second last one, where
 we get the two extreme corners of the window... I'll try to see if this
 could actually be a bug where the position of the invalidation is wrong and
 the intent was to get it at (726, 592) but it wasn't offset properly... But
 since we also have that same (0, 0, 17, 17) request in the no resizer rect
 scenario, I doubt it...

Otherwise, I'll try to compute the timings with a new paint message that
 receives an array of rects instead of a union...

 BYE
 MAD

 On Thu, Apr 30, 2009 at 1:59 PM, Marc-Andre Decoste m...@chromium.orgwrote:

 Salut Adam,
this is a theory that I'm currently validating... And I will try to
 change the IPC message code  to confirm that it will resolve the problem...
 So I guess you don't see any problem in this approach... So if I succeed,
 now I know who to ask for a code review :-)

 Thanks!

 BYE
 MAD


 On Thu, Apr 30, 2009 at 1:44 PM, Adam Langley a...@chromium.org wrote:

 On Thu, Apr 30, 2009 at 7:51 AM, Marc-Andre Decoste m...@chromium.org
 wrote:
 An alternative could be to send a bitmap the size of the union rect,
 but
  only paint the individual rects in it, and extract them individually on
 the
  other side of the IPC... But I wonder if it would be worth the added
  complexity and risk... Unless I missed something (which is most
 probably the
  case :-)...

 Forgive me if I'm misunderstanding here. But is the problem that the area
 of the
 union rectangle is significantly greater than the areas of the actually
 damaged
 regions, thus we're painting too much?

 If that's the case, we could well change the PaintRect and ScrollRect
 messages
 to carry a vector of rects and have them arranged in sequence in the
 TransportDIB. Since I'm currently to blame for much of the IPC painting
 code, I
 can do this if it'll be of benefit.


 AGL





--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---

[chromium-dev] Re: Need help in finding a performance problem...

[Marc-Andre Decoste]

Salut Evan,
  thanks, I will do that... And the results seems better than I initially
thought...

   Now I need a better way to validate the results, because I think the high
level evaluation I did previously was misleading... Looking at the data a
little closer, I seem to have reached a much better performance than I
thought, and the scenarios of with and without the resize corner are much
closer than I initially thought with these two changes...

   So I'm going to work a bit more on making sure I have my numbers
straight, and then get the gurus to validate my fixes :-)

Thanks again!

BYE
MAD

On Fri, May 1, 2009 at 11:54 AM, Evan Martin e...@chromium.org wrote:

 On Fri, May 1, 2009 at 8:36 AM, Marc-Andre Decoste m...@chromium.org
 wrote:
 So these don't seem to be THE reason for this slow down... So I keep
  digging... But I wonder if it would be worth it or not to commit these
  changes anyway, since they do provide a small performance improvement (at
  least in the scenario I'm working with, would be interesting to compare
 to
  other scenarios, I may try that later today)...

 If you have improved page load performance as measured by the page
 cyclers, it would seem very reasonable to commit it.  Though you
 probably want to run it by Darin first as there are a lot of subtle
 details to how painting works.


--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---

[chromium-dev] Re: Need help in finding a performance problem...

[Marc-Andre Decoste]

Salut Aaron,
   no, this is not recent... This is something that was encountered a while
back and put on a shelf because it wasn't critical... I'm now taking it off
the shelf and trying to put an end to it :-)

Thanks!

BYE
MAD

On Thu, Apr 30, 2009 at 1:56 PM, Aaron Boodman a...@chromium.org wrote:

 Was this a recent regression? I made some changes in that area
 recently to support transparent webviews. I tried not to change
 anything in the case where transparency is not needed, but I imagine
 this could have been me.

 Here is the change:


 http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/renderer_host/render_widget_host_unittest.cc?revision=14378view=markup

 - a

 On Thu, Apr 30, 2009 at 10:44 AM, Adam Langley a...@chromium.org wrote:
 
  On Thu, Apr 30, 2009 at 7:51 AM, Marc-Andre Decoste m...@chromium.org
 wrote:
 An alternative could be to send a bitmap the size of the union rect,
 but
  only paint the individual rects in it, and extract them individually on
 the
  other side of the IPC... But I wonder if it would be worth the added
  complexity and risk... Unless I missed something (which is most probably
 the
  case :-)...
 
  Forgive me if I'm misunderstanding here. But is the problem that the area
 of the
  union rectangle is significantly greater than the areas of the actually
 damaged
  regions, thus we're painting too much?
 
  If that's the case, we could well change the PaintRect and ScrollRect
 messages
  to carry a vector of rects and have them arranged in sequence in the
  TransportDIB. Since I'm currently to blame for much of the IPC painting
 code, I
  can do this if it'll be of benefit.
 
 
  AGL
 
   
 


--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---

[chromium-dev] Re: Need help in finding a performance problem...

[Marc-Andre Decoste]

Salut Adam,
   this is a theory that I'm currently validating... And I will try to
change the IPC message code  to confirm that it will resolve the problem...
So I guess you don't see any problem in this approach... So if I succeed,
now I know who to ask for a code review :-)

Thanks!

BYE
MAD

On Thu, Apr 30, 2009 at 1:44 PM, Adam Langley a...@chromium.org wrote:

 On Thu, Apr 30, 2009 at 7:51 AM, Marc-Andre Decoste m...@chromium.org
 wrote:
 An alternative could be to send a bitmap the size of the union rect,
 but
  only paint the individual rects in it, and extract them individually on
 the
  other side of the IPC... But I wonder if it would be worth the added
  complexity and risk... Unless I missed something (which is most probably
 the
  case :-)...

 Forgive me if I'm misunderstanding here. But is the problem that the area
 of the
 union rectangle is significantly greater than the areas of the actually
 damaged
 regions, thus we're painting too much?

 If that's the case, we could well change the PaintRect and ScrollRect
 messages
 to carry a vector of rects and have them arranged in sequence in the
 TransportDIB. Since I'm currently to blame for much of the IPC painting
 code, I
 can do this if it'll be of benefit.


 AGL


--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---

[chromium-dev] Re: Need help in finding a performance problem...

[Marc-Andre Decoste]

As an intermediate point of reference, looking at the calls
to RenderWidget::DidInvalidateRect() and tracing the cases where the new
rect doesn't intersect with the current paint_rect, I get the following
results:
Without the resizer rect:
(0, 0, 743, 633) not in (0, 0, 0, 0)
(362, 8, 1, 1) not in (0, 0, 0, 0)
(726, 0, 17, 609) not in (0, 0, 0, 0)
(0, 0, 17, 17) not in (0, 0, 0, 0)

With the resizer rect:
(0, 0, 743, 633) not in (0, 0, 0, 0)
(362, 8, 1, 1) not in (0, 0, 0, 0)
(726, 0, 17, 592) not in (0, 0, 0, 0)
(726, 592, 17, 17) not in (0, 0, 0, 0)
(0, 0, 17, 17) not in (726, 592, 17, 17)
(21, 149, 12, 25) not in (0, 0, 0, 0)

   So we do get a few more, and the scariest is the second last one, where
we get the two extreme corners of the window... I'll try to see if this
could actually be a bug where the position of the invalidation is wrong and
the intent was to get it at (726, 592) but it wasn't offset properly... But
since we also have that same (0, 0, 17, 17) request in the no resizer rect
scenario, I doubt it...

   Otherwise, I'll try to compute the timings with a new paint message that
receives an array of rects instead of a union...

BYE
MAD

On Thu, Apr 30, 2009 at 1:59 PM, Marc-Andre Decoste m...@chromium.orgwrote:

 Salut Adam,
this is a theory that I'm currently validating... And I will try to
 change the IPC message code  to confirm that it will resolve the problem...
 So I guess you don't see any problem in this approach... So if I succeed,
 now I know who to ask for a code review :-)

 Thanks!

 BYE
 MAD


 On Thu, Apr 30, 2009 at 1:44 PM, Adam Langley a...@chromium.org wrote:

 On Thu, Apr 30, 2009 at 7:51 AM, Marc-Andre Decoste m...@chromium.org
 wrote:
 An alternative could be to send a bitmap the size of the union rect,
 but
  only paint the individual rects in it, and extract them individually on
 the
  other side of the IPC... But I wonder if it would be worth the added
  complexity and risk... Unless I missed something (which is most probably
 the
  case :-)...

 Forgive me if I'm misunderstanding here. But is the problem that the area
 of the
 union rectangle is significantly greater than the areas of the actually
 damaged
 regions, thus we're painting too much?

 If that's the case, we could well change the PaintRect and ScrollRect
 messages
 to carry a vector of rects and have them arranged in sequence in the
 TransportDIB. Since I'm currently to blame for much of the IPC painting
 code, I
 can do this if it'll be of benefit.


 AGL




--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---

[chromium-dev] Last Known Good Revision

[Marc-Andre Decoste]

Salut Chromium-Dev'ers,
  if you are not interested in knowing about the last revisions that
successfully built, you can stop reading here (or you could actually go on
if you are curious because this email isn't that long anyway :-).

We just added a new builder status receiver to the build bot so that we
can post the revision number when a build succeeds all the way  to the unit
test on all platforms (win, mac and linux). You can access the latest good
revision from this URL: http://chromium-status.appspot.com/lkgr, which
simply returns the revision number (so you could easily use it in a script
automating your local build with the Last Known Good Revision). You can also
access the list of the last known good revisions here:
http://chromium-status.appspot.com/revisions.

We will soon start using the LKGR number on the try servers to make sure
that the patches you try are not affected by a failing HEAD build.

Enjoy!

BYE
MAD

--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---

[chromium-dev] Re: Is there a scroll bar?

[Marc-Andre Decoste]

OK, Thanks... I'll try to find the slowdown then...

BYE
MAD
On Tue, Feb 17, 2009 at 6:48 PM, Brett Wilson bre...@chromium.org wrote:

 On Tue, Feb 17, 2009 at 3:43 PM, Marc-Andre Decoste m...@google.com
 wrote:
  Salut,
 I'm looking for a way to know, at the BrowserView (or even at
  the RenderWidgetHostViewWin) level, if there is one, and only one scroll
 bar
  visible. I thought I could ask Windows with ::GetScrollBarInfo, but it
  doesn't work since the scroll bars are handled by WebKit and not by
 Windows.

 Scroll bars a non-native and rendered by WebKit. The browser doesn't
 know anything about it. I think you will have to do this at the WebKit
 level.

 Brett


--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---

[chromium-dev] Re: Adding an existing child to a parent view... should it crash?

[Marc-Andre Decoste]

Salut,
   I got a crash while playing with new code and I wonder if we should fix
the view parenting code to avoid this. The problem happens when you try to
add a child view to a parent view that already has this view as a child.

Here's how it goes:

First we call AddChildView with an already child view pointer:
void View::AddChildView(View* v) {
  AddChildView(static_castint(child_views_.size()), v, false);
}

the child_views_.size() value will include this child view...

void View::AddChildView(int index, View* v, bool floating_view) {
  // Remove the view from its current parent if any.
  if (v-GetParent())
v-GetParent()-RemoveChildView(v);

  if (!floating_view) {
// Sets the prev/next focus views.
InitFocusSiblings(v, index);
  }
[...]

since the child view already has this view as a parent, it will be removed
as a child, which is fine... But now, the child_views_.size() value will be
decremented, and index is now too big...

Now I'm wondering, should we simply return if we recognize ourselves as the
current parent of the child we were given? Or at least assert that we are
NOT the current parent? Or just leave it like that and document that it can
NOT be called with an already child view?

Thanks!

BYE
MAD

--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---

[chromium-dev] Re: What's missing in my view?

[Marc-Andre Decoste]

Salut Darin,

   I know, I have investigated that route, but the webkit code for the
resizer is very tied to HTML tag rendering, so I had a hard time finding how
to reuse it. Also, since I realized that I couldn't reuse it in the same way
when I get to draw over bottom shelves like the download shelf, I concluded
that it would be better to do it on our own...

   I have it all working when it is over the download shelf now, and looking
at the status bubble as suggested by Jo, I see that it has to create its own
window to set the status view as the content of that window... I think this
would be a little overkill for a resize corner, but if it is the only way to
do it over the tab content... so be it... Unless someone else has another
suggestion?

Thanks!

BYE
MAD

On Sat, Dec 13, 2008 at 11:37 PM, Darin Fisher da...@chromium.org wrote:

 By the way, WebKit has support for drawing the resizer.  So, if you are
 trying to add a resizer to windows that display web content, you might want
 to just enable the feature in WebKit (or at least the rendering portion of
 it).
 -Darin


 On Fri, Dec 12, 2008 at 1:32 PM, Marc-Andre Decoste m...@google.comwrote:

 Salut,

I'm working on the resize corner view and am having a little trouble
 getting it to work. I tried to use the sample code provided in
 http://dev.chromium.org/developers/design-documents/views-windowing, by
 doing as follows:


- in BrowserView::Init(), I create a label view and add it as a child
of the contents_container_
- in BrowserView::Layout(), I get the parent of the label, as well as
the preferred size of the label and then do as follows:
label-SetBounds((parent_view-width() - ps.width() )/ 2,
(parent_view-height() - ps.height()) / 2,
ps.width(), ps.height());
- Then I run with a break point in Label::Paint() and it seems to do
as it is told except...
I don't see the Hello World text drawn anywhere...
- I also tried my own BrowserResizerView class which also has mouse
event overrides and they never get called, only the Paint override which
seems to be painting on a /dev/nul canvas...

What am I doing wrong? Do we have more detailed documentation about
 this, or is
 http://dev.chromium.org/developers/design-documents/chromeviews the only
 source of info we currently have?

 Thanks!

 BYE
 MAD





 


--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
Chromium-dev group.
To post to this group, send email to chromium-dev@googlegroups.com
To unsubscribe from this group, send email to 
chromium-dev+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/chromium-dev?hl=en
-~--~~~~--~~--~--~---

[chromium-dev] Re: What's missing in my view?

[Marc-Andre Decoste]

ok, cool... Thanks Peter...

I thought the resizer needed to be a leaf in the view hierarchy to be
drawn on top of all the other views and receive the mouse events...

So now I guess there is a way for the browser view to specify that
this child has to be laid on top of the other child, right? Is it just
by the order they are added as children of a parent, or is there
another trick?

Thanks again...

BYE
MAD

On 12/13/08, Peter Kasting pkast...@google.com wrote:
 On Fri, Dec 12, 2008 at 9:14 PM, Marc-Andre Decoste m...@google.com wrote:

So I'll try to make the resizer object a TabContents object,


 This will be disaster, do not go this route.  (TabContents means the
 contents are a tab, i.e. a web page or one of our sson-to-be-disappearing
 native views.  This is not the case for your resizer.)

 You should probably just be a child of the main browser window, not of the
 content area.  This will make life much better when you're blending against
 things like the download shelf, which themselves are going to be owned by
 the browser window and not the content area.

 Ben is the right person to ask about this stuff.

 PK

 


--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
Chromium-dev group.
To post to this group, send email to chromium-dev@googlegroups.com
To unsubscribe from this group, send email to 
chromium-dev+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/chromium-dev?hl=en
-~--~~~~--~~--~--~---

[chromium-dev] Re: What's missing in my view?

[Marc-Andre Decoste]

Salut again,

   a little update. I got the drawing of my view to work within the download
shelf, and I think I found why I couldn't get it to work as a child of the
content_container_. This latter parent view seem to only be able to draw
TabContents objects...

   So I'll try to make the resizer object a TabContents object, hoping that
it will still be able to draw itself on top of the download shelf...

To be continued...

BYE
MAD

On Fri, Dec 12, 2008 at 4:32 PM, Marc-Andre Decoste m...@google.com wrote:

 Salut,

I'm working on the resize corner view and am having a little trouble
 getting it to work. I tried to use the sample code provided in
 http://dev.chromium.org/developers/design-documents/views-windowing, by
 doing as follows:


- in BrowserView::Init(), I create a label view and add it as a child
of the contents_container_
- in BrowserView::Layout(), I get the parent of the label, as well as
the preferred size of the label and then do as follows:
label-SetBounds((parent_view-width() - ps.width() )/ 2,
(parent_view-height() - ps.height()) / 2,
ps.width(), ps.height());
- Then I run with a break point in Label::Paint() and it seems to do as
it is told except...
I don't see the Hello World text drawn anywhere...
- I also tried my own BrowserResizerView class which also has mouse
event overrides and they never get called, only the Paint override which
seems to be painting on a /dev/nul canvas...

What am I doing wrong? Do we have more detailed documentation about
 this, or is
 http://dev.chromium.org/developers/design-documents/chromeviews the only
 source of info we currently have?

 Thanks!

 BYE
 MAD



--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
Chromium-dev group.
To post to this group, send email to chromium-dev@googlegroups.com
To unsubscribe from this group, send email to 
chromium-dev+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/chromium-dev?hl=en
-~--~~~~--~~--~--~---