980472 - Make SurfaceToPackedBGRA take a DataSourceSurface to avoid unnecessary surface conversion

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Jonathan Watt [:jwatt]]

Right now callers of SurfaceToPackedBGRA that happen to have a DataSourceSurface end up having GetDataSurface called on that surface by SurfaceToPackedBGRA. Since bug 943614 that involves a second, unnecessary conversion (to type DATA) for certain backends. We should have callers do the GetDataSurface call as necessary and pass a DataSourceSurface so that we're not doing an extra conversion for callers that already have a DataSourceSurface.

Comment 1

[Jonathan Watt [:jwatt]]

Attachment: patch

Comment 2

[Jonathan Watt [:jwatt]]

There's only one caller currently, and that already passes a DataSourceSurface. Another patch that I have that adds a second caller also does the same.

Comment 3

[Jonathan Watt [:jwatt]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2ac59d28974c

Comment 4

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/2ac59d28974c

Comment 5

[Jonathan Watt [:jwatt]]

(In reply to Jonathan Watt [:jwatt] from comment #0)
> Right now callers of SurfaceToPackedBGRA that happen to have a
> DataSourceSurface end up having GetDataSurface called on that surface by
> SurfaceToPackedBGRA. Since bug 943614 that involves a second, unnecessary
> conversion (to type DATA) for certain backends.

FWIW I'm told that this is wrong. The "second" call will just create a very lightweight wrapper for which GetType()==DATA, and that has virtually no overhead. That said, it's still good to have callers know that the call involves converting to a data SourceSurface (has readback overhead, potentially), so this is still not a bad change.