Bug 148307 – Clear cairo-gl surface for initialization

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Jinyoung Hur]

Clear cairo-gl surface for initialization

[Jinyoung Hur]

Created attachment 259615 [details]
Patch

[Jinyoung Hur]

The accelerated canvas backed by cairo-gl has a possible memory initialize problem.

A texture memory that is created by calling glTexImage2D with no data, is in undefined status[1] so it can have a garbage data.

And also, according to the comments[2] in cairo.h, cairo seems not cleaning the passed texture when it create a surface.

So it would be good to clean the just created cairo-gl surface by hand.


[1] https://www.opengl.org/sdk/docs/man/html/glTexImage2D.xhtml  
    "The image is undefined if the user tries..."
[2] "if the user passes in a reference to some backing storage and asks cairo to wrap that in a #cairo_surface_t, then the contents are not modified.

[Jinyoung Hur]

Created attachment 259616 [details]
Patch

[Gyuyoung Kim]

CC'ing Martin.

[Martin Robinson]

Comment on attachment 259616 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=259616&action=review

Looks okay to me, but the code has a few extra steps which I think are unnecessary.

> Source/WebCore/platform/graphics/cairo/ImageBufferCairo.cpp:80
> +    cairo_rectangle(cr.get(), 0, 0, size.width(), size.height());
> +    cairo_set_operator(cr.get(), CAIRO_OPERATOR_CLEAR);
> +    cairo_fill(cr.get());

You don't need to use cairo_rectangle + cairo_fill, you can simply do cairo_paint.

> Source/WebCore/platform/graphics/cairo/ImageBufferCairo.cpp:81
> +    cairo_restore(cr.get());

No need to save and restore the context state, you are just throwing it away afterward.

[Jinyoung Hur]

Created attachment 259798 [details]
Patch

[Jinyoung Hur]

(In reply to comment #5)
> Comment on attachment 259616 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=259616&action=review
> 
> Looks okay to me, but the code has a few extra steps which I think are
> unnecessary.
> 
> > Source/WebCore/platform/graphics/cairo/ImageBufferCairo.cpp:80
> > +    cairo_rectangle(cr.get(), 0, 0, size.width(), size.height());
> > +    cairo_set_operator(cr.get(), CAIRO_OPERATOR_CLEAR);
> > +    cairo_fill(cr.get());
> 
> You don't need to use cairo_rectangle + cairo_fill, you can simply do
> cairo_paint.
> 
> > Source/WebCore/platform/graphics/cairo/ImageBufferCairo.cpp:81
> > +    cairo_restore(cr.get());
> 
> No need to save and restore the context state, you are just throwing it away
> afterward.

Thank you for a helpful review!
I've modified the patch as you said.

[WebKit Commit Bot]

Comment on attachment 259798 [details]
Patch

Clearing flags on attachment: 259798

Committed r188902: <http://trac.webkit.org/changeset/188902>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.