SkTextBlob

include/core/SkTextBlob.h

Index: include/core/SkTextBlob.h
diff --git a/include/core/SkTextBlob.h b/include/core/SkTextBlob.h
new file mode 100644
index 0000000000000000000000000000000000000000..4077c61eb6138586023f0a6750094a2702bf8b67
--- /dev/null
+++ b/include/core/SkTextBlob.h
@@ -0,0 +1,182 @@
+/*
+ * Copyright 2014 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#ifndef SkTextBlob_DEFINED
+#define SkTextBlob_DEFINED
+
+#include "SkPaint.h"
+#include "SkRefCnt.h"
+#include "SkTArray.h"
+#include "SkTDArray.h"
+
+/** \class SkTextBlob
+
+    SkTextBlob combines multiple text runs into an immutable, ref-counted structure.
+*/
+class SkTextBlob : public SkRefCnt {
+public:
+    virtual ~SkTextBlob();
+
+    /** Specifies the glyph positioning mode.
+    */
+    enum Positioning {
+        kDefault_Positioning      = 0, // Default glyph advances -- zero scalars per glyph.
+        kHorizontal_Positioning   = 1, // Horizontal positioning -- one scalar per glyph.
+        kFull_Positioning         = 2  // Point positioning -- two scalars per glyph.
+    };
+
+    /** \struct RunInfo
+
+        A run is a sequence of (optionally positioned) glyphs sharing the same font metrics
+        and positioning mode.
+    */
+    struct RunInfo {
+        uint32_t        glyphCount;
+        Positioning     positioning;
+        const uint16_t* glyphs;
+        const SkScalar* pos;
+        const SkPaint*  font;

[jbroman, 2014/08/19 17:39:16]

nit: Why not "const SkPaint&"? Is this a sign that this can be null?

[f(malita), 2014/08/20 01:21:57]

Good point -- done.

+
+        RunInfo(uint32_t c, Positioning m, const uint16_t* g, const SkScalar* p, const SkPaint *f)
+            : glyphCount(c), positioning(m), glyphs(g), pos(p), font(f) { }
+    };
+
+    /**
+     *  Returns the blob bounding box.
+     */
+    const SkRect& bounds() const { return fBounds; }
+
+    /**
+     *  Return a non-zero, unique value representing the text blob.
+     */
+    uint32_t uniqueID() const;
+
+    /**
+     *  Returns the number of runs in this blob.
+     */
+    unsigned runs() const { return fRuns ? fRuns->count() : 0; }
+
+    /**
+     *  Returns a RunInfo struct for the given run index.
+     */
+    const RunInfo runInfo(unsigned runIndex) const;

[jbroman, 2014/08/19 17:39:16]

Returning a const struct is meaningless (it's the same as returning a non-const
one). Unless Skia does this elsewhere, it seems clearer to return
"SkTextBlob::RunInfo".

[f(malita), 2014/08/20 01:21:57]

Removed from the API.

+
+private:

[robertphillips, 2014/08/19 15:06:47]

// This is ... ?

[f(malita), 2014/08/20 01:21:57]

Done.

+    struct Run {
+        uint32_t    count;
+        uint32_t    glyphStart; // index into fGlyphBuffer
+        uint32_t    posStart;   // index into fPosBuffer
+        SkPaint     font;
+        Positioning positioning;
+    };
+
+    SkTextBlob(const uint16_t* glyphs, const SkScalar* pos, const SkTArray<Run>* runs,
+               const SkRect& bounds);
+    friend class SkTextBlobBuilder;
+
+    const uint16_t*      fGlyphBuffer;

[jbroman, 2014/08/19 17:39:16]

Any reason not to use SkAutoTMalloc or something similar here instead of writing
a destructor? If this is idiomatic Skia, disregard. (Here and below.)

[f(malita), 2014/08/20 01:21:57]

Done.

+    const SkScalar*      fPosBuffer;
+    const SkTArray<Run>* fRuns;
+    const SkRect         fBounds;
+
+    mutable uint32_t     fUniqueID;

[jbroman, 2014/08/19 17:39:16]

Why "mutable"? Making this lazy kind of scares me (what if two threads try to
get the unique ID at once?); why not just populate it on construction?

[f(malita), 2014/08/20 01:21:58]

(this is the same pattern used by other Skia ID-ed classes)

The ID generation logic needs to be thread-safe (the impl uses atomic_inc) =>
we're trying to avoid the synchronization overhead for objects which never use
their ID => we defer the actual initialization until someone asks for it.

+};
+
+/** \class SkTextBlobBuilder
+
+    Helper class for constructing SkTextBlobs.

[robertphillips, 2014/08/19 15:06:47]

// Expected use is:
//     SkTextBlobBuilder tbb;
//     for (textruns) {
//        RunBuilderBase rbb = tbb.startRun(...);
//        rbb.addGlyph(...);
//     }
//     SkAutoTUnref<SkTextBlob> result(tbb.build());
?

+ */
+class SkTextBlobBuilder {
+private:

[robertphillips, 2014/08/19 15:06:47]

// This class ... 
specifically what sort of API is this class provided (e.g., the addGlyph(s)
variants)
?

[f(malita), 2014/08/20 01:21:57]

Removed.

+    class RunBuilderBase {
+    public:
+        RunBuilderBase(SkTextBlobBuilder* blobBuilder) : fBlobBuilder(blobBuilder) { }
+
+    protected:
+        SkTextBlobBuilder* fBlobBuilder;
+    };
+
+public:
+    /**
+     *  @param runs The number of runs to be added, if known. This is a storage hint and
+     *              not a limit.
+     */
+    SkTextBlobBuilder(unsigned runs = 0);
+
+    ~SkTextBlobBuilder();
+
+    template <SkTextBlob::Positioning>
+    class RunBuilder : public RunBuilderBase { };
+
+    /**
+     *  Start a text run for the given font and positioning mode.
+     *
+     *  @param glyphCount The number of glyphs in this run, if known. This is a storage hint
+     *                    and not a limit.
+     *  @param bounds     The bounding box for this run, if known. If non-null, it will be used
+     *                    in the blob bounding box computation -- otherwise the builder will
+     *                    measure the run.
+     *  @return           A specialized builder for this run. The run builder is only valid
+     *                    until the next startRun() or build() call.
+     */
+    template <SkTextBlob::Positioning POS>
+    RunBuilder<POS> startRun(const SkPaint& font, unsigned glyphCount = 0,

[jbroman, 2014/08/19 17:39:15]

What do we get by making this a two-step process (start run, then supply glyph
positioning) as opposed to simply exposing addRun methods that take all the
needed information?

[jbroman, 2014/08/19 17:39:16]

Why document the glyphCount and bounds arguments, but not font?

[f(malita), 2014/08/20 01:21:57]

Done.

[f(malita), 2014/08/20 01:21:57]

The latter forces clients to format data in a Skia-friendly manner, possibly
introducing extra copies. For example, given a Blink GlyphBuffer holding
advances we need to allocate another buffer, convert advances -> positions and
then pass that to addRun.

Using builder managed storage we could avoid allocating the intermediate buffer
and feed glyphs+positions directly into the builder, one at a time, as the
conversion progresses.

But after discussing this with Mike, I think we'll go with the simpler/more
direct solution for now. We can add advance-aware builder methods or convert
Blink to store positions if needed, later.

+                             const SkRect* bounds = NULL) {

[robertphillips, 2014/08/19 15:06:47]

this->

[f(malita), 2014/08/20 01:21:57]

Done.

+        ensureRun(font, POS, glyphCount, bounds);
+        return RunBuilder<POS>(this);
+    }
+
+    /**
+     *  Returns an immutable SkTextBlob for the current runs/glyphs. The builder is reset and
+     *  can be reused.
+     */
+    const SkTextBlob* build();
+
+private:
+    void ensureRun(const SkPaint& font, SkTextBlob::Positioning positioning, unsigned count,
+                   const SkRect* runBounds);
+    void updateRunBounds();
+
+    SkTDArray<uint16_t>        fGlyphBuffer;
+    SkTDArray<SkScalar>        fPosBuffer;
+    SkTArray<SkTextBlob::Run>* fRuns;
+
+    SkRect                     fBounds;
+    bool                       fDeferredRunBounds;
+};
+
+template<>
+class SkTextBlobBuilder::RunBuilder<SkTextBlob::kDefault_Positioning> : public RunBuilderBase {
+public:
+    RunBuilder(SkTextBlobBuilder* blobBuilder) : RunBuilderBase(blobBuilder) { }
+
+    void addGlyph(uint16_t glyph);
+    void addGlyphs(const uint16_t glyphs[], size_t count);
+};
+

