Closed Bug 1046550 Opened 6 years ago Closed 5 years ago

Allow using Direct2D 1.1 for layer rendering

Categories

(Core :: Graphics, defect)

x86_64
Windows 7
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla35

People

(Reporter: mattwoodrow, Assigned: mattwoodrow)

References

(Depends on 1 open bug)

Details

Attachments

(3 files, 4 obsolete files)

Attached patch allow-d2d11 (obsolete) — Splinter Review
Patch that gets this somewhat working attached. There's still a lot of bugs and we hit asserts frequently.
Attachment #8465219 - Flags: review?(bas)
A couple of things that I saw:

DrawTargetD2D1 and SourceSurfaceD2D1 can both hold references to each other, causing cycles. It also causes us to fail this assertion http://mxr.mozilla.org/mozilla-central/source/gfx/layers/d3d11/TextureD3D11.cpp#248

Nothing appears to ever remove things from DrawTargetD2D1::mDependentTargets, so we hit this assertion: http://mxr.mozilla.org/mozilla-central/source/gfx/2d/DrawTargetD2D1.cpp#681
Comment on attachment 8465219 [details] [diff] [review]
allow-d2d11

Review of attachment 8465219 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/layers/d3d11/TextureD3D11.cpp
@@ +165,4 @@
>      MOZ_ASSERT(mDrawTarget->refCount() == 1);
> +    if (mTexture) {
> +      LockD3DTexture(mTexture.get());
> +    } else {

MOZ_ASSERT(mTexture10)

etc.

::: gfx/thebes/gfxPrefs.h
@@ +157,5 @@
>    DECL_GFX_PREF(Live, "gfx.color_management.rendering_intent", CMSRenderingIntent, int32_t, 0);
>  
>    DECL_GFX_PREF(Once, "gfx.direct2d.disabled",                 Direct2DDisabled, bool, false);
>    DECL_GFX_PREF(Once, "gfx.direct2d.force-enabled",            Direct2DForceEnabled, bool, false);
> +  DECL_GFX_PREF(Once, "gfx.direct2d.use1_1",                   Direct2DUse1_1, bool, false);

We might as well make it live I suppose?

::: gfx/thebes/moz.build
@@ +193,5 @@
>              'gfxDWriteTextAnalysis.cpp',
>          ]
>  
> +    if CONFIG['MOZ_ENABLE_DIRECT2D1_1']:
> +        DEFINES['USE_D2D1_1'] = True

It's unclear to me what we need this for? :) I'm guessing in some intermediate stage you had something conditional inside gfxWindowsPlatform?
Attachment #8465219 - Flags: review?(bas) → review+
gfxWindowsPlatform includes 2D.h which checks USE_D2D1_1.
Attached patch Allow d2d1_1 (obsolete) — Splinter Review
Updated patch without the racy device.
Attachment #8465938 - Flags: review?(bas)
Slightly cleaned up and splitted out Content device additions.
Attachment #8469227 - Flags: review?(matt.woodrow)
This is basically Matt Woodrow's patch split up and rebased. Carrying my t+.
Attachment #8469296 - Flags: review+
Last bit of Matt's patch, rebased. Carrying my r+.
Attachment #8465219 - Attachment is obsolete: true
Attachment #8465938 - Attachment is obsolete: true
Attachment #8465938 - Flags: review?(bas)
Attachment #8469302 - Flags: review+
Comment on attachment 8469227 [details] [diff] [review]
Part 1: Add content device to gfxWindowsPlatform

Review of attachment 8469227 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/thebes/gfxWindowsPlatform.cpp
@@ +1495,5 @@
> +  if (FAILED(hr)) {
> +    return;
> +  }
> +
> +  hr = d3d11CreateDevice(adapter, D3D_DRIVER_TYPE_UNKNOWN, nullptr,

You might want to #ifdef this piece in USE_D3D1_1 to avoid allocating it if it won't ever be used. Not that it matters much.
Attachment #8469227 - Flags: review?(matt.woodrow) → review+
This adds a bunch of code to assure temporary DTs created are also of the right backend.
Attachment #8469302 - Attachment is obsolete: true
Attachment #8488164 - Flags: review?(matt.woodrow)
The right patch this time.
Attachment #8488165 - Flags: review?(matt.woodrow)
Attachment #8488164 - Attachment is obsolete: true
Attachment #8488164 - Flags: review?(matt.woodrow)
Comment on attachment 8488165 [details] [diff] [review]
Part 3: Use Direct2D 1.1 when preffed on in D3D11 layers v2

Review of attachment 8488165 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/layers/moz.build
@@ +99,5 @@
>              'd3d11/CompositorD3D11.cpp',
>              'd3d11/ReadbackManagerD3D11.cpp',
> +        ]         
> +        if CONFIG['MOZ_ENABLE_DIRECT2D1_1']:
> +            DEFINES['USE_D2D1_1'] = True 

Whitespace!
Attachment #8488165 - Flags: review?(matt.woodrow) → review+
Depends on: 1068590
Comment on attachment 8488165 [details] [diff] [review]
Part 3: Use Direct2D 1.1 when preffed on in D3D11 layers v2

>+      if (gfxPrefs::Direct2DUse1_1() && Factory::SupportsD2D1()) {
>+        contentMask |= BackendTypeBit(BackendType::DIRECT2D1_1);
>+        defaultBackend = BackendType::DIRECT2D1_1;
>+      } else {
>+        defaultBackend = BackendType::DIRECT2D;
>+      }
SupportsD2D1() is #ifdef in 2D.h so the compile fails if you don't have it. Perhaps you could write
#ifdef USE_D2D1_1
      if (gfxPrefs::Direct2DUse1_1() && Factory::SupportsD2D1()) {
        contentMask |= BackendTypeBit(BackendType::DIRECT2D1_1);
        defaultBackend = BackendType::DIRECT2D1_1;
      } else
#endif
        defaultBackend = BackendType::DIRECT2D;
My bad, I now see bug 1068590 exists for that.
Depends on: 1077071
Depends on: 1077157
Depends on: 1076349
Depends on: 1153609
Depends on: 1153530
Depends on: 1270023
Depends on: 1310611
You need to log in before you can comment on or make changes to this bug.