Fix the painting of document markers under zoom and variable DPI

third_party/WebKit/Source/platform/graphics/GraphicsContext.cpp

 /*
  * Copyright (C) 2003, 2004, 2005, 2006, 2009 Apple Inc. All rights reserved.
  * Copyright (C) 2013 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
@@ skipping 491 matching lines @@
     drawRect(r1, fillPaint);
     drawRect(r2, fillPaint);
   }
 
   adjustLineToPixelBoundaries(p1, p2, width, penStyle);
   m_canvas->drawLine(p1.x(), p1.y(), p2.x(), p2.y(), paint);
 }
 
 void GraphicsContext::drawLineForDocumentMarker(const FloatPoint& pt,
                                                 float width,
-                                                DocumentMarkerLineStyle style) {
+                                                DocumentMarkerLineStyle style,
+                                                float zoom) {
   if (contextDisabled())
     return;
 
   // Use 2x resources for a device scale factor of 1.5 or above.
+  // This modifes the bitmaps used for the marker, not the overall
+  // scaling of the marker.
   int deviceScaleFactor = m_deviceScaleFactor > 1.5f ? 2 : 1;
 
   // Create the pattern we'll use to draw the underline.
   int index = style == DocumentMarkerGrammarLineStyle ? 1 : 0;
   static SkBitmap* misspellBitmap1x[2] = {0, 0};
   static SkBitmap* misspellBitmap2x[2] = {0, 0};
   SkBitmap** misspellBitmap =
       deviceScaleFactor == 2 ? misspellBitmap2x : misspellBitmap1x;

[f(malita), 2017/02/03 15:53:55]

Looks like we're not taking |zoom| into account when selecting the bitmap, so
we're going to use the low-res version and scale it up at raster time => blurry
results on hidpi?

I presume this is a problem related to --enable-use-zoom-for-dsf, which changed
how DSF is communicated (now a zoom factor instead of m_deviceScaleFactor). 
Would it not suffice to fold it into the deviceScaleFactor calculation above? 
E.g.:

  int deviceScaleFactor = m_deviceScaleFactor * zoom > 1.5f ? 2 : 1;

[Stephen Chennney, 2017/02/03 16:27:45]

That's a good idea. I'll do that.

I also thought of moving away from the bitmaps, but it seems we historical had
good reasons for doing so and I didn't want to mess with that for now. Of
course, that "good reason" may be Apple's, so I'll do some historical analysis
while I wait for baselines.

   if (!misspellBitmap[index]) {
 #if OS(MACOSX)
     // Match the artwork used by the Mac.
     const int rowPixels = 4 * deviceScaleFactor;
     const int colPixels = 3 * deviceScaleFactor;
     SkBitmap bitmap;
     if (!bitmap.tryAllocN32Pixels(rowPixels, colPixels))
       return;
 
     bitmap.eraseARGB(0, 0, 0, 0);
@@ skipping 42 matching lines @@
         row[5] = colors[index][x * 3];
         row[6] = transparentColor;
         row[7] = transparentColor;
       }
     } else
       ASSERT_NOT_REACHED();
 
     misspellBitmap[index] = new SkBitmap(bitmap);
 #else
     // We use a 2-pixel-high misspelling indicator because that seems to be
-    // what WebKit is designed for, and how much room there is in a typical
+    // what Blink is designed for, and how much room there is in a typical
     // page for it.
     const int rowPixels =
         32 * deviceScaleFactor;  // Must be multiple of 4 for pattern below.
     const int colPixels = 2 * deviceScaleFactor;
     SkBitmap bitmap;
     if (!bitmap.tryAllocN32Pixels(rowPixels, colPixels))
       return;
 
     bitmap.eraseARGB(0, 0, 0, 0);
     if (deviceScaleFactor == 1)
       draw1xMarker(&bitmap, index);
     else if (deviceScaleFactor == 2)
       draw2xMarker(&bitmap, index);
     else
       ASSERT_NOT_REACHED();
 
     misspellBitmap[index] = new SkBitmap(bitmap);
 #endif
   }
 
 #if OS(MACOSX)
-  SkScalar originX = WebCoreFloatToSkScalar(pt.x()) * deviceScaleFactor;
-  SkScalar originY = WebCoreFloatToSkScalar(pt.y()) * deviceScaleFactor;
+  // Position already includes zoom and device scale factor.
+  SkScalar originX = WebCoreFloatToSkScalar(pt.x());
+  SkScalar originY = WebCoreFloatToSkScalar(pt.y());
 
-  // Make sure to draw only complete dots.
-  int rowPixels = misspellBitmap[index]->width();
-  float widthMod = fmodf(width * deviceScaleFactor, rowPixels);
-  if (rowPixels - widthMod > deviceScaleFactor)
-    width -= widthMod / deviceScaleFactor;
+  // Make sure to draw only complete dots, and finish inside the marked text.
+  float rowPixels = misspellBitmap[index]->width() * zoom / deviceScaleFactor;
+  float dotCount = floor(width / rowPixels);
+  width = dotCount * rowPixels;
 #else
   SkScalar originX = WebCoreFloatToSkScalar(pt.x());
 
   // Offset it vertically by 1 so that there's some space under the text.
   SkScalar originY = WebCoreFloatToSkScalar(pt.y()) + 1;
-  originX *= deviceScaleFactor;
-  originY *= deviceScaleFactor;
 #endif
 
   SkMatrix localMatrix;
-  localMatrix.setTranslate(originX, originY);
+  localMatrix.setScale(zoom / deviceScaleFactor, zoom / deviceScaleFactor);
+  localMatrix.postTranslate(originX, originY);
 
   SkPaint paint;
   paint.setShader(SkShader::MakeBitmapShader(
       *misspellBitmap[index], SkShader::kRepeat_TileMode,
       SkShader::kRepeat_TileMode, &localMatrix));
 
   SkRect rect;
-  rect.set(originX, originY,
-           originX + WebCoreFloatToSkScalar(width) * deviceScaleFactor,
-           originY + SkIntToScalar(misspellBitmap[index]->height()));
+  rect.set(
+      originX, originY, originX + WebCoreFloatToSkScalar(width),
+      originY +
+          SkIntToScalar(misspellBitmap[index]->height() / deviceScaleFactor) *
+              zoom);
 
-  if (deviceScaleFactor == 2) {
-    save();
-    scale(0.5, 0.5);
-  }
   drawRect(rect, paint);
-  if (deviceScaleFactor == 2)
-    restore();
 }
 
 void GraphicsContext::drawLineForText(const FloatPoint& pt, float width) {
   if (contextDisabled())
     return;
 
   if (width <= 0)
     return;
 
   SkPaint paint;
@@ skipping 758 matching lines @@
   static const SkPMColor colors[] = {
       SkPreMultiplyARGB(0x60, 0xFF, 0x00, 0x00),  // More transparent red
       SkPreMultiplyARGB(0x60, 0xC0, 0xC0, 0xC0)   // More transparent gray
   };
 
   return colors[index];
 }
 #endif
 
 }  // namespace blink