1039568 - Add a capture DrawTarget to Moz2D

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Bas Schouten (:bas.schouten)]

Attachment: Add DrawTargetCapture to Moz2D

For tiling we'd like the ability to capture a series of drawing commands and replay them in a consistent fashion across multiple tiles. This patch aims to add that support.

Comment 1

[Bas Schouten (:bas.schouten)]

Attachment: Add tests for DrawTargetCapture to Moz2D

Comment 2

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8456945 [details] [diff] [review]
Add DrawTargetCapture to Moz2D

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

::: DrawTargetCapture.h
@@ +138,5 @@
> +
> +  // This storage system was used to minimize the amount of heap allocations
> +  // that are required while recording.
> +  template<typename T>
> +  void AppendToCommandList(const T& aDrawingCommand)

We could avoid a copy by returning the pointer and then constructing in-place from the caller. Unless the compiler can optimize to that from this code, but I'm not convinced that it can.

@@ +150,5 @@
> +  RefPtr<DrawTarget> mRefDT;
> +
> +  IntSize mSize;
> +
> +  std::vector<uint8_t> mDrawCommandStorage;

Won't we leak everything in here when it's destructed?

It seems like we need to iterate over the area again (possibly combined with the final replay) and invoke the destructors.

Or we could store raw pointers to things in mDrawCommandDrawStorage and keep an array of RefPtr<SourceSurface> etc separately as lifetime management.

::: SourceSurfaceRawData.h
@@ +32,5 @@
>                          SurfaceFormat aFormat,
>                          bool aOwnData);
>  
> +  virtual void GuaranteePersistance();
> +

We could probably also add a method for users of this code to tell the SourceSurfaceRawData to take ownership of existing data. I expect in a lot of cases where we might hit the copy path we could use that instead. Not that easy to find where we need it other than just looking at profiles though.

Comment 3

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8456945 [details] [diff] [review]
Add DrawTargetCapture to Moz2D

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

::: DrawTargetCapture.h
@@ +143,5 @@
> +  {
> +    size_t oldSize = mDrawCommandStorage.size();
> +    mDrawCommandStorage.resize(mDrawCommandStorage.size() + sizeof(T) + sizeof(uint32_t));
> +    uint8_t* nextDrawLocation = &mDrawCommandStorage.front() + oldSize;
> +    *(uint32_t*)(nextDrawLocation) = sizeof(T) + sizeof(uint32_t);

This can cause unaligned accesses. I'm not sure how much it matters (I suppose you could have a compiler that was using vector instructions and assuming alignment of particular things), but it's at least worth a comment, or just fix it to memcpy in and out of the buffer. The sizes are small enough that I don't think the copies will matter... Up to you.

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/150cdebe837f