[robertphillips, 2014/08/19 15:06:47]

// The Y-value will be supplied in the using SkCanvas::drawTextBlob call ?
// All the x values are relative to the x value passed to drawTextBlob ?

+template<>
+class SkTextBlobBuilder::RunBuilder<SkTextBlob::kHorizontal_Positioning> : public RunBuilderBase {
+public:
+    RunBuilder(SkTextBlobBuilder* blobBuilder) : RunBuilderBase(blobBuilder) { }
+
+    void addGlyph(uint16_t glyph, SkScalar x);

[jbroman, 2014/08/19 17:39:16]

Would we ever want to add a singular glyph? If it's intended that users always
supply whole runs, it seems easier not to expand the API prematurely.

+    void addGlyphs(const uint16_t glyphs[], const SkScalar xPos[], size_t count);
+};
+

[robertphillips, 2014/08/19 15:06:47]

// All the supplied positions are relative to the x,y supplied to the
drawTextBlob call ?

+template<>
+class SkTextBlobBuilder::RunBuilder<SkTextBlob::kFull_Positioning> : public RunBuilderBase {
+public:
+    RunBuilder(SkTextBlobBuilder* blobBuilder) : RunBuilderBase(blobBuilder) { }
+
+    void addGlyph(uint16_t glyph, SkScalar x, SkScalar y);

[robertphillips, 2014/08/19 15:06:48]

const SkPoint&

+    void addGlyph(uint16_t glyph, SkPoint pos);
+    void addGlyphs(const uint16_t glyphs[], const SkScalar xPos[], const SkScalar yPos[],
+                   size_t count);

[robertphillips, 2014/08/19 15:06:47]

pox ?

+    void addGlyphs(const uint16_t glyphs[], const SkPoint pox[], size_t count);

[jbroman, 2014/08/19 17:39:16]

What does this method do, given this is the "full positioning" case?

+};
+
+#endif // SkTextBlob_DEFINED