Re: [webkit-dev] Canvas performance and memory usage

2010-08-04 Thread Christophe Public
Hi,

Actually, I was questioning also the necessity of this extra buffer.
I'll just give an update and some numbers. We have a very large canvas (few
MB) and our updates are very frequent but very small (clip area is for
example 3x3 pixels), it takes about 25ms for our system to handle the
expose. Once the double buffer is removed, the expose takes less than 1ms.

We applied our change to image(). When the BitmapImage is created, we now
pass the m_data.m_surface after increasing its reference count through
cairo_surface_reference and removed the copy.

Our tests didn't detect any issue after this change.

Christophe

On Wed, Aug 4, 2010 at 11:07 AM, David Hyatt

hy...@apple.com wrote:

 I'm confused why a special method is needed though.  Can't image() just
 avoid the full copy?  Given how we use image() in WebKit, I don't think
 there's any reason to be concerned if image() continues to reflect the
 contents of the ImageBuffer.  I think we should just switch to that model
 for the CG port also anyway, since I'm unconvinced we're truly avoiding a
 copy, and return an image with a custom data provider that feeds the current
 contents of the ImageBuffer to the image.

 dave
 (hy...@apple.com)

 On Aug 3, 2010, at 8:55 PM, Martin Robinson wrote:

  Resent from the proper address:
 
  On Tue, Aug 3, 2010 at 6:00 PM, Martin Robinson
  martin.james.robin...@gmail.com wrote:
  I notice that Qt added imageForRendering() and felt they could not use
  image() for some reason.  I'd be curious if a Qt expert could weigh in
 on
  that, since maybe with a redesign a separate call would not be needed.
 
  I'm not a Qt expert, but just based on a quick look, it seems that
  imageForRendering  avoids the full QPixmap copy. Christophe,
  when you open a bug for this issue,  please CC me, as I have a
  small patch in my tree which has the same imageForRendering
  pecialization, but for Cairo.
 
  Martin
  ___
  webkit-dev mailing list
  webkit-dev@lists.webkit.org
  http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

 ___
 webkit-dev mailing list
 webkit-dev@lists.webkit.org
 http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] Canvas performance and memory usage

2010-08-04 Thread Christophe Public
Thanks for the clarification. If this is the main reason, then it seems a
valuable optimization to do.
Christophe

On Wed, Aug 4, 2010 at 2:52 PM, Oliver Hunt oli...@apple.com wrote:

 The reason ImageBuffer::image() makes a copy (be it a deep copy, or CoW) is
 almost exclusively for the purpose of ensuring correct behaviour in the case
 where a canvas is drawn onto itself, eg.

 context = myCanvas.getContext(2d);
 context.drawImage(myCanvas, 0, 0);

 Off hand I can think of no other case where this would be necessary, so it
 seems like the best solution would be to make the default behaviour for
 image() be to return a reference to a potentially mutable image, and to give
 the canvas a distinct method for getting a copied image in the case where
 it's actually necessary.

 --Oliver

 On Aug 4, 2010, at 2:45 PM, Christophe Public wrote:

 Hi,

 Actually, I was questioning also the necessity of this extra buffer.
 I'll just give an update and some numbers. We have a very large canvas (few
 MB) and our updates are very frequent but very small (clip area is for
 example 3x3 pixels), it takes about 25ms for our system to handle the
 expose. Once the double buffer is removed, the expose takes less than 1ms.

 We applied our change to image(). When the BitmapImage is created, we now
 pass the m_data.m_surface after increasing its reference count through
 cairo_surface_reference and removed the copy.

 Our tests didn't detect any issue after this change.

 Christophe

 On Wed, Aug 4, 2010 at 11:07 AM, David Hyatt

 hy...@apple.com wrote:

 I'm confused why a special method is needed though.  Can't image() just
 avoid the full copy?  Given how we use image() in WebKit, I don't think
 there's any reason to be concerned if image() continues to reflect the
 contents of the ImageBuffer.  I think we should just switch to that model
 for the CG port also anyway, since I'm unconvinced we're truly avoiding a
 copy, and return an image with a custom data provider that feeds the current
 contents of the ImageBuffer to the image.

 dave
 (hy...@apple.com)

 On Aug 3, 2010, at 8:55 PM, Martin Robinson wrote:

  Resent from the proper address:
 
  On Tue, Aug 3, 2010 at 6:00 PM, Martin Robinson
  martin.james.robin...@gmail.com wrote:
  I notice that Qt added imageForRendering() and felt they could not use
  image() for some reason.  I'd be curious if a Qt expert could weigh in
 on
  that, since maybe with a redesign a separate call would not be needed.
 
  I'm not a Qt expert, but just based on a quick look, it seems that
  imageForRendering  avoids the full QPixmap copy. Christophe,
  when you open a bug for this issue,  please CC me, as I have a
  small patch in my tree which has the same imageForRendering
  pecialization, but for Cairo.
 
  Martin
  ___
  webkit-dev mailing list
  webkit-dev@lists.webkit.org
  http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

 ___
 webkit-dev mailing list
 webkit-dev@lists.webkit.org
 http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


 ___
 webkit-dev mailing list
 webkit-dev@lists.webkit.org
 http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev



___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


[webkit-dev] Canvas performance and memory usage

2010-08-03 Thread Christophe Public
Hi,

There is a performance hit when using HTML canvas when using
ImageBufferCairo:

- As soon as something is rendered in the canvas, the
HTMLCanvasElement::willDraw method is called which in turn calls clearImage
on the imageBuffer if one is already present.
- When the render tree is traversed (HTMLCanvasElement::paint gets called),
the image containing the canvas is drawn using GraphicsContext::drawImage.
Unfortunately, ImageBuffer::image() (in BufferImageCairo.cpp) allocates a
new surface, and copies the image surface into it. When very frequent
updates are done in the canvas, this double buffering causes significant
performance degradation that worsens with large canvas sizes. In addition,
the extra memory consumption can become quite expensive especially on
embedded devices.

Can anybody explain why this double buffering is necessary for ImageBuffer
specifically for the Cairo port?

We are looking at disabling the double buffering at least for the canvas
because of both performance and memory issues. It would also be nice to
disable it for images as well but we would like to understand why in the
first place it was added. Note that a code comment states This seems silly,
but is the way the CG port works: image() is intended to be used only when
rendering is complete.

Any suggestion on how to fix this issue is welcome. If it is considered a
bug, then I will create a bug as well to keep track of it.

Regards,
Christophe
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev