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)
Tracking
()
RESOLVED
FIXED
mozilla37
People
(Reporter: bas.schouten, Assigned: bas.schouten)
References
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
BenWa
:
review+
|
Details | Diff | 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)
Comment 1•10 years ago
|
||
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 2•10 years ago
|
||
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-
Assignee | ||
Comment 3•10 years ago
|
||
Attachment #8526319 -
Attachment is obsolete: true
Attachment #8526960 -
Flags: review?(bgirard)
Comment 4•10 years ago
|
||
(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 5•10 years ago
|
||
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 6•10 years ago
|
||
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()) {|?
Comment 7•10 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
Updated•10 years ago
|
Summary: Use D3D11 for composition everywhere on Win7+ → Use D3D11 for composition everywhere on Win7+ using WARP if needed
Assignee | ||
Comment 8•10 years ago
|
||
(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.
You need to log in
before you can comment on or make changes to this bug.
Description
•