Closed Bug 1023350 Opened 6 years ago Closed 6 years ago

The way TextureClient exposes a DrawTarget is confusing

Categories

(Core :: Graphics: Layers, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla33

People

(Reporter: nical, Assigned: nical)

Details

Attachments

(1 file)

TextureClient has Lock/Unlock semantics that also behaves like Map/Unmap. This means that the DrawTarget that you can get out of a TextureClient is only valid between Lock and Unlock. "GetAsDrawTarget" isn't a great name because it doesn't convey the fact that all external references to the DrawTarget must be cleared before Unlock. Moreover, GetAsDrawTarget returns a TemporaryRef, which makes it look like it is a good idea to store the DrawTarget in a RefPtr, which should be discouraged, since the DrawTarget is kept alive by the TextureClient until Unlock, and the reference should not be kept any longer.
The combination of these two things has led to confusion and a few bugs, so let's make the fact that the DrawTarget is temporary and borrowed explicit.
This patch renames GetAsDrawTarget in BorrowDrawTarget and makes it return a pointer rather than a TemporaryRef.
While doing so I noticed that TextureClientX11 is not caching its DrawTarget until Lock (which it should do), so I fixed it as well.
Assignee: nobody → nical.bugzilla
Attachment #8437738 - Flags: review?(bjacob)
Comment on attachment 8437738 [details] [diff] [review]
Rename TextureClient::GetAsDrawTarget into TextureClient::BorrowDrawTarget and fix TextureClientX11

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

::: gfx/layers/basic/TextureClientX11.cpp
@@ +107,5 @@
>      return nullptr;
>    }
>  
>    IntSize size = ToIntSize(mSurface->GetSize());
> +  mDrawTarget = Factory::CreateDrawTargetForCairoSurface(mSurface->CairoSurface(), size);

Just put if (!mDrawTarget) around this? (So no need for 2 return statements)
Attachment #8437738 - Flags: review?(bjacob) → review+
https://hg.mozilla.org/mozilla-central/rev/a3dcd96e1b84
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
You need to log in before you can comment on or make changes to this bug.