Closed Bug 1296658 Opened 6 years ago Closed 6 years ago

BasicLayerManager disregards its final destination when creating buffer surfaces

Categories

(Core :: Graphics, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla51
Tracking Status
firefox51 --- fixed

People

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

References

Details

Attachments

(1 file)

This can cause it to create D2D surfaces which it then has to draw to a non-D2D target.
Comment on attachment 8782945 [details]
Bug 1296658: Attempt to create the correct BackendType buffers for a LayerManager.

https://reviewboard.mozilla.org/r/72944/#review70724

::: gfx/layers/basic/BasicPaintedLayer.cpp:232
(Diff revision 2)
>  already_AddRefed<PaintedLayer>
>  BasicLayerManager::CreatePaintedLayer()
>  {
>    NS_ASSERTION(InConstruction(), "Only allowed in construction phase");
> -  RefPtr<PaintedLayer> layer = new BasicPaintedLayer(this);
> +
> +  BackendType backend = gfxPlatform::GetPlatform()->GetDefaultContentBackend();

You're clobbering this assignment on both branches of the if below. Is this intentional?
Flags: needinfo?(bas)
Comment on attachment 8782945 [details]
Bug 1296658: Attempt to create the correct BackendType buffers for a LayerManager.

https://reviewboard.mozilla.org/r/72944/#review70724

> You're clobbering this assignment on both branches of the if below. Is this intentional?

Yes, because there's a default branch where the original assignment is used, where there's !mDefaultTarget and mType != BLM_WIDGET, i.e. a regular browser window on first draw.
See reply.
Flags: needinfo?(bas)
Comment on attachment 8782945 [details]
Bug 1296658: Attempt to create the correct BackendType buffers for a LayerManager.

https://reviewboard.mozilla.org/r/72944/#review71708
Attachment #8782945 - Flags: review?(jmuizelaar) → review+
Bas, this has r+, is it ready to land?
Flags: needinfo?(bas)
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #7)
> Bas, this has r+, is it ready to land?

Yes but another bug (1296657) isn't reviewed yet.
Depends on: 1296657
Flags: needinfo?(bas)
Comment on attachment 8782945 [details]
Bug 1296658: Attempt to create the correct BackendType buffers for a LayerManager.

https://reviewboard.mozilla.org/r/72944/#review76738

::: gfx/layers/client/ContentClient.cpp:117
(Diff revision 2)
>    }
>  }
>  
>  // We pass a null pointer for the ContentClient Forwarder argument, which means
>  // this client will not have a ContentHost on the other side.
> -ContentClientBasic::ContentClientBasic()
> +ContentClientBasic::ContentClientBasic(gfx::BackendType aBackend)

ContentClient.h needs to be updated to include this argument in the function signature.

::: gfx/layers/client/ContentClient.cpp:120
(Diff revision 2)
>  // We pass a null pointer for the ContentClient Forwarder argument, which means
>  // this client will not have a ContentHost on the other side.
> -ContentClientBasic::ContentClientBasic()
> +ContentClientBasic::ContentClientBasic(gfx::BackendType aBackend)
>    : ContentClient(nullptr)
>    , RotatedContentBuffer(ContainsVisibleBounds)
> +  , mBackend(aBackend)

The member variable definition for mBackend is missing from ContentClient.h.
Can this and the bug that it depends on now land with the noted issues fixed?
Flags: needinfo?(bas)
Flags: needinfo?(bas)
Pushed by bschouten@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b2372a86671c
Attempt to create the correct BackendType buffers for a LayerManager. r=jrmuizel
Pushed by bschouten@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1032461dcb88
Followup: Mark constructor explicit. r=bustage
https://hg.mozilla.org/mozilla-central/rev/b2372a86671c
https://hg.mozilla.org/mozilla-central/rev/1032461dcb88
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Depends on: 1303570
You need to log in before you can comment on or make changes to this bug.