Closed Bug 1419767 Opened 4 years ago Closed 4 years ago

Ensure SVGs prefer data surfaces whenever possible


(Core :: Graphics: WebRender, enhancement, P1)




Tracking Status
firefox59 --- fixed


(Reporter: aosmond, Assigned: aosmond)


(Whiteboard: [wr-mvp])


(1 file, 2 obsolete files)

In imgFrame::InitWithDrawable, we call gfxPlatform::CanRenderContentToDataSurface() to determine whether or not we should render the SVG to a DataSourceSurface. imgFrame is the only consumer of this API. When we cannot render to a DataSourceSurface, we need to take a snapshot of the DrawTarget after rendering the SVG to get a SourceSurface to put into the SurfaceCache. It by default returns false, and exceptions are made for Mac and Android.

Linux should always return true, because it uses Skia as its content backend just like Mac and Android. It is only on Windows that we could potentially use D2D.

For WebRender however, it would be preferable if we could always render to a DataSourceSurface, because that allows us to use SourceSurfaceSharedData. That in turn allows us to avoid copies of the image data itself in both content and GPU processes, and avoid duplicate ImageKeys for the same image because we had to fallback to using blob images or ImageBridgeChild. This will have the side effect of using Skia to rasterize SVGs on Windows whereas before we may have used D2D if available.
Assignee: nobody → aosmond
Priority: -- → P1
Whiteboard: [wr-mvp]
I didn't think clearly when I rewrote the now obsoleted patch. The original method didn't need the backend type passed in as it as all hard coded to return true/false based on platform. Now it properly passes in the desired backend to use with data surfaces. Hopefully this greens the reftests.

Attachment #8930920 - Attachment is obsolete: true
As it stands, I expect the Windows QR SVG reftests will need some fuzz because of this, since it ends up using Skia instead of D2D to rasterize SVGs now.

There are two alternatives that come to mind: one, take out the check for if we are using WR; it will at least start using shared surfaces on Linux and on Windows if D2D was disabled in the content process. I expect that would mean SVGs will use PImageBridge/Textures to shuttle the D2D surfaces across.

Or two, continue using D2D to generate the surface, but do what the texture code will do anyways, and copy *that* into a shared surface. This does at least have the advantage over textures by sharing the memory instead of duping it in both the content and GPU processes, as well as enabling sharing the image keys across display items.
Let's not get too ambitious with this. For the moment I just want it to be correct without changing the world :). Right now on Linux we don't use shared surfaces (because nobody implemented CanRenderContentToDataSurface for it) and we don't use volatile surfaces on Windows if it disabled D2D. Let's switch to Factory::DoesBackendSupportDataDrawtarget straight up, as that is maintained and does what we want better.
Attachment #8931075 - Attachment is obsolete: true
Attachment #8931472 - Flags: review?(nical.bugzilla)
Summary: Ensure SVGs prefer data surfaces with WebRender → Ensure SVGs prefer data surfaces whenever possible
Attachment #8931472 - Flags: review?(nical.bugzilla) → review+
Pushed by
Remove gfxPlatform::CanRenderContentToDataSurface. r=nical
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
You need to log in before you can comment on or make changes to this bug.