From 3036904b537e48dbe04b2e4fef5d957536294ad1 Mon Sep 17 00:00:00 2001 From: James Ruan Date: Thu, 28 Mar 2024 16:25:36 +0800 Subject: [PATCH 1/5] allow `SkImage::asyncRescaleAndReadPixels()` to work async on WebGL Reading back pixels is blocking on WebGL. This is because skia lacks pixel transfer support with out mapping. WebGL has PBO for async reading, however it use `glGetBufferSubData()` to read back from PIXEL_PACK_BUFFER instead of mapping - add GetBufferSubData to GrGLInterface - add GrGpuBuffer::getData() - TAsyncReadResult use getData() to copy pixels to cpu plane --- include/gpu/gl/GrGLFunctions.h | 1 + include/gpu/gl/GrGLInterface.h | 1 + src/gpu/AsyncReadTypes.h | 15 +++++++++++++-- src/gpu/ganesh/GrGpuBuffer.cpp | 4 ++++ src/gpu/ganesh/GrGpuBuffer.h | 10 ++++++++++ .../ganesh/gl/GrGLAssembleGLInterfaceAutogen.cpp | 1 + .../gl/GrGLAssembleWebGLInterfaceAutogen.cpp | 1 + src/gpu/ganesh/gl/GrGLBuffer.cpp | 12 ++++++++++++ src/gpu/ganesh/gl/GrGLBuffer.h | 1 + src/gpu/ganesh/gl/GrGLCaps.cpp | 10 ++++++++++ src/gpu/ganesh/gl/GrGLCaps.h | 3 +++ 11 files changed, 57 insertions(+), 2 deletions(-) diff --git a/include/gpu/gl/GrGLFunctions.h b/include/gpu/gl/GrGLFunctions.h index b0673e13b19a..f50d8ed8ee5a 100644 --- a/include/gpu/gl/GrGLFunctions.h +++ b/include/gpu/gl/GrGLFunctions.h @@ -95,6 +95,7 @@ using GrGLGenSamplersFn = GrGLvoid GR_GL_FUNCTION_TYPE(GrGLsizei count, GrGLuint using GrGLGenTexturesFn = GrGLvoid GR_GL_FUNCTION_TYPE(GrGLsizei n, GrGLuint* textures); using GrGLGenVertexArraysFn = GrGLvoid GR_GL_FUNCTION_TYPE(GrGLsizei n, GrGLuint* arrays); using GrGLGetBufferParameterivFn = GrGLvoid GR_GL_FUNCTION_TYPE(GrGLenum target, GrGLenum pname, GrGLint* params); +using GrGLGetBufferSubDataFn = GrGLvoid GR_GL_FUNCTION_TYPE(GrGLenum target, GrGLintptr offset, GrGLsizeiptr size, GrGLvoid* data); using GrGLGetErrorFn = GrGLenum GR_GL_FUNCTION_TYPE(); using GrGLGetFramebufferAttachmentParameterivFn = GrGLvoid GR_GL_FUNCTION_TYPE(GrGLenum target, GrGLenum attachment, GrGLenum pname, GrGLint* params); using GrGLGetFloatvFn = GrGLvoid GR_GL_FUNCTION_TYPE(GrGLenum pname, GrGLfloat* params); diff --git a/include/gpu/gl/GrGLInterface.h b/include/gpu/gl/GrGLInterface.h index 862178ec55c0..9e5ab9e219ac 100644 --- a/include/gpu/gl/GrGLInterface.h +++ b/include/gpu/gl/GrGLInterface.h @@ -165,6 +165,7 @@ struct SK_API GrGLInterface : public SkRefCnt { GrGLFunction fGenTextures; GrGLFunction fGenVertexArrays; GrGLFunction fGetBufferParameteriv; + GrGLFunction fGetBufferSubData; GrGLFunction fGetError; GrGLFunction fGetFramebufferAttachmentParameteriv; GrGLFunction fGetFloatv; diff --git a/src/gpu/AsyncReadTypes.h b/src/gpu/AsyncReadTypes.h index 52d7961b6d4f..f04c7a958190 100644 --- a/src/gpu/AsyncReadTypes.h +++ b/src/gpu/AsyncReadTypes.h @@ -153,11 +153,22 @@ class TAsyncReadResult : public SkImage::AsyncReadResult { size_t rowBytes, TClientMappedBufferManager* manager) { const void* mappedData = result.fTransferBuffer->map(); + size_t size = rowBytes*dimensions.height(); if (!mappedData) { - return false; + sk_sp data = SkData::MakeUninitialized(size); + if (!result.fTransferBuffer->getData(data->writable_data(), 0, size)) { + return false; + } + if (result.fPixelConverter) { + sk_sp out = SkData::MakeUninitialized(size); + result.fPixelConverter(out->writable_data(), data->data()); + this->addCpuPlane(std::move(out), rowBytes); + } else { + this->addCpuPlane(std::move(data), rowBytes); + } + return true; } if (result.fPixelConverter) { - size_t size = rowBytes*dimensions.height(); sk_sp data = SkData::MakeUninitialized(size); result.fPixelConverter(data->writable_data(), mappedData); this->addCpuPlane(std::move(data), rowBytes); diff --git a/src/gpu/ganesh/GrGpuBuffer.cpp b/src/gpu/ganesh/GrGpuBuffer.cpp index 68c0d583bcac..3afb93c55ed2 100644 --- a/src/gpu/ganesh/GrGpuBuffer.cpp +++ b/src/gpu/ganesh/GrGpuBuffer.cpp @@ -83,6 +83,10 @@ bool GrGpuBuffer::updateData(const void* src, size_t offset, size_t size, bool p return this->onUpdateData(src, offset, size, preserve); } +bool GrGpuBuffer::getData(void* dst, size_t offset, size_t size) { + return this->onGetData(dst, offset, size); +} + void GrGpuBuffer::ComputeScratchKeyForDynamicBuffer(size_t size, GrGpuBufferType intendedType, skgpu::ScratchKey* key) { diff --git a/src/gpu/ganesh/GrGpuBuffer.h b/src/gpu/ganesh/GrGpuBuffer.h index 200d78b5ccbb..e7a092cb7d18 100644 --- a/src/gpu/ganesh/GrGpuBuffer.h +++ b/src/gpu/ganesh/GrGpuBuffer.h @@ -96,6 +96,15 @@ class GrGpuBuffer : public GrGpuResource, public GrBuffer { */ bool updateData(const void* src, size_t offset, size_t size, bool preserve); + /** + * Get the buffer data. + * + * WebGL's buffer can't be mapped to client side. Use this function to get the buffer data. + * + * @return returns true if the update succeeds, false otherwise. + */ + bool getData(void* dst, size_t offset, size_t size); + GrGpuBufferType intendedType() const { return fIntendedType; } protected: @@ -129,6 +138,7 @@ class GrGpuBuffer : public GrGpuResource, public GrBuffer { virtual void onUnmap(MapType) = 0; virtual bool onClearToZero() = 0; virtual bool onUpdateData(const void* src, size_t offset, size_t size, bool preserve) = 0; + virtual bool onGetData(void* dst, size_t offset, size_t size) { return false; } size_t onGpuMemorySize() const override { return fSizeInBytes; } void onSetLabel() override{} diff --git a/src/gpu/ganesh/gl/GrGLAssembleGLInterfaceAutogen.cpp b/src/gpu/ganesh/gl/GrGLAssembleGLInterfaceAutogen.cpp index bbc15630d119..43bd9d1e0e83 100644 --- a/src/gpu/ganesh/gl/GrGLAssembleGLInterfaceAutogen.cpp +++ b/src/gpu/ganesh/gl/GrGLAssembleGLInterfaceAutogen.cpp @@ -93,6 +93,7 @@ sk_sp GrGLMakeAssembledGLInterface(void *ctx, GrGLGetProc g GET_PROC(GenBuffers); GET_PROC(GenTextures); GET_PROC(GetBufferParameteriv); + GET_PROC(GetBufferSubData); GET_PROC(GetError); GET_PROC(GetFloatv); GET_PROC(GetIntegerv); diff --git a/src/gpu/ganesh/gl/GrGLAssembleWebGLInterfaceAutogen.cpp b/src/gpu/ganesh/gl/GrGLAssembleWebGLInterfaceAutogen.cpp index 62f8da5f7f54..05ba590d2407 100644 --- a/src/gpu/ganesh/gl/GrGLAssembleWebGLInterfaceAutogen.cpp +++ b/src/gpu/ganesh/gl/GrGLAssembleWebGLInterfaceAutogen.cpp @@ -215,6 +215,7 @@ sk_sp GrGLMakeAssembledWebGLInterface(void *ctx, GrGLGetPro if (glVer >= GR_GL_VER(2,0)) { GET_PROC(CopyBufferSubData); + GET_PROC(GetBufferSubData); } if (glVer >= GR_GL_VER(2,0)) { diff --git a/src/gpu/ganesh/gl/GrGLBuffer.cpp b/src/gpu/ganesh/gl/GrGLBuffer.cpp index 39080ea9b8dc..94faedb9c194 100644 --- a/src/gpu/ganesh/gl/GrGLBuffer.cpp +++ b/src/gpu/ganesh/gl/GrGLBuffer.cpp @@ -275,6 +275,18 @@ bool GrGLBuffer::onUpdateData(const void* src, size_t offset, size_t size, bool return true; } +bool GrGLBuffer::onGetData(void* dst, size_t offset, size_t size) { + SkASSERT(fBufferID); + + // bindbuffer handles dirty context + GrGLenum target = this->glGpu()->bindBuffer(fIntendedType, this); + if (this->glCaps().getBufferSubDataSupport()) { + GL_CALL(GetBufferSubData(target, offset, size, dst)); + return true; + } + return false; +} + void GrGLBuffer::onSetLabel() { SkASSERT(fBufferID); if (!this->getLabel().empty()) { diff --git a/src/gpu/ganesh/gl/GrGLBuffer.h b/src/gpu/ganesh/gl/GrGLBuffer.h index d6ec2cb5df31..bcb61c2a6301 100644 --- a/src/gpu/ganesh/gl/GrGLBuffer.h +++ b/src/gpu/ganesh/gl/GrGLBuffer.h @@ -51,6 +51,7 @@ class GrGLBuffer : public GrGpuBuffer { void onUnmap(MapType) override; bool onClearToZero() override; bool onUpdateData(const void* src, size_t offset, size_t size, bool preserve) override; + bool onGetData(void* dst, size_t offset, size_t size) override; void onSetLabel() override; diff --git a/src/gpu/ganesh/gl/GrGLCaps.cpp b/src/gpu/ganesh/gl/GrGLCaps.cpp index 7bdbe4f72c47..ab75232cf5a1 100644 --- a/src/gpu/ganesh/gl/GrGLCaps.cpp +++ b/src/gpu/ganesh/gl/GrGLCaps.cpp @@ -100,6 +100,7 @@ GrGLCaps::GrGLCaps(const GrContextOptions& contextOptions, fFBFetchRequiresEnablePerSample = false; fSRGBWriteControl = false; fSkipErrorChecks = false; + fGetBufferSubDataSupport = false; fShaderCaps = std::make_unique(); @@ -400,6 +401,10 @@ void GrGLCaps::init(const GrContextOptions& contextOptions, return SkToBool(contextFlags & GR_GL_CONTEXT_FLAG_PROTECTED_CONTENT_BIT_EXT); }(); + if (GR_IS_GR_GL(standard) || GR_IS_GR_WEBGL(standard)) { + fGetBufferSubDataSupport = version >= GR_GL_VER(2, 0); + } + /************************************************************************** * GrShaderCaps fields **************************************************************************/ @@ -557,6 +562,11 @@ void GrGLCaps::init(const GrContextOptions& contextOptions, } else if (GR_IS_GR_WEBGL(standard)) { // explicitly removed https://www.khronos.org/registry/webgl/specs/2.0/#5.14 fMapBufferFlags = kNone_MapFlags; + if (version >= GR_GL_VER(2, 0)) { + fTransferFromBufferToTextureSupport = true; + fTransferFromSurfaceToBufferSupport = true; + fTransferBufferType = TransferBufferType::kARB_PBO; + } } // Buffers have more restrictions in WebGL than GLES. For example, diff --git a/src/gpu/ganesh/gl/GrGLCaps.h b/src/gpu/ganesh/gl/GrGLCaps.h index 16227714260a..23dfb5e14394 100644 --- a/src/gpu/ganesh/gl/GrGLCaps.h +++ b/src/gpu/ganesh/gl/GrGLCaps.h @@ -515,6 +515,8 @@ class GrGLCaps : public GrCaps { bool clientCanDisableMultisample() const { return fClientCanDisableMultisample; } + bool getBufferSubDataSupport() const { return fGetBufferSubDataSupport; } + GrBackendFormat getBackendFormatFromCompressionType(SkTextureCompressionType) const override; skgpu::Swizzle getWriteSwizzle(const GrBackendFormat&, GrColorType) const override; @@ -630,6 +632,7 @@ class GrGLCaps : public GrCaps { bool fSRGBWriteControl : 1; bool fSkipErrorChecks : 1; bool fClientCanDisableMultisample : 1; + bool fGetBufferSubDataSupport: 1; // Driver workarounds bool fDoManualMipmapping : 1; From 68c9d5a3c6de734fb55cc59dba3e5d04f85f740e Mon Sep 17 00:00:00 2001 From: James Ruan Date: Fri, 29 Mar 2024 09:37:06 +0800 Subject: [PATCH 2/5] use autogen tool --- src/gpu/ganesh/gl/GrGLAssembleGLInterfaceAutogen.cpp | 3 ++- src/gpu/ganesh/gl/GrGLAssembleWebGLInterfaceAutogen.cpp | 5 ++++- src/gpu/ganesh/gl/GrGLInterfaceAutogen.cpp | 8 ++++++++ tools/gpu/gl/interface/interface.json5 | 9 +++++++++ 4 files changed, 23 insertions(+), 2 deletions(-) diff --git a/src/gpu/ganesh/gl/GrGLAssembleGLInterfaceAutogen.cpp b/src/gpu/ganesh/gl/GrGLAssembleGLInterfaceAutogen.cpp index 43bd9d1e0e83..41548e704852 100644 --- a/src/gpu/ganesh/gl/GrGLAssembleGLInterfaceAutogen.cpp +++ b/src/gpu/ganesh/gl/GrGLAssembleGLInterfaceAutogen.cpp @@ -93,7 +93,6 @@ sk_sp GrGLMakeAssembledGLInterface(void *ctx, GrGLGetProc g GET_PROC(GenBuffers); GET_PROC(GenTextures); GET_PROC(GetBufferParameteriv); - GET_PROC(GetBufferSubData); GET_PROC(GetError); GET_PROC(GetFloatv); GET_PROC(GetIntegerv); @@ -152,6 +151,8 @@ sk_sp GrGLMakeAssembledGLInterface(void *ctx, GrGLGetProc g GET_PROC(DrawBuffer); GET_PROC(PolygonMode); + GET_PROC(GetBufferSubData); + if (glVer >= GR_GL_VER(3,0)) { GET_PROC(GetStringi); } diff --git a/src/gpu/ganesh/gl/GrGLAssembleWebGLInterfaceAutogen.cpp b/src/gpu/ganesh/gl/GrGLAssembleWebGLInterfaceAutogen.cpp index 05ba590d2407..ed394e8b8e39 100644 --- a/src/gpu/ganesh/gl/GrGLAssembleWebGLInterfaceAutogen.cpp +++ b/src/gpu/ganesh/gl/GrGLAssembleWebGLInterfaceAutogen.cpp @@ -137,6 +137,10 @@ sk_sp GrGLMakeAssembledWebGLInterface(void *ctx, GrGLGetPro GET_PROC(VertexAttribPointer); GET_PROC(Viewport); + if (glVer >= GR_GL_VER(2,0)) { + GET_PROC(GetBufferSubData); + } + if (glVer >= GR_GL_VER(2,0)) { GET_PROC(GetStringi); } @@ -215,7 +219,6 @@ sk_sp GrGLMakeAssembledWebGLInterface(void *ctx, GrGLGetPro if (glVer >= GR_GL_VER(2,0)) { GET_PROC(CopyBufferSubData); - GET_PROC(GetBufferSubData); } if (glVer >= GR_GL_VER(2,0)) { diff --git a/src/gpu/ganesh/gl/GrGLInterfaceAutogen.cpp b/src/gpu/ganesh/gl/GrGLInterfaceAutogen.cpp index 5d8bd80f6024..4580c1a85855 100644 --- a/src/gpu/ganesh/gl/GrGLInterfaceAutogen.cpp +++ b/src/gpu/ganesh/gl/GrGLInterfaceAutogen.cpp @@ -188,6 +188,14 @@ bool GrGLInterface::validate() const { } } + if (GR_IS_GR_GL(fStandard) || + (GR_IS_GR_WEBGL(fStandard) && ( + (glVer >= GR_GL_VER(2,0))))) { + if (!fFunctions.fGetBufferSubData) { + RETURN_FALSE_INTERFACE; + } + } + if ((GR_IS_GR_GL(fStandard) && ( (glVer >= GR_GL_VER(3,0)))) || (GR_IS_GR_GL_ES(fStandard) && ( diff --git a/tools/gpu/gl/interface/interface.json5 b/tools/gpu/gl/interface/interface.json5 index 44e5163ceb73..04702dd6a009 100644 --- a/tools/gpu/gl/interface/interface.json5 +++ b/tools/gpu/gl/interface/interface.json5 @@ -55,6 +55,15 @@ "DrawBuffer", "PolygonMode", ], }, + { + "GL": [{"ext": ""}], + "GLES": null, + "WebGL": [{"min_version": [2, 0], "ext": ""}], + + "functions": [ + "GetBufferSubData", + ], + }, { "GL": [{"min_version": [3, 0], "ext": ""}], "GLES": [{"min_version": [3, 0], "ext": ""}], From 8289c8b7e92acf5166851c47553487f23c4ffbbb Mon Sep 17 00:00:00 2001 From: James Ruan Date: Fri, 29 Mar 2024 09:47:48 +0800 Subject: [PATCH 3/5] fix GrGLBuffer logic --- src/gpu/ganesh/GrGpuBuffer.cpp | 7 +++++++ src/gpu/ganesh/GrGpuBuffer.h | 4 +++- src/gpu/ganesh/gl/GrGLBuffer.cpp | 11 ++++++----- 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/src/gpu/ganesh/GrGpuBuffer.cpp b/src/gpu/ganesh/GrGpuBuffer.cpp index 3afb93c55ed2..e268feb78405 100644 --- a/src/gpu/ganesh/GrGpuBuffer.cpp +++ b/src/gpu/ganesh/GrGpuBuffer.cpp @@ -84,6 +84,13 @@ bool GrGpuBuffer::updateData(const void* src, size_t offset, size_t size, bool p } bool GrGpuBuffer::getData(void* dst, size_t offset, size_t size) { + const void* mapped = this->map(); + if (mapped != nullptr) { + memcpy(dst, (const char*)mapped + offset, size); + this->unmap(); + return true; + } + return this->onGetData(dst, offset, size); } diff --git a/src/gpu/ganesh/GrGpuBuffer.h b/src/gpu/ganesh/GrGpuBuffer.h index e7a092cb7d18..3077fd540470 100644 --- a/src/gpu/ganesh/GrGpuBuffer.h +++ b/src/gpu/ganesh/GrGpuBuffer.h @@ -101,7 +101,9 @@ class GrGpuBuffer : public GrGpuResource, public GrBuffer { * * WebGL's buffer can't be mapped to client side. Use this function to get the buffer data. * - * @return returns true if the update succeeds, false otherwise. + * The data is always copied to client side. Will try copy from mapped if supported. + * + * @return returns true if succeeds, false otherwise. */ bool getData(void* dst, size_t offset, size_t size); diff --git a/src/gpu/ganesh/gl/GrGLBuffer.cpp b/src/gpu/ganesh/gl/GrGLBuffer.cpp index 94faedb9c194..7c81bccc8c0c 100644 --- a/src/gpu/ganesh/gl/GrGLBuffer.cpp +++ b/src/gpu/ganesh/gl/GrGLBuffer.cpp @@ -278,13 +278,14 @@ bool GrGLBuffer::onUpdateData(const void* src, size_t offset, size_t size, bool bool GrGLBuffer::onGetData(void* dst, size_t offset, size_t size) { SkASSERT(fBufferID); + if (!this->glCaps().getBufferSubDataSupport()) { + return false; + } + // bindbuffer handles dirty context GrGLenum target = this->glGpu()->bindBuffer(fIntendedType, this); - if (this->glCaps().getBufferSubDataSupport()) { - GL_CALL(GetBufferSubData(target, offset, size, dst)); - return true; - } - return false; + GL_CALL(GetBufferSubData(target, offset, size, dst)); + return true; } void GrGLBuffer::onSetLabel() { From fd155849b4c0825991d677bacab86a78486a80ee Mon Sep 17 00:00:00 2001 From: James Ruan Date: Sun, 31 Mar 2024 11:27:43 +0800 Subject: [PATCH 4/5] add tests and fix caps detection --- src/gpu/AsyncReadTypes.h | 44 +++++++++++++++------------ src/gpu/ganesh/GrGpuBuffer.cpp | 8 +++++ src/gpu/ganesh/GrGpuBuffer.h | 5 ++++ src/gpu/ganesh/gl/GrGLCaps.cpp | 23 +++++++------- tests/TransferPixelsTest.cpp | 55 +++++++++++++++++----------------- 5 files changed, 79 insertions(+), 56 deletions(-) diff --git a/src/gpu/AsyncReadTypes.h b/src/gpu/AsyncReadTypes.h index f04c7a958190..14eccd505072 100644 --- a/src/gpu/AsyncReadTypes.h +++ b/src/gpu/AsyncReadTypes.h @@ -152,30 +152,36 @@ class TAsyncReadResult : public SkImage::AsyncReadResult { SkISize dimensions, size_t rowBytes, TClientMappedBufferManager* manager) { - const void* mappedData = result.fTransferBuffer->map(); size_t size = rowBytes*dimensions.height(); - if (!mappedData) { - sk_sp data = SkData::MakeUninitialized(size); - if (!result.fTransferBuffer->getData(data->writable_data(), 0, size)) { - return false; + auto doConvert = [converter{result.fPixelConverter}, size](const void* src) -> sk_sp { + if (converter) { + sk_sp data = SkData::MakeUninitialized(size); + converter(data->writable_data(), src); + return data; } - if (result.fPixelConverter) { - sk_sp out = SkData::MakeUninitialized(size); - result.fPixelConverter(out->writable_data(), data->data()); - this->addCpuPlane(std::move(out), rowBytes); + return nullptr; + }; + + const void* mappedData = result.fTransferBuffer->map(); + + if (mappedData) { + if (auto converted = doConvert(mappedData)) { + this->addCpuPlane(std::move(converted), rowBytes); + result.fTransferBuffer->unmap(); } else { - this->addCpuPlane(std::move(data), rowBytes); + manager->insert(result.fTransferBuffer); + this->addMappedPlane(mappedData, rowBytes, std::move(result.fTransferBuffer)); } - return true; - } - if (result.fPixelConverter) { - sk_sp data = SkData::MakeUninitialized(size); - result.fPixelConverter(data->writable_data(), mappedData); - this->addCpuPlane(std::move(data), rowBytes); - result.fTransferBuffer->unmap(); } else { - manager->insert(result.fTransferBuffer); - this->addMappedPlane(mappedData, rowBytes, std::move(result.fTransferBuffer)); + sk_sp tmp = SkData::MakeUninitialized(size); + if (!result.fTransferBuffer->getData(tmp->writable_data(), 0, size)) { + return false; + } + if (auto converted = doConvert(tmp->data())) { + this->addCpuPlane(std::move(converted), rowBytes); + } else { + this->addCpuPlane(std::move(tmp), rowBytes); + } } return true; } diff --git a/src/gpu/ganesh/GrGpuBuffer.cpp b/src/gpu/ganesh/GrGpuBuffer.cpp index e268feb78405..e178192d90ef 100644 --- a/src/gpu/ganesh/GrGpuBuffer.cpp +++ b/src/gpu/ganesh/GrGpuBuffer.cpp @@ -84,6 +84,14 @@ bool GrGpuBuffer::updateData(const void* src, size_t offset, size_t size, bool p } bool GrGpuBuffer::getData(void* dst, size_t offset, size_t size) { + SkASSERT(!this->isMapped()); + SkASSERT(size > 0 && offset + size <= fSizeInBytes); + SkASSERT(dst); + + if (this->wasDestroyed()) { + return false; + } + const void* mapped = this->map(); if (mapped != nullptr) { memcpy(dst, (const char*)mapped + offset, size); diff --git a/src/gpu/ganesh/GrGpuBuffer.h b/src/gpu/ganesh/GrGpuBuffer.h index 3077fd540470..867230f07339 100644 --- a/src/gpu/ganesh/GrGpuBuffer.h +++ b/src/gpu/ganesh/GrGpuBuffer.h @@ -103,6 +103,11 @@ class GrGpuBuffer : public GrGpuResource, public GrBuffer { * * The data is always copied to client side. Will try copy from mapped if supported. * + * The buffer must not be mapped. + * + * Note that buffer updates do not go through GrContext and therefore are not serialized with + * other operations. + * * @return returns true if succeeds, false otherwise. */ bool getData(void* dst, size_t offset, size_t size); diff --git a/src/gpu/ganesh/gl/GrGLCaps.cpp b/src/gpu/ganesh/gl/GrGLCaps.cpp index ab75232cf5a1..365bcd091d1a 100644 --- a/src/gpu/ganesh/gl/GrGLCaps.cpp +++ b/src/gpu/ganesh/gl/GrGLCaps.cpp @@ -562,18 +562,8 @@ void GrGLCaps::init(const GrContextOptions& contextOptions, } else if (GR_IS_GR_WEBGL(standard)) { // explicitly removed https://www.khronos.org/registry/webgl/specs/2.0/#5.14 fMapBufferFlags = kNone_MapFlags; - if (version >= GR_GL_VER(2, 0)) { - fTransferFromBufferToTextureSupport = true; - fTransferFromSurfaceToBufferSupport = true; - fTransferBufferType = TransferBufferType::kARB_PBO; - } } - // Buffers have more restrictions in WebGL than GLES. For example, - // https://www.khronos.org/registry/webgl/specs/latest/2.0/#BUFFER_OBJECT_BINDING - // We therefore haven't attempted to support mapping or transfers between buffers and surfaces - // or between buffers. - if (GR_IS_GR_GL(standard)) { if (version >= GR_GL_VER(2, 1) || ctxInfo.hasExtension("GL_ARB_pixel_buffer_object") || ctxInfo.hasExtension("GL_EXT_pixel_buffer_object")) { @@ -599,6 +589,12 @@ void GrGLCaps::init(const GrContextOptions& contextOptions, // fTransferFromSurfaceToBufferSupport = false; // fTransferBufferType = TransferBufferType::kChromium; } + } else if (GR_IS_GR_WEBGL(standard)) { + if (version >= GR_GL_VER(2, 0)) { + fTransferFromBufferToTextureSupport = true; + fTransferFromSurfaceToBufferSupport = true; + fTransferBufferType = TransferBufferType::kARB_PBO; + } } if (GR_IS_GR_GL(standard) && @@ -607,6 +603,13 @@ void GrGLCaps::init(const GrContextOptions& contextOptions, } else if (GR_IS_GR_GL_ES(standard) && (version >= GR_GL_VER(3, 0) || ctxInfo.hasExtension("GL_NV_copy_buffer"))) { fTransferFromBufferToBufferSupport = true; + } else if (GR_IS_GR_WEBGL(standard)) { + if (version >= GR_GL_VER(2, 0)) { + // WebGL has more restriction about buffer binding + // https://registry.khronos.org/webgl/specs/latest/2.0/#COPYING_BUFFERS + // TODO: make sure index buffer is handled properly to enable this + // fTransferFromBufferToBufferSupport = true; + } } // On many GPUs, map memory is very expensive, so we effectively disable it here by setting the diff --git a/tests/TransferPixelsTest.cpp b/tests/TransferPixelsTest.cpp index fead34d340ec..570c87e8e042 100644 --- a/tests/TransferPixelsTest.cpp +++ b/tests/TransferPixelsTest.cpp @@ -198,13 +198,25 @@ void basic_transfer_to_test(skiatest::Reporter* reporter, if (!buffer) { return; } - void* data = buffer->map(); - if (!buffer) { - ERRORF(reporter, "Could not map buffer"); - return; - } - memcpy(data, srcData.get(), size); - buffer->unmap(); + auto updateData = [buffer, caps, gpu, reporter] (const void* src, size_t size, bool preserve) { + if (GrCaps::kNone_MapFlags != caps->mapBufferFlags()) { + void *map = buffer->map(); + REPORTER_ASSERT(reporter, map); + if (!map) { + ERRORF(reporter, "Failed to map transfer buffer."); + return; + } + memcpy(map, src, size); + buffer->unmap(); + } else { + if (!buffer->updateData(src, 0, size, preserve)) { + ERRORF(reporter, "Could not updateData"); + } + gpu->submitToGpu(GrSyncCpu::kYes); + } + }; + + updateData(srcData.get(), size, false); ////////////////////////// // transfer full data @@ -278,9 +290,7 @@ void basic_transfer_to_test(skiatest::Reporter* reporter, // change color of subrectangle fill_transfer_data(left, top, width, height, srcRowBytes, allowedSrc.fColorType, srcData.get()); - data = buffer->map(); - memcpy(data, srcData.get(), size); - buffer->unmap(); + updateData(srcData.get(), size, true); result = gpu->transferPixelsTo(tex.get(), SkIRect::MakeXYWH(left, top, width, height), @@ -393,7 +403,7 @@ void basic_transfer_from_test(skiatest::Reporter* reporter, const sk_gpu_test::C sk_sp buffer = resourceProvider->createBuffer(bufferSize, GrGpuBufferType::kXferGpuToCpu, - kDynamic_GrAccessPattern, + kStream_GrAccessPattern, GrResourceProvider::ZeroInit::kNo); REPORTER_ASSERT(reporter, buffer); if (!buffer) { @@ -422,15 +432,12 @@ void basic_transfer_from_test(skiatest::Reporter* reporter, const sk_gpu_test::C } // Copy the transfer buffer contents to a temporary so we can manipulate it. - const auto* map = reinterpret_cast(buffer->map()); - REPORTER_ASSERT(reporter, map); - if (!map) { - ERRORF(reporter, "Failed to map transfer buffer."); - return; + size_t copy_size = kTexDims.fHeight * fullBufferRowBytes; + std::unique_ptr transferData(new char[copy_size]); + + if (!buffer->getData(transferData.get(), 0, copy_size)) { + ERRORF(reporter, "Could not getData"); } - std::unique_ptr transferData(new char[kTexDims.fHeight * fullBufferRowBytes]); - memcpy(transferData.get(), map, fullBufferRowBytes * kTexDims.fHeight); - buffer->unmap(); GrImageInfo transferInfo(allowedRead.fColorType, kUnpremul_SkAlphaType, nullptr, kTexDims); @@ -468,15 +475,9 @@ void basic_transfer_from_test(skiatest::Reporter* reporter, const sk_gpu_test::C gpu->submitToGpu(GrSyncCpu::kYes); } - map = reinterpret_cast(buffer->map()); - REPORTER_ASSERT(reporter, map); - if (!map) { - ERRORF(reporter, "Failed to map transfer buffer."); - return; + if (!buffer->getData(transferData.get(), partialReadOffset, partialBufferRowBytes * kTexDims.fHeight)) { + ERRORF(reporter, "Could not getData"); } - const char* bufferStart = reinterpret_cast(map) + partialReadOffset; - memcpy(transferData.get(), bufferStart, partialBufferRowBytes * kTexDims.fHeight); - buffer->unmap(); transferInfo = transferInfo.makeWH(kPartialWidth, kPartialHeight); const char* textureDataStart = From d537ec67e7f06d37582d4f05cacaa54dc8de5a02 Mon Sep 17 00:00:00 2001 From: James Ruan Date: Thu, 4 Apr 2024 00:47:05 +0800 Subject: [PATCH 5/5] fix --- src/gpu/ganesh/gl/GrGLCaps.cpp | 4 +++- tests/TransferPixelsTest.cpp | 5 ++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/gpu/ganesh/gl/GrGLCaps.cpp b/src/gpu/ganesh/gl/GrGLCaps.cpp index 365bcd091d1a..8362ea23df11 100644 --- a/src/gpu/ganesh/gl/GrGLCaps.cpp +++ b/src/gpu/ganesh/gl/GrGLCaps.cpp @@ -401,7 +401,9 @@ void GrGLCaps::init(const GrContextOptions& contextOptions, return SkToBool(contextFlags & GR_GL_CONTEXT_FLAG_PROTECTED_CONTENT_BIT_EXT); }(); - if (GR_IS_GR_GL(standard) || GR_IS_GR_WEBGL(standard)) { + if (GR_IS_GR_GL(standard)) { + fGetBufferSubDataSupport = true; + } else if (GR_IS_GR_WEBGL(standard)) { fGetBufferSubDataSupport = version >= GR_GL_VER(2, 0); } diff --git a/tests/TransferPixelsTest.cpp b/tests/TransferPixelsTest.cpp index 570c87e8e042..b14763310b1c 100644 --- a/tests/TransferPixelsTest.cpp +++ b/tests/TransferPixelsTest.cpp @@ -475,9 +475,8 @@ void basic_transfer_from_test(skiatest::Reporter* reporter, const sk_gpu_test::C gpu->submitToGpu(GrSyncCpu::kYes); } - if (!buffer->getData(transferData.get(), partialReadOffset, partialBufferRowBytes * kTexDims.fHeight)) { - ERRORF(reporter, "Could not getData"); - } + if (!buffer->getData(transferData.get(), partialReadOffset, partialBufferRowBytes * + kTexDims.fHeight)) { ERRORF(reporter, "Could not getData"); } transferInfo = transferInfo.makeWH(kPartialWidth, kPartialHeight); const char* textureDataStart =