1343814 - Factor a base class out of Compositor

Status: RESOLVED FIXED

(Core :: Graphics: Layers, enhancement)

Description

[David Anderson [:dvander] - inactive, e-mail if emergency]

It is difficult to introduce new HostLayerManagers and have them re-use existing texture/compositable code, because much of layers depends on a Compositor instance. This set of patches aims to factor texture-related code out of Compositor and into a TextureSourceProvider class.

Rather than add D3D11/OGL-specific TextureSourceProviders, the new class just directly exposes the ID3D11Device/GLContext and returns null if none exists. That turned out to be a lot easier and allows almost everything to stop doing crazy casting. A few classes still cast down to CompositorOGL since they need access to helper methods.

The major weirdness in the TextureHost/Compositor relationship is the SetCompositor call. It's inconsistent and messy. It gets called pretty much at random. Sometimes before using a texture we'll happen to call SetCompositor, just in case somewhere else we forgot to. Sometimes SetCompositor will reject a new compositor, sometimes not. Sometimes we'll remember to SetCompositor on sibling textures, sometimes not. Sometimes an invalid compositor causes stuff to get flushed, but not always. We handle setting a null compositor, but it's not clear why this would ever happen.

Anyway, these patches don't address any of that. I tried to preserve the behavior of existing SetCompositor calls.

Comment 1

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: part 1, TextureSourceProvider

This is the new base class for Compositor.

Comment 2

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: part 2, change TextureSource

Change TextureSource::SetCompositor to TextureSource::SetTextureSourceProvider. In almost every case we can stop storing the compositor and instead directly store the device. We reject a change in GLContext or ID3D11Device, but don't care about the actual provider.

Comment 3

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: part 3.1, change TextureHost API

This changes the TextureHost API, removing GetCompositor (we can just store the provider always), and changing SetCompositor to SetTextureSourceProvider.

This patch includes all of the boring syntactic changes. The material changes are separate for easier reviewing.

Comment 4

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: part 3.2, material changes to TextureHosts

This changes all of the platform-specific TextureHosts to use TextureSourceProvider. I tried to keep the reject/evict behavior the same for when the provider doesn't match.

One minor change is that the D3D11 code will now use the compositor's D3D11 device and not the system one (which could change randomly during a device reset).

Comment 5

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: part 4, change Composite() calls

Boring syntactic change - pass the Compositor into Composite calls, in preparation for removing CompositableHost::mCompositor.

Comment 6

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: part 6, rm CompositableHost::mCompositor

Switch CompositableHost and LayerTransactionParent to use TextureSourceProvider. This is mostly syntactic.

Comment 7

[Milan Sreckovic [:milan] (needinfo for best results)]

Let's coordinate these changes with the graphics branch when it comes time to land them.

Comment 8

[Kartikaya Gupta (email:[e-mail])]

(In reply to Milan Sreckovic [:milan] from comment #7)
> Let's coordinate these changes with the graphics branch when it comes time
> to land them.

Wes just pushed a merge of graphics back to m-c [1], so now is as good a time as any to land this, since graphics and m-c are identical. (And by "now" I really mean "wait until those graphics patches get merged over to autoland or inbound, and then rebase and land on top of it").

[1] https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&revision=942165e408f0241cf543c368db00fefe22a9997a

Comment 9

[Milan Sreckovic [:milan] (needinfo for best results)]

Modulo moving to 55 on Monday, that is :)

Comment 10

[David Anderson [:dvander] - inactive, e-mail if emergency]

No merge or build conflicts from pulling latest m-c, luckily.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4768fe2f6131
Factor texture methods out of Compositor into a TextureSourceProvider class. (bug 1343814 part 1, r=mattwoodrow)
https://hg.mozilla.org/integration/mozilla-inbound/rev/53c6fa699fa7
Change TextureSource::SetCompositor to use TextureSourceProvider. (bug 1343814 part 2, r=mattwoodrow)
https://hg.mozilla.org/integration/mozilla-inbound/rev/11811b48bbbe
Replace TextureHost compositor access with TextureSourceProvider. (bug 1343814 part 3.1, r=mattwoodrow)
https://hg.mozilla.org/integration/mozilla-inbound/rev/d55f871c503d
Material changes for TextureHost TextureSourceProvider support. (bug 1343814 part 3.2, r=mattwoodrow)
https://hg.mozilla.org/integration/mozilla-inbound/rev/3bf615dfeba0
Propagate the compositor through CompositableHost::Composite. (bug 1343814 part 4, r=mattwoodrow)
https://hg.mozilla.org/integration/mozilla-inbound/rev/64c73abd4190
Attach Compositables to TextureSourceProviders instead of Compositors. (bug 1343814 part 5, r=mattwoodrow)

Comment 12

[Iris Hsiao [:ihsiao]]

sorry had to back this out for bustage like https://treeherder.mozilla.org/logviewer.html#?job_id=82897726&repo=mozilla-inbound&lineNumber=6327

https://hg.mozilla.org/integration/mozilla-inbound/rev/3fda771e3444

Comment 13

[David Anderson [:dvander] - inactive, e-mail if emergency]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=fce7a1243036e940d8fd37b0bb03f2deb5b67590

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b8cd7d7075c6
Factor texture methods out of Compositor into a TextureSourceProvider class. (bug 1343814 part 1, r=mattwoodrow)
https://hg.mozilla.org/integration/mozilla-inbound/rev/95a00dc5a1b6
Change TextureSource::SetCompositor to use TextureSourceProvider. (bug 1343814 part 2, r=mattwoodrow)
https://hg.mozilla.org/integration/mozilla-inbound/rev/53895c639823
Replace TextureHost compositor access with TextureSourceProvider. (bug 1343814 part 3.1, r=mattwoodrow)
https://hg.mozilla.org/integration/mozilla-inbound/rev/30cf9aea9a00
Material changes for TextureHost TextureSourceProvider support. (bug 1343814 part 3.2, r=mattwoodrow)
https://hg.mozilla.org/integration/mozilla-inbound/rev/ca398f2b75c7
Propagate the compositor through CompositableHost::Composite. (bug 1343814 part 4, r=mattwoodrow)
https://hg.mozilla.org/integration/mozilla-inbound/rev/45a0933af0d6
Attach Compositables to TextureSourceProviders instead of Compositors. (bug 1343814 part 5, r=mattwoodrow)

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/b8cd7d7075c6
https://hg.mozilla.org/mozilla-central/rev/95a00dc5a1b6
https://hg.mozilla.org/mozilla-central/rev/53895c639823
https://hg.mozilla.org/mozilla-central/rev/30cf9aea9a00
https://hg.mozilla.org/mozilla-central/rev/ca398f2b75c7
https://hg.mozilla.org/mozilla-central/rev/45a0933af0d6