SkTextBlob

include/core/SkTextBlob.h

+/*
+ * 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 "SkRect.h"
+#include "SkRefCnt.h"
+#include "SkTDArray.h"
+
+class SkBaseDevice;
+class SkDraw;
+struct SkPoint;
+
+/** \class SkTextChunk
+
+    A text chunk is a sequence of (optionally positioned glyphs) sharing the same
+    font data/metrics.
+*/
+class SK_API SkTextChunk {
+public:
+    static const SkTextChunk* Create(const uint16_t* glyphs, size_t count, const SkPaint& paint,
+                                     const SkRect* bounds = NULL);
+    static const SkTextChunk* Create(const uint16_t* glyphs, size_t count, const SkScalar* pos,

[mtklein, 2014/08/14 14:00:42]

Doesn't this need another SkScalar for the shared Y?

[f(malita), 2014/08/14 14:29:26]

I'm hoping most blobs will be relocatable (at least for simple text) such that
we don't have to rebuild them when the positioning attributes change.

To encourage that thinking, the current plan is to only store relative offsets
(=> shared Y == 0), and use an 'offset' param to position the blob at draw time.

+                                     const SkPaint& paint, const SkRect* bounds = NULL);
+    static const SkTextChunk* Create(const uint16_t* glyphs, size_t count, const SkPoint* pos,
+                                     const SkPaint& paint, const SkRect* bounds = NULL);
+
+    ~SkTextChunk();
+
+    void draw(SkBaseDevice*, const SkDraw&, const SkPaint&) const;
+
+    const SkRect& bounds() const;
+
+private:
+    SkTextChunk(const uint16_t* glyphs, size_t count, const SkPaint& paint, const SkRect* bounds);
+    SkTextChunk(const uint16_t* glyphs, size_t count, const SkScalar* pos, const SkPaint& paint,
+                const SkRect* bounds);
+    SkTextChunk(const uint16_t* glyphs, size_t count, const SkPoint* pos, const SkPaint& paint,
+                const SkRect* bounds);
+
+    void init(const uint16_t*, size_t, const SkRect*, const SkScalar*);
+
+    friend class SkPictureRecord;
+
+    size_t         fGlyphCount;
+    uint16_t*      fGlyphs;
+    SkScalar*      fPos;  // FIXME: merge glyphs/pos storage?
+
+    SkPaint        fFont; // FIXME: SkFont
+
+    mutable SkRect fBounds;

[mtklein, 2014/08/14 14:00:42]

We create TextChunks all at once right?  If so, let's proactively compute the
bounds?  Lazy things like this are really annoying to think about when dealing
with threads.

[f(malita), 2014/08/14 14:29:26]

It's not completely clear whether we *always* need bounds - I think the BBH
recorder uses heuristics for example and not sure if Blink computes them
unconditionally (at least not at layout time). Will need to look into it some
more to figure out the exact uses.

If we don't always compute them today, I'm somewhat worried about overhead --
although I'm inclined to heed your advice and revisit if it turns out to be a
problem. MikeR, WDYT?

[mtklein, 2014/08/14 14:38:45]

I'm going to try have pictures start querying the bounds of text all the time. 
Those heuristics we use today are... dicey.

[f(malita), 2014/08/14 16:13:36]

Awesome, that settles it :)

I think it also makes sense to store the bounds per blob, not per chunk. Done.

+    mutable bool   fBoundsDirty   : 1;

[mtklein, 2014/08/14 14:00:42]

I'd drop the bit packing for now?  It's particularly hard to think about what it
means to have mutable bits and immutable bits sharing the same byte when
considering thread safety.  I think at best we'll get yelled at by TSAN. :)

[f(malita), 2014/08/14 14:29:26]

Done.

+
+    unsigned       fScalarsPerPos : 2;
+};
+
+/** \class SkTextBlob
+
+    SkTextBlob combines multiple text chunks into an immutable, ref-counted structure.
+*/
+class SK_API SkTextBlob : public SkRefCnt {
+public:
+    static const SkTextBlob* Create(const SkTextChunk* chunk);
+    static const SkTextBlob* Create(const SkTDArray<const SkTextChunk*>& chunks);
+
+    ~SkTextBlob();
+
+    uint32_t uniqueID() const;
+
+    class SK_API Iter {
+    public:
+        Iter(const SkTextBlob* blob) : fBlob(blob), fIndex(0) { SkASSERT(blob); }
+        const SkTextChunk* next() {
+            return (fIndex < fBlob->fChunks.count()) ? fBlob->fChunks[fIndex++] : NULL;
+        }
+
+    private:
+        const SkTextBlob* fBlob;
+        int fIndex;
+    };
+
+private:
+    SkTextBlob(const SkTDArray<const SkTextChunk*>& chunks);
+
+    const SkTDArray<const SkTextChunk*>  fChunks;
+    mutable uint32_t                     fUniqueID;
+};
+
+/** \class SkTextBlobBuilder
+
+    Helper class for constructing SkTextBlobs.
+*/
+class SK_API SkTextBlobBuilder {
+public:
+    SkTextBlobBuilder();
+    ~SkTextBlobBuilder();
+
+    void addChunk(const SkTextChunk* chunk);
+
+    // FIXME: should we support glyph-wise ops? Not sure whether this is interesting
+    // for clients at this point -- they likely have a better idea of chunking.
+    /*
+    void addGlyph(uint16_t glyph, const SkPaint& font, const SkRect* bounds = NULL);
+    void addGlyph(uint16_t glyph, const SkScalar pos, const SkPaint& font,
+                  const SkRect* bounds = NULL);
+    void addGlyph(uint16_t glyph, const SkPoint pos, const SkPaint& font,
+                  const SkRect* bounds = NULL);
+    */
+
+    const SkTextBlob* build();

[jbroman, 2014/08/14 14:46:02]

nit: If SkTextBlobs are always immutable, why the "const" modifier here? I fear
this will just make us say "const SkTextBlob" everywhere and never simply
"SkTextBlob". Unsure.

[f(malita), 2014/08/14 16:13:36]

In general, I would actually find that desirable: forcing clients to be
const-explicit => more info for the compiler to take advantage of. Whether it
buys us much for naturally immutable objects, I'm not sure. I guess we can relax
the factory methods and see if anyone complains.

[mtklein, 2014/08/14 16:22:23]

I'm in (perhaps, am?) the the-more-const-the-better camp.  We have a couple
classes that are immutable but not passed as const (SkTypeface, SkXfermode) in
Skia, and I always have to think twice about whether or not the thing is going
to be changed by some function call I pass it to.  Even if it's technically
meaningless, const* is a nice readability signal.

Returning const SkTextBlob* does also leave us a bit more flexibility to mutate
SkTextBlobs internally before they escape the builder API.

+
+private:
+
+    SkTDArray<const SkTextChunk*>  fChunks;

[jbroman, 2014/08/14 14:46:02]

Random thought: pros/cons of having this approach vs just having an
SkTArray<SkTextChunk> and having the caller construct them on the stack (passing
in a const SkTextChunk&, or giving the constructor args to addChunk and having
it be constructed in place)?

There's something to be said about memory locality, but I'm not sure how the
tradeoffs play out, especially while this holds a full SkPaint.

[f(malita), 2014/08/14 16:13:36]

The reason it turned out this way is trying to avoid extra chunk copying and
making sure we don't overallocate the array (due to dynamic growth/internal
reserve). It's possible there are more elegant solutions.
Hopefully not for long :)

+};
+
+#endif // SkTextBlob_DEFINED