commit b5f5ac7b7314a917b4b6736aca51547ccfa46449
parent 6ea38474b2651b9f9e546293d3d21802722c712d
Author: Lee Salzman <lsalzman@mozilla.com>
Date: Sun, 12 Oct 2025 20:56:55 +0000
Bug 1993852 - Simplify AC2D surface mapping. r=aosmond
The mechanism of PreparedMaps immediately before a GetDataSurface call is somewhat superfluous
and just exposes the scenario to more possible failures. Instead, do the relevant mapping inside
GetDataSurface.
Differential Revision: https://phabricator.services.mozilla.com/D268325
Diffstat:
4 files changed, 58 insertions(+), 144 deletions(-)
diff --git a/gfx/layers/RecordedCanvasEventImpl.h b/gfx/layers/RecordedCanvasEventImpl.h
@@ -33,24 +33,23 @@ const EventType CANVAS_FLUSH = EventType(EventType::LAST + 2);
const EventType TEXTURE_LOCK = EventType(EventType::LAST + 3);
const EventType TEXTURE_UNLOCK = EventType(EventType::LAST + 4);
const EventType CACHE_DATA_SURFACE = EventType(EventType::LAST + 5);
-const EventType PREPARE_DATA_FOR_SURFACE = EventType(EventType::LAST + 6);
-const EventType GET_DATA_FOR_SURFACE = EventType(EventType::LAST + 7);
-const EventType ADD_SURFACE_ALIAS = EventType(EventType::LAST + 8);
-const EventType REMOVE_SURFACE_ALIAS = EventType(EventType::LAST + 9);
-const EventType DEVICE_CHANGE_ACKNOWLEDGED = EventType(EventType::LAST + 10);
-const EventType CANVAS_DRAW_TARGET_CREATION = EventType(EventType::LAST + 11);
-const EventType TEXTURE_DESTRUCTION = EventType(EventType::LAST + 12);
-const EventType CHECKPOINT = EventType(EventType::LAST + 13);
-const EventType PAUSE_TRANSLATION = EventType(EventType::LAST + 14);
-const EventType RECYCLE_BUFFER = EventType(EventType::LAST + 15);
-const EventType DROP_BUFFER = EventType(EventType::LAST + 16);
-const EventType PREPARE_SHMEM = EventType(EventType::LAST + 17);
-const EventType PRESENT_TEXTURE = EventType(EventType::LAST + 18);
-const EventType DEVICE_RESET_ACKNOWLEDGED = EventType(EventType::LAST + 19);
-const EventType AWAIT_TRANSLATION_SYNC = EventType(EventType::LAST + 20);
-const EventType RESOLVE_EXTERNAL_SNAPSHOT = EventType(EventType::LAST + 21);
-const EventType ADD_EXPORT_SURFACE = EventType(EventType::LAST + 22);
-const EventType REMOVE_EXPORT_SURFACE = EventType(EventType::LAST + 23);
+const EventType GET_DATA_FOR_SURFACE = EventType(EventType::LAST + 6);
+const EventType ADD_SURFACE_ALIAS = EventType(EventType::LAST + 7);
+const EventType REMOVE_SURFACE_ALIAS = EventType(EventType::LAST + 8);
+const EventType DEVICE_CHANGE_ACKNOWLEDGED = EventType(EventType::LAST + 9);
+const EventType CANVAS_DRAW_TARGET_CREATION = EventType(EventType::LAST + 10);
+const EventType TEXTURE_DESTRUCTION = EventType(EventType::LAST + 11);
+const EventType CHECKPOINT = EventType(EventType::LAST + 12);
+const EventType PAUSE_TRANSLATION = EventType(EventType::LAST + 13);
+const EventType RECYCLE_BUFFER = EventType(EventType::LAST + 14);
+const EventType DROP_BUFFER = EventType(EventType::LAST + 15);
+const EventType PREPARE_SHMEM = EventType(EventType::LAST + 16);
+const EventType PRESENT_TEXTURE = EventType(EventType::LAST + 17);
+const EventType DEVICE_RESET_ACKNOWLEDGED = EventType(EventType::LAST + 18);
+const EventType AWAIT_TRANSLATION_SYNC = EventType(EventType::LAST + 19);
+const EventType RESOLVE_EXTERNAL_SNAPSHOT = EventType(EventType::LAST + 20);
+const EventType ADD_EXPORT_SURFACE = EventType(EventType::LAST + 21);
+const EventType REMOVE_EXPORT_SURFACE = EventType(EventType::LAST + 22);
const EventType LAST_CANVAS_EVENT_TYPE = REMOVE_EXPORT_SURFACE;
class RecordedCanvasBeginTransaction final
@@ -235,8 +234,11 @@ RecordedTextureUnlock::RecordedTextureUnlock(S& aStream)
class RecordedCacheDataSurface final
: public RecordedEventDerived<RecordedCacheDataSurface> {
public:
- explicit RecordedCacheDataSurface(gfx::SourceSurface* aSurface)
- : RecordedEventDerived(CACHE_DATA_SURFACE), mSurface(aSurface) {}
+ explicit RecordedCacheDataSurface(const gfx::SourceSurface* aSurface,
+ bool aForceData = false)
+ : RecordedEventDerived(CACHE_DATA_SURFACE),
+ mSurface(aSurface),
+ mForceData(aForceData) {}
template <class S>
MOZ_IMPLICIT RecordedCacheDataSurface(S& aStream);
@@ -250,88 +252,43 @@ class RecordedCacheDataSurface final
private:
ReferencePtr mSurface;
+ bool mForceData = false;
};
inline bool RecordedCacheDataSurface::PlayCanvasEvent(
CanvasTranslator* aTranslator) const {
+ if (RefPtr<gfx::DataSourceSurface> dataSurface =
+ aTranslator->LookupDataSurface(mSurface)) {
+ if (mForceData) {
+ (void)dataSurface->GetData();
+ }
+ return true;
+ }
+
gfx::SourceSurface* surface = aTranslator->LookupSourceSurface(mSurface);
if (!surface) {
return false;
}
-
- RefPtr<gfx::DataSourceSurface> dataSurface = surface->GetDataSurface();
-
- aTranslator->AddDataSurface(mSurface, std::move(dataSurface));
+ if (RefPtr<gfx::DataSourceSurface> dataSurface = surface->GetDataSurface()) {
+ if (mForceData) {
+ (void)dataSurface->GetData();
+ }
+ aTranslator->AddDataSurface(mSurface, std::move(dataSurface));
+ }
return true;
}
template <class S>
void RecordedCacheDataSurface::Record(S& aStream) const {
WriteElement(aStream, mSurface);
+ WriteElement(aStream, mForceData);
}
template <class S>
RecordedCacheDataSurface::RecordedCacheDataSurface(S& aStream)
: RecordedEventDerived(CACHE_DATA_SURFACE) {
ReadElement(aStream, mSurface);
-}
-
-class RecordedPrepareDataForSurface final
- : public RecordedEventDerived<RecordedPrepareDataForSurface> {
- public:
- explicit RecordedPrepareDataForSurface(const gfx::SourceSurface* aSurface)
- : RecordedEventDerived(PREPARE_DATA_FOR_SURFACE), mSurface(aSurface) {}
-
- template <class S>
- MOZ_IMPLICIT RecordedPrepareDataForSurface(S& aStream);
-
- bool PlayCanvasEvent(CanvasTranslator* aTranslator) const;
-
- template <class S>
- void Record(S& aStream) const;
-
- std::string GetName() const final { return "RecordedPrepareDataForSurface"; }
-
- private:
- ReferencePtr mSurface;
-};
-
-inline bool RecordedPrepareDataForSurface::PlayCanvasEvent(
- CanvasTranslator* aTranslator) const {
- RefPtr<gfx::DataSourceSurface> dataSurface =
- aTranslator->LookupDataSurface(mSurface);
- if (!dataSurface) {
- gfx::SourceSurface* surface = aTranslator->LookupSourceSurface(mSurface);
- if (!surface) {
- return false;
- }
-
- dataSurface = surface->GetDataSurface();
- if (!dataSurface) {
- return false;
- }
- }
-
- auto preparedMap = MakeUnique<gfx::DataSourceSurface::ScopedMap>(
- dataSurface, gfx::DataSourceSurface::READ);
- if (!preparedMap->IsMapped()) {
- return false;
- }
-
- aTranslator->SetPreparedMap(mSurface, std::move(preparedMap));
-
- return true;
-}
-
-template <class S>
-void RecordedPrepareDataForSurface::Record(S& aStream) const {
- WriteElement(aStream, mSurface);
-}
-
-template <class S>
-RecordedPrepareDataForSurface::RecordedPrepareDataForSurface(S& aStream)
- : RecordedEventDerived(PREPARE_DATA_FOR_SURFACE) {
- ReadElement(aStream, mSurface);
+ ReadElement(aStream, mForceData);
}
class RecordedGetDataForSurface final
@@ -979,7 +936,6 @@ RecordedRemoveExportSurface::RecordedRemoveExportSurface(S& aStream)
f(TEXTURE_LOCK, RecordedTextureLock); \
f(TEXTURE_UNLOCK, RecordedTextureUnlock); \
f(CACHE_DATA_SURFACE, RecordedCacheDataSurface); \
- f(PREPARE_DATA_FOR_SURFACE, RecordedPrepareDataForSurface); \
f(GET_DATA_FOR_SURFACE, RecordedGetDataForSurface); \
f(ADD_SURFACE_ALIAS, RecordedAddSurfaceAlias); \
f(REMOVE_SURFACE_ALIAS, RecordedRemoveSurfaceAlias); \
diff --git a/gfx/layers/ipc/CanvasChild.cpp b/gfx/layers/ipc/CanvasChild.cpp
@@ -634,7 +634,7 @@ already_AddRefed<gfx::DataSourceSurface> CanvasChild::GetDataSurface(
}
}
- RecordEvent(RecordedPrepareDataForSurface(aSurface));
+ RecordEvent(RecordedCacheDataSurface(aSurface, true));
if (!EnsureDataSurfaceShmem(ssSize, ssFormat)) {
return nullptr;
diff --git a/gfx/layers/ipc/CanvasTranslator.cpp b/gfx/layers/ipc/CanvasTranslator.cpp
@@ -358,22 +358,29 @@ void CanvasTranslator::GetDataSurface(uint64_t aSurfaceRef) {
MOZ_ASSERT(IsInTaskQueue());
ReferencePtr surfaceRef = reinterpret_cast<void*>(aSurfaceRef);
- gfx::SourceSurface* surface = LookupSourceSurface(surfaceRef);
- if (!surface) {
- return;
+ RefPtr<gfx::DataSourceSurface> dataSurface = LookupDataSurface(surfaceRef);
+ if (!dataSurface) {
+ gfx::SourceSurface* surface = LookupSourceSurface(surfaceRef);
+ if (!surface) {
+ return;
+ }
+ dataSurface = surface->GetDataSurface();
+ if (!dataSurface) {
+ return;
+ }
}
-
- UniquePtr<gfx::DataSourceSurface::ScopedMap> map = GetPreparedMap(surfaceRef);
- if (!map) {
+ gfx::DataSourceSurface::ScopedMap map(dataSurface,
+ gfx::DataSourceSurface::READ);
+ if (!map.IsMapped()) {
return;
}
- auto dstSize = surface->GetSize();
- auto srcSize = map->GetSurface()->GetSize();
- gfx::SurfaceFormat format = surface->GetFormat();
+ auto dstSize = dataSurface->GetSize();
+ auto srcSize = map.GetSurface()->GetSize();
+ gfx::SurfaceFormat format = dataSurface->GetFormat();
int32_t bpp = BytesPerPixel(format);
int32_t dataFormatWidth = dstSize.width * bpp;
- int32_t srcStride = map->GetStride();
+ int32_t srcStride = map.GetStride();
if (dataFormatWidth > srcStride || srcSize != dstSize) {
return;
}
@@ -387,7 +394,7 @@ void CanvasTranslator::GetDataSurface(uint64_t aSurfaceRef) {
}
uint8_t* dst = mDataSurfaceShmem.DataAs<uint8_t>();
- const uint8_t* src = map->GetData();
+ const uint8_t* src = map.GetData();
const uint8_t* endSrc = src + (srcSize.height * srcStride);
while (src < endSrc) {
memcpy(dst, src, dataFormatWidth);
@@ -1889,26 +1896,5 @@ void CanvasTranslator::RemoveDataSurface(gfx::ReferencePtr aRefPtr) {
mDataSurfaces.Remove(aRefPtr);
}
-void CanvasTranslator::SetPreparedMap(
- gfx::ReferencePtr aSurface,
- UniquePtr<gfx::DataSourceSurface::ScopedMap> aMap) {
- mMappedSurface = aSurface;
- mPreparedMap = std::move(aMap);
-}
-
-UniquePtr<gfx::DataSourceSurface::ScopedMap> CanvasTranslator::GetPreparedMap(
- gfx::ReferencePtr aSurface) {
- if (!mPreparedMap) {
- // We might fail to set the map during, for example, device resets.
- return nullptr;
- }
-
- MOZ_RELEASE_ASSERT(mMappedSurface == aSurface,
- "aSurface must match previously stored surface.");
-
- mMappedSurface = nullptr;
- return std::move(mPreparedMap);
-}
-
} // namespace layers
} // namespace mozilla
diff --git a/gfx/layers/ipc/CanvasTranslator.h b/gfx/layers/ipc/CanvasTranslator.h
@@ -243,10 +243,6 @@ class CanvasTranslator final : public gfx::InlineTranslator,
*/
void AddSourceSurface(gfx::ReferencePtr aRefPtr,
gfx::SourceSurface* aSurface) final {
- if (mMappedSurface == aRefPtr) {
- mPreparedMap = nullptr;
- mMappedSurface = nullptr;
- }
RemoveDataSurface(aRefPtr);
InlineTranslator::AddSourceSurface(aRefPtr, aSurface);
}
@@ -258,10 +254,6 @@ class CanvasTranslator final : public gfx::InlineTranslator,
* @param aRefPtr the key to the objects to remove
*/
void RemoveSourceSurface(gfx::ReferencePtr aRefPtr) final {
- if (mMappedSurface == aRefPtr) {
- mPreparedMap = nullptr;
- mMappedSurface = nullptr;
- }
RemoveDataSurface(aRefPtr);
InlineTranslator::RemoveSourceSurface(aRefPtr);
}
@@ -301,24 +293,6 @@ class CanvasTranslator final : public gfx::InlineTranslator,
*/
void RemoveDataSurface(gfx::ReferencePtr aRefPtr);
- /**
- * Sets a ScopedMap, to be used in a later event.
- *
- * @param aSurface the associated surface in the other process
- * @param aMap the ScopedMap to store
- */
- void SetPreparedMap(gfx::ReferencePtr aSurface,
- UniquePtr<gfx::DataSourceSurface::ScopedMap> aMap);
-
- /**
- * Gets the ScopedMap stored using SetPreparedMap.
- *
- * @param aSurface must match the surface from the SetPreparedMap call
- * @returns the ScopedMap if aSurface matches otherwise nullptr
- */
- UniquePtr<gfx::DataSourceSurface::ScopedMap> GetPreparedMap(
- gfx::ReferencePtr aSurface);
-
void PrepareShmem(const RemoteTextureOwnerId aTextureOwnerId);
void RecycleBuffer();
@@ -600,8 +574,6 @@ class CanvasTranslator final : public gfx::InlineTranslator,
void RemoveTextureKeepAlive(const RemoteTextureOwnerId& aId);
nsRefPtrHashtable<nsPtrHashKey<void>, gfx::DataSourceSurface> mDataSurfaces;
- gfx::ReferencePtr mMappedSurface;
- UniquePtr<gfx::DataSourceSurface::ScopedMap> mPreparedMap;
Atomic<bool> mDeactivated{false};
Atomic<bool> mBlocked{false};
Atomic<bool> mIPDLClosed{false};
