From ba1879d9f1b32522689326628818f186107530c9 Mon Sep 17 00:00:00 2001 From: John Stiles Date: Tue, 11 Aug 2020 13:58:32 -0400 Subject: [PATCH] Add `dumpTreeInfo` debug method to GrFragmentProcessor. This is a net reduction in code size because this idea was already implemented separately in two places: - dump_fragment_processor_tree (GrProcessorSet) - describe_fp (ProcessorTest) This consolidates the implementations. This CL also fixes a handful of dumpInfo() methods that were not sufficiently descriptive--e.g. the FP name was missing, or the implementation was just buggy. Change-Id: If34ac46c97e9ae431c7c64b1247fc619703580b2 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309324 Commit-Queue: Brian Salomon Reviewed-by: Brian Salomon Auto-Submit: John Stiles --- src/gpu/GrFragmentProcessor.cpp | 24 ++++++ src/gpu/GrFragmentProcessor.h | 8 ++ src/gpu/GrProcessorSet.cpp | 20 +---- src/gpu/ccpr/GrCCCoverageProcessor.h | 5 -- src/gpu/effects/GrBlendFragmentProcessor.cpp | 4 +- .../GrGaussianConvolutionFragmentProcessor.h | 5 +- src/gpu/effects/GrYUVtoRGBEffect.cpp | 10 +-- tests/ProcessorTest.cpp | 74 ++++++++----------- 8 files changed, 71 insertions(+), 79 deletions(-) diff --git a/src/gpu/GrFragmentProcessor.cpp b/src/gpu/GrFragmentProcessor.cpp index de2f6efb0e..05f69a9882 100644 --- a/src/gpu/GrFragmentProcessor.cpp +++ b/src/gpu/GrFragmentProcessor.cpp @@ -75,6 +75,30 @@ const GrTextureEffect* GrFragmentProcessor::asTextureEffect() const { return nullptr; } +#ifdef SK_DEBUG +static void recursive_dump_tree_info(const GrFragmentProcessor& fp, + SkString indent, + SkString* text) { + for (int index = 0; index < fp.numChildProcessors(); ++index) { + text->appendf("\n%s(#%d) -> ", indent.c_str(), index); + if (const GrFragmentProcessor* childFP = fp.childProcessor(index)) { + text->append(childFP->dumpInfo()); + indent.append("\t"); + recursive_dump_tree_info(*childFP, indent, text); + } else { + text->append("null"); + } + } +} + +SkString GrFragmentProcessor::dumpTreeInfo() const { + SkString text = this->dumpInfo(); + recursive_dump_tree_info(*this, SkString("\t"), &text); + text.append("\n"); + return text; +} +#endif + GrGLSLFragmentProcessor* GrFragmentProcessor::createGLSLInstance() const { GrGLSLFragmentProcessor* glFragProc = this->onCreateGLSLInstance(); glFragProc->fChildProcessors.push_back_n(fChildProcessors.count()); diff --git a/src/gpu/GrFragmentProcessor.h b/src/gpu/GrFragmentProcessor.h index ea1f6622c7..5e603c98e8 100644 --- a/src/gpu/GrFragmentProcessor.h +++ b/src/gpu/GrFragmentProcessor.h @@ -250,6 +250,14 @@ public: GrTextureEffect* asTextureEffect(); const GrTextureEffect* asTextureEffect() const; +#ifdef SK_DEBUG + // Generates debug info for this processor tree by recursively calling dumpInfo() on this + // processor and its children. + SkString dumpTreeInfo() const; +#else + SkString dumpTreeInfo() const { return dumpInfo(); } +#endif + // A pre-order traversal iterator over a hierarchy of FPs. It can also iterate over all the FP // hierarchies rooted in a GrPaint, GrProcessorSet, or GrPipeline. For these collections it // iterates the tree rooted at each color FP and then each coverage FP. diff --git a/src/gpu/GrProcessorSet.cpp b/src/gpu/GrProcessorSet.cpp index 7dbae261e3..385284a0bc 100644 --- a/src/gpu/GrProcessorSet.cpp +++ b/src/gpu/GrProcessorSet.cpp @@ -50,33 +50,17 @@ GrProcessorSet::~GrProcessorSet() { } #ifdef SK_DEBUG -SkString dump_fragment_processor_tree(const GrFragmentProcessor* fp, int indentCnt) { - SkString result; - SkString indentString; - for (int i = 0; i < indentCnt; ++i) { - indentString.append(" "); - } - result.appendf("%s%s %s \n", indentString.c_str(), fp ? fp->name() : "null", - fp ? fp->dumpInfo().c_str() : ""); - if (fp && fp->numChildProcessors()) { - for (int i = 0; i < fp->numChildProcessors(); ++i) { - result += dump_fragment_processor_tree(fp->childProcessor(i), indentCnt + 1); - } - } - return result; -} - SkString GrProcessorSet::dumpProcessors() const { SkString result; if (this->hasColorFragmentProcessor()) { result.append("Color Fragment Processor:\n"); - result += dump_fragment_processor_tree(this->colorFragmentProcessor(), 1); + result += this->colorFragmentProcessor()->dumpTreeInfo(); } else { result.append("No color fragment processor.\n"); } if (this->hasColorFragmentProcessor()) { result.append("Coverage Fragment Processor:\n"); - result += dump_fragment_processor_tree(this->coverageFragmentProcessor(), 1); + result += this->coverageFragmentProcessor()->dumpTreeInfo(); } else { result.append("No coverage fragment processors.\n"); } diff --git a/src/gpu/ccpr/GrCCCoverageProcessor.h b/src/gpu/ccpr/GrCCCoverageProcessor.h index 36835f6d29..f734453e5e 100644 --- a/src/gpu/ccpr/GrCCCoverageProcessor.h +++ b/src/gpu/ccpr/GrCCCoverageProcessor.h @@ -89,11 +89,6 @@ public: // GrPrimitiveProcessor overrides. const char* name() const override { return PrimitiveTypeName(fPrimitiveType); } -#ifdef SK_DEBUG - SkString dumpInfo() const override { - return SkStringPrintf("%s\n%s", this->name(), this->INHERITED::dumpInfo().c_str()); - } -#endif void getGLSLProcessorKey(const GrShaderCaps&, GrProcessorKeyBuilder* b) const override { SkDEBUGCODE(this->getDebugBloatKey(b)); b->add32((int)fPrimitiveType); diff --git a/src/gpu/effects/GrBlendFragmentProcessor.cpp b/src/gpu/effects/GrBlendFragmentProcessor.cpp index e1a7dead2b..bbe87256dc 100644 --- a/src/gpu/effects/GrBlendFragmentProcessor.cpp +++ b/src/gpu/effects/GrBlendFragmentProcessor.cpp @@ -53,9 +53,7 @@ public: #ifdef SK_DEBUG SkString dumpInfo() const override { - SkString str; - str.appendf("Mode: %s", SkBlendMode_Name(fMode)); - return str; + return SkStringPrintf("BlendFragmentProcessor(fMode=%s)", SkBlendMode_Name(fMode)); } #endif diff --git a/src/gpu/effects/GrGaussianConvolutionFragmentProcessor.h b/src/gpu/effects/GrGaussianConvolutionFragmentProcessor.h index 5eb0516073..5743642bb0 100644 --- a/src/gpu/effects/GrGaussianConvolutionFragmentProcessor.h +++ b/src/gpu/effects/GrGaussianConvolutionFragmentProcessor.h @@ -42,9 +42,8 @@ public: #ifdef SK_DEBUG SkString dumpInfo() const override { - SkString str; - str.appendf("dir: %s radius: %d", Direction::kX == fDirection ? "X" : "Y", fRadius); - return str; + return SkStringPrintf("GaussianConvolutionFragmentProcessor(dir=%s, radius=%d)", + Direction::kX == fDirection ? "X" : "Y", fRadius); } #endif diff --git a/src/gpu/effects/GrYUVtoRGBEffect.cpp b/src/gpu/effects/GrYUVtoRGBEffect.cpp index 203e097b2a..0516a82f95 100644 --- a/src/gpu/effects/GrYUVtoRGBEffect.cpp +++ b/src/gpu/effects/GrYUVtoRGBEffect.cpp @@ -185,13 +185,13 @@ GrYUVtoRGBEffect::GrYUVtoRGBEffect(std::unique_ptr planeFPs #ifdef SK_DEBUG SkString GrYUVtoRGBEffect::dumpInfo() const { - SkString str; + SkString str("YUVtoRGBEffect("); for (int i = 0; i < 4; ++i) { - str.appendf("yuvindex_%d: %d %d\n", i, fYUVAIndices->fIndex, - static_cast(fYUVAIndices->fChannel)); + str.appendf("YUVAIndices[%d]=%d %d, ", + i, fYUVAIndices[i].fIndex, static_cast(fYUVAIndices[i].fChannel)); } - str.appendf("cs: %d\n", static_cast(fYUVColorSpace)); - str.appendf("snap x: %d snap y: %d\n", fSnap[0], fSnap[1]); + str.appendf("YUVColorSpace=%d, snap=(%d, %d))", + static_cast(fYUVColorSpace), fSnap[0], fSnap[1]); return str; } #endif diff --git a/tests/ProcessorTest.cpp b/tests/ProcessorTest.cpp index 65398b3cb0..623e9ca0ce 100644 --- a/tests/ProcessorTest.cpp +++ b/tests/ProcessorTest.cpp @@ -608,8 +608,8 @@ DEF_GPUTEST_FOR_GL_RENDERING_CONTEXTS(ProcessorOptimizationValidationTest, repor if (trial >= kMaximumTrials) { SkDebugf("Abandoning ProcessorOptimizationValidationTest after %d trials. " - "Seed: 0x%08x, processor: %s.", - kMaximumTrials, fpGenerator.initialSeed(), fp->name()); + "Seed: 0x%08x, processor:\n%s", + kMaximumTrials, fpGenerator.initialSeed(), fp->dumpTreeInfo().c_str()); break; } } @@ -684,10 +684,10 @@ DEF_GPUTEST_FOR_GL_RENDERING_CONTEXTS(ProcessorOptimizationValidationTest, repor passing = false; if (coverageMessage.isEmpty()) { coverageMessage.printf( - "\"Modulating\" processor %s did not match " - "alpha-modulation nor color-modulation rules. " + "\"Modulating\" processor did not match alpha-modulation " + "nor color-modulation rules.\n" "Input: 0x%08x, Output: 0x%08x, pixel (%d, %d).", - fp->name(), input, output, x, y); + input, output, x, y); } } } @@ -705,12 +705,13 @@ DEF_GPUTEST_FOR_GL_RENDERING_CONTEXTS(ProcessorOptimizationValidationTest, repor if (constMessage.isEmpty()) { passing = false; - constMessage.printf("Processor %s claimed output for const input " - "doesn't match actual output. Error: %f, Tolerance: %f, " - "input: (%f, %f, %f, %f), actual: (%f, %f, %f, %f), " - "expected(%f, %f, %f, %f)", fp->name(), - std::max(rDiff, std::max(gDiff, std::max(bDiff, aDiff))), kTol, - input4f.fR, input4f.fG, input4f.fB, input4f.fA, + constMessage.printf( + "Processor claimed output for const input doesn't match " + "actual output.\n" + "Error: %f, Tolerance: %f, input: (%f, %f, %f, %f), " + "actual: (%f, %f, %f, %f), expected(%f, %f, %f, %f).", + std::max(rDiff, std::max(gDiff, std::max(bDiff, aDiff))), + kTol, input4f.fR, input4f.fG, input4f.fB, input4f.fA, output4f.fR, output4f.fG, output4f.fB, output4f.fA, expected4f.fR, expected4f.fG, expected4f.fB, expected4f.fA); } @@ -720,9 +721,10 @@ DEF_GPUTEST_FOR_GL_RENDERING_CONTEXTS(ProcessorOptimizationValidationTest, repor passing = false; if (opaqueMessage.isEmpty()) { - opaqueMessage.printf("Processor %s claimed opaqueness is preserved but " + opaqueMessage.printf( + "Processor claimed opaqueness is preserved but " "it is not. Input: 0x%08x, Output: 0x%08x.", - fp->name(), input, output); + input, output); } } @@ -737,10 +739,11 @@ DEF_GPUTEST_FOR_GL_RENDERING_CONTEXTS(ProcessorOptimizationValidationTest, repor // Finished analyzing the entire image, see if the number of pixel failures meets the // threshold for an FP violating the optimization requirements. if (failedPixelCount > kMaxAcceptableFailedPixels) { - ERRORF(reporter, "Processor violated %d of %d pixels, seed: 0x%08x, processor: %s" - ", first failing pixel details are below:", + ERRORF(reporter, + "Processor violated %d of %d pixels, seed: 0x%08x.\n" + "Processor:\n%s\nFirst failing pixel details are below:", failedPixelCount, kRenderSize * kRenderSize, fpGenerator.initialSeed(), - fp->dumpInfo().c_str()); + fp->dumpTreeInfo().c_str()); // Print first failing pixel's details. if (!coverageMessage.isEmpty()) { @@ -793,46 +796,27 @@ DEF_GPUTEST_FOR_GL_RENDERING_CONTEXTS(ProcessorOptimizationValidationTest, repor } } -static void describe_fp_children(const GrFragmentProcessor& fp, - std::string indent, - SkString* text) { - for (int index = 0; index < fp.numChildProcessors(); ++index) { - const GrFragmentProcessor* childFP = fp.childProcessor(index); - text->appendf("\n%s(#%d) -> %s", indent.c_str(), index, childFP ? childFP->name() : "null"); - if (childFP) { - describe_fp_children(*childFP, indent + "\t", text); - } - } -} - -static SkString describe_fp(const GrFragmentProcessor& fp) { - SkString text; - text.printf("\n%s", fp.name()); - describe_fp_children(fp, "\t", &text); - return text; -} - static void assert_processor_equality(skiatest::Reporter* reporter, const GrFragmentProcessor& fp, const GrFragmentProcessor& clone) { REPORTER_ASSERT(reporter, !strcmp(fp.name(), clone.name()), - "%s\n", describe_fp(fp).c_str()); + "\n%s", fp.dumpTreeInfo().c_str()); REPORTER_ASSERT(reporter, fp.compatibleWithCoverageAsAlpha() == clone.compatibleWithCoverageAsAlpha(), - "%s\n", describe_fp(fp).c_str()); + "\n%s", fp.dumpTreeInfo().c_str()); REPORTER_ASSERT(reporter, fp.isEqual(clone), - "%s\n", describe_fp(fp).c_str()); + "\n%s", fp.dumpTreeInfo().c_str()); REPORTER_ASSERT(reporter, fp.preservesOpaqueInput() == clone.preservesOpaqueInput(), - "%s\n", describe_fp(fp).c_str()); + "\n%s", fp.dumpTreeInfo().c_str()); REPORTER_ASSERT(reporter, fp.hasConstantOutputForConstantInput() == clone.hasConstantOutputForConstantInput(), - "%s\n", describe_fp(fp).c_str()); + "\n%s", fp.dumpTreeInfo().c_str()); REPORTER_ASSERT(reporter, fp.numChildProcessors() == clone.numChildProcessors(), - "%s\n", describe_fp(fp).c_str()); + "\n%s", fp.dumpTreeInfo().c_str()); REPORTER_ASSERT(reporter, fp.usesVaryingCoords() == clone.usesVaryingCoords(), - "%s\n", describe_fp(fp).c_str()); + "\n%s", fp.dumpTreeInfo().c_str()); REPORTER_ASSERT(reporter, fp.referencesSampleCoords() == clone.referencesSampleCoords(), - "%s\n", describe_fp(fp).c_str()); + "\n%s", fp.dumpTreeInfo().c_str()); } static bool verify_identical_render(skiatest::Reporter* reporter, int renderSize, @@ -942,7 +926,7 @@ DEF_GPUTEST_FOR_GL_RENDERING_CONTEXTS(ProcessorCloneTest, reporter, ctxInfo) { fpGenerator.make(i, /*randomTreeDepth=*/1, /*inputFP=*/nullptr); std::unique_ptr clone = fp->clone(); if (!clone) { - ERRORF(reporter, "Clone of processor %s failed.", fp->name()); + ERRORF(reporter, "Clone of processor %s failed.", fp->dumpTreeInfo().c_str()); continue; } assert_processor_equality(reporter, *fp, *clone); @@ -958,7 +942,7 @@ DEF_GPUTEST_FOR_GL_RENDERING_CONTEXTS(ProcessorCloneTest, reporter, ctxInfo) { readDataFP.data(), readDataClone.data())) { // Dump a description from the regenerated processor (since the original FP has // already been consumed). - ERRORF(reporter, "FP hierarchy:\n%s\n", describe_fp(*regen).c_str()); + ERRORF(reporter, "FP hierarchy:\n%s", regen->dumpTreeInfo().c_str()); // Render and readback output from the regenerated FP. If this also mismatches, the // FP itself doesn't generate consistent output. This could happen if: