From f2ec024c445ed17e80a5d351c797841f93ce8a7e Mon Sep 17 00:00:00 2001 From: Robert Phillips Date: Thu, 1 Mar 2018 16:51:25 -0500 Subject: [PATCH] Make use of the buffer data null hint a GrContextOption TBR=bsalomon@google.com Change-Id: I5a3fd18479ca8c95e1bc8c087c28346264049eb0 Reviewed-on: https://skia-review.googlesource.com/111604 Commit-Queue: Robert Phillips Reviewed-by: Greg Daniel --- include/gpu/GrContextOptions.h | 13 +++++++++++ src/gpu/GrCaps.cpp | 1 + src/gpu/gl/GrGLBuffer.cpp | 42 +++++++++++++++++----------------- src/gpu/gl/GrGLCaps.cpp | 8 +++++++ src/gpu/gl/GrGLCaps.h | 3 +++ 5 files changed, 46 insertions(+), 21 deletions(-) diff --git a/include/gpu/GrContextOptions.h b/include/gpu/GrContextOptions.h index 74ec7feb1f..69ea181bbb 100644 --- a/include/gpu/GrContextOptions.h +++ b/include/gpu/GrContextOptions.h @@ -131,6 +131,19 @@ struct GrContextOptions { */ bool fAvoidStencilBuffers = false; + /** + * When specifing new data for a vertex/index buffer that replaces old data Ganesh can give + * a hint to the driver that the previous data will not be used in future draws like this: + * glBufferData(GL_..._BUFFER, size, NULL, usage); //<--hint, NULL means + * glBufferSubData(GL_..._BUFFER, 0, lessThanSize, data) // old data can't be + * // used again. + * However, this can be an unoptimization on some platforms, esp. Chrome. + * Chrome's cmd buffer will create a new allocation and memset the whole thing + * to zero (for security reasons). + * Defaults to the value of GR_GL_USE_BUFFER_DATA_NULL_HINT #define (which is, by default, 1). + */ + Enable fUseGLBufferDataNullHint = Enable::kDefault; + /** * If true, texture fetches from mip-mapped textures will be biased to read larger MIP levels. * This has the effect of sharpening those textures, at the cost of some aliasing, and possible diff --git a/src/gpu/GrCaps.cpp b/src/gpu/GrCaps.cpp index 8cfbc5faea..e8e098ae4e 100644 --- a/src/gpu/GrCaps.cpp +++ b/src/gpu/GrCaps.cpp @@ -172,6 +172,7 @@ void GrCaps::dumpJSON(SkJSONWriter* writer) const { writer->appendBool("Blacklist Coverage Counting Path Renderer [workaround]", fBlacklistCoverageCounting); writer->appendBool("Prefer VRAM Use over flushes [workaround]", fPreferVRAMUseOverFlushes); + writer->appendBool("Avoid stencil buffers [workaround]", fAvoidStencilBuffers); if (this->advancedBlendEquationSupport()) { writer->appendHexU32("Advanced Blend Equation Blacklist", fAdvBlendEqBlacklist); diff --git a/src/gpu/gl/GrGLBuffer.cpp b/src/gpu/gl/GrGLBuffer.cpp index 180dc39958..074dc662bf 100644 --- a/src/gpu/gl/GrGLBuffer.cpp +++ b/src/gpu/gl/GrGLBuffer.cpp @@ -175,7 +175,7 @@ void GrGLBuffer::onMap() { case GrGLCaps::kMapBuffer_MapBufferType: { GrGLenum target = this->glGpu()->bindBuffer(fIntendedType, this); // Let driver know it can discard the old data - if (GR_GL_USE_BUFFER_DATA_NULL_HINT || fGLSizeInBytes != this->sizeInBytes()) { + if (this->glCaps().useBufferDataNullHint() || fGLSizeInBytes != this->sizeInBytes()) { GL_CALL(BufferData(target, this->sizeInBytes(), nullptr, fUsage)); } GL_CALL_RET(fMapPtr, MapBuffer(target, readOnly ? GR_GL_READ_ONLY : GR_GL_WRITE_ONLY)); @@ -257,28 +257,28 @@ bool GrGLBuffer::onUpdateData(const void* src, size_t srcSizeInBytes) { // bindbuffer handles dirty context GrGLenum target = this->glGpu()->bindBuffer(fIntendedType, this); -#if GR_GL_USE_BUFFER_DATA_NULL_HINT - if (this->sizeInBytes() == srcSizeInBytes) { - GL_CALL(BufferData(target, (GrGLsizeiptr) srcSizeInBytes, src, fUsage)); + if (this->glCaps().useBufferDataNullHint()) { + if (this->sizeInBytes() == srcSizeInBytes) { + GL_CALL(BufferData(target, (GrGLsizeiptr) srcSizeInBytes, src, fUsage)); + } else { + // Before we call glBufferSubData we give the driver a hint using + // glBufferData with nullptr. This makes the old buffer contents + // inaccessible to future draws. The GPU may still be processing + // draws that reference the old contents. With this hint it can + // assign a different allocation for the new contents to avoid + // flushing the gpu past draws consuming the old contents. + // TODO I think we actually want to try calling bufferData here + GL_CALL(BufferData(target, this->sizeInBytes(), nullptr, fUsage)); + GL_CALL(BufferSubData(target, 0, (GrGLsizeiptr) srcSizeInBytes, src)); + } + fGLSizeInBytes = this->sizeInBytes(); } else { - // Before we call glBufferSubData we give the driver a hint using - // glBufferData with nullptr. This makes the old buffer contents - // inaccessible to future draws. The GPU may still be processing - // draws that reference the old contents. With this hint it can - // assign a different allocation for the new contents to avoid - // flushing the gpu past draws consuming the old contents. - // TODO I think we actually want to try calling bufferData here - GL_CALL(BufferData(target, this->sizeInBytes(), nullptr, fUsage)); - GL_CALL(BufferSubData(target, 0, (GrGLsizeiptr) srcSizeInBytes, src)); + // Note that we're cheating on the size here. Currently no methods + // allow a partial update that preserves contents of non-updated + // portions of the buffer (map() does a glBufferData(..size, nullptr..)) + GL_CALL(BufferData(target, srcSizeInBytes, src, fUsage)); + fGLSizeInBytes = srcSizeInBytes; } - fGLSizeInBytes = this->sizeInBytes(); -#else - // Note that we're cheating on the size here. Currently no methods - // allow a partial update that preserves contents of non-updated - // portions of the buffer (map() does a glBufferData(..size, nullptr..)) - GL_CALL(BufferData(target, srcSizeInBytes, src, fUsage)); - fGLSizeInBytes = srcSizeInBytes; -#endif VALIDATE(); return true; } diff --git a/src/gpu/gl/GrGLCaps.cpp b/src/gpu/gl/GrGLCaps.cpp index 9068525148..e70cb9d25b 100644 --- a/src/gpu/gl/GrGLCaps.cpp +++ b/src/gpu/gl/GrGLCaps.cpp @@ -48,6 +48,7 @@ GrGLCaps::GrGLCaps(const GrContextOptions& contextOptions, fPartialFBOReadIsSlow = false; fMipMapLevelAndLodControlSupport = false; fRGBAToBGRAReadbackConversionsAreSlow = false; + fUseBufferDataNullHint = SkToBool(GR_GL_USE_BUFFER_DATA_NULL_HINT); fDoManualMipmapping = false; fSRGBDecodeDisableAffectsMipmaps = false; fClearToBoundaryValuesIsBroken = false; @@ -260,6 +261,12 @@ void GrGLCaps::init(const GrContextOptions& contextOptions, // vis-versa. fRGBAToBGRAReadbackConversionsAreSlow = isMESA || isMAC; + if (GrContextOptions::Enable::kNo == contextOptions.fUseGLBufferDataNullHint) { + fUseBufferDataNullHint = false; + } else if (GrContextOptions::Enable::kYes == contextOptions.fUseGLBufferDataNullHint) { + fUseBufferDataNullHint = true; + } + if (kGL_GrGLStandard == standard) { if (version >= GR_GL_VER(4,4) || ctxInfo.hasExtension("GL_ARB_clear_texture")) { fClearTextureSupport = true; @@ -1145,6 +1152,7 @@ void GrGLCaps::onDumpJSON(SkJSONWriter* writer) const { writer->appendBool("Texture swizzle support", fTextureSwizzleSupport); writer->appendBool("BGRA to RGBA readback conversions are slow", fRGBAToBGRAReadbackConversionsAreSlow); + writer->appendBool("Use buffer data null hint", fUseBufferDataNullHint); writer->appendBool("Draw To clear color", fUseDrawToClearColor); writer->appendBool("Draw To clear stencil clip", fUseDrawToClearStencilClip); writer->appendBool("Intermediate texture for partial updates of unorm textures ever bound to FBOs", diff --git a/src/gpu/gl/GrGLCaps.h b/src/gpu/gl/GrGLCaps.h index 770ebca946..c4a10b3cd9 100644 --- a/src/gpu/gl/GrGLCaps.h +++ b/src/gpu/gl/GrGLCaps.h @@ -351,6 +351,8 @@ public: return fRGBAToBGRAReadbackConversionsAreSlow; } + bool useBufferDataNullHint() const { return fUseBufferDataNullHint; } + // Certain Intel GPUs on Mac fail to clear if the glClearColor is made up of only 1s and 0s. bool clearToBoundaryValuesIsBroken() const { return fClearToBoundaryValuesIsBroken; } @@ -480,6 +482,7 @@ private: bool fTextureSwizzleSupport : 1; bool fMipMapLevelAndLodControlSupport : 1; bool fRGBAToBGRAReadbackConversionsAreSlow : 1; + bool fUseBufferDataNullHint : 1; bool fClearTextureSupport : 1; bool fProgramBinarySupport : 1;