1180326 - Create a new ContentClient that supports async transaction without minimum size

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[Matt Woodrow (:mattwoodrow)]

Most of our current ContentClients require sync transactions, except for tiling which has a fixed minimum size (that we want to increase).

We want a new simple ContentClient implementation that supports async transactions for small layers that we can use for non-scrollable layers.

Comment 1

[Matt Woodrow (:mattwoodrow)]

Attachment: Part 1: Abstract parts of the client side tiling that we need

Comment 2

[Matt Woodrow (:mattwoodrow)]

Attachment: Part 2: Add support for variable tile sizes

Comment 3

[Matt Woodrow (:mattwoodrow)]

Attachment: Part 3: Move repeated code into GetBackBuffer

Comment 4

[Matt Woodrow (:mattwoodrow)]

Attachment: Part 4: Add new content client

Comment 5

[Matt Woodrow (:mattwoodrow)]

Attachment: Part 1: Abstract parts of the client side tiling that we need

Comment 6

[Matt Woodrow (:mattwoodrow)]

Attachment: Part 2: Add support for variable tile sizes

Comment 7

[Matt Woodrow (:mattwoodrow)]

Attachment: Part 3: Move repeated code into GetBackBuffer

Comment 8

[Matt Woodrow (:mattwoodrow)]

Attachment: Part 4: Add new content client

Comment 9

[Matt Woodrow (:mattwoodrow)]

Attachment: Part 5: Support HWC with tiling when we only have 1 tile

Comment 10

[Matt Woodrow (:mattwoodrow)]

Attachment: Part 6: Use SingleTiledContentClient for non-scrollable layers on b2g and OSX

Comment 11

[Matt Woodrow (:mattwoodrow)]

Attachment: Part 1: Abstract parts of the client side tiling that we need

Comment 12

[Matt Woodrow (:mattwoodrow)]

Attachment: Part 2: Add support for variable tile sizes

Comment 13

[Matt Woodrow (:mattwoodrow)]

Attachment: Part 3: Move repeated code into GetBackBuffer

Comment 14

[Matt Woodrow (:mattwoodrow)]

Attachment: Part 4: Add new content client

Comment 15

[Matt Woodrow (:mattwoodrow)]

Attachment: Part 5: Support HWC with tiling when we only have 1 tile

Comment 16

[Matt Woodrow (:mattwoodrow)]

Attachment: Part 6: Use SingleTiledContentClient for non-scrollable layers on b2g and OSX

Comment 17

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8631315 [details] [diff] [review]
Part 2: Add support for variable tile sizes

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

::: gfx/layers/TiledLayerBuffer.h
@@ +193,5 @@
>    nsTArray<Tile>  mRetainedTiles;
>    TilesPlacement  mTiles;
>    float           mResolution;
>    gfx::IntSize    mTileSize;
> +  gfx::IntPoint   mTileOrigin;

Include a coordinate space in the type here.

Comment 18

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8631318 [details] [diff] [review]
Part 4: Add new content client

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

::: gfx/layers/client/ClientTiledPaintedLayer.cpp
@@ +233,5 @@
>    if (!gfxPlatform::GetPlatform()->UseProgressivePaint()) {
>      // pref is disabled, so never do progressive
>      return false;
>    }
> +      

Whitespace

::: gfx/layers/client/SingleTiledContentClient.cpp
@@ +143,5 @@
> +
> +  extraPainted.MoveBy(mTilingOrigin);
> +  extraPainted.And(extraPainted, aNewValidRegion);
> +  mPaintedRegion.OrWith(paintRegion);
> +  mPaintedRegion.Or(mPaintedRegion, extraPainted);

mPaintedRegion.OrWith(extraPainted);

@@ +184,5 @@
> +  }
> +
> +  // Mark the area we just drew into the back buffer as invalid in the front buffer as they're
> +  // now out of sync.
> +  mTile.mInvalidFront.Or(mTile.mInvalidFront, paintRegion);

OrWith

::: gfx/layers/client/SingleTiledContentClient.h
@@ +126,5 @@
> +  virtual ClientTiledLayerBuffer* GetTiledBuffer() override { return mTiledBuffer; }
> +  virtual ClientTiledLayerBuffer* GetLowPrecisionTiledBuffer() override { return nullptr; }
> +
> +private:
> +  nsRefPtr<ClientSingleTiledLayerBuffer> mTiledBuffer;

Can't this just be:
ClientSingleTiledLayerBuffer mTiledBuffer;

It seems like the relationship is always 1:1

::: gfx/layers/client/TiledContentClient.h
@@ +411,5 @@
>                     const nsIntRegion& aPaintRegion,
>                     LayerManager::DrawPaintedLayerCallback aCallback,
>                     void* aCallbackData) = 0;
>  
> +  virtual bool SupportsProgressiveUpdate() { return true; }

It probably makes sense for this to be pure virtual like ProgressiveUpdate.

Comment 19

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/24d32648073c
https://hg.mozilla.org/integration/mozilla-inbound/rev/e1bfa923a761
https://hg.mozilla.org/integration/mozilla-inbound/rev/6b9ace2a57ab
https://hg.mozilla.org/integration/mozilla-inbound/rev/9bd5b37c0168
https://hg.mozilla.org/integration/mozilla-inbound/rev/0f0c427490da
https://hg.mozilla.org/integration/mozilla-inbound/rev/69871c00379b

Comment 20

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/24d32648073c
https://hg.mozilla.org/mozilla-central/rev/e1bfa923a761
https://hg.mozilla.org/mozilla-central/rev/6b9ace2a57ab
https://hg.mozilla.org/mozilla-central/rev/9bd5b37c0168
https://hg.mozilla.org/mozilla-central/rev/0f0c427490da
https://hg.mozilla.org/mozilla-central/rev/69871c00379b

Comment 21

[KTucker [:KTucker][Inactive 3/4/2016]]

We will need this backed out asap. This is causing a smoketest blocker. Please see bug 1189261.

Comment 22

[Sotaro Ikeda [:sotaro]]

This also break hwc composition on flame-kk. Local backout fixed the problem.

Comment 23

[Martin Shuman [:Marty]]

The fix for this bug is causing many identified issues on both Flame and Aries.

- Status bar can disappear or flicker in many apps (dialer, messages, music). bug 1189261
- Keyboard is flickering, flashes black: bug 1189399
- FTU Cellular Data screen shows black bars and cuts off part of the toggle switch. (Flame only) bug 1189417
- Card view will shift around and tile when swiping cards. (Flame only)
- Video player UI will flicker heavily.
- SIM selection in dialer when using multiple SIMs flickers heavily (Flame only)
- Camera video recording UI will flicker (possibly Flame only)
- Full screen dialogs 'jump' up when appearing on screen, specifically seen when changing screen time-out in Display settings.
- Email Setup for a new account, Display notifications for new messages toggle has shaded UI overlapping

Comment 24

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Patch to disable these patches are in bug bug 1189399.  I asked Sotaro to review that patch.