Closed Bug 1023350 Opened 6 years ago Closed 6 years ago
The way Texture
Client exposes a Draw Target is confusing
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+
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.