From abc9bb55ddfeb4b1a7acc335a34841fddcd22d27 Mon Sep 17 00:00:00 2001 From: rmistry Date: Mon, 23 Jun 2014 05:39:26 -0700 Subject: [PATCH] Revert of Fix SkPaint::measureText for stroked hairline text (https://codereview.chromium.org/335603003/) Reason for revert: Caused many shadertext GM failures Original issue's description: > Fix SkPaint::measureText for stroked hairline text > > SkPaint::measureText and text drawing used different criteria for > determining whether text should be drawn as paths or not. > > Adds tests glyph_pos_(h/n)_(s/f/b) to test the text rendering and the glyph > positioning in the rendering. Mainly added in order to define what is the > expected text rendering when hairline stroke is used with various transform > options. > > The testcase also tries to note or highlight the fact that SkPaint::measureText > is not expected to produce intuitively matching results when compared to a > rendering, if the rendering is done so that the device ends up having a device > transform. > > This fixes the glyph_pos_h_s (hairline, stroked) test-case. > > Ignore shadertext2_pdf-poppler.png gm on > Test-Ubuntu13.10-ShuttleA-NoGPU-x86_64-Debug temporarily, as that fails. > > Committed: https://skia.googlesource.com/skia/+/196af738027c5e18c3eb792dbcaf90ef27821793 R=jvanverth@google.com, reed@google.com, kkinnunen@nvidia.com TBR=jvanverth@google.com, kkinnunen@nvidia.com, reed@google.com NOTREECHECKS=true NOTRY=true Author: rmistry@google.com Review URL: https://codereview.chromium.org/354433002 --- .../expected-results.json | 3 +- gm/glyph_pos.cpp | 204 ------------------ gyp/gmslides.gypi | 1 - include/core/SkPaint.h | 3 + src/core/SkPaint.cpp | 12 +- 5 files changed, 14 insertions(+), 209 deletions(-) delete mode 100644 gm/glyph_pos.cpp diff --git a/expectations/gm/Test-Ubuntu13.10-ShuttleA-NoGPU-x86_64-Debug/expected-results.json b/expectations/gm/Test-Ubuntu13.10-ShuttleA-NoGPU-x86_64-Debug/expected-results.json index 30558811de..09d6c5810e 100644 --- a/expectations/gm/Test-Ubuntu13.10-ShuttleA-NoGPU-x86_64-Debug/expected-results.json +++ b/expectations/gm/Test-Ubuntu13.10-ShuttleA-NoGPU-x86_64-Debug/expected-results.json @@ -9210,7 +9210,6 @@ 5483462587049856153 ] ], - "ignore-failure": true, "reviewed-by-human": true }, "shadertext3_565.png": { @@ -10861,4 +10860,4 @@ "reviewed-by-human": false } } -} +} \ No newline at end of file diff --git a/gm/glyph_pos.cpp b/gm/glyph_pos.cpp deleted file mode 100644 index b0451016ac..0000000000 --- a/gm/glyph_pos.cpp +++ /dev/null @@ -1,204 +0,0 @@ -/* - * Copyright 2014 Google Inc. - * - * Use of this source code is governed by a BSD-style license that can be - * found in the LICENSE file. - */ - -#include "gm.h" -#include "SkCanvas.h" -#include "SkTypeface.h" - -/* This test tries to define the effect of using hairline strokes on text. - * Provides non-hairline images for reference and consistency checks. - * glyph_pos_(h/n)_(s/f/b) - * -> test hairline/non-hairline stroke/fill/stroke+fill. - */ -static const SkScalar kTextHeight = 14.0f; -static const char kText[] = "Proportional Hamburgefons #% fi"; - -namespace skiagm { - -class GlyphPosGM : public GM { -public: - GlyphPosGM(SkScalar strokeWidth, SkPaint::Style strokeStyle) - : fStrokeWidth(strokeWidth) - , fStrokeStyle(strokeStyle) { - } - -protected: - virtual uint32_t onGetFlags() const SK_OVERRIDE { - return kSkipTiled_Flag; - } - - virtual SkString onShortName() SK_OVERRIDE { - SkString str("glyph_pos"); - if (fStrokeWidth == 0.0f) { - str.append("_h"); // h == Hairline. - } else { - str.append("_n"); // n == Normal. - } - if (fStrokeStyle == SkPaint::kStroke_Style) { - str.append("_s"); - } else if (fStrokeStyle == SkPaint::kFill_Style) { - str.append("_f"); - } else { - str.append("_b"); // b == Both. - } - return str; - } - - virtual SkISize onISize() { return SkISize::Make(800, 600); } - - virtual void onDraw(SkCanvas* canvas) SK_OVERRIDE { - if (!fProp) { - fProp.reset(SkTypeface::CreateFromName("Helvetica", SkTypeface::kNormal)); - } - - // There's a black pixel at 40, 40 for reference. - canvas->drawPoint(40.0f, 40.0f, SK_ColorBLACK); - - // Two reference images. - canvas->translate(50.0f, 50.0f); - drawTestCase(canvas, 1.0f); - - canvas->translate(0.0f, 50.0f); - drawTestCase(canvas, 3.0f); - - // Uniform scaling test. - canvas->translate(0.0f, 100.0f); - canvas->save(); - canvas->scale(3.0f, 3.0f); - drawTestCase(canvas, 1.0f); - canvas->restore(); - - // Non-uniform scaling test. - canvas->translate(0.0f, 100.0f); - canvas->save(); - canvas->scale(3.0f, 6.0f); - drawTestCase(canvas, 1.0f); - canvas->restore(); - - // Skew test. - canvas->translate(0.0f, 80.0f); - canvas->save(); - canvas->scale(3.0f, 3.0f); - SkMatrix skew; - skew.setIdentity(); - skew.setSkewX(SkScalarDiv(8.0f, - 25.0f)); - skew.setSkewY(SkScalarDiv(2.0f, - 25.0f)); - canvas->concat(skew); - drawTestCase(canvas, 1.0f); - canvas->restore(); - - // Perspective test. - canvas->translate(0.0f, 80.0f); - canvas->save(); - SkMatrix perspective; - perspective.setIdentity(); - perspective.setPerspX(-SkScalarDiv(SK_Scalar1, 340.0f)); - perspective.setSkewX(SkScalarDiv(8.0f, - 25.0f)); - perspective.setSkewY(SkScalarDiv(2.0f, - 25.0f)); - - - canvas->concat(perspective); - drawTestCase(canvas, 1.0f); - canvas->restore(); - } - - void drawTestCase(SkCanvas* canvas, SkScalar textScale) { - SkPaint paint; - paint.setColor(SK_ColorBLACK); - paint.setAntiAlias(true); - paint.setTextSize(kTextHeight * textScale); - paint.setTypeface(fProp); - paint.setDevKernText(true); - paint.setStrokeWidth(fStrokeWidth); - paint.setStyle(fStrokeStyle); - - // This demonstrates that we can not measure the text if there's a device transform. The - // canvas total matrix will end up being a device transform. - bool drawRef = !(canvas->getTotalMatrix().getType() & - ~(SkMatrix::kIdentity_Mask | SkMatrix::kTranslate_Mask)); - - SkRect bounds; - if (drawRef) { - SkScalar advance = paint.measureText(kText, sizeof(kText) - 1, &bounds); - - paint.setStrokeWidth(0.0f); - paint.setStyle(SkPaint::kStroke_Style); - - // Green box is the measured text bounds. - paint.setColor(SK_ColorGREEN); - canvas->drawRect(bounds, paint); - - // Red line is the measured advance from the 0,0 of the text position. - paint.setColor(SK_ColorRED); - canvas->drawLine(0.0f, 0.0f, advance, 0.0f, paint); - } - - // Black text is the testcase, eg. the text. - paint.setColor(SK_ColorBLACK); - paint.setStrokeWidth(fStrokeWidth); - paint.setStyle(fStrokeStyle); - canvas->drawText(kText, sizeof(kText) - 1, 0.0f, 0.0f, paint); - - if (drawRef) { - SkScalar widths[sizeof(kText) - 1]; - paint.getTextWidths(kText, sizeof(kText) - 1, widths, NULL); - - paint.setStrokeWidth(0.0f); - paint.setStyle(SkPaint::kStroke_Style); - - // Magenta lines are the positions for the characters. - paint.setColor(SK_ColorMAGENTA); - SkScalar w = bounds.x(); - for (size_t i = 0; i < sizeof(kText) - 1; ++i) { - canvas->drawLine(w, 0.0f, w, 5.0f, paint); - w += widths[i]; - } - } - } - -private: - SkAutoTUnref fProp; - SkScalar fStrokeWidth; - SkPaint::Style fStrokeStyle; - - typedef GM INHERITED; -}; - -////////////////////////////////////////////////////////////////////////////// - -static GM* GlyphPosHairlineStrokeAndFillFactory(void*) { - return new GlyphPosGM(0.0f, SkPaint::kStrokeAndFill_Style); -} -static GM* GlyphPosStrokeAndFillFactory(void*) { - return new GlyphPosGM(1.2f, SkPaint::kStrokeAndFill_Style); -} -static GM* GlyphPosHairlineStrokeFactory(void*) { - return new GlyphPosGM(0.0f, SkPaint::kStroke_Style); -} -static GM* GlyphPosStrokeFactory(void*) { - return new GlyphPosGM(1.2f, SkPaint::kStroke_Style); -} -static GM* GlyphPosHairlineFillFactory(void*) { - return new GlyphPosGM(0.0f, SkPaint::kFill_Style); -} -static GM* GlyphPosFillFactory(void*) { - return new GlyphPosGM(1.2f, SkPaint::kFill_Style); -} - -static GMRegistry reg1(GlyphPosHairlineStrokeAndFillFactory); -static GMRegistry reg2(GlyphPosStrokeAndFillFactory); -static GMRegistry reg3(GlyphPosHairlineStrokeFactory); -static GMRegistry reg4(GlyphPosStrokeFactory); -static GMRegistry reg5(GlyphPosHairlineFillFactory); -static GMRegistry reg6(GlyphPosFillFactory); - - -} diff --git a/gyp/gmslides.gypi b/gyp/gmslides.gypi index 694ec72f40..18e589b3f4 100644 --- a/gyp/gmslides.gypi +++ b/gyp/gmslides.gypi @@ -83,7 +83,6 @@ '../gm/gammatext.cpp', '../gm/getpostextpath.cpp', '../gm/giantbitmap.cpp', - '../gm/glyph_pos.cpp', '../gm/gradients.cpp', '../gm/gradients_2pt_conical.cpp', '../gm/gradients_no_texture.cpp', diff --git a/include/core/SkPaint.h b/include/core/SkPaint.h index a73faec42e..f766ca1c7f 100644 --- a/include/core/SkPaint.h +++ b/include/core/SkPaint.h @@ -1117,6 +1117,9 @@ private: static bool TooBigToUseCache(const SkMatrix& ctm, const SkMatrix& textM); + bool tooBigToUseCache() const; + bool tooBigToUseCache(const SkMatrix& ctm) const; + // Set flags/hinting/textSize up to use for drawing text as paths. // Returns scale factor to restore the original textSize, since will will // have change it to kCanonicalTextSizeForPaths. diff --git a/src/core/SkPaint.cpp b/src/core/SkPaint.cpp index 86b54f22c8..16d8bb2e4e 100644 --- a/src/core/SkPaint.cpp +++ b/src/core/SkPaint.cpp @@ -11,7 +11,6 @@ #include "SkColorFilter.h" #include "SkData.h" #include "SkDeviceProperties.h" -#include "SkDraw.h" #include "SkFontDescriptor.h" #include "SkFontHost.h" #include "SkGlyphCache.h" @@ -507,6 +506,15 @@ bool SkPaint::TooBigToUseCache(const SkMatrix& ctm, const SkMatrix& textM) { return tooBig(matrix, MaxCacheSize2()); } +bool SkPaint::tooBigToUseCache(const SkMatrix& ctm) const { + SkMatrix textM; + return TooBigToUseCache(ctm, *this->setTextMatrix(&textM)); +} + +bool SkPaint::tooBigToUseCache() const { + SkMatrix textM; + return tooBig(*this->setTextMatrix(&textM), MaxCacheSize2()); +} /////////////////////////////////////////////////////////////////////////////// @@ -989,7 +997,7 @@ SkScalar SkPaint::setupForAsPaths() { class SkCanonicalizePaint { public: SkCanonicalizePaint(const SkPaint& paint) : fPaint(&paint), fScale(0) { - if (paint.isLinearText() || SkDraw::ShouldDrawTextAsPaths(paint, SkMatrix::I())) { + if (paint.isLinearText() || paint.tooBigToUseCache()) { SkPaint* p = fLazy.set(paint); fScale = p->setupForAsPaths(); fPaint = p;