diff --git a/expectations/gm/ignored-tests.txt b/expectations/gm/ignored-tests.txt index 67c734921e..1499accc4a 100644 --- a/expectations/gm/ignored-tests.txt +++ b/expectations/gm/ignored-tests.txt @@ -43,6 +43,3 @@ dropshadowimagefilter # senorblanco https://codereview.chromium.org/637283009/ # quality improvements to imagemagnifier GM imagemagnifier - -#joshualitt single pixel mismatch in msaa16 -gradients_view_perspective diff --git a/include/gpu/gl/GrGLConfig.h b/include/gpu/gl/GrGLConfig.h index 4b3ab8b258..444be00f15 100644 --- a/include/gpu/gl/GrGLConfig.h +++ b/include/gpu/gl/GrGLConfig.h @@ -46,6 +46,12 @@ * GR_GL_CHECK_ERROR_START: controls the initial value of gCheckErrorGL * when GR_GL_CHECK_ERROR is 1. Defaults to 1. * + * GR_GL_NO_CONSTANT_ATTRIBUTES: if this evaluates to true then the GL backend + * will use uniforms instead of attributes in all cases when there is not + * per-vertex data. This is important when the underlying GL implementation + * doesn't actually support immediate style attribute values (e.g. when + * the GL stream is converted to DX as in ANGLE on Chrome). Defaults to 0. + * * GR_GL_USE_BUFFER_DATA_NULL_HINT: 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: @@ -120,6 +126,10 @@ #define GR_GL_CHECK_ERROR_START 1 #endif +#if !defined(GR_GL_NO_CONSTANT_ATTRIBUTES) + #define GR_GL_NO_CONSTANT_ATTRIBUTES 0 +#endif + #if !defined(GR_GL_USE_BUFFER_DATA_NULL_HINT) #define GR_GL_USE_BUFFER_DATA_NULL_HINT 1 #endif diff --git a/include/gpu/gl/GrGLConfig_chrome.h b/include/gpu/gl/GrGLConfig_chrome.h index ee875b7f22..acad90450c 100644 --- a/include/gpu/gl/GrGLConfig_chrome.h +++ b/include/gpu/gl/GrGLConfig_chrome.h @@ -12,12 +12,16 @@ #define GR_GL_CHECK_ERROR_START 0 #if defined(SK_BUILD_FOR_WIN32) +// ANGLE creates a temp VB for vertex attributes not specified per-vertex. +#define GR_GL_NO_CONSTANT_ATTRIBUTES 1 + // For RGBA teximage/readpixels ANGLE will sw-convert to/from BGRA. #define GR_GL_RGBA_8888_PIXEL_OPS_SLOW 1 // ANGLE can go faster if the entire fbo is read rather than a subrect #define GR_GL_FULL_READPIXELS_FASTER_THAN_PARTIAL 1 #else +#define GR_GL_NO_CONSTANT_ATTRIBUTES 0 #define GR_GL_RGBA_8888_PIXEL_OPS_SLOW 0 #define GR_GL_FULL_READPIXELS_FASTER_THAN_PARTIAL 0 #endif diff --git a/src/gpu/effects/GrYUVtoRGBEffect.cpp b/src/gpu/effects/GrYUVtoRGBEffect.cpp index 703c672cfd..f02c1b2295 100644 --- a/src/gpu/effects/GrYUVtoRGBEffect.cpp +++ b/src/gpu/effects/GrYUVtoRGBEffect.cpp @@ -109,7 +109,7 @@ private: virtual void onComputeInvariantOutput(InvariantOutput* inout) const SK_OVERRIDE { // YUV is opaque inout->setToOther(kA_GrColorComponentFlag, 0xFF << GrColor_SHIFT_A, - InvariantOutput::kWillNot_ReadInput); + InvariantOutput::kWill_ReadInput); } GrCoordTransform fCoordTransform; diff --git a/src/gpu/gl/GrGLProgram.cpp b/src/gpu/gl/GrGLProgram.cpp index bfa5f3cbc1..2d9b569333 100644 --- a/src/gpu/gl/GrGLProgram.cpp +++ b/src/gpu/gl/GrGLProgram.cpp @@ -129,12 +129,13 @@ void GrGLProgram::bindTextures(const GrGLInstalledProc* ip, const GrProcessor& p void GrGLProgram::setData(const GrOptDrawState& optState, GrGpu::DrawType drawType, - const GrDeviceCoordTexture* dstCopy) { + const GrDeviceCoordTexture* dstCopy, + SharedGLState* sharedState) { GrColor color = optState.getColor(); GrColor coverage = optState.getCoverageColor(); - this->setColor(optState, color); - this->setCoverage(optState, coverage); + this->setColor(optState, color, sharedState); + this->setCoverage(optState, coverage, sharedState); this->setMatrixAndRenderTargetHeight(drawType, optState); if (dstCopy) { @@ -200,49 +201,80 @@ void GrGLProgram::didSetData(GrGpu::DrawType drawType) { SkASSERT(!GrGpu::IsPathRenderingDrawType(drawType)); } -void GrGLProgram::setColor(const GrOptDrawState& optState, GrColor color) { +void GrGLProgram::setColor(const GrOptDrawState& optState, + GrColor color, + SharedGLState* sharedState) { const GrGLProgramDesc::KeyHeader& header = fDesc.getHeader(); - switch (header.fColorInput) { - case GrGLProgramDesc::kAttribute_ColorInput: - // Attribute case is handled in GrGpuGL::setupGeometry - break; - case GrGLProgramDesc::kUniform_ColorInput: - if (fColor != color && fBuiltinUniformHandles.fColorUni.isValid()) { - // OpenGL ES doesn't support unsigned byte varieties of glUniform - GrGLfloat c[4]; - GrColorToRGBAFloat(color, c); - fProgramDataManager.set4fv(fBuiltinUniformHandles.fColorUni, 1, c); - fColor = color; - } - break; - case GrGLProgramDesc::kAllOnes_ColorInput: - // Handled by shader creation - break; - default: - SkFAIL("Unexpected color type."); + if (!optState.hasColorVertexAttribute()) { + switch (header.fColorInput) { + case GrGLProgramDesc::kAttribute_ColorInput: + SkASSERT(-1 != header.fColorAttributeIndex); + if (sharedState->fConstAttribColor != color || + sharedState->fConstAttribColorIndex != header.fColorAttributeIndex) { + // OpenGL ES only supports the float varieties of glVertexAttrib + GrGLfloat c[4]; + GrColorToRGBAFloat(color, c); + GL_CALL(VertexAttrib4fv(header.fColorAttributeIndex, c)); + sharedState->fConstAttribColor = color; + sharedState->fConstAttribColorIndex = header.fColorAttributeIndex; + } + break; + case GrGLProgramDesc::kUniform_ColorInput: + if (fColor != color && fBuiltinUniformHandles.fColorUni.isValid()) { + // OpenGL ES doesn't support unsigned byte varieties of glUniform + GrGLfloat c[4]; + GrColorToRGBAFloat(color, c); + fProgramDataManager.set4fv(fBuiltinUniformHandles.fColorUni, 1, c); + fColor = color; + } + sharedState->fConstAttribColorIndex = -1; + break; + case GrGLProgramDesc::kAllOnes_ColorInput: + sharedState->fConstAttribColorIndex = -1; + break; + default: + SkFAIL("Unexpected color type."); + } + } else { + sharedState->fConstAttribColorIndex = -1; } } -void GrGLProgram::setCoverage(const GrOptDrawState& optState, GrColor coverage) { +void GrGLProgram::setCoverage(const GrOptDrawState& optState, + GrColor coverage, + SharedGLState* sharedState) { const GrGLProgramDesc::KeyHeader& header = fDesc.getHeader(); - switch (header.fCoverageInput) { - case GrGLProgramDesc::kAttribute_ColorInput: - // Attribute case is handled in GrGpuGL::setupGeometry - break; - case GrGLProgramDesc::kUniform_ColorInput: - if (fCoverage != coverage) { - // OpenGL ES doesn't support unsigned byte varieties of glUniform - GrGLfloat c[4]; - GrColorToRGBAFloat(coverage, c); - fProgramDataManager.set4fv(fBuiltinUniformHandles.fCoverageUni, 1, c); - fCoverage = coverage; - } - break; - case GrGLProgramDesc::kAllOnes_ColorInput: - // Handled by shader creation - break; - default: - SkFAIL("Unexpected coverage type."); + if (!optState.hasCoverageVertexAttribute()) { + switch (header.fCoverageInput) { + case GrGLProgramDesc::kAttribute_ColorInput: + if (sharedState->fConstAttribCoverage != coverage || + sharedState->fConstAttribCoverageIndex != header.fCoverageAttributeIndex) { + // OpenGL ES only supports the float varieties of glVertexAttrib + GrGLfloat c[4]; + GrColorToRGBAFloat(coverage, c); + GL_CALL(VertexAttrib4fv(header.fCoverageAttributeIndex, c)); + sharedState->fConstAttribCoverage = coverage; + sharedState->fConstAttribCoverageIndex = header.fCoverageAttributeIndex; + } + break; + case GrGLProgramDesc::kUniform_ColorInput: + if (fCoverage != coverage) { + // OpenGL ES doesn't support unsigned byte varieties of glUniform + GrGLfloat c[4]; + GrColorToRGBAFloat(coverage, c); + fProgramDataManager.set4fv(fBuiltinUniformHandles.fCoverageUni, 1, c); + fCoverage = coverage; + } + sharedState->fConstAttribCoverageIndex = -1; + break; + case GrGLProgramDesc::kAllOnes_ColorInput: + sharedState->fConstAttribCoverageIndex = -1; + break; + default: + SkFAIL("Unexpected coverage type."); + } + } else { + sharedState->fConstAttribCoverageIndex = -1; } } diff --git a/src/gpu/gl/GrGLProgram.h b/src/gpu/gl/GrGLProgram.h index ca75e206f1..e8aef35f1d 100644 --- a/src/gpu/gl/GrGLProgram.h +++ b/src/gpu/gl/GrGLProgram.h @@ -59,6 +59,27 @@ public: */ virtual bool hasVertexShader() const { return true; } + /** + * Some GL state that is relevant to programs is not stored per-program. In particular color + * and coverage attributes can be global state. This struct is read and updated by + * GrGLProgram::setColor and GrGLProgram::setCoverage to allow us to avoid setting this state + * redundantly. + */ + struct SharedGLState { + GrColor fConstAttribColor; + int fConstAttribColorIndex; + GrColor fConstAttribCoverage; + int fConstAttribCoverageIndex; + + SharedGLState() { this->invalidate(); } + void invalidate() { + fConstAttribColor = GrColor_ILLEGAL; + fConstAttribColorIndex = -1; + fConstAttribCoverage = GrColor_ILLEGAL; + fConstAttribCoverageIndex = -1; + } + }; + /** * The GrDrawState's view matrix along with the aspects of the render target determine the * matrix sent to GL. The size of the render target affects the GL matrix because we must @@ -131,7 +152,8 @@ public: */ void setData(const GrOptDrawState&, GrGpu::DrawType, - const GrDeviceCoordTexture* dstCopy /* can be NULL*/); + const GrDeviceCoordTexture* dstCopy, // can be NULL + SharedGLState*); protected: typedef GrGLProgramDataManager::UniformHandle UniformHandle; @@ -151,11 +173,11 @@ protected: // Helper for setData(). Makes GL calls to specify the initial color when there is not // per-vertex colors. - void setColor(const GrOptDrawState&, GrColor color); + void setColor(const GrOptDrawState&, GrColor color, SharedGLState*); // Helper for setData(). Makes GL calls to specify the initial coverage when there is not // per-vertex coverages. - void setCoverage(const GrOptDrawState&, GrColor coverage); + void setCoverage(const GrOptDrawState&, GrColor coverage, SharedGLState*); // A templated helper to loop over effects, set the transforms(via subclass) and bind textures void setFragmentData(const GrOptDrawState&); diff --git a/src/gpu/gl/GrGLProgramDesc.cpp b/src/gpu/gl/GrGLProgramDesc.cpp index 4a274c6bb1..79088137ca 100644 --- a/src/gpu/gl/GrGLProgramDesc.cpp +++ b/src/gpu/gl/GrGLProgramDesc.cpp @@ -257,27 +257,25 @@ bool GrGLProgramDesc::Build(const GrOptDrawState& optState, header->fEmitsPointSize = GrGpu::kDrawPoints_DrawType == drawType; - bool isPathRendering = GrGpu::IsPathRenderingDrawType(drawType); - if (gpu->caps()->pathRenderingSupport() && isPathRendering) { - header->fUseNvpr = true; + if (gpu->caps()->pathRenderingSupport() && + GrGpu::IsPathRenderingDrawType(drawType) && + gpu->glPathRendering()->texturingMode() == GrGLPathRendering::FixedFunction_TexturingMode) { + header->fUseFragShaderOnly = true; SkASSERT(!optState.hasGeometryProcessor()); } else { - header->fUseNvpr = false; + header->fUseFragShaderOnly = false; } - bool hasUniformColor = inputColorIsUsed && - (isPathRendering || !optState.hasColorVertexAttribute()); - - bool hasUniformCoverage = inputCoverageIsUsed && - (isPathRendering || !optState.hasCoverageVertexAttribute()); + bool defaultToUniformInputs = GrGpu::IsPathRenderingDrawType(drawType) || + GR_GL_NO_CONSTANT_ATTRIBUTES; if (!inputColorIsUsed) { header->fColorInput = kAllOnes_ColorInput; - } else if (hasUniformColor) { + } else if (defaultToUniformInputs && !optState.hasColorVertexAttribute()) { header->fColorInput = kUniform_ColorInput; } else { header->fColorInput = kAttribute_ColorInput; - SkASSERT(!header->fUseNvpr); + SkASSERT(!header->fUseFragShaderOnly); } bool covIsSolidWhite = !optState.hasCoverageVertexAttribute() && @@ -285,11 +283,11 @@ bool GrGLProgramDesc::Build(const GrOptDrawState& optState, if (covIsSolidWhite || !inputCoverageIsUsed) { header->fCoverageInput = kAllOnes_ColorInput; - } else if (hasUniformCoverage) { + } else if (defaultToUniformInputs && !optState.hasCoverageVertexAttribute()) { header->fCoverageInput = kUniform_ColorInput; } else { header->fCoverageInput = kAttribute_ColorInput; - SkASSERT(!header->fUseNvpr); + SkASSERT(!header->fUseFragShaderOnly); } if (optState.readsDst()) { diff --git a/src/gpu/gl/GrGLProgramDesc.h b/src/gpu/gl/GrGLProgramDesc.h index d97bdfded5..4e1be5b2f9 100644 --- a/src/gpu/gl/GrGLProgramDesc.h +++ b/src/gpu/gl/GrGLProgramDesc.h @@ -94,7 +94,7 @@ private: // effects that read the fragment position. // Otherwise, 0. - SkBool8 fUseNvpr; + SkBool8 fUseFragShaderOnly; SkBool8 fEmitsPointSize; ColorInput fColorInput : 8; diff --git a/src/gpu/gl/GrGpuGL.cpp b/src/gpu/gl/GrGpuGL.cpp index a85548ea64..7b892913c5 100644 --- a/src/gpu/gl/GrGpuGL.cpp +++ b/src/gpu/gl/GrGpuGL.cpp @@ -342,6 +342,7 @@ void GrGpuGL::onResetContext(uint32_t resetBits) { if (resetBits & kProgram_GrGLBackendState) { fHWProgramID = 0; + fSharedGLProgramState.invalidate(); } } diff --git a/src/gpu/gl/GrGpuGL.h b/src/gpu/gl/GrGpuGL.h index dc0d076a2d..973a568981 100644 --- a/src/gpu/gl/GrGpuGL.h +++ b/src/gpu/gl/GrGpuGL.h @@ -288,6 +288,8 @@ private: int fHWActiveTextureUnitIdx; GrGLuint fHWProgramID; + GrGLProgram::SharedGLState fSharedGLProgramState; + enum TriState { kNo_TriState, kYes_TriState, diff --git a/src/gpu/gl/GrGpuGL_program.cpp b/src/gpu/gl/GrGpuGL_program.cpp index a1259c9bf8..bf73f0059c 100644 --- a/src/gpu/gl/GrGpuGL_program.cpp +++ b/src/gpu/gl/GrGpuGL_program.cpp @@ -256,7 +256,7 @@ bool GrGpuGL::flushGraphicsState(DrawType type, this->flushBlend(*optState.get(), kDrawLines_DrawType == type, srcCoeff, dstCoeff); - fCurrentProgram->setData(*optState.get(), type, dstCopy); + fCurrentProgram->setData(*optState.get(), type, dstCopy, &fSharedGLProgramState); } GrGLRenderTarget* glRT = static_cast(optState->getRenderTarget()); diff --git a/src/gpu/gl/builders/GrGLProgramBuilder.cpp b/src/gpu/gl/builders/GrGLProgramBuilder.cpp index 65a7cdaa63..28d1517423 100644 --- a/src/gpu/gl/builders/GrGLProgramBuilder.cpp +++ b/src/gpu/gl/builders/GrGLProgramBuilder.cpp @@ -55,9 +55,7 @@ GrGLProgram* GrGLProgramBuilder::CreateProgram(const GrOptDrawState& optState, // if we have a vertex shader(we don't only if we are using NVPR or NVPR ES), then we may have // to setup a few more things like builtin vertex attributes - bool hasVertexShader = !(header.fUseNvpr && - gpu->glPathRendering()->texturingMode() == - GrGLPathRendering::FixedFunction_TexturingMode); + bool hasVertexShader = !header.fUseFragShaderOnly; if (hasVertexShader) { pb->fVS.setupLocalCoords(); pb->fVS.transformGLToSkiaCoords(); @@ -94,15 +92,18 @@ GrGLProgramBuilder::CreateProgramBuilder(const GrGLProgramDesc& desc, GrGpu::DrawType drawType, bool hasGeometryProcessor, GrGpuGL* gpu) { - if (desc.getHeader().fUseNvpr) { + if (desc.getHeader().fUseFragShaderOnly) { SkASSERT(gpu->glCaps().pathRenderingSupport()); + SkASSERT(gpu->glPathRendering()->texturingMode() == + GrGLPathRendering::FixedFunction_TexturingMode); SkASSERT(!hasGeometryProcessor); - if (gpu->glPathRendering()->texturingMode() == - GrGLPathRendering::FixedFunction_TexturingMode) { - return SkNEW_ARGS(GrGLLegacyNvprProgramBuilder, (gpu, optState, desc)); - } else { - return SkNEW_ARGS(GrGLNvprProgramBuilder, (gpu, optState, desc)); - } + return SkNEW_ARGS(GrGLLegacyNvprProgramBuilder, (gpu, optState, desc)); + } else if (GrGpu::IsPathRenderingDrawType(drawType)) { + SkASSERT(gpu->glCaps().pathRenderingSupport()); + SkASSERT(gpu->glPathRendering()->texturingMode() == + GrGLPathRendering::SeparableShaders_TexturingMode); + SkASSERT(!hasGeometryProcessor); + return SkNEW_ARGS(GrGLNvprProgramBuilder, (gpu, optState, desc)); } else { return SkNEW_ARGS(GrGLProgramBuilder, (gpu, optState, desc)); } @@ -419,9 +420,7 @@ GrGLProgram* GrGLProgramBuilder::finalize() { this->cleanupProgram(programID, shadersToDelete); return NULL; } - if (!(this->header().fUseNvpr && - fGpu->glPathRendering()->texturingMode() == - GrGLPathRendering::FixedFunction_TexturingMode)) { + if (!this->header().fUseFragShaderOnly) { if (!fVS.compileAndAttachShaders(programID, &shadersToDelete)) { this->cleanupProgram(programID, shadersToDelete); return NULL;