Skip to content

Commit

Permalink
Fast SkPackedGlyphID CTOR
Browse files Browse the repository at this point in the history
Performance measurments showed that SkPackedGlyphID was too much
time.

SkDiffBench-lorem_ipsum 955µs -> 873µs

BUG=chromium:881505

Change-Id: I8d42f02d87817777cc7c95404c73c723f0bc100c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/251818
Commit-Queue: Herb Derby <herb@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
  • Loading branch information
herbderby authored and Skia Commit-Bot committed Oct 31, 2019
1 parent b4577fb commit f5ad3f4
Show file tree
Hide file tree
Showing 6 changed files with 95 additions and 10 deletions.
2 changes: 2 additions & 0 deletions src/core/SkGlyph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
#include "src/pathops/SkPathOpsCubic.h"
#include "src/pathops/SkPathOpsQuad.h"

constexpr SkIPoint SkPackedGlyphID::kXYFieldMask;

SkMask SkGlyph::mask() const {
// getMetrics had to be called.
SkASSERT(fMaskFormat != MASK_FORMAT_UNKNOWN);
Expand Down
25 changes: 23 additions & 2 deletions src/core/SkGlyph.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include "include/private/SkChecksum.h"
#include "include/private/SkFixed.h"
#include "include/private/SkTo.h"
#include "include/private/SkVx.h"
#include "src/core/SkMask.h"

class SkArenaAlloc;
Expand Down Expand Up @@ -47,14 +48,19 @@ struct SkPackedGlyphID {
kFixedPointSubPixelPosBits = kFixedPointBinaryPointPos - kSubPixelPosLen,
};

static constexpr SkIPoint kXYFieldMask{3u << kSubPixelX, 3u << kSubPixelY};

constexpr explicit SkPackedGlyphID(SkGlyphID glyphID)
: fID{(uint32_t)glyphID << kGlyphID} { }

constexpr SkPackedGlyphID(SkGlyphID glyphID, SkFixed x, SkFixed y)
: fID {PackIDXY(glyphID, x, y)} { }

constexpr SkPackedGlyphID(SkGlyphID glyphID, SkIPoint pt)
: SkPackedGlyphID(glyphID, pt.fX, pt.fY) { }
constexpr SkPackedGlyphID(SkGlyphID glyphID, uint32_t x, uint32_t y)
: fID {PackIDSubXSubY(glyphID, x, y)} { }

SkPackedGlyphID(SkGlyphID glyphID, SkPoint pt, SkIPoint mask)
: fID{PackIDSkPoint(glyphID, pt, mask)} { }

constexpr explicit SkPackedGlyphID(uint32_t v) : fID{v & kMaskAll} { }

Expand Down Expand Up @@ -104,6 +110,21 @@ struct SkPackedGlyphID {
return (x << kSubPixelX) | (y << kSubPixelY) | (glyphID << kGlyphID);
}

// Assumptions: pt is properly rounded. mask is set for the x or y fields.
static uint32_t PackIDSkPoint(SkGlyphID glyphID, SkPoint pt, SkIPoint mask) {
using namespace skvx;
using XY = Vec<2, float>;
// The 4.f moves 1/2 bit and 1/4 bit into the 2 and 1 bits. The shift move those two bits
// into the right place in the masks.
const XY magic = {4.f * (1u << kSubPixelX), 4.f * (1u << kSubPixelY)};
XY pos{pt.x(), pt.y()};
Vec<2, int> sub = cast<int>(floor(pos * magic)) & Vec<2, int> {mask.x(), mask.y()};
SkASSERT(sub[0] / (1u << kSubPixelX) < 4);
SkASSERT(sub[1] / (1u << kSubPixelY) < 4);

return (glyphID << kGlyphID) | sub[0] | sub[1];
}

static constexpr uint32_t PackIDXY(SkGlyphID glyphID, SkFixed x, SkFixed y) {
return PackIDSubXSubY(glyphID, FixedToSub(x), FixedToSub(y));
}
Expand Down
6 changes: 2 additions & 4 deletions src/core/SkGlyphBuffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ void SkDrawableGlyphBuffer::startDevice(
matrix.mapPoints(fPositions, positions.data(), positions.size());

// Mask for controlling axis alignment.
SkIPoint mask = roundingSpec.ignorePositionMask;
SkIPoint mask = roundingSpec.ignorePositionFieldMask;

// Convert glyph ids and positions to packed glyph ids.
SkZip<const SkGlyphID, const SkPoint> withMappedPos =
Expand All @@ -68,9 +68,7 @@ void SkDrawableGlyphBuffer::startDevice(
for (auto t : withMappedPos) {
SkGlyphID glyphID; SkPoint pos;
std::tie(glyphID, pos) = t;
SkFixed subX = SkScalarToFixed(pos.x()) & mask.x(),
subY = SkScalarToFixed(pos.y()) & mask.y();
*packedIDCursor++ = SkPackedGlyphID{glyphID, subX, subY};
*packedIDCursor++ = SkPackedGlyphID{glyphID, pos, mask};
}
SkDEBUGCODE(fPhase = kInput);
}
Expand Down
16 changes: 12 additions & 4 deletions src/core/SkGlyphRunPainter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -720,8 +720,16 @@ SkIPoint SkGlyphPositionRoundingSpec::IgnorePositionMask(
(!isSubpixel || axisAlignment == kX_SkAxisAlignment) ? 0 : ~0);
}

SkGlyphPositionRoundingSpec::SkGlyphPositionRoundingSpec(bool isSubpixel,
SkAxisAlignment axisAlignment)
: halfAxisSampleFreq{HalfAxisSampleFreq(isSubpixel, axisAlignment)}
, ignorePositionMask{IgnorePositionMask(isSubpixel, axisAlignment)} {
SkIPoint SkGlyphPositionRoundingSpec::IgnorePositionFieldMask(bool isSubpixel,
SkAxisAlignment axisAlignment) {
SkIPoint ignoreMask = IgnorePositionMask(isSubpixel, axisAlignment);
SkIPoint answer{ignoreMask.x() & SkPackedGlyphID::kXYFieldMask.x(),
ignoreMask.y() & SkPackedGlyphID::kXYFieldMask.y()};
return answer;
}

SkGlyphPositionRoundingSpec::SkGlyphPositionRoundingSpec(
bool isSubpixel,SkAxisAlignment axisAlignment)
: halfAxisSampleFreq{HalfAxisSampleFreq(isSubpixel, axisAlignment)}
, ignorePositionMask{IgnorePositionMask(isSubpixel, axisAlignment)}
, ignorePositionFieldMask {IgnorePositionFieldMask(isSubpixel, axisAlignment)}{ }
2 changes: 2 additions & 0 deletions src/core/SkGlyphRunPainter.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,12 @@ struct SkGlyphPositionRoundingSpec {
SkGlyphPositionRoundingSpec(bool isSubpixel, SkAxisAlignment axisAlignment);
const SkVector halfAxisSampleFreq;
const SkIPoint ignorePositionMask;
const SkIPoint ignorePositionFieldMask;

private:
static SkVector HalfAxisSampleFreq(bool isSubpixel, SkAxisAlignment axisAlignment);
static SkIPoint IgnorePositionMask(bool isSubpixel, SkAxisAlignment axisAlignment);
static SkIPoint IgnorePositionFieldMask(bool isSubpixel, SkAxisAlignment axisAlignment);
};

class SkStrikeCommon {
Expand Down
54 changes: 54 additions & 0 deletions tests/SkGlyphBufferTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,60 @@
#include "src/core/SkScalerContext.h"
#include "tests/Test.h"

DEF_TEST(SkPackedGlyphIDCtor, reporter) {
// x and y are in 1/16.
const float step = 1.f / 16.f;
{
// Normal sub-pixel with y-axis snapping.
auto roundingSpec = SkGlyphPositionRoundingSpec(true, kX_SkAxisAlignment);
SkIPoint mask = roundingSpec.ignorePositionFieldMask;
for (int y = -32; y < 33; y++) {
for (int x = -32; x < 33; x++) {
float fx = x * step, fy = y * step;
SkPoint roundedPos = SkPoint{fx, fy} + roundingSpec.halfAxisSampleFreq;
SkPackedGlyphID packedID{3, roundedPos, mask};
uint32_t subX = ((x + 2) & 0b1100u) >> 2;
uint32_t subY = ((y + 8) & 0b1100u) >> 4;
SkPackedGlyphID correctID(3, subX, subY);
REPORTER_ASSERT(reporter, packedID == correctID);
}
}
}

{
// Subpixel with no axis snapping.
auto roundingSpec = SkGlyphPositionRoundingSpec(true, kNone_SkAxisAlignment);
SkIPoint mask = roundingSpec.ignorePositionFieldMask;
for (int y = -32; y < 33; y++) {
for (int x = -32; x < 33; x++) {
float fx = x * step, fy = y * step;
SkPoint roundedPos = SkPoint{fx, fy} + roundingSpec.halfAxisSampleFreq;
SkPackedGlyphID packedID{3, roundedPos, mask};
uint32_t subX = ((x + 2) & 0b1100u) >> 2;
uint32_t subY = ((y + 2) & 0b1100u) >> 2;
SkPackedGlyphID correctID(3, subX, subY);
REPORTER_ASSERT(reporter, packedID == correctID);
}
}
}

{
// No subpixel positioning.
auto roundingSpec = SkGlyphPositionRoundingSpec(false, kNone_SkAxisAlignment);
SkIPoint mask = roundingSpec.ignorePositionFieldMask;
for (int y = -32; y < 33; y++) {
for (int x = -32; x < 33; x++) {
float fx = x * step, fy = y * step;
SkPoint roundedPos = SkPoint{fx, fy} + roundingSpec.halfAxisSampleFreq;
SkPackedGlyphID packedID{3, roundedPos, mask};
uint32_t subX = ((x + 8) & 0b1100u) >> 4;
uint32_t subY = ((y + 8) & 0b1100u) >> 4;
SkPackedGlyphID correctID(3, subX, subY);
REPORTER_ASSERT(reporter, packedID == correctID);
}
}
}
}

DEF_TEST(SkSourceGlyphBufferBasic, reporter) {
SkSourceGlyphBuffer rejects;
Expand Down

0 comments on commit f5ad3f4

Please sign in to comment.