From 8702a092627702b00bbdd656d622793c8fb8b8b8 Mon Sep 17 00:00:00 2001 From: Bogdan Szekely Date: Tue, 12 Jul 2022 12:03:35 +0300 Subject: [PATCH] Backed out changeset 458aae4c5d0a (bug 1777872) for causing mochitest failures on Shmem.cpp CLOSED TREE --- dom/canvas/ClientWebGLContext.cpp | 54 ++------------------ dom/canvas/ClientWebGLContext.h | 4 +- dom/canvas/DrawTargetWebgl.cpp | 83 ++++++------------------------- dom/canvas/DrawTargetWebgl.h | 16 ------ dom/canvas/PWebGL.ipdl | 9 +--- dom/canvas/TexUnpackBlob.cpp | 46 +++-------------- dom/canvas/WebGLParent.cpp | 11 +--- dom/canvas/WebGLParent.h | 3 +- 8 files changed, 29 insertions(+), 197 deletions(-) diff --git a/dom/canvas/ClientWebGLContext.cpp b/dom/canvas/ClientWebGLContext.cpp index 7e952cf8dd0b..b2c1cfe1f75a 100644 --- a/dom/canvas/ClientWebGLContext.cpp +++ b/dom/canvas/ClientWebGLContext.cpp @@ -4358,35 +4358,11 @@ void ClientWebGLContext::TexImage(uint8_t funcDims, GLenum imageTarget, } } -void ClientWebGLContext::RawTexImage(uint32_t level, GLenum respecFormat, - uvec3 offset, const webgl::PackingInfo& pi, - webgl::TexUnpackBlobDesc&& desc) const { +void ClientWebGLContext::RawTexImage( + uint32_t level, GLenum respecFormat, uvec3 offset, + const webgl::PackingInfo& pi, const webgl::TexUnpackBlobDesc& desc) const { const FuncScope funcScope(*this, "tex(Sub)Image[23]D"); if (IsContextLost()) return; - if (desc.sd) { - // Shmems are stored in Buffer surface descriptors. We need to ensure first - // that all queued commands are flushed and then send the Shmem over IPDL. - const auto& sd = *(desc.sd); - if (sd.type() == layers::SurfaceDescriptor::TSurfaceDescriptorBuffer && - sd.get_SurfaceDescriptorBuffer().data().type() == - layers::MemoryOrShmem::TShmem) { - const auto& inProcess = mNotLost->inProcess; - if (inProcess) { - inProcess->TexImage(level, respecFormat, offset, pi, desc); - } else { - const auto& child = mNotLost->outOfProcess; - child->FlushPendingCmds(); - (void)child->SendTexImage(level, respecFormat, offset, pi, - std::move(desc)); - } - } else { - NS_WARNING( - "RawTexImage with SurfaceDescriptor only supports " - "SurfaceDescriptorBuffer with Shmem"); - } - return; - } - Run(level, respecFormat, offset, pi, desc); } @@ -4963,30 +4939,6 @@ bool ClientWebGLContext::DoReadPixels(const webgl::ReadPixelsDesc& desc, return true; } -bool ClientWebGLContext::DoReadPixels(const webgl::ReadPixelsDesc& desc, - const mozilla::ipc::Shmem& shmem) const { - const auto notLost = - mNotLost; // Hold a strong-ref to prevent LoseContext=>UAF. - if (!notLost) return false; - const auto& inProcess = notLost->inProcess; - if (inProcess) { - const auto& shmemBytes = shmem.Range(); - inProcess->ReadPixelsInto(desc, shmemBytes); - return true; - } - const auto& child = notLost->outOfProcess; - child->FlushPendingCmds(); - webgl::ReadPixelsResultIpc res = {}; - // We assume the input is an unsafe shmem which won't be consumed by this - // request. Since SendReadPixels expects a Shmem rvalue, we must create a copy - // to provide it that can be consumed instead of the original descriptor. - mozilla::ipc::Shmem dest = shmem; - if (!child->SendReadPixels(desc, dest, &res)) { - res = {}; - } - return res.byteStride > 0; -} - bool ClientWebGLContext::ReadPixels_SharedPrecheck( dom::CallerType aCallerType, ErrorResult& out_error) const { if (IsContextLost()) return false; diff --git a/dom/canvas/ClientWebGLContext.h b/dom/canvas/ClientWebGLContext.h index 38018f9e4393..b1f44a7f9712 100644 --- a/dom/canvas/ClientWebGLContext.h +++ b/dom/canvas/ClientWebGLContext.h @@ -1054,8 +1054,6 @@ class ClientWebGLContext final : public nsICanvasRenderingContextInternal, RefPtr BackBufferSnapshot(); [[nodiscard]] bool DoReadPixels(const webgl::ReadPixelsDesc&, Range) const; - [[nodiscard]] bool DoReadPixels(const webgl::ReadPixelsDesc&, - const mozilla::ipc::Shmem&) const; uvec2 DrawingBufferSize(); // - @@ -1557,7 +1555,7 @@ class ClientWebGLContext final : public nsICanvasRenderingContextInternal, // Primitive tex upload functions void RawTexImage(uint32_t level, GLenum respecFormat, uvec3 offset, const webgl::PackingInfo& pi, - webgl::TexUnpackBlobDesc&&) const; + const webgl::TexUnpackBlobDesc&) const; void TexImage(uint8_t funcDims, GLenum target, GLint level, GLenum respecFormat, const ivec3& offset, const Maybe& size, GLint border, diff --git a/dom/canvas/DrawTargetWebgl.cpp b/dom/canvas/DrawTargetWebgl.cpp index a8480492d833..81de2e2d721a 100644 --- a/dom/canvas/DrawTargetWebgl.cpp +++ b/dom/canvas/DrawTargetWebgl.cpp @@ -15,10 +15,8 @@ #include "mozilla/gfx/Logging.h" #include "mozilla/gfx/PathSkia.h" #include "mozilla/gfx/Swizzle.h" -#include "mozilla/layers/ImageDataSerializer.h" #include "ClientWebGLContext.h" -#include "WebGLChild.h" #include "gfxPlatform.h" @@ -216,12 +214,6 @@ void DrawTargetWebgl::ClearSnapshot(bool aCopyOnWrite, bool aNeedHandle) { DrawTargetWebgl::~DrawTargetWebgl() { ClearSnapshot(false); if (mSharedContext) { - if (mShmem.IsWritable()) { - auto* child = mSharedContext->mWebgl->GetChild(); - if (child && child->CanSend()) { - child->DeallocShmem(mShmem); - } - } if (mFramebuffer) { mSharedContext->mWebgl->DeleteFramebuffer(mFramebuffer); } @@ -357,23 +349,8 @@ bool DrawTargetWebgl::Init(const IntSize& size, const SurfaceFormat format) { return false; } - auto* child = mSharedContext->mWebgl->GetChild(); - if (child && child->CanSend()) { - size_t byteSize = layers::ImageDataSerializer::ComputeRGBBufferSize( - mSize, SurfaceFormat::B8G8R8A8); - if (byteSize) { - (void)child->AllocUnsafeShmem(byteSize, &mShmem); - } - } mSkia = new DrawTargetSkia; - if (mShmem.IsWritable()) { - auto stride = layers::ImageDataSerializer::ComputeRGBStride( - SurfaceFormat::B8G8R8A8, size.width); - if (!mSkia->Init(mShmem.get(), size, stride, - SurfaceFormat::B8G8R8A8, true)) { - return false; - } - } else if (!mSkia->Init(size, SurfaceFormat::B8G8R8A8)) { + if (!mSkia->Init(size, SurfaceFormat::B8G8R8A8)) { return false; } SetPermitSubpixelAA(IsOpaque(format)); @@ -692,15 +669,8 @@ bool DrawTargetWebgl::SharedContext::ReadInto(uint8_t* aDstData, desc.srcOffset = *ivec2::From(aBounds); desc.size = *uvec2::FromSize(aBounds); desc.packState.rowLength = aDstStride / 4; - - bool success = false; - if (mCurrentTarget->mShmem.IsWritable() && - aDstData == mCurrentTarget->mShmem.get()) { - success = mWebgl->DoReadPixels(desc, mCurrentTarget->mShmem); - } else { - Range range = {aDstData, size_t(aDstStride) * aBounds.height}; - success = mWebgl->DoReadPixels(desc, range); - } + Range range = {aDstData, size_t(aDstStride) * aBounds.height}; + bool success = mWebgl->DoReadPixels(desc, range); // Restore the actual framebuffer after reading is done. if (aHandle && mCurrentTarget) { @@ -1255,30 +1225,19 @@ bool DrawTargetWebgl::SharedContext::UploadSurface(DataSourceSurface* aData, } int32_t stride = map.GetStride(); int32_t bpp = BytesPerPixel(aFormat); - if (mCurrentTarget->mShmem.IsWritable() && - map.GetData() == mCurrentTarget->mShmem.get()) { - texDesc.sd = Some(layers::SurfaceDescriptorBuffer( - layers::RGBDescriptor(mCurrentTarget->mSize, SurfaceFormat::R8G8B8A8), - mCurrentTarget->mShmem)); - texDesc.structuredSrcSize = - uvec2::From(stride / bpp, mCurrentTarget->mSize.height); - texDesc.unpacking.skipPixels = aSrcRect.x; - texDesc.unpacking.skipRows = aSrcRect.y; - mWaitForShmem = true; - } else { - // Get the data pointer range considering the sampling rect offset and - // size. - Range range( - map.GetData() + aSrcRect.y * size_t(stride) + aSrcRect.x * bpp, - std::max(aSrcRect.height - 1, 0) * size_t(stride) + - aSrcRect.width * bpp); - texDesc.cpuData = Some(RawBuffer(range)); - } + // Get the data pointer range considering the sampling rect offset and + // size. + Range range( + map.GetData() + aSrcRect.y * size_t(stride) + aSrcRect.x * bpp, + std::max(aSrcRect.height - 1, 0) * size_t(stride) + + aSrcRect.width * bpp); + texDesc.cpuData = Some(RawBuffer(range)); // If the stride happens to be 4 byte aligned, assume that is the // desired alignment regardless of format (even A8). Otherwise, we // default to byte alignment. texDesc.unpacking.alignmentInTypeElems = stride % 4 ? 1 : 4; texDesc.unpacking.rowLength = stride / bpp; + texDesc.unpacking.imageHeight = aSrcRect.height; } else if (aZero) { // Create a PBO filled with zero data to initialize the texture data and // avoid slow initialization inside WebGL. @@ -1310,7 +1269,7 @@ bool DrawTargetWebgl::SharedContext::UploadSurface(DataSourceSurface* aData, // Do the (partial) upload for the shared or standalone texture. mWebgl->RawTexImage(0, aInit ? intFormat : 0, {uint32_t(aDstOffset.x), uint32_t(aDstOffset.y), 0}, - texPI, std::move(texDesc)); + texPI, texDesc); if (!aData && aZero) { mWebgl->BindBuffer(LOCAL_GL_PIXEL_UNPACK_BUFFER, 0); } @@ -2755,19 +2714,8 @@ void DrawTargetWebgl::FillGlyphs(ScaledFont* aFont, const GlyphBuffer& aBuffer, mSkia->FillGlyphs(aFont, aBuffer, aPattern, aOptions); } -void DrawTargetWebgl::SharedContext::WaitForShmem() { - if (mWaitForShmem) { - // GetError is a sync IPDL call that forces all dispatched commands to be - // flushed. Once it returns, we are certain that any commands processing - // the Shmem have finished. - (void)mWebgl->GetError(); - mWaitForShmem = false; - } -} - void DrawTargetWebgl::MarkSkiaChanged(const DrawOptions& aOptions) { if (SupportsLayering(aOptions)) { - WaitForShmem(); if (!mSkiaValid) { // If the Skia context needs initialization, clear it and enable layering. mSkiaValid = true; @@ -2949,9 +2897,10 @@ Maybe DrawTargetWebgl::GetFrontBuffer() { // Copy and swizzle the WebGL framebuffer to the swap chain front buffer. webgl::SwapChainOptions options; options.bgra = true; - // Allow async present to be toggled on for accelerated Canvas2D independent - // of WebGL via pref. - options.forceAsyncPresent = StaticPrefs::gfx_canvas_accelerated_async_present(); + // Allow async present to be toggled on for accelerated Canvas2D + // independent of WebGL via pref. + options.forceAsyncPresent = + StaticPrefs::gfx_canvas_accelerated_async_present(); mSharedContext->mWebgl->CopyToSwapChain(mFramebuffer, options); } } diff --git a/dom/canvas/DrawTargetWebgl.h b/dom/canvas/DrawTargetWebgl.h index 116efe4462fe..caff81240965 100644 --- a/dom/canvas/DrawTargetWebgl.h +++ b/dom/canvas/DrawTargetWebgl.h @@ -11,7 +11,6 @@ #include "mozilla/LinkedList.h" #include "mozilla/WeakPtr.h" #include "mozilla/ThreadLocal.h" -#include "mozilla/ipc/Shmem.h" #include namespace mozilla { @@ -69,8 +68,6 @@ class DrawTargetWebgl : public DrawTarget, public SupportsWeakPtr { RefPtr mFramebuffer; RefPtr mTex; RefPtr mSkia; - // The Shmem backing the Skia DT, if applicable. - mozilla::ipc::Shmem mShmem; // The currently cached snapshot of the WebGL context RefPtr mSnapshot; // Whether or not the Skia target has valid contents and is being drawn to @@ -201,10 +198,6 @@ class DrawTargetWebgl : public DrawTarget, public SupportsWeakPtr { // A memory pressure event may signal from another thread that caches should // be cleared if possible. Atomic mShouldClearCaches; - // Whether the Shmem is currently being processed by the remote side. If so, - // we need to wait for processing to complete before any further commands - // modifying the Skia DT can proceed. - bool mWaitForShmem = false; const Matrix& GetTransform() const { return mCurrentTarget->mTransform; } @@ -279,8 +272,6 @@ class DrawTargetWebgl : public DrawTarget, public SupportsWeakPtr { void ClearAllTextures(); void ClearEmptyTextureMemory(); void ClearCachesIfNecessary(); - - void WaitForShmem(); }; RefPtr mSharedContext; @@ -441,14 +432,7 @@ class DrawTargetWebgl : public DrawTarget, public SupportsWeakPtr { void FlattenSkia(); bool FlushFromSkia(); - void WaitForShmem() { - if (mSharedContext->mWaitForShmem) { - mSharedContext->WaitForShmem(); - } - } - void MarkSkiaChanged() { - WaitForShmem(); if (!mSkiaValid) { ReadIntoSkia(); } else if (mSkiaLayer) { diff --git a/dom/canvas/PWebGL.ipdl b/dom/canvas/PWebGL.ipdl index ad0ab139e519..1075e3bd2617 100644 --- a/dom/canvas/PWebGL.ipdl +++ b/dom/canvas/PWebGL.ipdl @@ -31,13 +31,6 @@ using mozilla::webgl::ShaderPrecisionFormat from "mozilla/dom/WebGLIpdl.h"; namespace mozilla { namespace dom { -union ReadPixelsBuffer { - // The buffer needs to be allocated, and the value specifies the maximum size. - uint64_t; - // The buffer is a pre-allocated Shmem. - Shmem; -}; - /** * Represents the connection between a WebGLChild actor that issues WebGL * command from the content process, and a WebGLParent in the compositor @@ -64,7 +57,7 @@ parent: sync GetBufferSubData(uint32_t target, uint64_t srcByteOffset, uint64_t byteSize) returns (Shmem ret); sync GetFrontBufferSnapshot() returns (FrontBufferSnapshotIpc ret); - sync ReadPixels(ReadPixelsDesc desc, ReadPixelsBuffer buffer) returns (ReadPixelsResultIpc ret); + sync ReadPixels(ReadPixelsDesc desc, uint64_t maxBytes) returns (ReadPixelsResultIpc ret); // - diff --git a/dom/canvas/TexUnpackBlob.cpp b/dom/canvas/TexUnpackBlob.cpp index 73746cca5e64..082c7121b12a 100644 --- a/dom/canvas/TexUnpackBlob.cpp +++ b/dom/canvas/TexUnpackBlob.cpp @@ -10,7 +10,6 @@ #include "mozilla/dom/Element.h" #include "mozilla/dom/HTMLCanvasElement.h" #include "mozilla/gfx/Logging.h" -#include "mozilla/layers/ImageDataSerializer.h" #include "mozilla/RefPtr.h" #include "nsLayoutUtils.h" #include "WebGLBuffer.h" @@ -291,14 +290,6 @@ static bool ValidateUnpackBytes(const WebGLContext* const webgl, //////////////////// -// Check if the surface descriptor describes a memory which contains a single -// RGBA data source. -static bool SDIsRGBBuffer(const layers::SurfaceDescriptor& sd) { - return sd.type() == layers::SurfaceDescriptor::TSurfaceDescriptorBuffer && - sd.get_SurfaceDescriptorBuffer().desc().type() == - layers::BufferDescriptor::TRGBDescriptor; -} - // static std::unique_ptr TexUnpackBlob::Create( const TexUnpackBlobDesc& desc) { @@ -320,11 +311,6 @@ std::unique_ptr TexUnpackBlob::Create( } if (desc.sd) { - // Shmem buffers need to be treated as if they were a DataSourceSurface. - // Otherwise, TexUnpackImage will try to blit the surface descriptor as - // if it can be mapped as a framebuffer, whereas the Shmem is still CPU - // data. - if (SDIsRGBBuffer(*desc.sd)) return new TexUnpackSurface(desc); return new TexUnpackImage(desc); } if (desc.dataSurf) { @@ -870,41 +856,21 @@ bool TexUnpackSurface::TexOrSubImage(bool isSubImage, bool needsRespec, GLenum* const out_error) const { const auto& webgl = tex->mContext; const auto& size = mDesc.size; - RefPtr surf; - if (mDesc.sd) { - // If we get here, we assume the SD describes an RGBA Shmem. - const auto& sd = *(mDesc.sd); - MOZ_ASSERT(SDIsRGBBuffer(sd)); - const auto& sdb = sd.get_SurfaceDescriptorBuffer(); - const auto& rgb = sdb.desc().get_RGBDescriptor(); - const auto& data = sdb.data(); - MOZ_ASSERT(data.type() == layers::MemoryOrShmem::TShmem); - const auto& shmem = data.get_Shmem(); - surf = gfx::Factory::CreateWrappingDataSourceSurface( - shmem.get(), layers::ImageDataSerializer::GetRGBStride(rgb), - rgb.size(), rgb.format()); - if (!surf) { - gfxCriticalError() << "TexUnpackSurface failed to create wrapping " - "DataSourceSurface for Shmem."; - return false; - } - } else { - surf = mDesc.dataSurf; - } + auto& surf = *(mDesc.dataSurf); //// WebGLTexelFormat srcFormat; uint8_t srcBPP; - if (!GetFormatForSurf(surf, &srcFormat, &srcBPP)) { + if (!GetFormatForSurf(&surf, &srcFormat, &srcBPP)) { webgl->ErrorImplementationBug( "GetFormatForSurf failed for" " WebGLTexelFormat::%u.", - uint32_t(surf->GetFormat())); + uint32_t(surf.GetFormat())); return false; } - gfx::DataSourceSurface::ScopedMap map(surf, + gfx::DataSourceSurface::ScopedMap map(&surf, gfx::DataSourceSurface::MapType::READ); if (!map.IsMapped()) { webgl->ErrorOutOfMemory("Failed to map source surface for upload."); @@ -918,7 +884,7 @@ bool TexUnpackSurface::TexOrSubImage(bool isSubImage, bool needsRespec, const auto dstFormat = FormatForPackingInfo(dstPI); const auto dstBpp = BytesPerPixel(dstPI); - const size_t dstUsedBytesPerRow = dstBpp * surf->GetSize().width; + const size_t dstUsedBytesPerRow = dstBpp * surf.GetSize().width; auto dstStride = dstUsedBytesPerRow; if (dstFormat == srcFormat) { dstStride = srcStride; // Try to match. @@ -951,7 +917,7 @@ bool TexUnpackSurface::TexOrSubImage(bool isSubImage, bool needsRespec, const uint8_t* dstBegin = srcBegin; UniqueBuffer tempBuffer; // clang-format off - if (!ConvertIfNeeded(webgl, surf->GetSize().width, surf->GetSize().height, + if (!ConvertIfNeeded(webgl, surf.GetSize().width, surf.GetSize().height, srcFormat, srcBegin, AutoAssertCast(srcStride), dstFormat, AutoAssertCast(dstUnpacking.metrics.bytesPerRowStride), &dstBegin, &tempBuffer)) { diff --git a/dom/canvas/WebGLParent.cpp b/dom/canvas/WebGLParent.cpp index edc2c813d2ec..c07f16f25d2f 100644 --- a/dom/canvas/WebGLParent.cpp +++ b/dom/canvas/WebGLParent.cpp @@ -166,7 +166,7 @@ IPCResult WebGLParent::RecvGetBufferSubData(const GLenum target, } IPCResult WebGLParent::RecvReadPixels(const webgl::ReadPixelsDesc& desc, - ReadPixelsBuffer&& buffer, + const uint64_t byteSize, webgl::ReadPixelsResultIpc* const ret) { AUTO_PROFILER_LABEL("WebGLParent::RecvReadPixels", GRAPHICS); *ret = {}; @@ -174,15 +174,6 @@ IPCResult WebGLParent::RecvReadPixels(const webgl::ReadPixelsDesc& desc, return IPC_FAIL(this, "HostWebGLContext is not initialized."); } - if (buffer.type() == ReadPixelsBuffer::TShmem) { - const auto& shmem = buffer.get_Shmem(); - const auto range = shmem.Range(); - const auto res = mHost->ReadPixelsInto(desc, range); - *ret = {res, {}}; - return IPC_OK(); - } - - const uint64_t byteSize = buffer.get_uint64_t(); const auto allocSize = std::max(1, byteSize); auto shmem = webgl::RaiiShmem::Alloc(this, allocSize); if (!shmem) { diff --git a/dom/canvas/WebGLParent.h b/dom/canvas/WebGLParent.h index 1a2d44ca1528..f86bee510b60 100644 --- a/dom/canvas/WebGLParent.h +++ b/dom/canvas/WebGLParent.h @@ -45,8 +45,7 @@ class WebGLParent : public PWebGLParent, public SupportsWeakPtr { IPCResult GetFrontBufferSnapshot(webgl::FrontBufferSnapshotIpc* ret, IProtocol* aProtocol); IPCResult RecvGetFrontBufferSnapshot(webgl::FrontBufferSnapshotIpc* ret); - IPCResult RecvReadPixels(const webgl::ReadPixelsDesc&, - ReadPixelsBuffer&& buffer, + IPCResult RecvReadPixels(const webgl::ReadPixelsDesc&, uint64_t byteSize, webgl::ReadPixelsResultIpc* ret); // -