1166173 - off-main-thread painting implementation

Status: RESOLVED WONTFIX

(Core :: Graphics: Layers, defect)

Description

[Ethan Lin[:ethlin]]

Move painting to another thread for improving performance. The idea should be similar as [1].
[1] https://wiki.mozilla.org/Gecko:OffMainThreadPainting

Comment 1

[Jet Villegas (inactive)]

Bas had a prototype several months ago but the GPU texture upload overhead negated the performance benefits (at least on Windows.) He may have some tips for you.

Comment 2

[Bas Schouten (:bas.schouten)]

Nicolas Silva is already working on code to draw all tiles off the main thread and in parallel. See bug 1083101.

Comment 3

[Nicolas Silva [:nical]]

(In reply to Bas Schouten (:bas.schouten) from comment #2)
> Nicolas Silva is already working on code to draw all tiles off the main
> thread and in parallel. See bug 1083101.

Note that what I am working on will help spreading the painting workload on several threads but the main thread is still blocked during paining. Ideally we would be driving the painting from a dedicated thread like servo does.

Comment 4

[Ethan Lin[:ethlin]]

(In reply to Nicolas Silva [:nical] from comment #3) 
> Note that what I am working on will help spreading the painting workload on
> several threads but the main thread is still blocked during paining. Ideally
> we would be driving the painting from a dedicated thread like servo does.
Right, I think the concepts of the two bugs are different. Off-main-thread painting means we can handle display items and painting parallelized. And multiple-thread painting is for reducing the time of painting. I have profiled performance of multiple-thread painting on flame-kk. I will update it in bug 1083101.

Comment 5

[u459114]

Current approach: put tile-base-paint-tasks, in FrameLayerBuilder::DrawPaintedLayer, into multiple-threads

Comment 6

[u459114]

For a ClientTiledPaintedLayer, it progressively paint each tile. We will have a FrameLayerBuilder::DrawPaintedLayer call for one tile painting.
If we can change Tile rendering from sequential rendering to parallel rendering(multi-threads or OpenCL kenels)
1. Less chance to see low-quality tiles, since we render high quality faster.
2. We have a change that get even better performance then no-tile-painting, since we put more then one threads on content painting.

The biggest problem that we need to overcome is how to keep thread safe in FrameLayerBuilder::DrawPaintedLayer.
For example, FrameLayerBuilder::DrawPaintedLayer calls aLayer->AddInvalidRect which is a mutable function. If we have multiple  FrameLayerBuilder::DrawPaintedLayer be executed, invalid region of aLayer might be corrupted.

Comment 7

[Markus Stange [:mstange]]

We talked about this a little in Whistler. The conclusion was:
Making FrameLayerBuilder::DrawPaintedLayer threadsafe is not a worthwhile approach. Instead, we should record the drawing commands on the main thread into a DrawTargetRecording and then execute those drawing commands on a paint thread.

Comment 8

[u459114]

(In reply to Markus Stange [:mstange] from comment #7)
> We talked about this a little in Whistler. The conclusion was:
> Making FrameLayerBuilder::DrawPaintedLayer threadsafe is not a worthwhile
> approach. Instead, we should record the drawing commands on the main thread
> into a DrawTargetRecording and then execute those drawing commands on a
> paint thread.
Hi Markus/ Jerry
I think Attachment 8620396 [details] [diff] matches your conclusion, and it should be the solution that we need.

Comment 9

[Markus Stange [:mstange]]

Well, mostly. With that patch, the main thread still blocks until painting has completed. I think what Ethan was requesting in comment 4 was a way to have painting of the previous frame run in parallel with requestAnimationFrame + display list building of the next frame. That part is not covered by bug 1083101.

Comment 10

[Nicolas Silva [:nical]]

(In reply to Markus Stange [:mstange] from comment #9)
> Well, mostly. With that patch, the main thread still blocks until painting
> has completed. I think what Ethan was requesting in comment 4 was a way to
> have painting of the previous frame run in parallel with
> requestAnimationFrame + display list building of the next frame. That part
> is not covered by bug 1083101.

Having a dedicated painting thread is a hard but really cool project. The multi-threaded DrawTargetTiled stuff will hopefully improve painting performance, but a separate painting thread would make it possible to avoid checkerboarding when the main thread is blocked by some heavy js/layout stuff.
What servo does is neat:
The layout thread sends a display list to the painting thread, which does the painting (they also spin worker threads to draw tiles in parallel but that's an implementation detail) and sends the tiles to the compositor thread using layers transactions like we do.
That makes it possible for their compositor to request some tiles to the painting thread during apz even if the layout or js threads are blocked.

Anyway, parallel tile painting and off-main-thread painting are two different things and can be done independently.

Comment 11

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: [WIP] multi-thread tiled layer painting

This is the wip idea before Whistler workweek. It's still having some painting error, but it's simple.
The performance chart for this patch will be uploaded later.
Then, I will have some discussion with :nical to find out the next step.

Comment 12

[Jerry Shih[:jerry] (UTC+8) (inactive)]

With bug 1083101, we don't need the patch attachment 8639960 [details] [diff] [review]. It does the similar thing(attachment 8639960 [details] [diff] [review] tries to paint each "tile" parallelly, and bug 1083101 tries to do the draw command parallelly).

So I turn to do the work as comment 10.

The timeline should like:

 -----> T    

Frame 0    Frame 0             
JS/layout  Paint               

           Frame 1    Frame 1  
           JS/Layout  Paint    

                      Frame 2  
                      JS/Layout

Comment 13

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: off-main-thread-painting.html

Here are two approches:

a) do the DLBI and layer creation at paint thread
DisplayItem should decouple with frame tree. Thus, we don't get the race when we process next frame tree.
After DLBI, should we pass the cached DisplayItem data back to frame tree for next frame tree generating? If yes, we need to wait the feedback cached displayItem data for next frame JS/layout update.

b) do the DLBI at layout thread(main thread)
If we can't do framelayerbuilder on the painting thread, we might need to pass the layer new/delete operation to painting thread. Then only painting thread could access the layer tree data.

Comment 14

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Thanks Jerry... can you change the font to be monospace? Also, in "Call FrameLayerBuilder at paint thread. Do DLBI and layer update at painting thread", what's the difference between "paint thread" and "painting thread"?

Comment 15

[Matt Woodrow (:mattwoodrow)]

I think keeping Layer building and DLBI on the layout thread is going to be much easier, and we can probably move that at a later time if we decide it's really necessary.

Making DisplayItems independent of the frames is probably going to have a fairly significant run-time cost (we access a lot of data that we'd need to copy), and there is also huge amounts of code that would need to be updated to draw in this way (all of nsCSSRendering, text, imagelib, style etc).

We could also simplify things by blocking on the painting thread before we start layer building. This way we never have multiple threads trying to access layers at once (but still allow JS/reflow to run at the same time as painting).

A high level 'lock' like that would avoid the need to proxy all layer edit operations between the main thread and painting thread.

Comment 16

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: off-main-thread-painting.html

Add monospace font-family for linux

Comment 17

[Jerry Shih[:jerry] (UTC+8) (inactive)]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #14)
> Thanks Jerry... can you change the font to be monospace? Also, in "Call
> FrameLayerBuilder at paint thread. Do DLBI and layer update at painting
> thread", what's the difference between "paint thread" and "painting thread"?

No, paint thread and painting thread are the same.
And the layout thread is the main thread.

Comment 18

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I think parallel and not-blocking-the-main-thread painting is important but I think probably the best way to do that is to have Moz2D handle it --- just like we need to do for 2D canvas.

Comment 19

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: proposed off-main thread painting.html

Comment 20

[Nicolas Silva [:nical]]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #18)
> I think parallel and not-blocking-the-main-thread painting is important but
> I think probably the best way to do that is to have Moz2D handle it --- just
> like we need to do for 2D canvas.

I agree.

We have to figure out how we handle layer transactions if some stuff happens on the main thread and the IPDL stuff is on the painting thread. The way layer transactions work today, we create a list of edits to the layer tree on the main thread and send it as a layer transaction. A lot of these edits are FrameLayerBuilder stuff that will have to happen on the main thread but we should just be able to build the list of edits related to layerization on the main thread, pass it to the painting thread and let the latter bundle that up with paint updates and send it all as a layer transaction. Basically, we should be able to not use the Layer classes themselves on the painting thread, and use the Compositable classes there instead.

I would like to get a bit more than "painting that does not block the main thread" out of this, thought: We should be able to record commands for a much larger region than what we paint, and let the compositor request new tiles during APZ even if the main thread is blocked.
One way to achieve that could be to use the recording DrawTarget on the main thread for each layer and send the recorded command lists to the painting thread. We might run into some issues if we have code that does things like painting into a DrawTarget and reading from a snapshot synchronously, though (hopefully we don't do that too much because that already kills the advantages of multi-threaded tile painting).

Comment 21

[Matt Woodrow (:mattwoodrow)]

(In reply to Nicolas Silva [:nical] from comment #20) 
> I agree.
> 
> We have to figure out how we handle layer transactions if some stuff happens
> on the main thread and the IPDL stuff is on the painting thread. The way
> layer transactions work today, we create a list of edits to the layer tree
> on the main thread and send it as a layer transaction. A lot of these edits
> are FrameLayerBuilder stuff that will have to happen on the main thread but
> we should just be able to build the list of edits related to layerization on
> the main thread, pass it to the painting thread and let the latter bundle
> that up with paint updates and send it all as a layer transaction.
> Basically, we should be able to not use the Layer classes themselves on the
> painting thread, and use the Compositable classes there instead.

This sounds right to me. We might need to make sure we allocate/destroy the PLayer instance (LayerChild etc) from the painting thread too.


> 
> I would like to get a bit more than "painting that does not block the main
> thread" out of this, thought: We should be able to record commands for a
> much larger region than what we paint, and let the compositor request new
> tiles during APZ even if the main thread is blocked.
> One way to achieve that could be to use the recording DrawTarget on the main
> thread for each layer and send the recorded command lists to the painting
> thread. We might run into some issues if we have code that does things like
> painting into a DrawTarget and reading from a snapshot synchronously, though
> (hopefully we don't do that too much because that already kills the
> advantages of multi-threaded tile painting).


We talked about this a bit last week.

Recording commands for a larger area is great for scrolling performance, but it'll add displaylist processing overhead which will hurt on pages that have other sorts of invalidations.

We might be able to mitigate that (by having variable displayport sizes based on heuristics), but we decided to do this as a separate step from the base OMTP implementation.

Comment 22

[Jerry Shih[:jerry] (UTC+8) (inactive)]

For my first step, I will try to reuse our current draw-command-recorder at [1], and replay all command using [2]. All tasks will be done at main thread. If all things are good, then move the replaying to another thread and do the layer transaction ipc related stuff.

[1]
https://hg.mozilla.org/mozilla-central/annotate/e47423c019643792e6de894cfcee598dead4d3ba/gfx/2d/2D.h#l973

[2]
https://hg.mozilla.org/mozilla-central/annotate/e47423c019643792e6de894cfcee598dead4d3ba/gfx/2d/2D.h#l729

Comment 23

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Our current draw-command-recorder implementation uses replacement new but it has memory alignment problem. I will have my own command recorder.

Comment 24

[Matt Woodrow (:mattwoodrow)]

Can you explain this a little more? Is it possible to fix rather than writing a new one?

Comment 25

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Yes, it's possible, but I need to add some additional interface in our current impl to fulfill my design. I will keep it in my mind and do that later.

Comment 26

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: Layer transaction model for off-main painting

Comment 27

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Hi all,

I'm stuck in traffic by the ipc problem. Main thread still sends a lot PCompositorChild, PLayer and PCompositor message. It's hard to rewrite all of them. So I try to update our ipdl generator for multi-thread usage.

Here is my proposal. The ipc code will like:

https://pastebin.mozilla.org/8848042

Our original ipc codes are like:
https://dxr.mozilla.org/mozilla-central/source/obj-x86_64-unknown-linux-gnu/ipc/ipdl/PLayerTransactionChild.cpp?from=PlayertransactionChild.cpp#373

After this, main-thread and painting-thread could use the same ipc channel.

Comment 28

[Jerry Shih[:jerry] (UTC+8) (inactive)]

I'm stuck in traffic by the ipc problem => I'm stuck by the ipc problem.

Comment 29

[Matt Woodrow (:mattwoodrow)]

I'd be worried about doing this since, since it makes it harder to reason about the order in which IPC messages will be sent.

I think we're ok for this case since we only have one other thread sending messages, and the painting thread should do all of it's work within a single task.

Other (future) users might not have this, so we lose by removing some of the safety from IPDL.

Can we instead just only use PCompositor/PLayer/PCompositor from the main thread, and PTexture/PCompositable from the painting thread?

Comment 30

[Jerry Shih[:jerry] (UTC+8) (inactive)]

(In reply to Matt Woodrow (:mattwoodrow) from comment #29)
> Can we instead just only use PCompositor/PLayer/PCompositor from the main
> thread, and PTexture/PCompositable from the painting thread?

I will refer the ImagebridgeChild about this approach. It needs to map the PTexture/PCompositable actor between the two messageChannel at main and painting thread.

Comment 31

[Nicolas Silva [:nical]]

In the ideal case we would not have any synchronous messages sent from the main thread to the compositor. This is clearly not the case but I wonder how far we are from this. it would make it easier to deal with the main thread not being the thread that sends layers transactions.
I would also prefer to keep the restriction of using IPDL from the IPDL thread only. Very bad patterns will emerge from using IPDL from any thread with synchronous proxies under the hood.

Comment 32

[Jerry Shih[:jerry] (UTC+8) (inactive)]

I still can't find a good way to handle the actor problem. In my prototype, I try to use ImageBridge to send the layerTransaction at painting thread. I make ImageBridge to manage PLayerTransaction, but it's complicate.

So, there is another try. Each SendXXX function can generate their "raw message" data(including ipc actor info). Then pass this raw message with ImageBridgeChild at painting thread. Finally, pass the raw message to PLayerTransactoinParent at ImageBridgeParent. The raw message still have the actor info, so it can find the correct actor pair.


1. Get raw message from SendXXX function
2. pass raw message with another ipc channel
3. pack a new message and send to corresponding Protocol::OnMessageReceived()

Comment 33

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: off-main painting ipc.html

I hack our ipc system. Right now, I can pass the message for PLayerTransaction::SendUpdateNoSwap through PImageBridge.

There still have problem for the actor deserialize at parent side. When we try to use PImageBridge to pass the message, the actor still can send the actor delete message through PLayerTransaction at main thread. In this case, we hit the crash when deserialize the deleted actor.
I'm checking our PTexture recycling mechanism.

Comment 34

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Here is another implementation for the off-main painting ipc problem. Unlike comment 33, this impl doesn't need to modify the ipc code generator.

If there is a "UpdateNoSwap" message, save all data into queue and do not pass these parameter into RecvUpdate(). When all draw commands are done, FlushPendingTransaction() will be called and then update all pending layer data into chrome side's layer tree.

The LayerTransaction message deferring part is done and it works fine. My next step is trying to merge my moz2d commend recorder patch with this ipc change. 
I think there are still have PTexture life-cycle problem. We'll see.

----
bool
LayerTransactionParent::RecvUpdateNoSwap(.......,
                                         const bool& aPendingUpdate)
{
  if (aPendingUpdate) {
    // save all layer transaction data
    mPendingTransactionMessage.reset(
        new PendingTransactionMessage(......));
    return true;
  }

  return RecvUpdate(........);
}

void
LayerTransactionParent::FlushPendingTransaction()
{
  RecvUpdate(mPendingTransactionMessage->mEdit,
             ........);

  mPendingTransactionMessage.reset(nullptr);
}

Comment 35

[Jerry Shih[:jerry] (UTC+8) (inactive)]

This is the current impl for off-main ipc problem.
https://github.com/JerryShih/gecko-dev/commits/defer-layer-transaction-at-message-channel-parent-side

The layer transaction message passing will be:
----
main thread:
  layerTransactionParent->GetIPCChannel()->StartPendingMessage(); //<==Start message deferring
  layerTransactionParent->SendUpdateNoSwap(); //<==this message will be deferred until EndPendingMessage()
  PostDrawTaskToPaintingThread();

painting thread:
  DrawCommand();
  layerTransactionParent->GetIPCChannel()->EndPendingMessage(); //<==Notify parent side to process all
                                                                //pending messages.
 
----

With these, I think I don't need to care about the non-layer-transaction-op message(e.g. PLayer::__delete__) when we process the draw command at painting thread. If we send that message during off-main-painting, we will hit the layer tree data race problem.


The next step will be trying to merge bug 1083101, and do the actual painting task at painting thread.

Comment 36

[Zimon Dai [:zimonD]]

I am eagerly waiting for this feature landed. Working on a webpage which has about 45,000 svg nodes and every paint takes about 500ms, which is really painful. Off mainthread painting is the best way I could find to improve the performance.

Comment 37

[Jerry Shih[:jerry] (UTC+8) (inactive)]

The current working item are:
a) handle off-main-painting layer transaction
  defer ipc message until all rendering task done.
  WIP:
    https://github.com/JerryShih/gecko-dev/commits/defer-layer-transaction-at-message-channel-parent-side
  
  IPC change:
    https://github.com/JerryShih/doc/blob/master/off-main-painting/off-main.md
b) do the actual painting task at another thread(recording all draw call at main thread, and replay them off-main)
  the recorder is done. I'm checking for a good way to preserve TextureClient and DrawTarget life-time for painting at another thread.

Hope this rough impl could be done in one weeks.

Comment 38

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: The off-main-painting profile

This is the profile result for off-main-painting at b2g on nexus5-l.
There are still some artifacts for homescreen display, and there are a lot of memory copy for wrapping DataSourceSurface. I will clean up the wip and find some guys for feedback.

Comment 39

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: off-main painting pref

Comment 40

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: turn on off-main painting pref

Comment 41

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: SourceSurfaceRawData::GuaranteePersistance profiler label

Comment 42

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: moz_off_main_painting configure

Comment 43

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: LivePref for bool type

Comment 44

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: Copy constructor for moz2d Patterns.

Comment 45

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: DrawTargetAsync impl

Comment 46

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: DrawTargetAsyncManager impl

Comment 47

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: P3: Handle borrow context for DrawTargetCairo and DrawTargetCG from DrawTargetAsync.

Comment 48

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: P4: Handle async drawing mode in TextureClient

Comment 49

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: P5: Handle off-main-painting transaction in ClientLayerManager

Comment 50

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: P6: Handle off-main-painting transaction in ShadowLayerManager

Comment 51

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: P7: Off-main-painting hint for ClientTiledPaintedLayer.

Comment 52

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: P8: Create DrawTargetAsync for tiled drawTarget.

Comment 53

[Jerry Shih[:jerry] (UTC+8) (inactive)]

These patches try to do some moz2d command at another thread. It might improve the concurrency for main thread.

When we try to borrow a drawTarget from textureClient, it will return a drawTargetAsync instead.
The drawTargetAsync could record moz2d draw commands at main thread and replay them witn drawTargetAsyncManager at another thread.

Currently, drawTargetAsync uses some gecko code, I will try to replace them with exist function in moz2d.

Comment 54

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Comment on attachment 8721265 [details] [diff] [review]
SourceSurfaceRawData::GuaranteePersistance profiler label

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

add a profiler label if we call memcpy during drawing.

Comment 55

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Comment on attachment 8721266 [details] [diff] [review]
moz_off_main_painting configure

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

I try to have a build configuration to export some def in cpp code and moz.build. Then we still build moz2d even thought we use gecko code in off-main-painting implementation. I will try to remove the gecko dependence as many as possible.

Comment 56

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Comment on attachment 8721267 [details] [diff] [review]
LivePref for bool type

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

extend the PreferenceAccess in moz2d for bool pref.

Comment 57

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Comment on attachment 8721268 [details] [diff] [review]
Copy constructor for moz2d Patterns.

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

I think we should not perform shallow copy with default copy constructor if we have RefPtr<xxx> type member.

e.g., the |RefPtr<SourceSurface> mSurface| in SurfacePattern.

Comment 58

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: DrawTargetAsync impl. v2

Comment 59

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: P1: DrawTargetAsync impl. v3

Comment 60

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Comment on attachment 8721295 [details] [diff] [review]
P1: DrawTargetAsync impl. v3

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

A DrawTarget which could record draw command.

::: gfx/2d/DrawTargetAsync.cpp
@@ +43,5 @@
> +  SETOPAQUERECT,
> +  SETPERMITSUBPIXELAA,
> +};
> +
> +class AsyncDrawCommand

I will try to reuse the exist draw command class in moz2d at next patch.

@@ +758,5 @@
> +  return mAsyncPaintData->GetDrawTarget()->GetType();
> +}
> +
> +already_AddRefed<SourceSurface>
> +DrawTargetAsync::Snapshot()

Flush draw command before returning the snapshot.

@@ +808,5 @@
> +#ifdef PROFILE_SURFACE_GUARANTEE_PERSISTANCE
> +    //PROFILER_LABEL("DrawTargetAsync", "DrawSurface::GuaranteePersistance", js::ProfileEntry::Category::GRAPHICS);
> +    ATRACE_NAME("DrawTargetAsync::DrawSurface::GuaranteePersistance");
> +#endif
> +    aSurface->GuaranteePersistance();

If the surface is a SourceSurfaceRawData, make a copy for that.

@@ +1028,5 @@
> +DrawTargetAsync::SetTransform(const Matrix& aTransform)
> +{
> +  mTransform = aTransform;
> +
> +  if (mAsyncPaintData->mPendingDrawCommand.size() && (mAsyncPaintData->mPendingDrawCommand.back()->GetType() == AsyncDrawCommandType::SETTRANSFORM)) {

If the previous draw command is SetTransform(), we could replace with the current transform.

@@ +1037,5 @@
> +  mAsyncPaintData->AppendDrawCommand<AsyncSetTransformCommand>(aTransform);
> +}
> +
> +void*
> +DrawTargetAsync::GetNativeSurface(NativeSurfaceType aType)

we can't make sure when the NativeSurface be used.

@@ +1115,5 @@
> +    return false;
> +  }
> +
> +  for (size_t i = 0; i < aTiles.mTileCount; ++i) {
> +    if (!aTiles.mTiles[i].mDrawTarget->IsAsyncDrawTarget()) {

Only DrawTargetAsync could hold the resource(textureClient or drawTarget) for painting at another thread. If the dt in TileSet is not DrawTargetAsync, we can't make sure the resource is still exist during painting.

@@ +1120,5 @@
> +      return false;
> +    }
> +  }
> +
> +  // replace tiles' dt with internal dt

We should replace the tileset dt with the internal dt. Otherwise, we still apply the draw command with a DrawTargetAsync and the draw command is still recorded.

::: gfx/2d/DrawTargetAsync.h
@@ +21,5 @@
> +class DrawTargetAsync;
> +class DrawTargetCairo;
> +class DrawTargetCG;
> +
> +class AsyncPaintData : public RefCounted<AsyncPaintData>

This is the base class to hold the resource(e.g. textureClient or DrawTarget) for off-main painting.

@@ +62,5 @@
> +  void InitPool();
> +  void ClearPoolDrawCommand();
> +  void ReleasePool();
> +
> +  PLArenaPool mPool;

It could be replaced with the memory arena in moz2d.

@@ +67,5 @@
> +  std::vector<AsyncDrawCommand*> mPendingDrawCommand;
> +  bool mIsPoolReady;
> +};
> +
> +class DrawTargetAsyncPaintData final : public mozilla::gfx::AsyncPaintData

Holding DrawTarget.

The TextureClient holder is at TextureClient.h at another patch.

@@ +86,5 @@
> +private:
> +  RefPtr<DrawTarget> mRefDrawTarget;
> +};
> +
> +class DrawTargetAsync : public DrawTarget

DrawTargetAsync will record draw command into AsyncPaintData. Then we can replay the command later.

@@ +242,5 @@
> +  RefPtr<AsyncPaintData> mAsyncPaintData;
> +  bool mDrawCommandApplied;
> +};
> +
> +class DrawTargetTiledAsync final : public DrawTargetAsync

