Subpixel text 3/8 of a pixel too far to the right.
http://codereview.appspot.com/5502097/
git-svn-id: http://skia.googlecode.com/svn/trunk@3037 2bbb7eff-a529-9590-31e7-b0007b416f81
diff --git a/gyp/tests.gyp b/gyp/tests.gyp
index 27a34a0..011030c 100644
--- a/gyp/tests.gyp
+++ b/gyp/tests.gyp
@@ -29,6 +29,7 @@
'../tests/DataRefTest.cpp',
'../tests/DequeTest.cpp',
'../tests/DrawBitmapRectTest.cpp',
+ '../tests/DrawTextTest.cpp',
'../tests/EmptyPathTest.cpp',
'../tests/FillPathTest.cpp',
'../tests/FlateTest.cpp',
diff --git a/src/core/SkDraw.cpp b/src/core/SkDraw.cpp
index c6fd406..d1b291f 100644
--- a/src/core/SkDraw.cpp
+++ b/src/core/SkDraw.cpp
@@ -13,6 +13,7 @@
#include "SkCanvas.h"
#include "SkColorPriv.h"
#include "SkDevice.h"
+#include "SkFixed.h"
#include "SkMaskFilter.h"
#include "SkPaint.h"
#include "SkPathEffect.h"
@@ -1477,9 +1478,9 @@
SkDraw1Glyph::Proc SkDraw1Glyph::init(const SkDraw* draw, SkBlitter* blitter,
SkGlyphCache* cache) {
fDraw = draw;
- fBounder = draw->fBounder;
- fBlitter = blitter;
- fCache = cache;
+ fBounder = draw->fBounder;
+ fBlitter = blitter;
+ fCache = cache;
if (hasCustomD1GProc(*draw)) {
// todo: fix this assumption about clips w/ custom
@@ -1577,17 +1578,21 @@
SkFixed fxMask = ~0;
SkFixed fyMask = ~0;
- if (paint.isSubpixelText()) {
+ if (cache->isSubpixel()) {
SkAxisAlignment baseline = SkComputeAxisAlignmentForHText(*matrix);
if (kX_SkAxisAlignment == baseline) {
fyMask = 0;
} else if (kY_SkAxisAlignment == baseline) {
fxMask = 0;
}
+
+ // apply bias here to avoid adding 1/2 the sampling frequency in the loop
+ fx += SK_FixedHalf >> SkGlyph::kSubBits;
+ fy += SK_FixedHalf >> SkGlyph::kSubBits;
+ } else {
+ fx += SK_FixedHalf;
+ fy += SK_FixedHalf;
}
- // apply the bias here, so we don't have to add 1/2 in the loop
- fx += SK_FixedHalf;
- fy += SK_FixedHalf;
SkAAClipBlitter aaBlitter;
SkAutoBlitterChoose blitterChooser;
@@ -1606,7 +1611,7 @@
SkDraw1Glyph::Proc proc = d1g.init(this, blitter, cache);
while (text < stop) {
- const SkGlyph& glyph = glyphCacheProc(cache, &text, fx & fxMask, fy & fyMask);
+ const SkGlyph& glyph = glyphCacheProc(cache, &text, fx & fxMask, fy & fyMask);
fx += autokern.adjust(glyph);
@@ -1757,12 +1762,12 @@
const char* stop = text + byteLength;
AlignProc alignProc = pick_align_proc(paint.getTextAlign());
- SkDraw1Glyph d1g;
- SkDraw1Glyph::Proc proc = d1g.init(this, blitter, cache);
+ SkDraw1Glyph d1g;
+ SkDraw1Glyph::Proc proc = d1g.init(this, blitter, cache);
TextMapState tms(*matrix, constY);
TextMapState::Proc tmsProc = tms.pickProc(scalarsPerPosition);
- if (paint.isSubpixelText()) {
+ if (cache->isSubpixel()) {
// maybe we should skip the rounding if linearText is set
SkAxisAlignment roundBaseline = SkComputeAxisAlignmentForHText(*matrix);
@@ -1771,8 +1776,13 @@
tmsProc(tms, pos);
+#ifdef SK_DRAW_POS_TEXT_IGNORE_SUBPIXEL_LEFT_ALIGN_FIX
SkFixed fx = SkScalarToFixed(tms.fLoc.fX);
SkFixed fy = SkScalarToFixed(tms.fLoc.fY);
+#else
+ SkFixed fx = SkScalarToFixed(tms.fLoc.fX) + (SK_FixedHalf >> SkGlyph::kSubBits);
+ SkFixed fy = SkScalarToFixed(tms.fLoc.fY) + (SK_FixedHalf >> SkGlyph::kSubBits);
+#endif
SkFixed fxMask = ~0;
SkFixed fyMask = ~0;
@@ -1807,8 +1817,8 @@
{
SkIPoint fixedLoc;
alignProc(tms.fLoc, *glyph, &fixedLoc);
- fx = fixedLoc.fX;
- fy = fixedLoc.fY;
+ fx = fixedLoc.fX + (SK_FixedHalf >> SkGlyph::kSubBits);
+ fy = fixedLoc.fY + (SK_FixedHalf >> SkGlyph::kSubBits);
if (kX_SkAxisAlignment == roundBaseline) {
fyMask = 0;
@@ -1840,8 +1850,10 @@
SkIPoint fixedLoc;
alignProc(tms.fLoc, glyph, &fixedLoc);
- proc(d1g, fixedLoc.fX + SK_FixedHalf,
- fixedLoc.fY + SK_FixedHalf, glyph);
+ proc(d1g,
+ fixedLoc.fX + SK_FixedHalf,
+ fixedLoc.fY + SK_FixedHalf,
+ glyph);
}
pos += scalarsPerPosition;
}
diff --git a/src/core/SkDrawProcs.h b/src/core/SkDrawProcs.h
index 74aa9bb..33b870d 100644
--- a/src/core/SkDrawProcs.h
+++ b/src/core/SkDrawProcs.h
@@ -14,19 +14,20 @@
class SkBlitter;
struct SkDraw1Glyph {
- const SkDraw* fDraw;
- SkBounder* fBounder;
- const SkRegion* fClip;
- const SkAAClip* fAAClip;
- SkBlitter* fBlitter;
- SkGlyphCache* fCache;
- SkIRect fClipBounds;
-
- // The fixed x,y have been pre-rounded (i.e. 1/2 has already been added),
- // so the impls need just trunc down to an int
- typedef void (*Proc)(const SkDraw1Glyph&, SkFixed x, SkFixed y, const SkGlyph&);
-
- Proc init(const SkDraw* draw, SkBlitter* blitter, SkGlyphCache* cache);
+ const SkDraw* fDraw;
+ SkBounder* fBounder;
+ const SkRegion* fClip;
+ const SkAAClip* fAAClip;
+ SkBlitter* fBlitter;
+ SkGlyphCache* fCache;
+ SkIRect fClipBounds;
+
+ // The fixed x,y are pre-rounded, so impls just trunc them down to ints.
+ // i.e. half the sampling frequency has been added.
+ // e.g. 1/2 or 1/(2^(SkGlyph::kSubBits+1)) has already been added.
+ typedef void (*Proc)(const SkDraw1Glyph&, SkFixed x, SkFixed y, const SkGlyph&);
+
+ Proc init(const SkDraw* draw, SkBlitter* blitter, SkGlyphCache* cache);
};
struct SkDrawProcs {
diff --git a/src/device/xps/SkXPSDevice.cpp b/src/device/xps/SkXPSDevice.cpp
index e2e23fa..84dcb8e 100644
--- a/src/device/xps/SkXPSDevice.cpp
+++ b/src/device/xps/SkXPSDevice.cpp
@@ -2176,9 +2176,15 @@
SkASSERT(skGlyph.fWidth > 0 && skGlyph.fHeight > 0);
SkXPSDrawProcs* procs = static_cast<SkXPSDrawProcs*>(state.fDraw->fProcs);
- //Draw pre-adds half for floor rounding.
- x -= SK_FixedHalf;
- y -= SK_FixedHalf;
+
+ //Draw pre-adds half the sampling frequency for floor rounding.
+ if (state.fCache->isSubpixel()) {
+ x -= (SK_FixedHalf >> SkGlyph::kSubBits);
+ y -= (SK_FixedHalf >> SkGlyph::kSubBits);
+ } else {
+ x -= SK_FixedHalf;
+ y -= SK_FixedHalf;
+ }
XPS_GLYPH_INDEX* xpsGlyph = procs->xpsGlyphs.append();
uint16_t glyphID = skGlyph.getGlyphID();
diff --git a/tests/DrawTextTest.cpp b/tests/DrawTextTest.cpp
new file mode 100644
index 0000000..aefe2f9
--- /dev/null
+++ b/tests/DrawTextTest.cpp
@@ -0,0 +1,115 @@
+/*
+ * Copyright 2011 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+#include "SkTypes.h"
+
+#include "Test.h"
+#include "SkBitmap.h"
+#include "SkCanvas.h"
+#include "SkColor.h"
+#include "SkPaint.h"
+#include "SkPoint.h"
+#include "SkRect.h"
+
+///////////////////////////////////////////////////////////////////////////////
+
+static const SkColor bgColor = SK_ColorWHITE;
+
+static void create(SkBitmap* bm, SkIRect bound, SkBitmap::Config config) {
+ bm->setConfig(config, bound.width(), bound.height());
+ bm->allocPixels();
+}
+
+static void drawBG(SkCanvas* canvas) {
+ canvas->drawColor(bgColor);
+}
+
+/** Assumes that the ref draw was completely inside ref canvas --
+ implies that everything outside is "bgColor".
+ Checks that all overlap is the same and that all non-overlap on the
+ ref is "bgColor".
+ */
+static bool compare(const SkBitmap& ref, const SkIRect& iref,
+ const SkBitmap& test, const SkIRect& itest)
+{
+ const int xOff = itest.fLeft - iref.fLeft;
+ const int yOff = itest.fTop - iref.fTop;
+
+ SkAutoLockPixels alpRef(ref);
+ SkAutoLockPixels alpTest(test);
+
+ for (int y = 0; y < test.height(); ++y) {
+ for (int x = 0; x < test.width(); ++x) {
+ SkColor testColor = test.getColor(x, y);
+ int refX = x + xOff;
+ int refY = y + yOff;
+ SkColor refColor;
+ if (refX >= 0 && refX < ref.width() &&
+ refY >= 0 && refY < ref.height())
+ {
+ refColor = ref.getColor(refX, refY);
+ } else {
+ refColor = bgColor;
+ }
+ if (refColor != testColor) {
+ return false;
+ }
+ }
+ }
+ return true;
+}
+
+static void test_drawText(skiatest::Reporter* reporter) {
+
+ SkPaint paint;
+ paint.setColor(SK_ColorGRAY);
+ paint.setTextSize(SkIntToScalar(20));
+
+ SkIRect drawTextRect = SkIRect::MakeWH(64, 64);
+ SkBitmap drawTextBitmap;
+ create(&drawTextBitmap, drawTextRect, SkBitmap::kARGB_8888_Config);
+ SkCanvas drawTextCanvas(drawTextBitmap);
+
+ SkIRect drawPosTextRect = SkIRect::MakeWH(64, 64);
+ SkBitmap drawPosTextBitmap;
+ create(&drawPosTextBitmap, drawPosTextRect, SkBitmap::kARGB_8888_Config);
+ SkCanvas drawPosTextCanvas(drawPosTextBitmap);
+
+ for (float offsetY = 0.0f; offsetY < 1.0f; offsetY += (1.0f / 16.0f)) {
+ for (float offsetX = 0.0f; offsetX < 1.0f; offsetX += (1.0f / 16.0f)) {
+ SkPoint point = SkPoint::Make(SkFloatToScalar(25.0f + offsetX),
+ SkFloatToScalar(25.0f + offsetY));
+
+ for (int align = 0; align < SkPaint::kAlignCount; ++align) {
+ paint.setTextAlign(static_cast<SkPaint::Align>(align));
+
+ for (unsigned int flags = 0; flags < (1 << 3); ++flags) {
+ static const unsigned int antiAliasFlag = 1;
+ static const unsigned int subpixelFlag = 1 << 1;
+ static const unsigned int lcdFlag = 1 << 2;
+
+ paint.setAntiAlias(SkToBool(flags & antiAliasFlag));
+ paint.setSubpixelText(SkToBool(flags & subpixelFlag));
+ paint.setLCDRenderText(SkToBool(flags & lcdFlag));
+
+ // Test: drawText and drawPosText draw the same.
+ drawBG(&drawTextCanvas);
+ drawTextCanvas.drawText("A", 1, point.fX, point.fY, paint);
+
+ drawBG(&drawPosTextCanvas);
+ drawPosTextCanvas.drawPosText("A", 1, &point, paint);
+
+ REPORTER_ASSERT(reporter,
+ compare(drawTextBitmap, drawTextRect,
+ drawPosTextBitmap, drawPosTextRect));
+ }
+ }
+ }
+ }
+}
+
+#include "TestClassDef.h"
+DEFINE_TESTCLASS("DrawText_DrawPosText", DrawTextTestClass, test_drawText)