Remove GrGLGpu/GrGLCaps readPixelsSupported.
This code checks the external format/type that will be used with
glReadPixels. It tests for the values inherently allowed by the GL
and for implementation values.
It would need to be refactored to use formats and color types rather
than GrPixelConfig.
However, the code that calls GrGpu::readPixels and
GrGpu::transferPixelsFrom already tests
GrCaps::supportedReadPixelsColorType. The GrGLCaps override only allows
format/type combinations allowed by ES without implementation queries,
which is less permissive than the deleted code.
We may get more permissive again but will probably follow a different
pattern and do this after the format/colortype tables are built out in
GrGLCaps.
Bug: skia:6718
Change-Id: I36d9ab496dfe71045dcbce84200a9de3e93440bf
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/226840
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
diff --git a/src/gpu/gl/GrGLCaps.cpp b/src/gpu/gl/GrGLCaps.cpp
index 979d544..802586e 100644
--- a/src/gpu/gl/GrGLCaps.cpp
+++ b/src/gpu/gl/GrGLCaps.cpp
@@ -969,79 +969,6 @@
return true;
}
-bool GrGLCaps::readPixelsSupported(GrPixelConfig surfaceConfig,
- GrPixelConfig readConfig,
- std::function<void (GrGLenum, GrGLint*)> getIntegerv,
- std::function<bool ()> bindRenderTarget,
- std::function<void ()> unbindRenderTarget) const {
- // If it's not possible to even have a color attachment of surfaceConfig then read pixels is
- // not supported regardless of readConfig.
- if (!this->canConfigBeFBOColorAttachment(surfaceConfig)) {
- return false;
- }
-
- GrGLenum readFormat;
- GrGLenum readType;
- if (!this->getReadPixelsFormat(surfaceConfig, readConfig, &readFormat, &readType)) {
- return false;
- }
-
- if (GR_IS_GR_GL(fStandard)) {
- // Some OpenGL implementations allow GL_ALPHA as a format to glReadPixels. However,
- // the manual (https://www.opengl.org/sdk/docs/man/) says only these formats are allowed:
- // GL_STENCIL_INDEX, GL_DEPTH_COMPONENT, GL_DEPTH_STENCIL, GL_RED, GL_GREEN, GL_BLUE,
- // GL_RGB, GL_BGR, GL_RGBA, and GL_BGRA. We check for the subset that we would use.
- // The manual does not seem to fully match the spec as the spec allows integer formats
- // when the bound color buffer is an integer buffer. It doesn't specify which integer
- // formats are allowed, so perhaps all of them are. We only use GL_RGBA_INTEGER currently.
- if (readFormat != GR_GL_RED && readFormat != GR_GL_RG && readFormat != GR_GL_RGB &&
- readFormat != GR_GL_RGBA && readFormat != GR_GL_BGRA &&
- readFormat != GR_GL_RGBA_INTEGER) {
- return false;
- }
- // There is also a set of allowed types, but all the types we use are in the set:
- // GL_UNSIGNED_BYTE, GL_BYTE, GL_UNSIGNED_SHORT, GL_SHORT, GL_UNSIGNED_INT, GL_INT,
- // GL_HALF_FLOAT, GL_FLOAT, GL_UNSIGNED_BYTE_3_3_2, GL_UNSIGNED_BYTE_2_3_3_REV,
- // GL_UNSIGNED_SHORT_5_6_5, GL_UNSIGNED_SHORT_5_6_5_REV, GL_UNSIGNED_SHORT_4_4_4_4,
- // GL_UNSIGNED_SHORT_4_4_4_4_REV, GL_UNSIGNED_SHORT_5_5_5_1, GL_UNSIGNED_SHORT_1_5_5_5_REV,
- // GL_UNSIGNED_INT_8_8_8_8, GL_UNSIGNED_INT_8_8_8_8_REV,GL_UNSIGNED_INT_10_10_10_2,
- // GL_UNSIGNED_INT_2_10_10_10_REV, GL_UNSIGNED_INT_24_8, GL_UNSIGNED_INT_10F_11F_11F_REV,
- // GL_UNSIGNED_INT_5_9_9_9_REV, or GL_FLOAT_32_UNSIGNED_INT_24_8_REV.
- return true;
- }
-
- // See Section 16.1.2 in the ES 3.2 specification.
- switch (fConfigTable[surfaceConfig].fFormatType) {
- case kNormalizedFixedPoint_FormatType:
- if (GR_GL_RGBA == readFormat && GR_GL_UNSIGNED_BYTE == readType) {
- return true;
- }
- break;
- case kFloat_FormatType:
- if (GR_GL_RGBA == readFormat && GR_GL_FLOAT == readType) {
- return true;
- }
- break;
- }
-
- if (0 == fConfigTable[surfaceConfig].fSecondReadPixelsFormat.fFormat) {
- ReadPixelsFormat* rpFormat =
- const_cast<ReadPixelsFormat*>(&fConfigTable[surfaceConfig].fSecondReadPixelsFormat);
- GrGLint format = 0, type = 0;
- if (!bindRenderTarget()) {
- return false;
- }
- getIntegerv(GR_GL_IMPLEMENTATION_COLOR_READ_FORMAT, &format);
- getIntegerv(GR_GL_IMPLEMENTATION_COLOR_READ_TYPE, &type);
- rpFormat->fFormat = format;
- rpFormat->fType = type;
- unbindRenderTarget();
- }
-
- return fConfigTable[surfaceConfig].fSecondReadPixelsFormat.fFormat == readFormat &&
- fConfigTable[surfaceConfig].fSecondReadPixelsFormat.fType == readType;
-}
-
void GrGLCaps::initFSAASupport(const GrContextOptions& contextOptions, const GrGLContextInfo& ctxInfo,
const GrGLInterface* gli) {
// We need dual source blending and the ability to disable multisample in order to support mixed
diff --git a/src/gpu/gl/GrGLCaps.h b/src/gpu/gl/GrGLCaps.h
index d2b08c5..839a4fa 100644
--- a/src/gpu/gl/GrGLCaps.h
+++ b/src/gpu/gl/GrGLCaps.h
@@ -337,13 +337,6 @@
SupportedRead supportedReadPixelsColorType(GrPixelConfig, const GrBackendFormat&,
GrColorType) const override;
- /// Does ReadPixels support reading readConfig pixels from a FBO that is surfaceConfig?
- bool readPixelsSupported(GrPixelConfig surfaceConfig,
- GrPixelConfig readConfig,
- std::function<void (GrGLenum, GrGLint*)> getIntegerv,
- std::function<bool ()> bindRenderTarget,
- std::function<void ()> unbindRenderTarget) const;
-
bool isCoreProfile() const { return fIsCoreProfile; }
bool bindFragDataLocationSupport() const { return fBindFragDataLocationSupport; }
diff --git a/src/gpu/gl/GrGLGpu.cpp b/src/gpu/gl/GrGLGpu.cpp
index 6a82183..fa92dff 100644
--- a/src/gpu/gl/GrGLGpu.cpp
+++ b/src/gpu/gl/GrGLGpu.cpp
@@ -2203,74 +2203,6 @@
fHWStencilSettings.invalidate();
}
-bool GrGLGpu::readPixelsSupported(GrRenderTarget* target, GrPixelConfig readConfig) {
-#ifdef SK_BUILD_FOR_MAC
- // Chromium may ask us to read back from locked IOSurfaces. Calling the command buffer's
- // glGetIntegerv() with GL_IMPLEMENTATION_COLOR_READ_FORMAT/_TYPE causes the command buffer
- // to make a call to check the framebuffer status which can hang the driver. So in Mac Chromium
- // we always use a temporary surface to test for read pixels support.
- // https://www.crbug.com/662802
- if (this->glContext().driver() == kChromium_GrGLDriver) {
- return this->readPixelsSupported(target->config(), readConfig);
- }
-#endif
- auto bindRenderTarget = [this, target]() -> bool {
- this->flushRenderTargetNoColorWrites(static_cast<GrGLRenderTarget*>(target));
- return true;
- };
- auto unbindRenderTarget = []{};
- auto getIntegerv = [this](GrGLenum query, GrGLint* value) {
- GR_GL_GetIntegerv(this->glInterface(), query, value);
- };
- GrPixelConfig rtConfig = target->config();
- return this->glCaps().readPixelsSupported(rtConfig, readConfig, getIntegerv, bindRenderTarget,
- unbindRenderTarget);
-}
-
-bool GrGLGpu::readPixelsSupported(GrPixelConfig rtConfig, GrPixelConfig readConfig) {
- sk_sp<GrTexture> temp;
- auto bindRenderTarget = [this, rtConfig, &temp]() -> bool {
- GrSurfaceDesc desc;
- desc.fConfig = rtConfig;
- desc.fWidth = desc.fHeight = 16;
- if (this->glCaps().isConfigRenderable(rtConfig)) {
- desc.fFlags = kRenderTarget_GrSurfaceFlag;
- temp = this->createTexture(desc, SkBudgeted::kNo);
- if (!temp) {
- return false;
- }
- GrGLRenderTarget* glrt = static_cast<GrGLRenderTarget*>(temp->asRenderTarget());
- this->flushRenderTargetNoColorWrites(glrt);
- return true;
- } else if (this->glCaps().canConfigBeFBOColorAttachment(rtConfig)) {
- temp = this->createTexture(desc, SkBudgeted::kNo);
- if (!temp) {
- return false;
- }
- this->bindSurfaceFBOForPixelOps(temp.get(), GR_GL_FRAMEBUFFER, kDst_TempFBOTarget);
- fHWBoundRenderTargetUniqueID.makeInvalid();
- return true;
- }
- return false;
- };
- auto unbindRenderTarget = [this, &temp]() {
- this->unbindTextureFBOForPixelOps(GR_GL_FRAMEBUFFER, temp.get());
- };
- auto getIntegerv = [this](GrGLenum query, GrGLint* value) {
- GR_GL_GetIntegerv(this->glInterface(), query, value);
- };
- return this->glCaps().readPixelsSupported(rtConfig, readConfig, getIntegerv, bindRenderTarget,
- unbindRenderTarget);
-}
-
-bool GrGLGpu::readPixelsSupported(GrSurface* surfaceForConfig, GrPixelConfig readConfig) {
- if (GrRenderTarget* rt = surfaceForConfig->asRenderTarget()) {
- return this->readPixelsSupported(rt, readConfig);
- } else {
- GrPixelConfig config = surfaceForConfig->config();
- return this->readPixelsSupported(config, readConfig);
- }
-}
bool GrGLGpu::readOrTransferPixelsFrom(GrSurface* surface, int left, int top, int width, int height,
GrColorType dstColorType, void* offsetOrPtr,
@@ -2285,10 +2217,6 @@
// TODO: Avoid this conversion by making GrGLCaps work with color types.
auto dstAsConfig = GrColorTypeToPixelConfig(dstColorType);
- if (!this->readPixelsSupported(surface, dstAsConfig)) {
- return false;
- }
-
GrGLenum externalFormat;
GrGLenum externalType;
if (!this->glCaps().getReadPixelsFormat(surface->config(), dstAsConfig, &externalFormat,
diff --git a/src/gpu/gl/GrGLGpu.h b/src/gpu/gl/GrGLGpu.h
index 027d571..50da366 100644
--- a/src/gpu/gl/GrGLGpu.h
+++ b/src/gpu/gl/GrGLGpu.h
@@ -229,20 +229,6 @@
GrGLTextureParameters::SamplerOverriddenState* initialState,
const void* data);
- // Checks whether glReadPixels can be called to get pixel values in readConfig from the
- // render target.
- bool readPixelsSupported(GrRenderTarget* target, GrPixelConfig readConfig);
-
- // Checks whether glReadPixels can be called to get pixel values in readConfig from a
- // render target that has renderTargetConfig. This may have to create a temporary
- // render target and thus is less preferable than the variant that takes a render target.
- bool readPixelsSupported(GrPixelConfig renderTargetConfig, GrPixelConfig readConfig);
-
- // Checks whether glReadPixels can be called to get pixel values in readConfig from a
- // render target that has the same config as surfaceForConfig. Calls one of the the two
- // variations above, depending on whether the surface is a render target or not.
- bool readPixelsSupported(GrSurface* surfaceForConfig, GrPixelConfig readConfig);
-
bool onReadPixels(GrSurface*, int left, int top, int width, int height, GrColorType,
void* buffer, size_t rowBytes) override;
