Nicolas Silva [:nical] Assignee
	Description • 9 years ago

The idea is to have a TextureData class that doesn't have any ownership semantic. It is uniquely owned by the TextureClient object which forwards backend-specific calls to the TextureData.

I want this to be able to:
* properly use the TextureClient's refcounting without horrible hacks when attempting to recycle textures
* simplify the deallocation code
* add the possibility to asynchronously deallocate textures on the client side.
* remove a lot of code duplication between TextureClient implementations

As a bonus, while working on this I noticed a bunch of duplicate functionalities inside TextureClient itself (like the Fence and SyncObject stuff), which I want to clean up in the process.

	Nicolas Silva [:nical] Assignee
	Comment 1 • 9 years ago

I added a temporary ClientTexture inheriting from TextureClient but they'll get merged back when all texture clients are based on TextureData.

	Matt Woodrow (:mattwoodrow)
	Comment 4 • 9 years ago

Comment on attachment 8655416 [details] [diff] [review]
add the TextureData interface and make the X11 implementation use it

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

Skimmed over the actual changes a bit since they all seem fairly mechanical.

I really like the concept :)

::: gfx/layers/client/TextureClient.h
@@ +576,5 @@
> +  virtual already_AddRefed<gfx::DrawTarget> BorrowDrawTarget() = 0;
> +
> +  virtual void Deallocate(ISurfaceAllocator* aAllocator) = 0;
> +
> +  virtual bool Serialize(SurfaceDescriptor& aDescrptor) = 0;

aDescriptor (typo).

@@ +586,5 @@
> +
> +  virtual bool UpdateFromSurface(gfx::SourceSurface* aSurface) = 0;
> +};
> +
> +class ClientTexture : public TextureClient {

This naming is fun :)

If it's going to survive for more than just this patch queue, then please add a comment explaining that it's a TextureClient impl that is backed by a TextureData object, and will go away when all TextureClients are that.

	Matt Woodrow (:mattwoodrow)
	Comment 5 • 9 years ago

Comment on attachment 8655417 [details] [diff] [review]
Make BufferTextureClient use TextureData

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

