Bug 1784315 - Ensure the clipping applied to glyph cache entries matches. r=aosmond,gfx-reviewers

If viewport clipping is applied to the bounds of glyphs, we can end up finding the
wrong entry if the clipping would differ, since we no longer compare the bounds to
the entry bounds when searching for a matching entry in an effort to reduce the
amount of bounds computations in the common case.

To workaround this, each entry now remembers its full, unclipped bounds. Upon
finding a candidate entry, we offset the full bounds to the new offset and reapply
clipping. If the bounds then match with the old clipped bounds with the new offset
applied, then we know the amount of applied clipping is the same, and it is safe
to reuse the entry.

Differential Revision: https://phabricator.services.mozilla.com/D154490

dom/canvas/DrawTargetWebgl.cpp

@@ -1912,11 +1912,13 @@ static inline bool HasMatchingScale(const Matrix& aTransform1,
 
 // Determines if an existing path cache entry matches an incoming path and
 // pattern.
-bool PathCacheEntry::MatchesPath(const SkPath& aPath, const Pattern* aPattern,
-                                 const StrokeOptions* aStrokeOptions,
-                                 const Matrix& aTransform,
-                                 const IntRect& aBounds, const Point& aOrigin,
-                                 HashNumber aHash, float aSigma) {
+inline bool PathCacheEntry::MatchesPath(const SkPath& aPath,
+                                        const Pattern* aPattern,
+                                        const StrokeOptions* aStrokeOptions,
+                                        const Matrix& aTransform,
+                                        const IntRect& aBounds,
+                                        const Point& aOrigin, HashNumber aHash,
+                                        float aSigma) {
   return aHash == mHash && HasMatchingScale(aTransform, mTransform) &&
          // Ensure the clipped relative bounds fit inside those of the entry
          aBounds.x - aOrigin.x >= mBounds.x - mOrigin.x &&
@@ -2495,11 +2497,10 @@ HashNumber GlyphCacheEntry::HashGlyphs(const GlyphBuffer& aBuffer,
 }
 
 // Determines if an existing glyph cache entry matches an incoming text run.
-bool GlyphCacheEntry::MatchesGlyphs(const GlyphBuffer& aBuffer,
-                                    const DeviceColor& aColor,
-                                    const Matrix& aTransform,
-                                    const IntPoint& aQuantizeScale,
-                                    HashNumber aHash) {
+inline bool GlyphCacheEntry::MatchesGlyphs(
+    const GlyphBuffer& aBuffer, const DeviceColor& aColor,
+    const Matrix& aTransform, const IntPoint& aQuantizeOffset,
+    const IntPoint& aBoundsOffset, const IntRect& aClipRect, HashNumber aHash) {
   // First check if the hash matches to quickly reject the text run before any
   // more expensive checking. If it matches, then check if the color and
   // transform are the same.
@@ -2507,33 +2508,41 @@ bool GlyphCacheEntry::MatchesGlyphs(const GlyphBuffer& aBuffer,
       aColor != mColor || !HasMatchingScale(aTransform, mTransform)) {
     return false;
   }
-  IntPoint offset = QuantizeOffset(aTransform, aQuantizeScale, aBuffer);
   // Finally check if all glyphs and their quantized positions match.
   for (size_t i = 0; i < aBuffer.mNumGlyphs; i++) {
     const Glyph& dst = mBuffer.mGlyphs[i];
     const Glyph& src = aBuffer.mGlyphs[i];
     if (dst.mIndex != src.mIndex ||
-        dst.mPosition !=
-            Point(QuantizePosition(aTransform, offset, src.mPosition))) {
+        dst.mPosition != Point(QuantizePosition(aTransform, aQuantizeOffset,
+                                                src.mPosition))) {
       return false;
     }
   }
-  return true;
+  // Verify that the full bounds, once translated and clipped, are equal to the
+  // clipped bounds.
+  return (mFullBounds + aBoundsOffset)
+      .Intersect(aClipRect)
+      .IsEqualEdges(GetBounds() + aBoundsOffset);
 }
 
 GlyphCacheEntry::GlyphCacheEntry(const GlyphBuffer& aBuffer,
                                  const DeviceColor& aColor,
                                  const Matrix& aTransform,
                                  const IntPoint& aQuantizeScale,
-                                 const IntRect& aBounds, HashNumber aHash)
+                                 const IntRect& aBounds,
+                                 const IntRect& aFullBounds, HashNumber aHash)
     : CacheEntryImpl<GlyphCacheEntry>(aTransform, aBounds, aHash),
-      mColor(aColor) {
+      mColor(aColor),
+      mFullBounds(aFullBounds) {
   // Store a copy of the glyph buffer with positions already quantized for fast
   // comparison later.
   Glyph* glyphs = new Glyph[aBuffer.mNumGlyphs];
   IntPoint offset = QuantizeOffset(aTransform, aQuantizeScale, aBuffer);
   // Make the bounds relative to the offset so we can add a new offset later.
-  mBounds -= IntPoint(offset.x / aQuantizeScale.x, offset.y / aQuantizeScale.y);
+  IntPoint boundsOffset(offset.x / aQuantizeScale.x,
+                        offset.y / aQuantizeScale.y);
+  mBounds -= boundsOffset;
+  mFullBounds -= boundsOffset;
   for (size_t i = 0; i < aBuffer.mNumGlyphs; i++) {
     Glyph& dst = glyphs[i];
     const Glyph& src = aBuffer.mGlyphs[i];
@@ -2552,10 +2561,13 @@ GlyphCacheEntry::~GlyphCacheEntry() { delete[] mBuffer.mGlyphs; }
 already_AddRefed<GlyphCacheEntry> GlyphCache::FindEntry(
     const GlyphBuffer& aBuffer, const DeviceColor& aColor,
     const Matrix& aTransform, const IntPoint& aQuantizeScale,
-    HashNumber aHash) {
+    const IntRect& aClipRect, HashNumber aHash) {
+  IntPoint offset = QuantizeOffset(aTransform, aQuantizeScale, aBuffer);
+  IntPoint boundsOffset(offset.x / aQuantizeScale.x,
+                        offset.y / aQuantizeScale.y);
   for (const RefPtr<GlyphCacheEntry>& entry : GetChain(aHash)) {
-    if (entry->MatchesGlyphs(aBuffer, aColor, aTransform, aQuantizeScale,
-                             aHash)) {
+    if (entry->MatchesGlyphs(aBuffer, aColor, aTransform, offset, boundsOffset,
+                             aClipRect, aHash)) {
       return do_AddRef(entry);
     }
   }
@@ -2566,9 +2578,9 @@ already_AddRefed<GlyphCacheEntry> GlyphCache::FindEntry(
 already_AddRefed<GlyphCacheEntry> GlyphCache::InsertEntry(
     const GlyphBuffer& aBuffer, const DeviceColor& aColor,
     const Matrix& aTransform, const IntPoint& aQuantizeScale,
-    const IntRect& aBounds, HashNumber aHash) {
+    const IntRect& aBounds, const IntRect& aFullBounds, HashNumber aHash) {
   RefPtr<GlyphCacheEntry> entry = new GlyphCacheEntry(
-      aBuffer, aColor, aTransform, aQuantizeScale, aBounds, aHash);
+      aBuffer, aColor, aTransform, aQuantizeScale, aBounds, aFullBounds, aHash);
   Insert(entry);
   return entry.forget();
 }
@@ -2665,8 +2677,9 @@ bool DrawTargetWebgl::SharedContext::FillGlyphsAccel(
       useBitmaps
           ? color
           : DeviceColor::Mask(aUseSubpixelAA ? 1 : 0, lightOnDark ? 1 : 0);
+  IntRect clipRect(IntPoint(), mViewportSize);
   RefPtr<GlyphCacheEntry> entry = cache->FindEntry(
-      aBuffer, colorOrMask, quantizeTransform, quantizeScale, hash);
+      aBuffer, colorOrMask, quantizeTransform, quantizeScale, clipRect, hash);
   if (!entry) {
     // For small text runs, bounds computations can be expensive relative to the
     // cost of looking up a cache result. Avoid doing local bounds computations
@@ -2678,16 +2691,19 @@ bool DrawTargetWebgl::SharedContext::FillGlyphsAccel(
     }
     // Transform the local bounds into device space so that we know how big
     // the cached texture will be.
-    Rect xformBounds = currentTransform.TransformBounds(*bounds).Intersect(
-        Rect(IntRect(IntPoint(), mViewportSize)));
+    Rect xformBounds = currentTransform.TransformBounds(*bounds);
+    // Check if the transform flattens out the bounds before rounding.
     if (xformBounds.IsEmpty()) {
       return true;
     }
-    // Ensure there is a clear border around the text.
-    xformBounds.Inflate(2);
-    IntRect intBounds = RoundedOut(xformBounds);
+    IntRect fullBounds = RoundedOut(currentTransform.TransformBounds(*bounds));
+    IntRect clipBounds = fullBounds.Intersect(clipRect);
+    // Check if the bounds are completely clipped out.
+    if (clipBounds.IsEmpty()) {
+      return true;
+    }
     entry = cache->InsertEntry(aBuffer, colorOrMask, quantizeTransform,
-                               quantizeScale, intBounds, hash);
+                               quantizeScale, clipBounds, fullBounds, hash);
     if (!entry) {
       return false;
     }
@@ -2702,6 +2718,9 @@ bool DrawTargetWebgl::SharedContext::FillGlyphsAccel(
       QuantizeOffset(quantizeTransform, quantizeScale, aBuffer);
   intBounds +=
       IntPoint(newOffset.x / quantizeScale.x, newOffset.y / quantizeScale.y);
+  // Ensure there is a clear border around the text. This must be applied only
+  // after clipping so that we always have some border texels for filtering.
+  intBounds.Inflate(2);
 
   RefPtr<TextureHandle> handle = entry->GetHandle();
   if (handle && handle->IsValid()) {

dom/canvas/DrawTargetWebglInternal.h

@@ -311,13 +311,15 @@ class GlyphCacheEntry : public CacheEntryImpl<GlyphCacheEntry> {
 
   GlyphCacheEntry(const GlyphBuffer& aBuffer, const DeviceColor& aColor,
                   const Matrix& aTransform, const IntPoint& aQuantizeScale,
-                  const IntRect& aBounds, HashNumber aHash);
+                  const IntRect& aBounds, const IntRect& aFullBounds,
+                  HashNumber aHash);
   ~GlyphCacheEntry();
 
   const GlyphBuffer& GetGlyphBuffer() const { return mBuffer; }
 
   bool MatchesGlyphs(const GlyphBuffer& aBuffer, const DeviceColor& aColor,
-                     const Matrix& aTransform, const IntPoint& aQuantizeScale,
+                     const Matrix& aTransform, const IntPoint& aQuantizeOffset,
+                     const IntPoint& aBoundsOffset, const IntRect& aClipRect,
                      HashNumber aHash);
 
   static HashNumber HashGlyphs(const GlyphBuffer& aBuffer,
@@ -329,6 +331,8 @@ class GlyphCacheEntry : public CacheEntryImpl<GlyphCacheEntry> {
   GlyphBuffer mBuffer = {nullptr, 0};
   // The color of the text run.
   DeviceColor mColor;
+  // The full bounds of the text run without any clipping applied.
+  IntRect mFullBounds;
 };
 
 // GlyphCache maintains a list of GlyphCacheEntry's representing previously
@@ -347,14 +351,13 @@ class GlyphCache : public LinkedListElement<GlyphCache>,
                                               const DeviceColor& aColor,
                                               const Matrix& aTransform,
                                               const IntPoint& aQuantizeScale,
+                                              const IntRect& aClipRect,
                                               HashNumber aHash);
 
-  already_AddRefed<GlyphCacheEntry> InsertEntry(const GlyphBuffer& aBuffer,
-                                                const DeviceColor& aColor,
-                                                const Matrix& aTransform,
-                                                const IntPoint& aQuantizeScale,
-                                                const IntRect& aBounds,
-                                                HashNumber aHash);
+  already_AddRefed<GlyphCacheEntry> InsertEntry(
+      const GlyphBuffer& aBuffer, const DeviceColor& aColor,
+      const Matrix& aTransform, const IntPoint& aQuantizeScale,
+      const IntRect& aBounds, const IntRect& aFullBounds, HashNumber aHash);
 
  private:
   // Weak pointer to the owning font