1027602 - Move TextureClient/Host backend-specific implementation into a class that can be re-used by both client and host code

Status: RESOLVED WONTFIX

(Core :: Graphics: Layers, defect)

Comment 1

[Nicolas Silva [:nical]]

There are several things I am trying to solve here:

A) Some boilerplate code is repeated in all TextureClient implementaions. For example the logic around storing the borrowed DrawTargets, keeping track of the locked/unlocked state, asserting that some calls are made in the right state, etc. We should separate the backend-specific code from this kind of boiler plate 
B) Some code is the same between the client and host, but right now we duplicate it.
C) Make it easy to support client-side deallocation asynchronously (right now all client-side texture deallocations are synchronous which is sad because it hurts performance a lot).
D) Make it easy to ensure that a TextureClient's shared data isn't deallocated befor ethe host can deserialize it. 

Items (C) or (D) would help fix Bug 1039535 which is why I bring this up now. The rest is more about having cleaner code which can always wait.


What I propose:

Have a backend-independent TextureClient implementation that owns a TextureData class. TextureData contains the backend-specific logic (ShmemTextureData, TextureDataD3D11, etc...)

The backend independent TextureClient has the same interface as before, and forwards all of its calls to its TextureData. Example:

DrawTarget*
GenericTextureClient::BorrowDrawTarget()
{
  // do all of the backend independent checks and book keeping here
  // ...
  if (!mBorrowedDrawTarget) {
    mBorrowedDrawTarget = mData->BorrowDrawTarget(); // TextureData implem
    // ... more boilerplate here ...
  }
  return mBorrowedDrawTarget;
}

As a first step this would help me keep the shared data alive longer than the TextureClient without synchronous messages which I need for DXGI TextureClient.

As a second step we would use TextureData for TextureHost too (to reuse code, but nothing really blocks on that right now)

As a third step, move this logic to the base-class of TextureClient and TextureHost (MozSurface or whatever we'll call it). This will remove the last bits of duplication and allow us to write some of the logic that manipulate textures independently of the client/host context. This came up during discussions with Sotaro about making a robust swap-chain design (which connects to some of the discussions of the paris work week too).

All of those steps are failry mechanical. Nothing changes in the current logic, we just split some code into different places. Step 1 can be done per-TextureClient and would help with the DXGI situation so I'd like to do it now, at least for the DXGI TextureClient.

Step 2 can be done per-TextureHost, and is easy as well, step 3 is trivial but requires step 1 and 2 to be complete.
The fact that I divide this into 3 steps makes it sound like a lot of work but it's actually pretty quick and simple to do.

This is based on discussions with Sotaro during the last team meetup in Toronto. Sotaro, am I missing something?

Comment 2

[Nicolas Silva [:nical]]

Attachment: diagram of what the classes look like after each step

ascii diagram to illustrate my previous comment.

Comment 3

[Nicolas Silva [:nical]]

I forgot to specify that I only need the DXGI TextureClient part of step 1 right now, and that the rest can come gradually later (easy enough to be a series of good first bugs).

Comment 4

[Nicolas Silva [:nical]]

Attachment: diagram of what the classes look like after each step

Fixed an error in the diagram.

Comment 5

[Nicolas Silva [:nical]]

Attachment: Add the surface SurfaceData interface

Here is the staring point of step 1: Add the SurfaceData interface. Since all TextureClients aren't going to be converted in one commit, add a GenericTextureClient implementation of TextureClient that does all of the checks and forwards the rest to SurfaceData. Once all TextureClients are based on SurfaceData, GenericTextureClient and TextureClient can be merged.

Comment 6

[Nicolas Silva [:nical]]

Attachment: Implement the D3D11 TextureClient as a SurfaceData implementation rather than subclassing TextureClient.

This pretty much only removes the checks in TextureClientD3D11 (since they are now done by GenericTextureClient) and rename TextureClientD3D11 into SurfaceDataD3D11.

Comment 7

[Nicolas Silva [:nical]]

I had a long discussion with Bas about this, and he has an interesting counter-proposal. 
Bas's approach mostly solves the same problems but is sufficiently different that going in the direction of what I propose in this bug is not useful if we later choose to go with Bas's design so I will put this on hold for now. We agreed that the DXGI issue can have a hacky fix in the short-medium term. We'll talk about his design proposal whenever the timing is right and decide where to go from there.

Comment 8

[Nicolas Silva [:nical]]

(let's not close this bug though, it contains useful reference for whenever that discussion happens)