::: gfx/layers/BufferTexture.cpp
@@ +88,5 @@
> +                          gfx::BackendType aMoz2DBackend, TextureFlags aFlags,
> +                          TextureAllocationFlags aAllocFlags,
> +                          ISurfaceAllocator* aAllocator)
> +{
> +  if (!aAllocator || aAllocator->IsSameProcess()) {

Do we want to support a nullptr allocator? The old code didn't.

@@ +152,5 @@
> +{
> +  switch (mFormat) {
> +    case gfx::SurfaceFormat::YUV:
> +    case gfx::SurfaceFormat::NV12:
> +    case gfx::SurfaceFormat::UNKNOWN:

Might be better to list the formats that we do support in case someone adds some other weird video one and forget to update this.

::: gfx/layers/BufferTexture.h
@@ +30,5 @@
> +  size_t mBufferSize;
> +  gfx::SurfaceFormat mFormat;
> +};
> +
> +class RawYCbCrTextureBuffer : public RefCounted<RawYCbCrTextureBuffer>

This appears to be unused?

::: gfx/layers/ipc/SharedRGBImage.cpp
@@ +96,2 @@
>    }
> +  return 0;

nullptr

	Sotaro Ikeda [:sotaro]
	Comment 6 • 9 years ago

Comment on attachment 8655419 [details] [diff] [review]
Gralloc part

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

::: widget/gonk/nativewindow/GonkBufferQueueJB.cpp
@@ +426,5 @@
> +        ISurfaceAllocator* allocator = ImageBridgeChild::GetSingleton();
> +        GrallocTextureData* texData = GrallocTextureData::Create(IntSize(w,h),
> +                                                                 SurfaceFormatForPixelFormat(format),
> +                                                                 gfx::BackendType::NONE, usage,
> +                                                                 allocator);

Does it support to allocate chip dependent YUV format buffers allocation? 'format' seems not directly used for gralloc allocation.

	Nicolas Silva [:nical] Assignee
	Comment 7 • 9 years ago

(In reply to Sotaro Ikeda [:sotaro] from comment #6)
> Does it support to allocate chip dependent YUV format buffers allocation?
> 'format' seems not directly used for gralloc allocation.

Hm, actually I might be loosing some format information in the process here (if we have pixel formats that are not handled in SurfaceFormatForPizelFormat which seems to be the case). I'll update the patch to take android's pixel format instead of SurfaceFormat as input.

	Nicolas Silva [:nical] Assignee
	Comment 11 • 9 years ago

This patch moves all of the deallocation logic into a single function and adds the possibility to deallocation TextureClients on the client side without waiting for the compositor.
The function handles dispatching the deallocation to the IPDL thread, blocking or not blocking, and makes sure the TextureData is deallocated after the host is gone (so we don't need the KeepAlive thing).

There are a few little things to figure out, like the RecycleCallback. I would like to use an interface like TextureRecycler or whatnot, but in practice it would not be very different from what we do with the function pointer today (just a bit more flexible, and no more ClearRecycleCallback for the callback: you create a texture with a recycler and the latter decides whether to recycle the TextureData or not).

	Nicolas Silva [:nical] Assignee
	Comment 12 • 9 years ago

A bug that took me a while to figure out was that BufferTextureClient exposes pointers to its raw buffer (with some metadata at the beginning of the buffer) whereas most users of BufferTextureClient really only want to get the address of the pixels themselves and I had a place where I'd give a pointer to the beginning of the buffer instead of the beginning of the pixels. This patch is mosty identical to the one that got r+ with the notable exception that TextureClient can now expose address to pixels using the method bool BorrowMappedData(MappedTextureData& descriptor) and its YCbCr equivalent, that provide access to the pixels independently of how they are stored in the buffer instead of exposing the buffer and letting the called figure out how do read from it. As a bonus, this will allow the gralloc texture to expose its pixels with the same interface which it couldn't do previously. I think that there are some software video decoding code paths that could remove an extra copy by using gralloc instead of BufferTextureClient thanks to this (need to check). 

Anyway, re-asking for a review because of the code changes related to BorrowMappedData.

	Matt Woodrow (:mattwoodrow)
	Comment 18 • 9 years ago

Comment on attachment 8678883 [details] [diff] [review]
TextureData: BufferTextureClient + improve the way we expose pointers to the texture's buffer

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

I'm trusting that most of the new BufferTextureData code is just copied from BufferTextureClient, I didn't check it line by line.

::: gfx/layers/client/TextureClient.cpp
@@ +345,5 @@
> +{
> +  MOZ_ASSERT(mValid);
> +
> +  // TODO - SharedRGBImage just accesses the buffer without properly locking
> +  // the texture. It's bad.

File a followup to fix this?

	Matt Woodrow (:mattwoodrow)
	Comment 19 • 9 years ago

Comment on attachment 8678891 [details] [diff] [review]
TextureData: android surface

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

::: gfx/layers/client/TextureClient.cpp
@@ +939,5 @@
>        uint8_t* dst = aDst.data + i * aDst.stride;
>        for (int32_t j = 0; j < size.width; ++j) {
>          *dst = *src;
>          src += 1 + skip;
> +        dst += 1 + aDst.skip;

I assume you'll fix this in the original patch, not here.

::: gfx/layers/opengl/TextureClientOGL.cpp
@@ +79,5 @@
> +  if (!aSurfTex || !XRE_IsParentProcess()) {
> +    return nullptr;
> +  }
> +
> +  // XXX - This is quite sad and slow.

Can you elaborate on this? (in the previous patch too). I doubt this is useful as it stands (except to you).

	Matt Woodrow (:mattwoodrow)
	Comment 20 • 9 years ago

Comment on attachment 8678893 [details] [diff] [review]
TextureData: MacIOSurface

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

::: gfx/layers/opengl/MacIOSurfaceTextureClientOGL.cpp
@@ +3,5 @@
>   * License, v. 2.0. If a copy of the MPL was not distributed with this
>   * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
>  
>  #include "MacIOSurfaceTextureClientOGL.h"
> +#include "mozilla/gfx/MacIOSurface.h" 

Whitespace.

	Nicolas Silva [:nical] Assignee
	Comment 22 • 9 years ago

SharedSurfaceTextureClient was trickier to convert because a lot of the SharedSurface code poke at the SharedTextureClient's internals, so I kept a special TextureClient class for it which is a bit awkward, but the patch remains reasonably small and we still have a dedicated TextureData object which is what I need to finally fix the destruction sequence and recycling logic.

The interaction between TextureClient and SharedSurface is still full of surprises and it'll eventually need to be improved in a way that doesn't require synchronous deallocation and where the data is really owned by the TextureClient. so I still consider it in a temporary state and there's plenty more to do in this area.

	Kelsey Gilbert [:jgilbert]
	Comment 23 • 9 years ago

Comment on attachment 8683668 [details] [diff] [review]
TextureData: SharedSurface

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

::: gfx/layers/client/TextureClientSharedSurface.h
@@ +55,5 @@
> +  virtual bool Serialize(SurfaceDescriptor& aOutDescriptor) override;
> +
> +  virtual void Deallocate(ISurfaceAllocator*) override;
> +
> +  gl::SharedSurface* Surf() const { return mSurf.get(); }

Why is this added? mSurf is const, so it is its own getter. No need to add a useless function.

	Nicolas Silva [:nical] Assignee
	Comment 24 • 9 years ago

There we go, the awkward ClientTexture class is gone.

	Nicolas Silva [:nical] Assignee
	Comment 25 • 9 years ago

This is one of the big wins from this patch queue. TextureClient's deallocation code used to be spread in different places including some duplicated parts in ShadowLayers.cpp and ImageBridgeChiold.cpp. Now all of the logic goes into one function which is much clearer. In addition, Textures can be deallocated on the client side asynchronously which was not possible before that and that will let us remove some really nasty and slow synchronous deallocation cases. And also we don't need the KeepUntilFullDeallocation stuff and other refcounting tricks anymore, because all of the TextureData objects are kept until TextureChild::ActorDestroy is called.

	Nicolas Silva [:nical] Assignee
	Comment 26 • 9 years ago

The previous version had a silly issue with synchronous deallocation.

	Nicolas Silva [:nical] Assignee
	Comment 27 • 9 years ago

Bas, in case that eases the reviews, the first patch (the one that introduces the ClientTexture class) adds some amount of ugliness which is removed in the last patch of the series. I had to do that to keep things working until all texture types were converted, but it looks good once all patches are in place.
The only thing to worry about is whether the XYZTextureData type converted from TextureClientXYZ has the same behavior.
Separating TextureClient and TextureData into several objects has several benefits, some of which are coming from the fact that the TextureData needs to stay alive for the entire deallocation handshake between the TextureChild and TextureParent, while the TextureHost object is destroyed earlier at the beginning of the handshake when its reference count reaches zero. It also reduces the amount of duplicated code since TextureClient takes care of checking inputs/states and the TextureData just implements the backend bits, but fixing the complexity and awfulness of how we currently destroy textures is the strongest motivator here.

	Matt Woodrow (:mattwoodrow)
	Comment 28 • 9 years ago

Comment on attachment 8684974 [details] [diff] [review]
Merge back TextureClient and ClientTexture

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

\o/

	Nicolas Silva [:nical] Assignee
	Comment 29 • 9 years ago

Review ping. The patchy queue is looking good on try and rebasing it is a long a risky process because of the amount of changes involved.

	Bas Schouten (:bas.schouten)
	Comment 30 • 9 years ago

Comment on attachment 8655416 [details] [diff] [review]
add the TextureData interface and make the X11 implementation use it

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

::: gfx/layers/client/TextureClient.cpp
@@ +209,5 @@
> +        RefPtr<DataSourceSurface> dataSurf = snapshot->GetDataSurface();
> +        mReadbackSink->ProcessReadback(dataSurf);
> +      }
> +    }
> +    mBorrowedDrawTarget = nullptr;

Currently we keep the DT around in a lot of cases even when the texture is unlocked I think.. are we sure we want to null this? Creating the DT can have its cost. Although I guess we can have mData decide to hold on to it or now.

::: gfx/layers/client/TextureClient.h
@@ +588,5 @@
> +};
> +
> +class ClientTexture : public TextureClient {
> +public:
> +  ClientTexture(TextureData* aData, TextureFlags aFlags, ISurfaceAllocator* aAllocator)

Since you're deleting the object pointed to by aData here in the destructor be sure to comment on the transfer of ownership and perhaps even use some C++ magic to add safety.

@@ +616,5 @@
> +
> +  virtual bool ToSurfaceDescriptor(SurfaceDescriptor& aOutDescriptor) override;
> +
> +  already_AddRefed<TextureClient>
> +  CreateSimilar(TextureFlags aFlags = TextureFlags::DEFAULT,

nit: indent

	Bas Schouten (:bas.schouten)
	Comment 31 • 9 years ago

Comment on attachment 8678885 [details] [diff] [review]
TextureData: D3D9 implementation

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

::: gfx/layers/d3d9/TextureD3D9.cpp
@@ +538,4 @@
>      return nullptr;
>    }
>  
> +  RefPtr<DrawTarget> dt;

It worries me that we'll now longer be caching this through lock/unlock pairs.

	Bas Schouten (:bas.schouten)
	Comment 32 • 9 years ago

Comment on attachment 8678884 [details] [diff] [review]
TextureData: D3D11 implementation

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

I'm assuming this is mostly code moving around, I haven't reviewed the bodies of a bunch of actual functions in detail, if this was a mistake let me know.

::: gfx/layers/d3d11/TextureD3D11.cpp
@@ +680,5 @@
>  }
>  
> +already_AddRefed<DrawTarget>
> +D3D11TextureData::BorrowDrawTarget()
> +{

It's good to see we're continuing to cache this one since we definitely have to do so here.

	Nicolas Silva [:nical] Assignee
	Comment 33 • 9 years ago

I tried to keep caching the DT in the places we used to but I'll double check and make sure we keep around the D3D9 one if we did before the patch.

I have a patch in the making to use UniquePtr with TextureData (should have done that in the first place)

	Nicolas Silva [:nical] Assignee
	Comment 34 • 9 years ago

Woops, I forgot to set the review flag for this one.

A bit too much got folded into this patch, sorry, but basically it consolidates the destruction logic in one place that is easier to understand and cover all corner cases (there are more of them than I expected). It also cleans up a whole bunch of stuff that was made useless by this patch and the previous ones, and contains the various fixes I needed to get things green on try.

	Matt Woodrow (:mattwoodrow)
	Comment 35 • 9 years ago

Comment on attachment 8689130 [details] [diff] [review]
Consolidate the texture deallocation code + fixes

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

::: gfx/2d/2D.h
@@ +1132,5 @@
>  {
>  public:
> +  struct Config {
> +    int32_t mMaxTextureSize;
> +    int32_t mMaxAllocSize;

What is this added for? I don't see it being used.

::: gfx/layers/client/TextureClient.cpp
@@ +225,5 @@
> +///
> +/// This funciton takes care of dispatching work to the right thread using
> +/// a synchronous proxy if needed, and handles client/host deallocation.
> +void
> +TextureClientDeallocation(TextureDeallocParams params)

DoTextureClientDeallocation, or DeallocateTextureClient.

	Nicolas Silva [:nical] Assignee
	Comment 36 • 9 years ago

(In reply to Matt Woodrow (:mattwoodrow) from comment #35)
> What is this added for? I don't see it being used.

Ah, darnit. That's me working on two patch sets at the same time. This has nothing to do here.

> DoTextureClientDeallocation, or DeallocateTextureClient.

ok

	Pulsebot
	Comment 37 • 9 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/fe2164aa1468
https://hg.mozilla.org/integration/mozilla-inbound/rev/6dc38a1e6073
https://hg.mozilla.org/integration/mozilla-inbound/rev/0092c9d7a86b
https://hg.mozilla.org/integration/mozilla-inbound/rev/a8cdfbf443d8
https://hg.mozilla.org/integration/mozilla-inbound/rev/6819f6d82287
https://hg.mozilla.org/integration/mozilla-inbound/rev/b555b130d439
https://hg.mozilla.org/integration/mozilla-inbound/rev/09e71ba15ea8
https://hg.mozilla.org/integration/mozilla-inbound/rev/a369a2983ceb
https://hg.mozilla.org/integration/mozilla-inbound/rev/9c27c8e2c021
https://hg.mozilla.org/integration/mozilla-inbound/rev/e55376bd2cf8
https://hg.mozilla.org/integration/mozilla-inbound/rev/4b88e8a3d8af
https://hg.mozilla.org/integration/mozilla-inbound/rev/c13228f84d85

	Pulsebot
	Comment 38 • 9 years ago

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

	Carsten Book [:Tomcat]
	Comment 39 • 9 years ago

(In reply to Pulsebot from comment #37)

sorry had to back this out since something seems have caused https://treeherder.mozilla.org/logviewer.html#?job_id=17609171&repo=mozilla-inbound here :(

	Pulsebot
	Comment 40 • 9 years ago

Backout:
https://hg.mozilla.org/integration/mozilla-inbound/rev/027edbd76ba2

	Pulsebot
	Comment 41 • 9 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/bb84403701b7
https://hg.mozilla.org/integration/mozilla-inbound/rev/8cc12f12f3d1
https://hg.mozilla.org/integration/mozilla-inbound/rev/0dc93424546c
https://hg.mozilla.org/integration/mozilla-inbound/rev/ed34bc528a1b
https://hg.mozilla.org/integration/mozilla-inbound/rev/ab326c34f1cf
https://hg.mozilla.org/integration/mozilla-inbound/rev/68aba6b39588
https://hg.mozilla.org/integration/mozilla-inbound/rev/add3fe9afc0c
https://hg.mozilla.org/integration/mozilla-inbound/rev/3ad5a4c457fe
https://hg.mozilla.org/integration/mozilla-inbound/rev/fc1fbb97c8eb
https://hg.mozilla.org/integration/mozilla-inbound/rev/7663208f1582
https://hg.mozilla.org/integration/mozilla-inbound/rev/65da564f952c
https://hg.mozilla.org/integration/mozilla-inbound/rev/cf8cf1a039dd

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 43 • 9 years ago

I had to back this all out to fix some b2g crashes https://treeherder.mozilla.org/logviewer.html#?job_id=17749266&repo=mozilla-inbound

https://hg.mozilla.org/integration/mozilla-inbound/rev/9783264529d3

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 44 • 9 years ago

It's possible this also caused an intermittent failure in web-platform-tests(1) like https://treeherder.mozilla.org/logviewer.html#?job_id=17751633&repo=mozilla-inbound but I'm less sure about that one.

	Pulsebot
	Comment 45 • 9 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/d3a6b8f4f2cd
https://hg.mozilla.org/integration/mozilla-inbound/rev/c2674884d1f7
https://hg.mozilla.org/integration/mozilla-inbound/rev/8cb40615eeb0
https://hg.mozilla.org/integration/mozilla-inbound/rev/981a6bc9b1ae
https://hg.mozilla.org/integration/mozilla-inbound/rev/4b2a5d9c0ccd
https://hg.mozilla.org/integration/mozilla-inbound/rev/d296b6686792
https://hg.mozilla.org/integration/mozilla-inbound/rev/0083654ce7df
https://hg.mozilla.org/integration/mozilla-inbound/rev/cccfd06730e9
https://hg.mozilla.org/integration/mozilla-inbound/rev/ca5f63ec86ec
https://hg.mozilla.org/integration/mozilla-inbound/rev/94e3e5bfbeb3
https://hg.mozilla.org/integration/mozilla-inbound/rev/971189684dd5
https://hg.mozilla.org/integration/mozilla-inbound/rev/49e11078c536

	Nigel Babu [:nigelb]
	Comment 46 • 9 years ago

Hello, this has caused build bustage on B2G emulator builds (as of now). Can you please look urgently?

	Pulsebot
	Comment 47 • 9 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/54693bad0ca1

	Pulsebot
	Comment 48 • 9 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/4847034e573e

	Carsten Book [:Tomcat]
	Comment 49 • 9 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/d3a6b8f4f2cd
https://hg.mozilla.org/mozilla-central/rev/c2674884d1f7
https://hg.mozilla.org/mozilla-central/rev/8cb40615eeb0
https://hg.mozilla.org/mozilla-central/rev/981a6bc9b1ae
https://hg.mozilla.org/mozilla-central/rev/4b2a5d9c0ccd
https://hg.mozilla.org/mozilla-central/rev/d296b6686792
https://hg.mozilla.org/mozilla-central/rev/0083654ce7df
https://hg.mozilla.org/mozilla-central/rev/cccfd06730e9
https://hg.mozilla.org/mozilla-central/rev/ca5f63ec86ec
https://hg.mozilla.org/mozilla-central/rev/94e3e5bfbeb3
https://hg.mozilla.org/mozilla-central/rev/971189684dd5
https://hg.mozilla.org/mozilla-central/rev/49e11078c536
https://hg.mozilla.org/mozilla-central/rev/54693bad0ca1
https://hg.mozilla.org/mozilla-central/rev/4847034e573e

	Vincent Liu[:vliu]
	Comment 50 • 9 years ago

Hi Nical,

Recently I'd working on 2d canvas in Worker thread, which the bug id is Bug 801176. It aims at operating 2d canvas in Worker thread. It seems that your recently patch landing in this bug changes behaviors with my working patch.

https://dxr.mozilla.org/mozilla-central/source/gfx/layers/client/TextureClient.cpp#481 

You put a check mechanism to make sure this function runs in main thread. May I know your concern if it runs in Worker? Really thanks for your comment. :)

	Nicolas Silva [:nical] Assignee
	Comment 51 • 9 years ago

(In reply to Vincent Liu[:vliu] from comment #50)
> Hi Nical,
> 
> Recently I'd working on 2d canvas in Worker thread, which the bug id is Bug
> 801176. It aims at operating 2d canvas in Worker thread. It seems that your
> recently patch landing in this bug changes behaviors with my working patch.
> 
> https://dxr.mozilla.org/mozilla-central/source/gfx/layers/client/
> TextureClient.cpp#481 
> 
> You put a check mechanism to make sure this function runs in main thread.
> May I know your concern if it runs in Worker? Really thanks for your
> comment. :)
Hi,

The video pipeline needed to update a TextureClient off the main thread and we had to add the TextureClient::UpdateFromSurface in order to do that. I think that this is because of a restriction with the D2D backend. I guess that we would be fine doing this with the cario-image backend but probably not with skia, and probably not with cairo-xlib either, I don't know for core graphics (Bas, probably knows more about the threading restrictions involved here with the different backends).

If you only need to copy the content of the canvas into the TextureClient, then you can use UpdateFromSurface. If the canvas draws directly into the TextureClient, then we need to figure something out (and probably restrict to cairo-image, and even with this, we'll run into issues rendering text (can't do that in parallel with our version of cairo, right now).

	Sotaro Ikeda [:sotaro]
	Comment 52 • 9 years ago

nical, jerry found that attachment 8657869 [details] [diff] [review] remove WaitForBufferOwnership(). It is necessary to wait the case when fence is not delivered to client side. It could cause a regression.

	Vincent Liu[:vliu]
	Comment 53 • 9 years ago

(In reply to Nicolas Silva [:nical] from comment #51)
> If you only need to copy the content of the canvas into the TextureClient,
> then you can use UpdateFromSurface. If the canvas draws directly into the
> TextureClient, then we need to figure something out (and probably restrict
> to cairo-image, and even with this, we'll run into issues rendering text
> (can't do that in parallel with our version of cairo, right now).

Please correct me if I got anything wrong. As your suggestion, I need to get SourceSurface as argument before calling UpdateFromSurface. To get it, I need to have DrawTarget. The thing is I can't get DrawTarget when calling BorrowDrawTarget. If so, could you please have more suggestions for the problem I met? Thanks.

	Nicolas Silva [:nical] Assignee
	Comment 54 • 9 years ago

(In reply to Vincent Liu[:vliu] from comment #53)
> To get it, I need to have DrawTarget.

You don't need a DrawTarget to get a SourceSurface, although that's one way to do it.
If the data you want to copy into the TextureClient is on the cpu, you are probably better off wrapping a temporary SourceSurface around with gfx::Factory::CreateWrappingDataSourceSurface.
What kind of backend are you dealing with off the main thread? is it exclusively cpu backends (cairo-image and skia) or can it also be a gpu backend like D2D?