Closed Bug 1102499 Opened 10 years ago Closed 10 years ago

Use D3D11 for composition everywhere on Win7+ using WARP if needed

Categories

(Core :: Graphics, defect)

x86_64
Windows 8.1
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla37

People

(Reporter: bas.schouten, Assigned: bas.schouten)

References

Details

Attachments

(1 file, 1 obsolete file)

Attached patch Use WARP for D3D11 when available (obsolete) (deleted) — Splinter Review
Theoretically we can use D3D11 for our composition everywhere when we're on Windows 7 and upwards, possibly using WARP then the GPU is blacklisted.
Attachment #8526319 - Flags: review?(bgirard)
We do a bunch of layers level optimizations when we have software composition (avoiding component alpha, only compositing changed areas of the screen, other misc changes), all of which have a big effect on performance. Are these going to matter with WARP?
Comment on attachment 8526319 [details] [diff] [review] Use WARP for D3D11 when available Review of attachment 8526319 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/thebes/gfxWindowsPlatform.cpp @@ +408,5 @@ > } > } > + if (NS_SUCCEEDED(gfxInfo->GetFeatureStatus(nsIGfxInfo::FEATURE_DIRECT3D_11_LAYERS, &status))) { > + if (status != nsIGfxInfo::FEATURE_STATUS_OK) { > + d2dBlocked = true; I don't see what this has to do with WARP. @@ +1653,5 @@ > return true; > } > > void > gfxWindowsPlatform::InitD3D11Devices() This function is bad and this patch makes it a lot worse. You have a bunch of decision intertwined that could be cleaned up and simplified. Feel free to ping me if you need convincing. You should be able to simplify the control flow of this function quite a bit.
Attachment #8526319 - Flags: review?(bgirard) → review-
Attachment #8526319 - Attachment is obsolete: true
Attachment #8526960 - Flags: review?(bgirard)
(In reply to Matt Woodrow (:mattwoodrow) from comment #1) > We do a bunch of layers level optimizations when we have software > composition (avoiding component alpha, only compositing changed areas of the > screen, other misc changes), all of which have a big effect on performance. > > Are these going to matter with WARP? I'm told the resolution was that it's worth taken WARP without these optimizations and that later we will composite&present only the damaged area.
Comment on attachment 8526960 [details] [diff] [review] Use WARP for D3D11 when available v2 Review of attachment 8526960 [details] [diff] [review]: ----------------------------------------------------------------- reluctant r+ with this addressed. ::: gfx/thebes/gfxPlatform.cpp @@ +2177,5 @@ > } > } > + if (!gfxPrefs::LayersD3D11DisableWARP()) { > + // Always support D3D11 when WARP is allowed. > + sLayersSupportsD3D11 = true; This isn't very nice. We're mixing d3d11 an d3d11_hardware/d3d11_warp_software in the code. It's hard to follow because you're reporting this and relying on a check later to ensure that we don't try d3d11_hardware later down the road. ::: gfx/thebes/gfxWindowsPlatform.cpp @@ +406,5 @@ > if (status != nsIGfxInfo::FEATURE_STATUS_OK) { > d2dBlocked = true; > } > } > + if (NS_SUCCEEDED(gfxInfo->GetFeatureStatus(nsIGfxInfo::FEATURE_DIRECT3D_11_LAYERS, &status))) { Make sure to remove this hunk since you moved it to another patch. @@ +1667,4 @@ > mD3D11DeviceInitialized = true; > > + bool useWARP = false; > + ScopedGfxFeatureReporter reporterWARP("D3D11-WARP", gfxPrefs::LayersD3D11ForceWARP()); Shouldn't this be further below in the WARP block? This will cause us to write D3D11-WARP - in code paths where we didn't attempt it because we got accelerated d3d11. @@ +1739,5 @@ > + adapter = nullptr; > + } > + } > + > + if (useWARP) { I'd be better to check if warp is disabled here instead of the two spots above. @@ +1740,5 @@ > + } > + } > + > + if (useWARP) { > + hr = d3d11CreateDevice(nullptr, D3D_DRIVER_TYPE_WARP, nullptr, MOZ_ASSERT(!gfxPrefs::LayersD3D11DisableWARP()); MOZ_ASSERT(!mD3D11Device); @@ +1764,5 @@ > + // We create our device for D2D content drawing here. Normally we don't use > + // D2D content drawing when using WARP. However when WARP is forced by > + // default we will let Direct2D use WARP as well. > + if (Factory::SupportsD2D1() && (!useWARP || gfxPrefs::LayersD3D11ForceWARP())) { > + hr = d3d11CreateDevice(adapter, useWARP ? D3D_DRIVER_TYPE_WARP : D3D_DRIVER_TYPE_UNKNOWN, nullptr, MOZ_ASSERT(adapter);?
Attachment #8526960 - Flags: review?(bgirard) → review+
Comment on attachment 8526960 [details] [diff] [review] Use WARP for D3D11 when available v2 Review of attachment 8526960 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/thebes/gfxWindowsPlatform.cpp @@ +1711,5 @@ > + if (!useWARP) { > + adapter = GetDXGIAdapter(); > + > + if (!adapter) { > + if (!gfxPrefs::LayersD3D11DisableWARP()) { Isn't this |if (gfxPrefs::LayersD3D11DisableWARP()) {|?
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
Depends on: 1107297
Summary: Use D3D11 for composition everywhere on Win7+ → Use D3D11 for composition everywhere on Win7+ using WARP if needed
Depends on: 1145143
(In reply to Masatoshi Kimura [:emk] from comment #6) > Comment on attachment 8526960 [details] [diff] [review] > Use WARP for D3D11 when available v2 > > Review of attachment 8526960 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: gfx/thebes/gfxWindowsPlatform.cpp > @@ +1711,5 @@ > > + if (!useWARP) { > > + adapter = GetDXGIAdapter(); > > + > > + if (!adapter) { > > + if (!gfxPrefs::LayersD3D11DisableWARP()) { > > Isn't this |if (gfxPrefs::LayersD3D11DisableWARP()) {|? You're absolutely right. This situation should be rare but I should fix it. Well spotted.
Depends on: 1149761
Depends on: 1167485
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: