Reorganize ligature caret to use external vector of coord values

c/addfeatures/hotconv/FeatVisitor.cpp

@@ -925,14 +925,14 @@ antlrcpp::Any FeatVisitor::visitGdefAttach(FeatParser::GdefAttachContext *ctx) {
 antlrcpp::Any FeatVisitor::visitGdefLigCaretPos(FeatParser::GdefLigCaretPosContext *ctx) {
     if ( stage != vExtract )
         return nullptr;
-    translateGdefLigCaret(ctx->lookupPattern(), ctx->NUM(), 1);
+    translateGdefLigCaret(ctx->lookupPattern(), ctx->NUM(), false);
     return nullptr;
 }
 
 antlrcpp::Any FeatVisitor::visitGdefLigCaretIndex(FeatParser::GdefLigCaretIndexContext *ctx) {
     if ( stage != vExtract )
         return nullptr;
-    translateGdefLigCaret(ctx->lookupPattern(), ctx->NUM(), 2);
+    translateGdefLigCaret(ctx->lookupPattern(), ctx->NUM(), true);
     return nullptr;
 }
 
@@ -1360,21 +1360,31 @@ void FeatVisitor::translateBaseScript(FeatParser::BaseScriptContext *ctx,
 
 void FeatVisitor::translateGdefLigCaret(FeatParser::LookupPatternContext *pctx,
                                  std::vector<antlr4::tree::TerminalNode *> nv,
-                                 unsigned short format) {
+                                 bool isPoints) {
     assert(stage == vExtract);
 
     GPat::SP gp = getLookupPattern(pctx, false);
     if (gp->patternLen() != 1)
         fc->featMsg(hotERROR, "Only one glyph|glyphClass may be present per"
                               " LigatureCaret statement");
 
-    std::vector<uint16_t> sv;
-    sv.reserve(nv.size());
-    for (auto n : nv)
-        sv.push_back(getNum<uint16_t>(TOK(n)->getText(), 10));
+    if (isPoints) {
+        std::vector<uint16_t> pv;
+        pv.reserve(nv.size());
+        for (auto n : nv)
+            pv.push_back(getNum<int16_t>(TOK(n)->getText(), 10));
 
-    for (GID gid : gp->classes[0].glyphs)
-        fc->g->ctx.GDEFp->addLigCaretEntry(gid, sv, format);
+        for (GID gid : gp->classes[0].glyphs)
+            fc->g->ctx.GDEFp->addLigCaretPoints(gid, pv);
+    } else {
+        std::vector<int16_t> cv;
+        cv.reserve(nv.size());
+        for (auto n : nv)
+            cv.push_back(getNum<uint16_t>(TOK(n)->getText(), 10));
+
+        for (GID gid : gp->classes[0].glyphs)
+            fc->g->ctx.GDEFp->addLigCaretCoords(gid, cv);
+    }
 }
 
 bool FeatVisitor::translateAnchor(FeatParser::AnchorContext *ctx,

c/addfeatures/hotconv/FeatVisitor.h

@@ -155,7 +155,7 @@ class FeatVisitor : public FeatParserBaseVisitor {
     void translateBaseScript(FeatParser::BaseScriptContext *ctx, bool vert, size_t cnt);
     void translateGdefLigCaret(FeatParser::LookupPatternContext *pctx,
                                std::vector<antlr4::tree::TerminalNode *> nv,
-                               unsigned short format);
+                               bool isPoints);
     bool translateAnchor(FeatParser::AnchorContext *ctx, int componentIndex);
 
 

c/addfeatures/hotconv/GDEF.cpp

@@ -188,20 +188,43 @@ bool GDEF::AttachTable::add(GID gid, uint16_t contour) {
     return false;
 }
 
-void GDEF::LigCaretTable::add(GID gid, std::vector<uint16_t> &caretValues, uint16_t format) {
+bool GDEF::LigCaretTable::warnGid(GID gid) {
     /* First, make sure that there is not another LGE for the same glyph
      We don't yet support format 3. */
     for (auto &lge : ligCaretEntries) {
         if (lge.gid == gid) {
             h.g->ctx.feat->dumpGlyph(lge.gid, 0, 0);
-            hotMsg(h.g, hotWARNING, "GDEF Ligature Caret List Table. Glyph '%s' gid '%d'.\n A glyph can have at most one ligature glyph entry. Skipping entry for format '%d'.", h.g->note.array, lge.gid, format);
-            return;
+            hotMsg(h.g, hotWARNING, "GDEF Ligature Caret List Table. Glyph '%s' gid '%d'.\n A glyph can have at most one ligature glyph entry. Skipping entry.", h.g->note.array, lge.gid);
+            return true;
         }
     }
-    LigGlyphEntry lge {gid, format};
+    return false;
+}
+
+void GDEF::LigCaretTable::addCoords(GID gid, std::vector<int16_t> &coords,
+                                    std::vector<int16_t> &values) {
+    if (warnGid(gid))
+        return;
 
-    for (auto cv : caretValues)
-        lge.caretTables.emplace_back(cv, format);
+    LigGlyphEntry lge {gid};
+
+    for (auto &c : coords) {
+        auto cctp = std::make_unique<LigGlyphEntry::CoordCaretTable>(values.size());
+        lge.caretTables.emplace_back(std::move(cctp));
+        values.push_back(c);
+    }
+
+    ligCaretEntries.emplace_back(std::move(lge));
+}
+
+void GDEF::LigCaretTable::addPoints(GID gid, std::vector<uint16_t> &points) {
+    if (warnGid(gid))
+        return;
+
+    LigGlyphEntry lge {gid};
+
+    for (auto &p : points)
+        lge.caretTables.emplace_back(std::make_unique<LigGlyphEntry::PointCaretTable>(p));
 
     ligCaretEntries.emplace_back(std::move(lge));
 }
@@ -265,16 +288,17 @@ Offset GDEF::LigCaretTable::fill(Offset o) {
     std::sort(ligCaretEntries.begin(), ligCaretEntries.end());
 
     cac.coverageBegin();
+    LigGlyphEntry::CaretTable::comparator ctc {h.values};
     for (auto &lge : ligCaretEntries) {
         lge.offset = sz;
         LOffset caretOffset = lge.size(lge.caretTables.size());
         sz += caretOffset;
         /* we will write the caret tables right after each lge entry */
-        std::sort(lge.caretTables.begin(), lge.caretTables.end());
+        std::stable_sort(lge.caretTables.begin(), lge.caretTables.end(), ctc);
         for (auto &ct : lge.caretTables) {
-            ct.offset = caretOffset;
-            caretOffset += ct.size();
-            sz += ct.size();
+            ct->offset = caretOffset;
+            caretOffset += ct->size();
+            sz += ct->size();
         }
         cac.coverageAddGlyph(lge.gid);
     }
@@ -387,19 +411,17 @@ void GDEF::LigCaretTable::write(GDEF *h) {
     OUT2(Coverage);
     OUT2((uint16_t) ligCaretEntries.size());
 
-    for (auto lge : ligCaretEntries)
+    for (auto &lge : ligCaretEntries)
         OUT2(lge.offset);
 
-    for (auto lge : ligCaretEntries) {
+    for (auto &lge : ligCaretEntries) {
         OUT2((uint16_t)lge.caretTables.size());
         /* write offsets */
-        for (auto ct : lge.caretTables)
-            OUT2(ct.offset);
+        for (auto &ct : lge.caretTables)
+            OUT2(ct->offset);
         /* then write caret tables for this lge */
-        for (auto ct : lge.caretTables) {
-            OUT2(ct.format);
-            OUT2(ct.CaretValue);
-        }
+        for (auto &ct : lge.caretTables)
+            ct->write(h, h->values);
     }
     cac.coverageWrite();
 }

c/addfeatures/hotconv/GDEF.h

@@ -6,8 +6,9 @@
 #define ADDFEATURES_HOTCONV_GDEF_H_
 
 #include <array>
-#include <utility>
+#include <memory>
 #include <vector>
+#include <utility>
 
 #include "common.h"
 #include "FeatCtx.h"
@@ -94,40 +95,73 @@ class GDEF {
     struct LigCaretTable {
         struct LigGlyphEntry {
             struct CaretTable {
-                CaretTable(uint16_t cv, uint16_t format) : CaretValue(cv), format(format) {}
-                bool operator < (const CaretTable &rhs) const {
-                    if (format != rhs.format)
-                        return format < rhs.format;
-                    if (format == 1)
-                        return (int16_t) CaretValue < (int16_t) rhs.CaretValue;
-                    else
-                        return (uint16_t) CaretValue < (uint16_t) rhs.CaretValue;
+                struct comparator {
+                    comparator() = delete;
+                    explicit comparator(std::vector<int16_t> &values) : values(values) {}
+                    bool operator()(const std::unique_ptr<CaretTable> &a,
+                                    const std::unique_ptr<CaretTable> &b) const {
+                        return a->sortValue(values) < b->sortValue(values);
+                    }
+                    std::vector<int16_t> &values;
+                };
+                virtual LOffset size() = 0;
+                virtual uint16_t format() = 0;
+                virtual void write(GDEF *h, std::vector<int16_t> &values) = 0;
+                virtual int16_t sortValue(std::vector<int16_t> &values) {
+                    return 0;
                 }
-                static LOffset size() { return sizeof(uint16_t) * 2; }
                 Offset offset {0};
-                uint16_t CaretValue {0};
-                uint16_t format {0};
             };
-
+            struct CoordCaretTable : public CaretTable {
+                CoordCaretTable() = delete;
+                explicit CoordCaretTable(uint32_t vi) : valueIndex(vi) {}
+                LOffset size() override {
+                    return sizeof(uint16_t) + sizeof(int16_t);
+                }
+                int16_t sortValue(std::vector<int16_t> &values) override {
+                    return values[valueIndex];
+                }
+                uint16_t format() override { return 1; }
+                void write(GDEF *h, std::vector<int16_t> &values) override {
+                    OUT2(format());
+                    OUT2(values[valueIndex]);
+                }
+                uint32_t valueIndex;
+            };
+            struct PointCaretTable : public CaretTable {
+                // Don't sort point records
+                PointCaretTable() = delete;
+                explicit PointCaretTable(uint16_t point) : point(point) {}
+                LOffset size() override { return 2 * sizeof(uint16_t); }
+                uint16_t format() override { return 2; }
+                void write(GDEF *h, std::vector<int16_t> &values) override {
+                    OUT2(format());
+                    OUT2(point);
+                }
+                uint16_t point;
+            };
             static LOffset size(uint32_t numCarets) {
                 return sizeof(uint16_t) * (1 + numCarets);
             }
             bool operator < (const LigGlyphEntry &rhs) const {
                 return gid < rhs.gid;
             }
-            LigGlyphEntry(GID gid, uint16_t format) : gid(gid), format(format) {}
+            LigGlyphEntry() = delete;
+            explicit LigGlyphEntry(GID gid) : gid(gid) {}
             GID gid {GID_UNDEF};
-            uint16_t format {0};
             Offset offset {0};
-            std::vector<CaretTable> caretTables;
+            std::vector<std::unique_ptr<CaretTable>> caretTables;
         };
 
         LigCaretTable() = delete;
         explicit LigCaretTable(GDEF &h) : cac(h.g), h(h) {}
         static LOffset size(uint32_t glyphCount) {
             return sizeof(uint16_t) * (2 + glyphCount);
         }
-        void add(GID gid, std::vector<uint16_t> &caretValues, uint16_t format);
+        bool warnGid(GID gid);
+        void addCoords(GID gid, std::vector<int16_t> &coords,
+                       std::vector<int16_t> &values);
+        void addPoints(GID gid, std::vector<uint16_t> &points);
         Offset fill(Offset offset);
         void write(GDEF *h);
 
@@ -183,9 +217,11 @@ class GDEF {
     bool addAttachEntry(GID gid, uint16_t contour) {
         return attachTable.add(gid, contour);
     }
-    void addLigCaretEntry(GID gid, std::vector<uint16_t> &caretValues,
-                          uint16_t format) {
-        ligCaretTable.add(gid, caretValues, format);
+    void addLigCaretCoords(GID gid, std::vector<int16_t> &coords) {
+        ligCaretTable.addCoords(gid, coords, values);
+    }
+    void addLigCaretPoints(GID gid, std::vector<uint16_t> &points) {
+        ligCaretTable.addPoints(gid, points);
     }
     uint16_t addGlyphMarkClass(GPat::ClassRec &&markClass) {
         return markAttachClassTable.add(std::move(markClass));
@@ -206,6 +242,8 @@ class GDEF {
     LigCaretTable ligCaretTable;
     MarkAttachClassTable markAttachClassTable;
     MarkSetFilteringTable markSetClassTable;
+
+    std::vector<int16_t> values;
 };
 
 #endif  // ADDFEATURES_HOTCONV_GDEF_H_

c/shared/include/varsupport.h

@@ -10,6 +10,7 @@
 #include <map>
 #include <memory>
 #include <vector>
+#include <utility>
 
 #include "sfntread.h"
 #include "supportfp.h"
@@ -166,15 +167,15 @@ class var_location {
         }
     };
     var_location() = delete;
-    var_location(std::vector<var_F2dot14> &l) : alocs(std::move(l)) {}
-    var_location(const std::vector<var_F2dot14> &l) : alocs(l) {}
+    explicit var_location(std::vector<var_F2dot14> &l) : alocs(std::move(l)) {}
+    explicit var_location(const std::vector<var_F2dot14> &l) : alocs(l) {}
     bool operator==(const var_location &o) const { return alocs == o.alocs; }
     bool operator<(const var_location &o) const { return alocs < o.alocs; }
     auto at(int i) const { return alocs.at(i); }
     auto size() const { return alocs.size(); }
     void toerr() const {
         int i {0};
-        for (auto f2d: alocs) {
+        for (auto f2d : alocs) {
             if (i++ > 0)
                 std::cerr << ", ";
             std::cerr << var_F2dot14ToFloat(f2d);
@@ -199,7 +200,7 @@ class var_location_map {
     }
     void toerr() {
         int i {0};
-        for (auto &loc: locvec) {
+        for (auto &loc : locvec) {
             std::cerr << i++ << " ";
             loc->toerr();
             std::cerr << std::endl;

c/shared/varsupport.cpp

@@ -261,7 +261,7 @@ bool var_axes::getAxis(uint16_t index, ctlTag *tag, Fixed *minValue,
 
 int16_t var_axes::getAxisIndex(ctlTag tag) {
     int i = 0;
-    for (auto &a: axes) {
+    for (auto &a : axes) {
         if (a.tag == tag)
             return i;
         i++;

tests/addfeatures_data/expected_output/fealib/LigatureCaretByIndex.ttx

@@ -1,5 +1,5 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<ttFont>
+<ttFont sfntVersion="OTTO" ttLibVersion="4.47">
 
   <GDEF>
     <Version value="0x00010000"/>
@@ -19,10 +19,10 @@
       <LigGlyph index="1">
         <!-- CaretCount=2 -->
         <CaretValue index="0" Format="2">
-          <CaretValuePoint value="33"/>
+          <CaretValuePoint value="66"/>
         </CaretValue>
         <CaretValue index="1" Format="2">
-          <CaretValuePoint value="66"/>
+          <CaretValuePoint value="33"/>
         </CaretValue>
       </LigGlyph>
       <LigGlyph index="2">