The Tiled version DrawTargetAsync.

Comment 61

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Comment on attachment 8721295 [details] [diff] [review]
P1: DrawTargetAsync impl. v3

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

::: gfx/2d/2D.h
@@ +1157,5 @@
>    virtual bool IsDualDrawTarget() const { return false; }
>    virtual bool IsTiledDrawTarget() const { return false; }
>    virtual bool SupportsRegionClipping() const { return true; }
>  
> +  virtual bool IsAsyncDrawTarget() const { return false; }

Check if this dt is DrawTargetAsync.

@@ +1174,5 @@
>     * this DrawTarget is first used for drawing. Either by the underlying surface
>     * being used as an input to external drawing, or Snapshot() being called.
>     * This rectangle is specified in device space.
>     */
> +  virtual void SetOpaqueRect(const IntRect &aRect) {

Change to virtual, then we can put setOpaqueRect op into draw command with DrawTargetAsync.

Comment 62

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Comment on attachment 8721271 [details] [diff] [review]
P3: Handle borrow context for DrawTargetCairo and DrawTargetCG from DrawTargetAsync.

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

If we try to create a context from DrawTargetAsync, flush all current pending draw command before return the internal DrawTargetCG and DrawTargetCairo.

Comment 63

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Comment on attachment 8721295 [details] [diff] [review]
P1: DrawTargetAsync impl. v3

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

::: gfx/2d/DrawTargetAsync.h
@@ +98,5 @@
> +  void ApplyPendingDrawCommand();
> +
> +  virtual bool IsAsyncDrawTarget() const override;
> +
> +  DrawTarget* GetInternalDrawTarget();

This function will return the actual DrawTarget(e.g. drawTargetCairo or drawTargetCG) which draw commands will apply to.

Comment 64

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Comment on attachment 8721272 [details] [diff] [review]
P4: Handle async drawing mode in TextureClient

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

When we setup a TextureClientRenderingAutoMode obj, all TextureClient will return a wrapping DrawTargetAsync if we call BorrowDrawTarget(). Then, all draw command could be recorded before TextureClientRenderingAutoMode obj destroyed.

Comment 65

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Comment on attachment 8721272 [details] [diff] [review]
P4: Handle async drawing mode in TextureClient

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

::: gfx/layers/client/TextureClient.cpp
@@ +74,5 @@
>  using namespace mozilla::gl;
>  using namespace mozilla::gfx;
>  
> +#ifdef MOZ_OFF_MAIN_PAINTING
> +class TextureClientAsyncPaintData final : public mozilla::gfx::AsyncPaintData

an AsyncPaintData which hold the TextureClient.

Comment 66

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Comment on attachment 8721273 [details] [diff] [review]
P5: Handle off-main-painting transaction in ClientLayerManager

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

::: gfx/layers/client/ClientLayerManager.cpp
@@ +285,5 @@
> +#ifdef MOZ_OFF_MAIN_PAINTING
> +  bool offMainPainting = gfxPrefs::ContentOffMainPainting();
> +
> +  // Only wait previous off-main transactions at the beginning of frame.
> +  if (!mIsRepeatTransaction) {

I assume that one layer is only be rendered once during a frame transaction. So, there is only one sync position here.

Comment 67

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Comment on attachment 8721274 [details] [diff] [review]
P6: Handle off-main-painting transaction in ShadowLayerManager

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

There are two function in DrawTargetAsyncManager to handle a transaction: ApplyLastTransaction() and PostApplyLastTransaction().

The ApplyLastTransaction() applies all pending draw commands in current transaction at main thread just like the original code flow.
The PostApplyLastTransaction() is different. It will create an task to do the draw command at another thread.

The impl detail is in DrawTargetAsyncManager.

Comment 68

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Comment on attachment 8721275 [details] [diff] [review]
P7: Off-main-painting hint for ClientTiledPaintedLayer.

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

::: gfx/layers/client/ClientTiledPaintedLayer.cpp
@@ +407,5 @@
>  void
>  ClientTiledPaintedLayer::RenderLayer()
>  {
> +#ifdef MOZ_OFF_MAIN_PAINTING
> +  TextureClientRenderingAutoMode renderingMode(TextureClientRenderingMode::DEFERRING);

Turn on off-main painting for tiled-painted layer.

Comment 69

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Comment on attachment 8721276 [details] [diff] [review]
P8: Create DrawTargetAsync for tiled drawTarget.

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

::: gfx/2d/Factory.cpp
@@ +465,5 @@
> +#else
> +  if (sOffMainPainting) {
> +    RefPtr<DrawTargetTiledAsync> drawTargetAsync = new DrawTargetTiledAsync(new DrawTargetAsyncPaintData(dt.get()));
> +
> +    if (drawTargetAsync->Init(aTileSet)) {

If we have off-main-painting, then we try to use DrawTargetTiledAsync first.

Comment 70

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: P2: DrawTargetAsyncManager impl. v2

Comment 71

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Comment on attachment 8721875 [details] [diff] [review]
P2: DrawTargetAsyncManager impl. v2

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

::: gfx/thebes/DrawTargetAsyncManager.cpp
@@ +107,5 @@
> +  MOZ_ASSERT(mOffManPaintingThread.get());
> +  MOZ_ASSERT(mOffManPaintingThread->message_loop());
> +
> +  printf_stderr("bignose StartPendingMessage");
> +  MOZ_ALWAYS_TRUE(aMessageChannel->StartPendingMessage());

Start IPC deferring mode.

@@ +130,5 @@
> +  }
> +
> +  for (int32_t i = mLastmTransactionIndex; i >= 0; --i) {
> +    if (mTransactions[i]->mFired && !mTransactions[i]->mWaitableEvent->IsSignaled()) {
> +      mTransactions[i]->mWaitableEvent->Wait();

Use chromium waitable_event for synchronization.

::: gfx/thebes/DrawTargetAsyncManager.h
@@ +36,5 @@
> +  virtual ~DrawTargetAsyncManager();
> +
> +  void BeginTransaction();
> +  void AppendAsyncPaintData(AsyncPaintData* aAsyncPaintData);
> +  void EndTransaction();

Every BeginTransaction() and EndTransaction() function call pair will create a Transaction data which maintain the draw commands and the necessary resource data(holding TextureClient or DT).

@@ +39,5 @@
> +  void AppendAsyncPaintData(AsyncPaintData* aAsyncPaintData);
> +  void EndTransaction();
> +
> +  void ApplyLastTransaction();
> +  void PostApplyLastTransaction(ipc::MessageChannel* aMessageChannel);

The ApplyLastTransaction() applies all pending draw commands in current transaction at main thread.
The PostApplyLastTransaction() create an task to do the transaction draw command at another thread. It also turns on ipc deferring mode.
Please reference Bug 1235018 and attachment 8719389 [details] for ipc deferring mode detail.

Comment 72

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: moz_off_main_painting configure. v2

add moz_off_main_painting configuration in configure.in

Comment 73

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: P0: DrawCommand update

Remove some std::vector usage.
Add PUSHLAYER, POPLAYER, SETOPAQUERECT and SETPERMITSUBPIXELAA command type.
Add missed DrawSurfaceWithShadowCommand and CopyRectCommand draw command.

Comment 74

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: P1: AsyncPaintData for draw command recording

The class which holds draw commands, DrawTarget and TextureClient.

Comment 75

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: P2: DrawTargetAsyncManager impl. v3

Comment 76

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: P3: DrawTargetAsyncPaintData for DrawTarget async painting

The AsyncPaintData which holds DrawTarget.

Comment 77

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: P4: DrawTargetAsync impl. v4

Comment 78

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: P5: DrawTargetTiledAsync impl

The tiled version of DrawTargetAsync.

Comment 79

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: P6: Handle borrow context for DrawTargetCairo and DrawTargetCG from DrawTargetAsync. v2

Comment 80

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: P7: Handle async drawing mode in TextureClient. v2

Comment 81

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: P8: Handle off-main-painting transaction in ClientLayerManager. v2

Comment 82

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: P9: Handle off-main-painting transaction in ShadowLayerManager. v2

Comment 83

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: P10: Off-main-painting hint for ClientTiledPaintedLayer. v2

Comment 84

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: P11: Create DrawTargetAsync for tiled drawTarget. v2

Comment 85

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: P12: moz2d use AtomicRefCounted for the type in draw command

Since the draw command is create from main thread and released at painting thread, some types in draw command should inherit from AtomicRefCounted.

Comment 86

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: P2: DrawTargetAsyncManager impl. v4

rebase