AuroraMiddleware/skia2
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Revert of Abstracted diffuse color map out of SkLightingShaderImpl into a generic SkShader (patchset #11 id:200001 of https://codereview.chromium.org/2062703003/ )

Browse Source 
Reason for revert:
Causing the Test-Ubuntu-GCC-ShuttleA-GPU-GTX550Ti-x86_64-Release-Valgrind builder to fail.

Started failing at this build:
https://uberchromegw.corp.google.com/i/client.skia/builders/Test-Ubuntu-GCC-ShuttleA-GPU-GTX550Ti-x86_64-Release-Valgrind/builds/1214

Snippet from build log:

==3016== Invalid read of size 4
==3016==    at 0xCD1901: GrFragmentProcessor::registerChildProcessor(sk_sp<GrFragmentProcessor>) (SkTArray.h:163)
==3016==    by 0xA58B47: _Z10sk_make_spI11NormalMapFPI5sk_spI19GrFragmentProcessorERK8SkMatrixEES1_IT_EDpOT0_ (SkNormalSource.cpp:84)
==3016==    by 0xA582E8: NormalMapSourceImpl::asFragmentProcessor(GrContext*, SkMatrix const&, SkMatrix const*, SkFilterQuality, SkSourceGammaTreatment) const (SkNormalSource.cpp:187)
==3016==    by 0xA0ADB5: SkLightingShaderImpl::asFragmentProcessor(GrContext*, SkMatrix const&, SkMatrix const*, SkFilterQuality, SkSourceGammaTreatment) const (SkLightingShader.cpp:268)
==3016==    by 0xDA17E6: skpaint_to_grpaint_impl(GrContext*, SkPaint const&, SkMatrix const&, sk_sp<GrFragmentProcessor>*, SkXfermode::Mode*, bool, bool, GrPaint*) (SkGr.cpp:552)
==3016==    by 0xDA4532: SkPaintToGrPaint(GrContext*, SkPaint const&, SkMatrix const&, bool, GrPaint*) (SkGr.cpp:668)
==3016==    by 0xD99B51: SkGpuDevice::drawRect(SkDraw const&, SkRect const&, SkPaint const&) (SkGpuDevice.cpp:534)
==3016==    by 0x9DDB1C: SkCanvas::onDrawRect(SkRect const&, SkPaint const&) (SkCanvas.cpp:2148)
==3016==    by 0x9DBF40: SkCanvas::drawRect(SkRect const&, SkPaint const&) (SkCanvas.cpp:1919)
==3016==    by 0x8D2A55: skiagm::LightingShaderGM::onDraw(SkCanvas*) (lightingshader.cpp:110)
==3016==    by 0x625659: skiagm::GM::drawContent(SkCanvas*) (gm.cpp:32)
==3016==    by 0x6256AD: skiagm::GM::draw(SkCanvas*) (gm.cpp:24)
==3016==    by 0x61B747: DM::GMSrc::draw(SkCanvas*) const (DMSrcSink.cpp:61)
==3016==    by 0x61F37B: DM::GPUSink::draw(DM::Src const&, SkBitmap*, SkWStream*, SkString*) const (DMSrcSink.cpp:1115)
==3016==    by 0x619261: dm_main() (DM.cpp:1016)
==3016==    by 0x619841: main (DM.cpp:1409)
==3016==  Address 0xf4 is not stack'd, malloc'd or (recently) free'd
==3016==

Caught signal 11 [Segmentation fault], was running:
	gpu gm  lightingshader

Original issue's description:
> Abstracted diffuse color map out of SkLightingShaderImpl into a generic SkShader
>
> Will not run until after landing https://codereview.chromium.org/2106893003/
>
> This CL's base is the CL for the addition of NormalSource to the API: https://codereview.chromium.org/2063793002/
>
> BUG=skia:
> GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2062703003
>
> Committed: https://skia.googlesource.com/skia/+/3f58cd0eb8bf4499c4f179aa6111d7e005809df8

TBR=egdaniel@google.com,robertphillips@google.com,dvonbeck@google.com
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=skia:
NOTRY=true

Review-Url: https://codereview.chromium.org/2137513002
This commit is contained in:
rmistry 2016-07-08 06:23:35 -07:00 committed by Commit bot
parent 86dc226b60
commit 0e56972e5f
2 changed files with 149 additions and 57 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

205
src/core/SkLightingShader.cpp
View File

@ -41,18 +41,22 @@
*/
class SkLightingShaderImpl : public SkShader {
public:
/** Create a new lighting shader that uses the provided normal map and
lights to light the diffuse bitmap.
@param diffuseShader the shader that provides the diffuse colors
@param diffuse the diffuse bitmap
@param lights the lights applied to the normal map
@param diffLocalM the local matrix for the diffuse coordinates
@param normalSource the source of normals for lighting computation
@param lights the lights applied to the geometry
*/
SkLightingShaderImpl(sk_sp<SkShader> diffuseShader,
sk_sp<SkNormalSource> normalSource,
const sk_sp<SkLights> lights)
: fDiffuseShader(std::move(diffuseShader))
, fNormalSource(std::move(normalSource))
, fLights(std::move(lights)) {}
SkLightingShaderImpl(const SkBitmap& diffuse,
const sk_sp<SkLights> lights,
const SkMatrix* diffLocalM,
sk_sp<SkNormalSource> normalSource)
: INHERITED(diffLocalM)
, fDiffuseMap(diffuse)
, fLights(std::move(lights))
, fNormalSource(std::move(normalSource)) {}
bool isOpaque() const override;
@ -69,9 +73,8 @@ public:
// The context takes ownership of the states. It will call their destructors
// but will NOT free the memory.
LightingShaderContext(const SkLightingShaderImpl&, const ContextRec&,
SkShader::Context* diffuseContext, SkNormalSource::Provider*,
SkBitmapProcState* diffuseState, SkNormalSource::Provider*,
void* heapAllocated);
~LightingShaderContext() override;
void shadeSpan(int x, int y, SkPMColor[], int count) override;
@ -79,7 +82,7 @@ public:
uint32_t getFlags() const override { return fFlags; }
private:
SkShader::Context* fDiffuseContext;
SkBitmapProcState* fDiffuseState;
SkNormalSource::Provider* fNormalProvider;
uint32_t fFlags;
@ -97,10 +100,11 @@ protected:
Context* onCreateContext(const ContextRec&, void*) const override;
private:
sk_sp<SkShader> fDiffuseShader;
sk_sp<SkNormalSource> fNormalSource;
SkBitmap fDiffuseMap;
sk_sp<SkLights> fLights;
sk_sp<SkNormalSource> fNormalSource;
friend class SkLightingShader;
typedef SkShader INHERITED;
@ -123,7 +127,12 @@ private:
class LightingFP : public GrFragmentProcessor {
public:
LightingFP(sk_sp<GrFragmentProcessor> normalFP, sk_sp<SkLights> lights) {
LightingFP(GrTexture* diffuse, const SkMatrix& diffMatrix, const GrTextureParams& diffParams,
sk_sp<SkLights> lights, sk_sp<GrFragmentProcessor> normalFP)
: fDiffDeviceTransform(kLocal_GrCoordSet, diffMatrix, diffuse, diffParams.filterMode())
, fDiffuseTextureAccess(diffuse, diffParams) {
this->addCoordTransform(&fDiffDeviceTransform);
this->addTextureAccess(&fDiffuseTextureAccess);
// fuse all ambient lights into a single one
fAmbientColor.set(0.0f, 0.0f, 0.0f);
@ -170,7 +179,11 @@ public:
kVec3f_GrSLType, kDefault_GrSLPrecision,
"AmbientColor", &ambientColorUniName);
fragBuilder->codeAppendf("vec4 diffuseColor = %s;", args.fInputColor);
fragBuilder->codeAppend("vec4 diffuseColor = ");
fragBuilder->appendTextureLookupAndModulate(args.fInputColor, args.fTexSamplers[0],
args.fCoords[0].c_str(),
args.fCoords[0].getType());
fragBuilder->codeAppend(";");
SkString dstNormalName("dstNormal");
this->emitChild(0, nullptr, &dstNormalName, args);
@ -245,11 +258,15 @@ private:
bool onIsEqual(const GrFragmentProcessor& proc) const override {
const LightingFP& lightingFP = proc.cast<LightingFP>();
return fLightDir == lightingFP.fLightDir &&
return fDiffDeviceTransform == lightingFP.fDiffDeviceTransform &&
fDiffuseTextureAccess == lightingFP.fDiffuseTextureAccess &&
fLightDir == lightingFP.fLightDir &&
fLightColor == lightingFP.fLightColor &&
fAmbientColor == lightingFP.fAmbientColor;
}
GrCoordTransform fDiffDeviceTransform;
GrTextureAccess fDiffuseTextureAccess;
SkVector3 fLightDir;
SkColor3f fLightColor;
SkColor3f fAmbientColor;
@ -257,21 +274,66 @@ private:
////////////////////////////////////////////////////////////////////////////
static bool make_mat(const SkBitmap& bm,
const SkMatrix& localMatrix1,
const SkMatrix* localMatrix2,
SkMatrix* result) {
result->setIDiv(bm.width(), bm.height());
SkMatrix lmInverse;
if (!localMatrix1.invert(&lmInverse)) {
return false;
}
if (localMatrix2) {
SkMatrix inv;
if (!localMatrix2->invert(&inv)) {
return false;
}
lmInverse.postConcat(inv);
}
result->preConcat(lmInverse);
return true;
}
sk_sp<GrFragmentProcessor> SkLightingShaderImpl::asFragmentProcessor(
GrContext* context,
const SkMatrix& viewM,
const SkMatrix* localMatrix,
SkFilterQuality filterQuality,
SkSourceGammaTreatment gammaTreatment) const {
// we assume diffuse and normal maps have same width and height
// TODO: support different sizes, will be addressed when diffuse maps are factored out of
// SkLightingShader in a future CL
SkMatrix diffM;
if (!make_mat(fDiffuseMap, this->getLocalMatrix(), localMatrix, &diffM)) {
return nullptr;
}
bool doBicubic;
GrTextureParams::FilterMode diffFilterMode = GrSkFilterQualityToGrFilterMode(
SkTMin(filterQuality, kMedium_SkFilterQuality),
viewM,
this->getLocalMatrix(),
&doBicubic);
SkASSERT(!doBicubic);
// TODO: support other tile modes
GrTextureParams diffParams(kClamp_TileMode, diffFilterMode);
SkAutoTUnref<GrTexture> diffuseTexture(GrRefCachedBitmapTexture(context, fDiffuseMap,
diffParams, gammaTreatment));
if (!diffuseTexture) {
SkErrorInternals::SetError(kInternalError_SkError, "Couldn't convert bitmap to texture.");
return nullptr;
}
sk_sp<GrFragmentProcessor> normalFP(
fNormalSource->asFragmentProcessor(context, viewM, localMatrix, filterQuality,
gammaTreatment));
sk_sp<GrFragmentProcessor> fpPipeline[] = {
fDiffuseShader->asFragmentProcessor(context, viewM, localMatrix, filterQuality,
gammaTreatment),
sk_make_sp<LightingFP>(std::move(normalFP), fLights)
};
sk_sp<GrFragmentProcessor> inner(GrFragmentProcessor::RunInSeries(fpPipeline, 2));
sk_sp<GrFragmentProcessor> inner (
new LightingFP(diffuseTexture, diffM, diffParams, fLights, std::move(normalFP)));
return GrFragmentProcessor::MulOutputByInputAlpha(std::move(inner));
}
@ -281,18 +343,18 @@ sk_sp<GrFragmentProcessor> SkLightingShaderImpl::asFragmentProcessor(
////////////////////////////////////////////////////////////////////////////
bool SkLightingShaderImpl::isOpaque() const {
return fDiffuseShader->isOpaque();
return fDiffuseMap.isOpaque();
}
SkLightingShaderImpl::LightingShaderContext::LightingShaderContext(
const SkLightingShaderImpl& shader, const ContextRec& rec,
SkShader::Context* diffuseContext, SkNormalSource::Provider* normalProvider,
void* heapAllocated)
const SkLightingShaderImpl& shader, const ContextRec& rec, SkBitmapProcState* diffuseState,
SkNormalSource::Provider* normalProvider, void* heapAllocated)
: INHERITED(shader, rec)
, fDiffuseContext(diffuseContext)
, fDiffuseState(diffuseState)
, fNormalProvider(normalProvider)
, fHeapAllocated(heapAllocated) {
bool isOpaque = shader.isOpaque();
const SkPixmap& pixmap = fDiffuseState->fPixmap;
bool isOpaque = pixmap.isOpaque();
// update fFlags
uint32_t flags = 0;
@ -304,9 +366,9 @@ SkLightingShaderImpl::LightingShaderContext::LightingShaderContext(
}
SkLightingShaderImpl::LightingShaderContext::~LightingShaderContext() {
// The dependencies have been created outside of the context on memory that was allocated by
// the onCreateContext() method. Call the destructors and free the memory.
fDiffuseContext->~Context();
// The bitmap proc states have been created outside of the context on memory that will be freed
// elsewhere. Call the destructors but leave the freeing of the memory to the caller.
fDiffuseState->~SkBitmapProcState();
fNormalProvider->~Provider();
sk_free(fHeapAllocated);
@ -336,23 +398,39 @@ static inline SkPMColor convert(SkColor3f color, U8CPU a) {
// larger is better (fewer times we have to loop), but we shouldn't
// take up too much stack-space (each one here costs 16 bytes)
#define BUFFER_MAX 16
#define TMP_COUNT 16
#define BUFFER_MAX ((int)(TMP_COUNT * sizeof(uint32_t)))
void SkLightingShaderImpl::LightingShaderContext::shadeSpan(int x, int y,
SkPMColor result[], int count) {
const SkLightingShaderImpl& lightShader = static_cast<const SkLightingShaderImpl&>(fShader);
SkPMColor diffuse[BUFFER_MAX];
uint32_t tmpColor[TMP_COUNT];
SkPMColor tmpColor2[2*TMP_COUNT];
SkBitmapProcState::MatrixProc diffMProc = fDiffuseState->getMatrixProc();
SkBitmapProcState::SampleProc32 diffSProc = fDiffuseState->getSampleProc32();
int max = fDiffuseState->maxCountForBufferSize(BUFFER_MAX);
SkASSERT(fDiffuseState->fPixmap.addr());
SkASSERT(max <= BUFFER_MAX);
SkPoint3 normals[BUFFER_MAX];
do {
int n = SkTMin(count, BUFFER_MAX);
int n = count;
if (n > max) {
n = max;
}
diffMProc(*fDiffuseState, tmpColor, n, x, y);
diffSProc(*fDiffuseState, tmpColor, n, tmpColor2);
fDiffuseContext->shadeSpan(x, y, diffuse, n);
fNormalProvider->fillScanLine(x, y, normals, n);
for (int i = 0; i < n; ++i) {
SkColor diffColor = SkUnPreMultiply::PMColorToColor(diffuse[i]);
SkColor diffColor = SkUnPreMultiply::PMColorToColor(tmpColor2[i]);
SkColor3f accum = SkColor3f::Make(0.0f, 0.0f, 0.0f);
// This is all done in linear unpremul color space (each component 0..255.0f though)
@ -391,10 +469,19 @@ void SkLightingShaderImpl::toString(SkString* str) const {
#endif
sk_sp<SkFlattenable> SkLightingShaderImpl::CreateProc(SkReadBuffer& buf) {
SkMatrix diffLocalM;
bool hasDiffLocalM = buf.readBool();
if (hasDiffLocalM) {
buf.readMatrix(&diffLocalM);
} else {
diffLocalM.reset();
}
// Discarding SkShader flattenable params
bool hasLocalMatrix = buf.readBool();
SkAssertResult(!hasLocalMatrix);
SkBitmap diffuse;
if (!buf.readBitmap(&diffuse)) {
return nullptr;
}
diffuse.setImmutable();
int numLights = buf.readInt();
@ -422,15 +509,16 @@ sk_sp<SkFlattenable> SkLightingShaderImpl::CreateProc(SkReadBuffer& buf) {
sk_sp<SkLights> lights(builder.finish());
sk_sp<SkNormalSource> normalSource(buf.readFlattenable<SkNormalSource>());
sk_sp<SkShader> diffuseShader(buf.readFlattenable<SkShader>());
return sk_make_sp<SkLightingShaderImpl>(std::move(diffuseShader), std::move(normalSource),
std::move(lights));
return sk_make_sp<SkLightingShaderImpl>(diffuse, std::move(lights), &diffLocalM,
std::move(normalSource));
}
void SkLightingShaderImpl::flatten(SkWriteBuffer& buf) const {
this->INHERITED::flatten(buf);
buf.writeBitmap(fDiffuseMap);
buf.writeInt(fLights->numLights());
for (int l = 0; l < fLights->numLights(); ++l) {
const SkLights::Light& light = fLights->light(l);
@ -445,7 +533,6 @@ void SkLightingShaderImpl::flatten(SkWriteBuffer& buf) const {
}
buf.writeFlattenable(fNormalSource.get());
buf.writeFlattenable(fDiffuseShader.get());
}
size_t SkLightingShaderImpl::onContextSize(const ContextRec& rec) const {
@ -454,27 +541,35 @@ size_t SkLightingShaderImpl::onContextSize(const ContextRec& rec) const {
SkShader::Context* SkLightingShaderImpl::onCreateContext(const ContextRec& rec,
void* storage) const {
size_t heapRequired = fDiffuseShader->contextSize(rec) +
fNormalSource->providerSize(rec);
SkMatrix diffTotalInv;
// computeTotalInverse was called in SkShader::createContext so we know it will succeed
SkAssertResult(this->computeTotalInverse(rec, &diffTotalInv));
size_t heapRequired = sizeof(SkBitmapProcState) + fNormalSource->providerSize(rec);
void* heapAllocated = sk_malloc_throw(heapRequired);
void* diffuseContextStorage = heapAllocated;
SkShader::Context* diffuseContext = fDiffuseShader->createContext(rec, diffuseContextStorage);
if (!diffuseContext) {
void* diffuseStateStorage = heapAllocated;
SkBitmapProcState* diffuseState = new (diffuseStateStorage) SkBitmapProcState(fDiffuseMap,
SkShader::kClamp_TileMode, SkShader::kClamp_TileMode,
SkMipMap::DeduceTreatment(rec));
SkASSERT(diffuseState);
if (!diffuseState->setup(diffTotalInv, *rec.fPaint)) {
diffuseState->~SkBitmapProcState();
sk_free(heapAllocated);
return nullptr;
}
void* normalProviderStorage = (char*)heapAllocated + sizeof(SkBitmapProcState);
void* normalProviderStorage = (char*)heapAllocated + fDiffuseShader->contextSize(rec);
SkNormalSource::Provider* normalProvider = fNormalSource->asProvider(rec,
normalProviderStorage);
if (!normalProvider) {
diffuseContext->~Context();
diffuseState->~SkBitmapProcState();
sk_free(heapAllocated);
return nullptr;
}
return new (storage) LightingShaderContext(*this, rec, diffuseContext, normalProvider,
return new (storage) LightingShaderContext(*this, rec, diffuseState, normalProvider,
heapAllocated);
}
@ -493,12 +588,8 @@ sk_sp<SkShader> SkLightingShader::Make(const SkBitmap& diffuse,
return nullptr;
}
// TODO: support other tile modes
sk_sp<SkShader> diffuseShader = SkBitmapProcShader::MakeBitmapShader(diffuse,
SkShader::kClamp_TileMode, SkShader::kClamp_TileMode, diffLocalM);
return sk_make_sp<SkLightingShaderImpl>(std::move(diffuseShader), std::move(normalSource),
std::move(lights));
return sk_make_sp<SkLightingShaderImpl>(diffuse, std::move(lights), diffLocalM,
std::move(normalSource));
}
///////////////////////////////////////////////////////////////////////////////

1
src/core/SkNormalSource.cpp
View File

@ -181,6 +181,7 @@ sk_sp<GrFragmentProcessor> NormalMapSourceImpl::asFragmentProcessor(
const SkMatrix *localMatrix,
SkFilterQuality filterQuality,
SkSourceGammaTreatment gammaTreatment) const {
sk_sp<GrFragmentProcessor> mapFP = fMapShader->asFragmentProcessor(context, viewM,
localMatrix, filterQuality, gammaTreatment);