638804 - Ensure that flush and markdirty are called for cairo surfaces

Status: RESOLVED WONTFIX

(Core :: Graphics, defect)

Description

[Benjamin Stover (:stechz)]

Corruptions can occur for surfaces if we don't tell cairo when a surface is being changed outside of cairo or if we don't have cairo flush any pending operations before a surface is changed outside of cairo.

Comment 1

[Benjamin Stover (:stechz)]

Attachment: Ensure that flush and makedirty are called for cairo surfaces

Comment 2

[Benjamin Stover (:stechz)]

Comment on attachment 516903 [details] [diff] [review]
Ensure that flush and makedirty are called for cairo surfaces

This catches:
* code in gfxUtils that touches surfaces
* code in IPC layers that flushes surfaces before forwarding a transaction
* code in gfxImageSurface that touches surfaces
* Android code that flushes cairo surfaces before giving Android the surface back

There still may be more. I'm looking at you, layout and canvas.

Comment 3

[Benjamin Stover (:stechz)]

Adding as blocker for bug 638594 because it may help fix ref tests.

Comment 4

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 516903 [details] [diff] [review]
Ensure that flush and makedirty are called for cairo surfaces

Overall, no objections. It seems like we will be doing some extra flushing/dirtying with no benefit here, but it does make us follow the api more closely.

> 
>   NS_ABORT_IF_FALSE(IsSurfaceDescriptorValid(mBackBuffer),
>                     "should have a back buffer by now");
>+  mBuffer.GetBuffer()->Flush();
>   BasicManager()->PaintedThebesBuffer(BasicManager()->Hold(this),
>                                       updatedRegion,
>                                       mBuffer.BufferRect(),
>                                       mBuffer.BufferRotation(),
>                                       mBackBuffer);

It's not obvious to me what these Flushes are for.

Comment 5

[Benjamin Stover (:stechz)]

> It's not obvious to me what these Flushes are for.

This is right before surfaces are getting shared to the other process. I'm not sure, but I figured that whatever cairo is coalescing would not come with the data that's being shared.

Comment 6

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 516903 [details] [diff] [review]
Ensure that flush and makedirty are called for cairo surfaces

Do we still want this?

Comment 7

[Benjamin Stover (:stechz)]

I'd say no.