Closed
Bug 1419767
Opened 7 years ago
Closed 7 years ago
Ensure SVGs prefer data surfaces whenever possible
Categories
(Core :: Graphics: WebRender, enhancement, P1)
Core
Graphics: WebRender
Tracking
()
RESOLVED
FIXED
mozilla59
Tracking | Status | |
---|---|---|
firefox59 | --- | fixed |
People
(Reporter: aosmond, Assigned: aosmond)
Details
(Whiteboard: [wr-mvp])
Attachments
(1 file, 2 obsolete files)
4.27 KB,
patch
|
nical
:
review+
|
Details | Diff | Splinter Review |
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 | ||
Updated•7 years ago
|
Assignee: nobody → aosmond
Status: NEW → ASSIGNED
Assignee | ||
Comment 1•7 years ago
|
||
try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=898ac370200d771303c9bf9b32584e52cf8b2c7a
Updated•7 years ago
|
Priority: -- → P1
Whiteboard: [wr-mvp]
Assignee | ||
Comment 2•7 years ago
|
||
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. try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=df4512f237325d9cf63f1c74ebe5b165b9b1df33
Attachment #8930920 -
Attachment is obsolete: true
Assignee | ||
Comment 3•7 years ago
|
||
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.
Assignee | ||
Comment 4•7 years ago
|
||
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)
Assignee | ||
Updated•7 years ago
|
Summary: Ensure SVGs prefer data surfaces with WebRender → Ensure SVGs prefer data surfaces whenever possible
Updated•7 years ago
|
Attachment #8931472 -
Flags: review?(nical.bugzilla) → review+
Pushed by aosmond@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/c3cd5881c7ee Remove gfxPlatform::CanRenderContentToDataSurface. r=nical
Comment 6•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/c3cd5881c7ee
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox59:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
You need to log in
before you can comment on or make changes to this bug.
Description
•