Bug 20838 – [CAIRO] needs getImageData and putImageData support

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Dirk Schulze]

Cairo needs getImageData() support for HTML 5 and Canvas.

[Dirk Schulze]

Created attachment 23408 [details]
Cairo getImageData()

Added getImageData support to Cairo. Has the same logic as the Cg port but uses the stored m_surface.

[Dirk Schulze]

Created attachment 23412 [details]
Cairo getImageData()

added copyright.

[Dirk Schulze]

Created attachment 23413 [details]
Cairo get/putImageData()

added putImageData() too.

[Alp Toker]

(In reply to comment #3)
> Created an attachment (id=23413) [edit]
> Cairo get/putImageData()
> 
> added putImageData() too.
> 

Can we use cairo_image_surface_get_stride() instead of cairo_format_stride_for_width() in some places here?

An assertion to ensure the surface passed in are image surfaces using cairo_surface_get_type() would be helpful too (to catch issues when we start to make use of more platform surfaces in future).

[Dirk Schulze]

(In reply to comment #4)
> Can we use cairo_image_surface_get_stride() instead of
> cairo_format_stride_for_width() in some places here?

You mean reaplce:

+int stride = cairo_format_stride_for_width(CAIRO_FORMAT_ARGB32, m_size.width());

by

+int stride;
+if (cairo_surface_get_type(m_surface) == CAIRO_SURFACE_TYPE_IMAGE)
+    stride = cairo_image_surface_get_stride(m_surface);
+else
+    stride = cairo_format_stride_for_width(CAIRO_FORMAT_ARGB32, m_size.width());

and expanse it if necessary?

[Alp Toker]

(In reply to comment #5)
> (In reply to comment #4)
> > Can we use cairo_image_surface_get_stride() instead of
> > cairo_format_stride_for_width() in some places here?
> 
> You mean reaplce:
> 
> +int stride = cairo_format_stride_for_width(CAIRO_FORMAT_ARGB32,
> m_size.width());
> 
> by
> 
> +int stride;
> +if (cairo_surface_get_type(m_surface) == CAIRO_SURFACE_TYPE_IMAGE)
> +    stride = cairo_image_surface_get_stride(m_surface);
> +else
> +    stride = cairo_format_stride_for_width(CAIRO_FORMAT_ARGB32,
> m_size.width());
> 
> and expanse it if necessary?
> 

Nope. I meant putting a cairo_surface_get_type() check is at the top as a debug ASSERT() since no cairo_image_surface_ functions all require an image surface.

I don't think you need to use cairo_format_stride_for_width() at all here. cairo_image_surface_get_stride() can be used both to get the source stride in one function and the destination stride in the other, right?

[Dirk Schulze]

Created attachment 23475 [details]
Cairo get/putImageData() with ASSERT for image_surface

Added the asserts to get/putImageData and took cairo_image_surface_get_stride().

[Darin Adler]

Comment on attachment 23475 [details]
Cairo get/putImageData() with ASSERT for image_surface

+    unsigned char* dataDst = result->data()->data().data();

LOL!

Looks fine, r=me

[Jan Alonzo]

landed in r36813