Bug 1838792: Add TryAddStoredObject to DrawEventRecorderPrivate and use in DrawTargetRecording. r=aosmond

This means we can do a combination of HasStoredObject and AddStoredObject with
only one call to ProcessPendingDeletions, which uses locking.

Differential Revision: https://phabricator.services.mozilla.com/D181963
This commit is contained in:
Bob Owen 2023-06-27 13:58:58 +00:00
parent 541b2ef29d
commit f2741a022a
2 changed files with 46 additions and 26 deletions

View File

@ -79,6 +79,22 @@ class DrawEventRecorderPrivate : public DrawEventRecorder {
mStoredObjects.insert(aObject);
}
/**
* This is a combination of HasStoredObject and AddStoredObject, so that we
* only have to call ProcessPendingDeletions once, which involves locking.
* @param aObject the object to store if not already stored
* @return true if the object was not already stored, false if it was
*/
bool TryAddStoredObject(const ReferencePtr aObject) {
ProcessPendingDeletions();
if (mStoredObjects.find(aObject) != mStoredObjects.end()) {
return false;
}
mStoredObjects.insert(aObject);
return true;
}
void AddPendingDeletion(std::function<void()>&& aPendingDeletion) {
auto lockedPendingDeletions = mPendingDeletions.Lock();
lockedPendingDeletions->emplace_back(std::move(aPendingDeletion));
@ -120,9 +136,13 @@ class DrawEventRecorderPrivate : public DrawEventRecorder {
mStoredSurfaces.erase(aSurface);
}
#if defined(DEBUG)
// Only used within debug assertions.
bool HasStoredObject(const ReferencePtr aObject) {
ProcessPendingDeletions();
return mStoredObjects.find(aObject) != mStoredObjects.end();
}
#endif
void AddStoredFontData(const uint64_t aFontDataKey) {
mStoredFontData.insert(aFontDataKey);

View File

@ -47,17 +47,16 @@ static void RecordingSourceSurfaceUserDataFunc(void* aUserData) {
});
}
static void EnsureSurfaceStoredRecording(DrawEventRecorderPrivate* aRecorder,
static bool EnsureSurfaceStoredRecording(DrawEventRecorderPrivate* aRecorder,
SourceSurface* aSurface,
const char* reason) {
if (aRecorder->HasStoredObject(aSurface)) {
return;
}
// It's important that AddStoredObject is called first because that will
// It's important that TryAddStoredObject is called first because that will
// run any pending processing required by recorded objects that have been
// deleted off the main thread.
aRecorder->AddStoredObject(aSurface);
if (!aRecorder->TryAddStoredObject(aSurface)) {
// Surface is already stored.
return false;
}
aRecorder->StoreSourceSurfaceRecording(aSurface, reason);
aRecorder->AddSourceSurface(aSurface);
@ -66,6 +65,7 @@ static void EnsureSurfaceStoredRecording(DrawEventRecorderPrivate* aRecorder,
userData->recorder = aRecorder;
aSurface->AddUserData(reinterpret_cast<UserDataKey*>(aRecorder), userData,
&RecordingSourceSurfaceUserDataFunc);
return true;
}
class SourceSurfaceRecording : public SourceSurface {
@ -572,27 +572,26 @@ already_AddRefed<SourceSurface> DrawTargetRecording::OptimizeSourceSurface(
if (strongRef) {
return do_AddRef(strongRef);
}
}
} else {
if (!EnsureSurfaceStoredRecording(mRecorder, aSurface,
"OptimizeSourceSurface")) {
// Surface was already stored, but doesn't have UserData so must be one
// of our recording surfaces.
MOZ_ASSERT(aSurface->GetType() == SurfaceType::RECORDING);
return do_AddRef(aSurface);
}
if (aSurface->GetType() == SurfaceType::RECORDING &&
mRecorder->HasStoredObject(aSurface)) {
// aSurface is already optimized for our recorder.
return do_AddRef(aSurface);
userData = static_cast<RecordingSourceSurfaceUserData*>(
aSurface->GetUserData(reinterpret_cast<UserDataKey*>(mRecorder.get())));
MOZ_ASSERT(userData,
"User data should always have been set by "
"EnsureSurfaceStoredRecording.");
}
EnsureSurfaceStoredRecording(mRecorder, aSurface, "OptimizeSourceSurface");
RefPtr<SourceSurface> retSurf = new SourceSurfaceRecording(
aSurface->GetSize(), aSurface->GetFormat(), mRecorder, aSurface);
mRecorder->RecordEvent(
RecordedOptimizeSourceSurface(aSurface, this, retSurf));
userData = static_cast<RecordingSourceSurfaceUserData*>(
aSurface->GetUserData(reinterpret_cast<UserDataKey*>(mRecorder.get())));
MOZ_ASSERT(
userData,
"User data should always have been set by EnsureSurfaceStoredRecording.");
userData->optimizedSurface = retSurf;
return retSurf.forget();
@ -715,7 +714,8 @@ already_AddRefed<PathRecording> DrawTargetRecording::EnsurePathStored(
if (aPath->GetBackendType() == BackendType::RECORDING) {
pathRecording =
const_cast<PathRecording*>(static_cast<const PathRecording*>(aPath));
if (mRecorder->HasStoredObject(aPath)) {
if (!mRecorder->TryAddStoredObject(pathRecording)) {
// Path is already stored.
return pathRecording.forget();
}
} else {
@ -725,12 +725,12 @@ already_AddRefed<PathRecording> DrawTargetRecording::EnsurePathStored(
new PathBuilderRecording(mFinalDT->GetBackendType(), fillRule);
aPath->StreamToSink(builderRecording);
pathRecording = builderRecording->Finish().downcast<PathRecording>();
mRecorder->AddStoredObject(pathRecording);
}
// It's important that AddStoredObject is called first because that will
// run any pending processing required by recorded objects that have been
// deleted off the main thread.
mRecorder->AddStoredObject(pathRecording);
// It's important that AddStoredObject or TryAddStoredObject is called before
// this because that will run any pending processing required by recorded
// objects that have been deleted off the main thread.
mRecorder->RecordEvent(RecordedPathCreation(pathRecording.get()));
pathRecording->mStoredRecorders.push_back(mRecorder);