From ee92063f9e1a28e301fe6cdc804dca0ccc837f67 Mon Sep 17 00:00:00 2001 From: dvonbeck Date: Thu, 11 Aug 2016 14:17:59 -0700 Subject: [PATCH] LightingShader and NormalSource comment and style fixes BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2237963002 Review-Url: https://codereview.chromium.org/2237963002 --- src/core/SkLightingShader.h | 5 +---- src/core/SkNormalBevelSource.cpp | 8 +++++--- src/core/SkNormalFlatSource.cpp | 8 +++----- src/core/SkNormalMapSource.cpp | 16 ++++++++-------- src/core/SkNormalSource.h | 8 +++++--- src/gpu/glsl/GrGLSLProgramBuilder.cpp | 2 +- 6 files changed, 23 insertions(+), 24 deletions(-) diff --git a/src/core/SkLightingShader.h b/src/core/SkLightingShader.h index 41e3ca2394..aa90710aa4 100644 --- a/src/core/SkLightingShader.h +++ b/src/core/SkLightingShader.h @@ -20,9 +20,6 @@ public: /** Returns a shader that lights the shape, colored by the diffuseShader, using the normals from normalSource, with the set of lights provided. - It returns a shader with a reference count of 1. - The caller should decrement the shader's reference count when done with the shader. - It is an error for count to be < 2. @param diffuseShader the shader that provides the colors. If nullptr, uses the paint's color. @param normalSource the source for the shape's normals. If nullptr, assumes straight @@ -30,7 +27,7 @@ public: @param lights the lights applied to the normals The lighting equation is currently: - result = LightColor * DiffuseColor * (Normal * LightDir) + AmbientColor + result = (LightColor * dot(Normal, LightDir) + AmbientColor) * DiffuseColor */ static sk_sp Make(sk_sp diffuseShader, sk_sp normalSource, diff --git a/src/core/SkNormalBevelSource.cpp b/src/core/SkNormalBevelSource.cpp index f4bd031c4c..4a08728a98 100644 --- a/src/core/SkNormalBevelSource.cpp +++ b/src/core/SkNormalBevelSource.cpp @@ -245,6 +245,7 @@ private: sk_sp SkNormalBevelSourceImpl::asFragmentProcessor( const SkShader::AsFPArgs& args) const { + // This assumes a uniform scale. Anisotropic scaling might not be handled gracefully. SkScalar maxScale = args.fViewMatrix->getMaxScale(); // Providing device-space width and height @@ -260,7 +261,7 @@ SkNormalBevelSourceImpl::Provider::Provider() {} SkNormalBevelSourceImpl::Provider::~Provider() {} SkNormalSource::Provider* SkNormalBevelSourceImpl::asProvider(const SkShader::ContextRec &rec, - void *storage) const { + void *storage) const { return new (storage) Provider(); } @@ -268,8 +269,9 @@ size_t SkNormalBevelSourceImpl::providerSize(const SkShader::ContextRec&) const return sizeof(Provider); } +// TODO Implement feature for the CPU pipeline void SkNormalBevelSourceImpl::Provider::fillScanLine(int x, int y, SkPoint3 output[], - int count) const { + int count) const { for (int i = 0; i < count; i++) { output[i] = {0.0f, 0.0f, 1.0f}; } @@ -297,7 +299,7 @@ void SkNormalBevelSourceImpl::flatten(SkWriteBuffer& buf) const { //////////////////////////////////////////////////////////////////////////// sk_sp SkNormalSource::MakeBevel(BevelType type, SkScalar width, SkScalar height) { - /* TODO make sure this checks are tolerant enough to account for loss of conversion when GPUs + /* TODO make sure these checks are tolerant enough to account for loss of conversion when GPUs use 16-bit float types. We don't want to assume stuff is non-zero on the GPU and be wrong.*/ SkASSERT(width > 0.0f && !SkScalarNearlyZero(width)); if (SkScalarNearlyZero(height)) { diff --git a/src/core/SkNormalFlatSource.cpp b/src/core/SkNormalFlatSource.cpp index bdd65b03e9..c30cca14d1 100644 --- a/src/core/SkNormalFlatSource.cpp +++ b/src/core/SkNormalFlatSource.cpp @@ -57,9 +57,7 @@ public: private: GrGLSLFragmentProcessor* onCreateGLSLInstance() const override { return new GLSLNormalFlatFP; } - bool onIsEqual(const GrFragmentProcessor& proc) const override { - return true; - } + bool onIsEqual(const GrFragmentProcessor&) const override { return true; } }; sk_sp SkNormalFlatSourceImpl::asFragmentProcessor( @@ -77,7 +75,7 @@ SkNormalFlatSourceImpl::Provider::Provider() {} SkNormalFlatSourceImpl::Provider::~Provider() {} SkNormalSource::Provider* SkNormalFlatSourceImpl::asProvider(const SkShader::ContextRec &rec, - void *storage) const { + void *storage) const { return new (storage) Provider(); } @@ -86,7 +84,7 @@ size_t SkNormalFlatSourceImpl::providerSize(const SkShader::ContextRec&) const { } void SkNormalFlatSourceImpl::Provider::fillScanLine(int x, int y, SkPoint3 output[], - int count) const { + int count) const { for (int i = 0; i < count; i++) { output[i] = {0.0f, 0.0f, 1.0f}; } diff --git a/src/core/SkNormalMapSource.cpp b/src/core/SkNormalMapSource.cpp index c2cda099ab..25c533cdbf 100644 --- a/src/core/SkNormalMapSource.cpp +++ b/src/core/SkNormalMapSource.cpp @@ -73,8 +73,7 @@ public: fragBuilder->codeAppend( "}"); } - static void GenKey(const GrProcessor& proc, const GrGLSLCaps&, - GrProcessorKeyBuilder* b) { + static void GenKey(const GrProcessor&, const GrGLSLCaps&, GrProcessorKeyBuilder* b) { b->add32(0x0); } @@ -92,6 +91,7 @@ public: } private: + // Upper-right 2x2 corner of the inverse of the CTM in column-major form float fColumnMajorInvCTM22[4]; GrGLSLProgramDataManager::UniformHandle fXformUni; }; @@ -134,8 +134,8 @@ sk_sp SkNormalMapSourceImpl::asFragmentProcessor( //////////////////////////////////////////////////////////////////////////// SkNormalMapSourceImpl::Provider::Provider(const SkNormalMapSourceImpl& source, - SkShader::Context* mapContext, - SkPaint* overridePaint) + SkShader::Context* mapContext, + SkPaint* overridePaint) : fSource(source) , fMapContext(mapContext) , fOverridePaint(overridePaint) {} @@ -145,8 +145,8 @@ SkNormalMapSourceImpl::Provider::~Provider() { fOverridePaint->~SkPaint(); } -SkNormalSource::Provider* SkNormalMapSourceImpl::asProvider( - const SkShader::ContextRec &rec, void *storage) const { +SkNormalSource::Provider* SkNormalMapSourceImpl::asProvider(const SkShader::ContextRec &rec, + void *storage) const { SkMatrix normTotalInv; if (!this->computeNormTotalInverse(rec, &normTotalInv)) { return nullptr; @@ -173,7 +173,7 @@ size_t SkNormalMapSourceImpl::providerSize(const SkShader::ContextRec& rec) cons } bool SkNormalMapSourceImpl::computeNormTotalInverse(const SkShader::ContextRec& rec, - SkMatrix* normTotalInverse) const { + SkMatrix* normTotalInverse) const { SkMatrix total; total.setConcat(*rec.fMatrix, fMapShader->getLocalMatrix()); @@ -187,7 +187,7 @@ bool SkNormalMapSourceImpl::computeNormTotalInverse(const SkShader::ContextRec& #define BUFFER_MAX 16 void SkNormalMapSourceImpl::Provider::fillScanLine(int x, int y, SkPoint3 output[], - int count) const { + int count) const { SkPMColor tmpNormalColors[BUFFER_MAX]; do { diff --git a/src/core/SkNormalSource.h b/src/core/SkNormalSource.h index 4b09d0b411..84aa9c4466 100644 --- a/src/core/SkNormalSource.h +++ b/src/core/SkNormalSource.h @@ -68,7 +68,7 @@ public: */ static sk_sp MakeFromNormalMap(sk_sp map, const SkMatrix& ctm); - /** Returns a normal source that provides straight-up normals only (0, 0, 1). + /** Returns a normal source that provides straight-up normals only <0, 0, 1>. */ static sk_sp MakeFlat(); @@ -108,8 +108,10 @@ public: */ kRoundedIn }; - /** Returns a normal source that generates a bevel for the given shape. UNIMPLEMENTED: Will - return straight-up normals only. + + /** Returns a normal source that generates a bevel for the shape being drawn. Currently this is + not implemented on CPU rendering. On GPU this currently only works for anti-aliased circles + and rectangles. @param type the type of bevel to add. @param width the width of the bevel, in source space. Must be positive. diff --git a/src/gpu/glsl/GrGLSLProgramBuilder.cpp b/src/gpu/glsl/GrGLSLProgramBuilder.cpp index d0d813d4c2..37c20deeb9 100644 --- a/src/gpu/glsl/GrGLSLProgramBuilder.cpp +++ b/src/gpu/glsl/GrGLSLProgramBuilder.cpp @@ -90,7 +90,7 @@ void GrGLSLProgramBuilder::emitAndInstallPrimProc(const GrPrimitiveProcessor& pr const char* distanceVectorName = nullptr; if (this->fPipeline.usesDistanceVectorField() && proc.implementsDistanceVector()) { distanceVectorName = fFS.distanceVectorName(); - fFS.codeAppend( "// Normalized vector to the closest geometric edge (in source space)\n"); + fFS.codeAppend( "// Normalized vector to the closest geometric edge (in device space)\n"); fFS.codeAppend( "// Distance to the edge encoded in the z-component\n"); fFS.codeAppendf("vec3 %s;", distanceVectorName); }