Conservative blob bounds cleanup

src/core/SkTextBlob.cpp

 /*
  * Copyright 2014 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "SkTextBlob.h"
 
 #include "SkReadBuffer.h"
@@ skipping 288 matching lines @@
 
 SkTextBlobBuilder::~SkTextBlobBuilder() {
     if (NULL != fStorage.get()) {
         // We are abandoning runs and must destruct the associated font data.
         // The easiest way to accomplish that is to use the blob destructor.
         build()->unref();
     }
 }
 
 SkRect SkTextBlobBuilder::TightRunBounds(const SkTextBlob::RunRecord& run) {
+    SkASSERT(SkTextBlob::kDefault_Positioning == run.positioning());
+
     SkRect bounds;
-
-    if (SkTextBlob::kDefault_Positioning == run.positioning()) {
-        run.font().measureText(run.glyphBuffer(), run.glyphCount() * sizeof(uint16_t), &bounds);
-        return bounds;

[f(malita), 2015/01/29 19:19:38]

Previously missing the run offset here.

-    }
-
-    SkASSERT(SkTextBlob::kFull_Positioning == run.positioning() ||
-             SkTextBlob::kHorizontal_Positioning == run.positioning());
-
-    SkAutoSTArray<16, SkRect> glyphBounds(run.glyphCount());
-    run.font().getTextWidths(run.glyphBuffer(),
-                             run.glyphCount() * sizeof(uint16_t),
-                             NULL,
-                             glyphBounds.get());
-
-    bounds = SkRect::MakeEmpty();
-    SkScalar* glyphPos = run.posBuffer();
-    for (unsigned i = 0; i < run.glyphCount(); ++i) {
-        if (SkTextBlob::kFull_Positioning == run.positioning()) {
-            // [ x, y, x, y... ]
-            glyphBounds[i].offset(glyphPos[0], glyphPos[1]);
-            SkASSERT(2 == SkTextBlob::ScalarsPerGlyph(run.positioning()));
-            glyphPos += 2;
-        } else {
-            // [ x, x, x... ], const y applied by runBounds.offset(run->offset()) later.
-            glyphBounds[i].offset(glyphPos[0], 0);
-            SkASSERT(1 == SkTextBlob::ScalarsPerGlyph(run.positioning()));
-            glyphPos += 1;
-        }
-
-        bounds.join(glyphBounds[i]);
-    }
-
-    SkASSERT((void*)glyphPos <= SkTextBlob::RunRecord::Next(&run));
+    run.font().measureText(run.glyphBuffer(), run.glyphCount() * sizeof(uint16_t), &bounds);
 
     return bounds.makeOffset(run.offset().x(), run.offset().y());
 }
 
 SkRect SkTextBlobBuilder::ConservativeRunBounds(const SkTextBlob::RunRecord& run) {
-    const SkScalar* glyphPos = run.posBuffer();
-    int posScalars = SkTextBlob::ScalarsPerGlyph(run.positioning());
-
-    SkASSERT(1 == posScalars || 2 == posScalars);
     SkASSERT(run.glyphCount() > 0);
-    SkASSERT((void*)(glyphPos + run.glyphCount() * posScalars) <=
-             SkTextBlob::RunRecord::Next(&run));
+    SkASSERT(SkTextBlob::kFull_Positioning == run.positioning() ||
+             SkTextBlob::kHorizontal_Positioning == run.positioning());
 
     // First, compute the glyph position bbox.
-    SkRect bounds = SkRect::MakeXYWH(glyphPos[0], (2 == posScalars) ? glyphPos[1] : 0, 0, 0);
-    for (unsigned i = 1; i < run.glyphCount(); ++i) {
-        SkScalar xpos = glyphPos[i * posScalars];
-        SkScalar ypos = (2 == posScalars) ? glyphPos[i * posScalars + 1] : 0;
-        bounds.growToInclude(xpos, ypos);
+    const SkScalar* glyphPos = run.posBuffer();
+    SkRect bounds = SkRect::MakeXYWH(glyphPos[0],

[mtklein, 2015/01/29 19:52:03]

SkRect bounds;

switch(..) {
  case H:
  SkScalar l,r = points[0];
  min/max
  bounds = { l, y, r, y };
  break;

  case XY:
  bounds.set(points);
  break;

?

[f(malita), 2015/01/29 20:18:31]

Done.

+        SkTextBlob::kFull_Positioning == run.positioning() ? glyphPos[1] : 0, 0, 0);
+
+    switch (run.positioning()) {
+    case SkTextBlob::kHorizontal_Positioning: {
+        SkASSERT((void*)(glyphPos + run.glyphCount()) <= SkTextBlob::RunRecord::Next(&run));
+        for (unsigned i = 1; i < run.glyphCount(); ++i) {
+            SkScalar x = glyphPos[i];
+            bounds.fLeft  = SkMinScalar(x, bounds.fLeft);
+            bounds.fRight = SkMaxScalar(x, bounds.fRight);
+        }
+    } break;
+    case SkTextBlob::kFull_Positioning: {
+        const SkPoint* glyphPosPts = reinterpret_cast<const SkPoint*>(glyphPos);
+        SkASSERT((void*)(glyphPosPts + run.glyphCount()) <= SkTextBlob::RunRecord::Next(&run));
+        bounds.growToInclude(glyphPosPts + 1, run.glyphCount() - 1);
+    } break;
+    default:
+        SkFAIL("unsupported positioning mode");
     }
 
     // Expand by typeface glyph bounds.
     const SkRect fontBounds = run.font().getFontBounds();
     bounds.fLeft   += fontBounds.left();
     bounds.fTop    += fontBounds.top();
     bounds.fRight  += fontBounds.right();
     bounds.fBottom += fontBounds.bottom();
 
     // Offset by run position.
     return bounds.makeOffset(run.offset().x(), run.offset().y());
 }
 
 void SkTextBlobBuilder::updateDeferredBounds() {
     SkASSERT(!fDeferredBounds || fRunCount > 0);
 
     if (!fDeferredBounds) {
         return;
     }
 
     SkASSERT(fLastRun >= sizeof(SkTextBlob));
     SkTextBlob::RunRecord* run = reinterpret_cast<SkTextBlob::RunRecord*>(fStorage.get() +
                                                                           fLastRun);
     SkASSERT(SkPaint::kGlyphID_TextEncoding == run->font().getTextEncoding());
 
-    SkRect runBounds;
-#ifdef SK_SUPPORT_LEGACY_BLOB_BOUNDS
-    runBounds = TightRunBounds(*run);
-#else
-    // FIXME: conservative bounds for default positioning?
-    if (SkTextBlob::kDefault_Positioning == run->positioning()) {
-        runBounds = TightRunBounds(*run);
-    } else {
-        runBounds = ConservativeRunBounds(*run);
-    }
-#endif
-
+    // FIXME: we should also use conservative bounds for kDefault_Positioning.
+    SkRect runBounds = SkTextBlob::kDefault_Positioning == run->positioning() ?
+                       TightRunBounds(*run) : ConservativeRunBounds(*run);
     fBounds.join(runBounds);
     fDeferredBounds = false;
 }
 
 void SkTextBlobBuilder::reserve(size_t size) {
     // We don't currently pre-allocate, but maybe someday...
     if (fStorageUsed + size <= fStorageSize) {
         return;
     }
 
@@ skipping 151 matching lines @@
 
     fStorageUsed = 0;
     fStorageSize = 0;
     fRunCount = 0;
     fLastRun = 0;
     fBounds.setEmpty();
 
     return blob;
 }