BitmapTextBatch and BitmapTextBlob

src/gpu/GrBatchFontCache.h

+/*
+ * Copyright 2015 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#ifndef GrBatchFontCache_DEFINED
+#define GrBatchFontCache_DEFINED
+
+#include "GrBatchAtlas.h"
+#include "GrDrawTarget.h"
+#include "GrFontScaler.h"
+#include "GrGlyph.h"
+#include "SkTDynamicHash.h"
+#include "SkVarAlloc.h"
+
+class GrBatchFontCache;
+class GrBatchTarget;
+class GrGpu;
+
+/**
+ *  The textstrike maps a hostfontscaler instance to a dictionary of
+ *  glyphid->strike

[bsalomon, 2015/03/26 17:06:56]

I don't understand the gr or sk font code much at all but it occurs to me that
this says this maps glyphids to strikes but it seems like the function that
takes a glyph id returns a GrGlyph not a "strike".

Looking at GrGlyph it appears to refer to a rectangle in a plot and some
metadata.

I'm not trying to be pedantic or at all blame this change for the confusion. The
reason I mention it is that I've always had a hard time understanding our font
code and part of that is the sparse and not very descriptive comments and jargon
that doesn't seem intuitive to me. It's been like that forever and is probably
better now than it used to be. I'd like us to make it better every time we touch
any of this code.

To that end:

Does this map glyphIds for a given strike to GrGlyphs?

Should GrGlyph be renamed? GrGlyphAtlasEntry? GrGlyphAtlasLocation? I'd rather
make it a little more verbose if it adds clarity.

[joshualitt, 2015/03/26 18:21:43]

I added a comment.  I think we need to overhaul how all of these classes work
together in a follow on CL

+ */
+class GrBatchTextStrike {
+public:
+    SK_DECLARE_INTERNAL_LLIST_INTERFACE(GrBatchTextStrike);
+    GrBatchTextStrike(GrBatchFontCache*, const GrFontDescKey* fontScalerKey);
+    ~GrBatchTextStrike();
+
+    const GrFontDescKey* getFontScalerKey() const { return fFontScalerKey; }
+    GrBatchFontCache* getBatchFontCache() const { return fBatchFontCache; }
+
+    inline GrGlyph* getGlyph(GrGlyph::PackedID packed, GrFontScaler* scaler) {
+        GrGlyph* glyph = fCache.find(packed);
+        if (NULL == glyph) {
+            glyph = this->generateGlyph(packed, scaler);
+        }
+        return glyph;
+    }
+
+    // returns true if glyph (or glyph+padding for distance field)
+    // is too large to ever fit in texture atlas subregions (GrPlots)
+    bool glyphTooLargeForAtlas(GrGlyph*);
+    // returns true if glyph successfully added to texture atlas, false otherwise
+    bool addGlyphToAtlas(GrBatchTarget*, GrGlyph*, GrFontScaler*);
+
+    // testing
+    int countGlyphs() const { return fCache.count(); }
+
+    // remove any references to this plot
+    void removeID(GrBatchAtlas::AtlasID);
+
+    static const GrFontDescKey& GetKey(const GrBatchTextStrike& ts) {
+        return *(ts.fFontScalerKey);
+    }
+    static uint32_t Hash(const GrFontDescKey& key) {
+        return key.getHash();
+    }
+
+private:
+    SkTDynamicHash<GrGlyph, GrGlyph::PackedID> fCache;
+    SkAutoTUnref<const GrFontDescKey> fFontScalerKey;
+    SkVarAlloc fPool;
+
+    GrBatchFontCache* fBatchFontCache;
+    int fAtlasedGlyphs;
+
+    GrGlyph* generateGlyph(GrGlyph::PackedID packed, GrFontScaler* scaler);
+
+    friend class GrBatchFontCache;
+};
+
+class GrBatchFontCache {

[bsalomon, 2015/03/26 17:06:56]

/** This class manages a set GrBatchAtlases that store cached glyphs...  ...It
uses GrGrBatchStrike for mumble mumble... distance field, bitmap (grayscale,
color, lcd)... mumble */

[joshualitt, 2015/03/26 18:21:43]

Acknowledged.

+public:
+    GrBatchFontCache(GrContext*);
+    ~GrBatchFontCache();
+
+    inline GrBatchTextStrike* getStrike(GrFontScaler* scaler) {
+        SkDEBUGCODE(this->validate();)
+        
+        GrBatchTextStrike* strike = fCache.find(*(scaler->getKey()));
+        if (NULL == strike) {
+            strike = this->generateStrike(scaler);
+        } else {
+            this->makeMRU(strike);
+        }
+        SkDEBUGCODE(this->validate();)
+        return strike;
+    }
+
+    bool hasGlyph(GrGlyph* glyph);
+    void refGlyph(GrGlyph*, GrBatchAtlas::BatchToken);

[bsalomon, 2015/03/26 17:06:56]

I think ref has a pretty specific meaning in Skia nomenclature. Can we rename
this?

Haven't looked at the impl yet, but maybe something like "glyphUsedWithToken()"?
"recordGlyphToken()"? "setLastGlyphToken()"?

Also, it's kind of asking for documentation.

[joshualitt, 2015/03/26 18:21:43]

Acknowledged.

+
+    // add to texture atlas that matches this format
+    bool addToAtlas(GrBatchTextStrike*, GrBatchAtlas::AtlasID*, GrBatchTarget*,
+                    GrMaskFormat, int width, int height, const void* image,
+                    SkIPoint16* loc);
+
+    uint32_t atlasGeneration(GrMaskFormat) const;

[bsalomon, 2015/03/26 17:06:56]

doc?

[joshualitt, 2015/03/26 18:21:43]

Acknowledged.

+
+    void freeAll();
+
+    inline void makeMRU(GrBatchTextStrike* strike) {
+        if (fStrikeList.head() == strike) {
+            return;
+        }
+
+        fStrikeList.remove(strike);
+        fStrikeList.addToHead(strike);
+    }
+
+    GrTexture* getTexture(GrMaskFormat);
+    GrPixelConfig getPixelConfig(GrMaskFormat) const;
+
+    SkDEBUGCODE(void validate() const;)
+
+    void dump() const;
+
+private:
+    enum AtlasType {

[bsalomon, 2015/03/26 17:06:56]

Maybe comment about how this relates to mask type? I assume it really refers to
the config of the backing texture but not the interpretation of the values
(coverage, color).

[joshualitt, 2015/03/26 18:21:43]

I deleted this enum

+        kA8_AtlasType,   //!< 1-byte per pixel
+        k565_AtlasType,  //!< 2-bytes per pixel
+        k8888_AtlasType, //!< 4-bytes per pixel
+
+        kLast_AtlasType = k8888_AtlasType
+    };
+
+    static const int kAtlasCount = kLast_AtlasType + 1;
+
+    static int MaskFormatToAtlasIndex(GrMaskFormat);
+    static GrMaskFormat AtlasIndexToMaskFormat(int atlasIndex);

[bsalomon, 2015/03/26 17:06:56]

Is this 1 to 1?

[joshualitt, 2015/03/26 18:21:43]

Acknowledged.

+
+    GrBatchTextStrike* generateStrike(GrFontScaler*);
+
+    void purgeStrike(GrBatchTextStrike* strike);
+    inline GrBatchAtlas* getAtlas(GrMaskFormat) const;
+
+    static void HandleEviction(GrBatchAtlas::AtlasID, void*);
+
+    typedef SkTInternalLList<GrBatchTextStrike> GrBatchStrikeList;
+
+    SkTDynamicHash<GrBatchTextStrike, GrFontDescKey> fCache;
+    GrBatchStrikeList fStrikeList;
+
+    GrBatchAtlas* fAtlases[kAtlasCount];
+    GrBatchTextStrike* fPreserveStrike;
+};
+
+#endif