947038 - Switch the basic compositor to use new textures

Status: RESOLVED DUPLICATE of bug 989904

(Core :: Graphics: Layers, defect)

Description

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

The software compositor still uses deprecated textures, it should be okay to force new textures for LAYERS_BASIC everywhere.

Comment 1

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

Attachment: ensure-deprecated-textures.patch

Comment 2

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8344132 [details] [diff] [review]
ensure-deprecated-textures.patch

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

::: gfx/layers/ipc/CompositorParent.cpp
@@ +596,5 @@
>  
>    mCompositionManager->ComputeRotation();
>  
>  #ifdef MOZ_DUMP_PAINTING
> +  static bool gDumpCompositorTree = true;

Don't land this!

Comment 3

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

Comment on attachment 8344132 [details] [diff] [review]
ensure-deprecated-textures.patch

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

David, what's the intent behind this change? Perhaps comments in the code or in the bug would help, but I can't tell why SetCompositorBackend() belongs in gfxPlatform and why it's "set only once", why the strange code in ContentClient.cpp, what of this is temporary, what is meant to stay, etc.

Comment 4

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

The intent is to remove dependencies on deprecated paths in the software compositor. I'm not sure why that's strange. Most of the code here is contained in areas that will clearly go away when deprecated textures are finally removed.

https://hg.mozilla.org/integration/mozilla-inbound/rev/b6f9dbc91dc4

Comment 5

[Phil Ringnalda (:philor)]

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/6bc49355df2b - a nice variety of crashtests (https://tbpl.mozilla.org/php/getParsedLog.php?id=31921008&tree=Mozilla-Inbound, https://tbpl.mozilla.org/php/getParsedLog.php?id=31920349&tree=Mozilla-Inbound, https://tbpl.mozilla.org/php/getParsedLog.php?id=31917208&tree=Mozilla-Inbound, https://tbpl.mozilla.org/php/getParsedLog.php?id=31918885&tree=Mozilla-Inbound, https://tbpl.mozilla.org/php/getParsedLog.php?id=31919681&tree=Mozilla-Inbound, https://tbpl.mozilla.org/php/getParsedLog.php?id=31921151&tree=Mozilla-Inbound) but all crashing Cipc in mozilla::layers::BufferTextureClient::AllocateForSurface.

Comment 6

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

Thanks for the links. It's not immediately clear to me how GetBuffer() could return something invalid after Allocate() succeeds. Will debug tomorrow.

Comment 7

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

(In reply to David Anderson [:dvander] from comment #4)
> The intent is to remove dependencies on deprecated paths in the software
> compositor. I'm not sure why that's strange. Most of the code here is
> contained in areas that will clearly go away when deprecated textures are
> finally removed.
> 
> https://hg.mozilla.org/integration/mozilla-inbound/rev/b6f9dbc91dc4

Having a function with "deprecated textures" in it marks it pretty clearly for removal when the deprecated textures are gone.  Having an API to set the compositor backend layer type on gfxPlatform, and only do it once (so that the second call to that function is a no-op, assert non-withstanding) isn't really obviously in need of a removal when deprecated textures go away.

Comment 8

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

Nical, where is this bug in this picture: Nical, where is this bug in this picture: https://wiki.mozilla.org/Platform/GFX/textures#Planned_work ?

Comment 9

[Nicolas Silva [:nical]]

(In reply to Milan Sreckovic [:milan] from comment #8)
> Nical, where is this bug in this picture: Nical, where is this bug in this
> picture: https://wiki.mozilla.org/Platform/GFX/textures#Planned_work ?

That needs to be added to the diagram. What platforms uses basic OMTC these days?

Comment 10

[Nick Cameron [:nrc]]

(In reply to Nicolas Silva [:nical] from comment #9)
> (In reply to Milan Sreckovic [:milan] from comment #8)
> > Nical, where is this bug in this picture: Nical, where is this bug in this
> > picture: https://wiki.mozilla.org/Platform/GFX/textures#Planned_work ?
> 
> That needs to be added to the diagram. What platforms uses basic OMTC these
> days?

e10s

Comment 11

[Tom S [:evilpie]]

Attachment: remove-deprecated

Comment 12

[Tom S [:evilpie]]

Looks like on Windows, when you driver is blocked, we actually use the basic compositor even without the pref.

Comment 13

[Tom S [:evilpie]]

Seems like this was done in bug 989904. Ping here if something else needs to be done.