<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[161577] trunk/Source</title>
</head>
<body>

<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt;  }
#msg dl a { font-weight: bold}
#msg dl a:link    { color:#fc3; }
#msg dl a:active  { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff  {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/161577">161577</a></dd>
<dt>Author</dt> <dd>bburg@apple.com</dd>
<dt>Date</dt> <dd>2014-01-09 13:21:53 -0800 (Thu, 09 Jan 2014)</dd>
</dl>

<h3>Log Message</h3>
<pre>REGRESSION (<a href="http://trac.webkit.org/projects/webkit/changeset/160152">r160152</a>): Selection drag snapshot doesn't appear or has the wrong content on Retina
https://bugs.webkit.org/show_bug.cgi?id=125375

Reviewed by Darin Adler.

Source/WebCore:

Move scaling of drag images by the device scale factor out of DragClient
and into WebCore. This removes several redundant copies and scaling operations.

Fix scaling bugs that were cancelled out by over-allocating the backing store.

* page/DragController.cpp:
(WebCore::DragController::startDrag): Scale the drag image for a link
according to the device scale factor before giving it to the OS.

(WebCore::DragController::doImageDrag): Scale the drag image for an image
according to the device scale factor before giving it to the OS.

* page/FrameSnapshotting.cpp:
(WebCore::snapshotFrameRect): Don't pre-scale or clip the snapshot. The
ImageBuffer does this already.

* platform/DragImage.cpp:
(WebCore::createDragImageFromSnapshot): Don't scale the backing store
when copying an ImageBuffer into an Image.

* platform/graphics/cg/ImageBufferCG.cpp:
(WebCore::ImageBuffer::copyImage): Draw the image in user-space coordinates,
not in backing-store coordinates. Remove unnecessary assertions. Crop the
buffer before drawing the image into it.

Source/WebKit2:

Remove scaling from WebDragClient because it is now selectively
performed by WebCore according to the drag image source.

* WebProcess/WebCoreSupport/mac/WebDragClientMac.mm:
(WebKit::WebDragClient::startDrag): Don't scale the provided drag image.</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCorepageDragControllercpp">trunk/Source/WebCore/page/DragController.cpp</a></li>
<li><a href="#trunkSourceWebCorepageFrameSnapshottingcpp">trunk/Source/WebCore/page/FrameSnapshotting.cpp</a></li>
<li><a href="#trunkSourceWebCoreplatformDragImagecpp">trunk/Source/WebCore/platform/DragImage.cpp</a></li>
<li><a href="#trunkSourceWebCoreplatformgraphicscgImageBufferCGcpp">trunk/Source/WebCore/platform/graphics/cg/ImageBufferCG.cpp</a></li>
<li><a href="#trunkSourceWebKit2ChangeLog">trunk/Source/WebKit2/ChangeLog</a></li>
<li><a href="#trunkSourceWebKit2WebProcessWebCoreSupportmacWebDragClientMacmm">trunk/Source/WebKit2/WebProcess/WebCoreSupport/mac/WebDragClientMac.mm</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (161576 => 161577)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog        2014-01-09 21:19:26 UTC (rev 161576)
+++ trunk/Source/WebCore/ChangeLog        2014-01-09 21:21:53 UTC (rev 161577)
</span><span class="lines">@@ -1,3 +1,35 @@
</span><ins>+2014-01-09  Brian Burg  &lt;bburg@apple.com&gt;
+
+        REGRESSION (r160152): Selection drag snapshot doesn't appear or has the wrong content on Retina
+        https://bugs.webkit.org/show_bug.cgi?id=125375
+
+        Reviewed by Darin Adler.
+
+        Move scaling of drag images by the device scale factor out of DragClient
+        and into WebCore. This removes several redundant copies and scaling operations.
+
+        Fix scaling bugs that were cancelled out by over-allocating the backing store.
+
+        * page/DragController.cpp:
+        (WebCore::DragController::startDrag): Scale the drag image for a link
+        according to the device scale factor before giving it to the OS.
+
+        (WebCore::DragController::doImageDrag): Scale the drag image for an image
+        according to the device scale factor before giving it to the OS.
+
+        * page/FrameSnapshotting.cpp:
+        (WebCore::snapshotFrameRect): Don't pre-scale or clip the snapshot. The
+        ImageBuffer does this already.
+
+        * platform/DragImage.cpp:
+        (WebCore::createDragImageFromSnapshot): Don't scale the backing store
+        when copying an ImageBuffer into an Image.
+
+        * platform/graphics/cg/ImageBufferCG.cpp:
+        (WebCore::ImageBuffer::copyImage): Draw the image in user-space coordinates,
+        not in backing-store coordinates. Remove unnecessary assertions. Crop the
+        buffer before drawing the image into it.
+
</ins><span class="cx"> 2014-01-09  Myles C. Maxfield  &lt;mmaxfield@apple.com&gt;
</span><span class="cx"> 
</span><span class="cx">         Narrow underlines are too tall
</span></span></pre></div>
<a id="trunkSourceWebCorepageDragControllercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/page/DragController.cpp (161576 => 161577)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/page/DragController.cpp        2014-01-09 21:19:26 UTC (rev 161576)
+++ trunk/Source/WebCore/page/DragController.cpp        2014-01-09 21:21:53 UTC (rev 161577)
</span><span class="lines">@@ -828,6 +828,8 @@
</span><span class="cx">             IntSize size = dragImageSize(dragImage);
</span><span class="cx">             m_dragOffset = IntPoint(-size.width() / 2, -LinkDragBorderInset);
</span><span class="cx">             dragLoc = IntPoint(mouseDraggedPoint.x() + m_dragOffset.x(), mouseDraggedPoint.y() + m_dragOffset.y());
</span><ins>+            // Later code expects the drag image to be scaled by device's scale factor.
+            dragImage = scaleDragImage(dragImage, FloatSize(m_page.deviceScaleFactor(), m_page.deviceScaleFactor()));
</ins><span class="cx">         }
</span><span class="cx">         doSystemDrag(dragImage, dragLoc, mouseDraggedPoint, clipboard, src, true);
</span><span class="cx">     } else if (state.type == DragSourceActionDHTML) {
</span><span class="lines">@@ -869,6 +871,7 @@
</span><span class="cx">         dragImage = fitDragImageToMaxSize(dragImage, layoutRect.size(), maxDragImageSize());
</span><span class="cx">         IntSize fittedSize = dragImageSize(dragImage);
</span><span class="cx"> 
</span><ins>+        dragImage = scaleDragImage(dragImage, FloatSize(m_page.deviceScaleFactor(), m_page.deviceScaleFactor()));
</ins><span class="cx">         dragImage = dissolveDragImageToFraction(dragImage, DragImageAlpha);
</span><span class="cx"> 
</span><span class="cx">         // Properly orient the drag image and orient it differently if it's smaller than the original.
</span></span></pre></div>
<a id="trunkSourceWebCorepageFrameSnapshottingcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/page/FrameSnapshotting.cpp (161576 => 161577)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/page/FrameSnapshotting.cpp        2014-01-09 21:19:26 UTC (rev 161576)
+++ trunk/Source/WebCore/page/FrameSnapshotting.cpp        2014-01-09 21:21:53 UTC (rev 161577)
</span><span class="lines">@@ -90,17 +90,12 @@
</span><span class="cx">     // Other paint behaviors are set by paintContentsForSnapshot.
</span><span class="cx">     frame.view()-&gt;setPaintBehavior(paintBehavior);
</span><span class="cx"> 
</span><del>-    float deviceScaleFactor = frame.page()-&gt;deviceScaleFactor();
-    IntRect usedRect(imageRect);
-    usedRect.scale(deviceScaleFactor);
-
-    std::unique_ptr&lt;ImageBuffer&gt; buffer = ImageBuffer::create(usedRect.size(), deviceScaleFactor, ColorSpaceDeviceRGB);
</del><ins>+    std::unique_ptr&lt;ImageBuffer&gt; buffer = ImageBuffer::create(imageRect.size(), frame.page()-&gt;deviceScaleFactor(), ColorSpaceDeviceRGB);
</ins><span class="cx">     if (!buffer)
</span><span class="cx">         return nullptr;
</span><del>-    buffer-&gt;context()-&gt;translate(-usedRect.x(), -usedRect.y());
-    buffer-&gt;context()-&gt;clip(FloatRect(0, 0, usedRect.maxX(), usedRect.maxY()));
</del><ins>+    buffer-&gt;context()-&gt;translate(-imageRect.x(), -imageRect.y());
</ins><span class="cx"> 
</span><del>-    frame.view()-&gt;paintContentsForSnapshot(buffer-&gt;context(), usedRect, shouldIncludeSelection, coordinateSpace);
</del><ins>+    frame.view()-&gt;paintContentsForSnapshot(buffer-&gt;context(), imageRect, shouldIncludeSelection, coordinateSpace);
</ins><span class="cx">     return buffer;
</span><span class="cx"> }
</span><span class="cx"> 
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformDragImagecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/DragImage.cpp (161576 => 161577)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/platform/DragImage.cpp        2014-01-09 21:19:26 UTC (rev 161576)
+++ trunk/Source/WebCore/platform/DragImage.cpp        2014-01-09 21:21:53 UTC (rev 161577)
</span><span class="lines">@@ -108,7 +108,7 @@
</span><span class="cx"> #else
</span><span class="cx">     UNUSED_PARAM(node);
</span><span class="cx"> #endif
</span><del>-    RefPtr&lt;Image&gt; image = snapshot-&gt;copyImage(ImageBuffer::fastCopyImageMode());
</del><ins>+    RefPtr&lt;Image&gt; image = snapshot-&gt;copyImage(ImageBuffer::fastCopyImageMode(), Unscaled);
</ins><span class="cx">     if (!image)
</span><span class="cx">         return nullptr;
</span><span class="cx">     return createDragImageFromImage(image.get(), orientation);
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformgraphicscgImageBufferCGcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/graphics/cg/ImageBufferCG.cpp (161576 => 161577)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/platform/graphics/cg/ImageBufferCG.cpp        2014-01-09 21:19:26 UTC (rev 161576)
+++ trunk/Source/WebCore/platform/graphics/cg/ImageBufferCG.cpp        2014-01-09 21:21:53 UTC (rev 161577)
</span><span class="lines">@@ -212,24 +212,22 @@
</span><span class="cx"> PassRefPtr&lt;Image&gt; ImageBuffer::copyImage(BackingStoreCopy copyBehavior, ScaleBehavior scaleBehavior) const
</span><span class="cx"> {
</span><span class="cx">     RetainPtr&lt;CGImageRef&gt; image;
</span><del>-    if (m_resolutionScale == 1 || scaleBehavior == Unscaled)
</del><ins>+    if (m_resolutionScale == 1 || scaleBehavior == Unscaled) {
</ins><span class="cx">         image = copyNativeImage(copyBehavior);
</span><del>-    else {
</del><ins>+        image = createCroppedImageIfNecessary(image.get(), internalSize());
+    } else {
</ins><span class="cx">         image = copyNativeImage(DontCopyBackingStore);
</span><span class="cx">         RetainPtr&lt;CGContextRef&gt; context = adoptCF(CGBitmapContextCreate(0, logicalSize().width(), logicalSize().height(), 8, 4 * logicalSize().width(), deviceRGBColorSpaceRef(), kCGImageAlphaPremultipliedLast));
</span><span class="cx">         CGContextSetBlendMode(context.get(), kCGBlendModeCopy);
</span><del>-        CGContextDrawImage(context.get(), CGRectMake(0, 0, m_data.m_backingStoreSize.width(), m_data.m_backingStoreSize.height()), image.get());
</del><ins>+        CGContextClipToRect(context.get(), FloatRect(FloatPoint::zero(), logicalSize()));
+        FloatSize imageSizeInUserSpace = scaleSizeToUserSpace(logicalSize(), m_data.m_backingStoreSize, internalSize());
+        CGContextDrawImage(context.get(), FloatRect(FloatPoint::zero(), imageSizeInUserSpace), image.get());
</ins><span class="cx">         image = adoptCF(CGBitmapContextCreateImage(context.get()));
</span><span class="cx">     }
</span><del>-    
-    image = createCroppedImageIfNecessary(image.get(), internalSize());
</del><span class="cx"> 
</span><span class="cx">     if (!image)
</span><del>-        return 0;
</del><ins>+        return nullptr;
</ins><span class="cx"> 
</span><del>-    ASSERT(CGImageGetWidth(image.get()) == static_cast&lt;size_t&gt;(m_logicalSize.width()));
-    ASSERT(CGImageGetHeight(image.get()) == static_cast&lt;size_t&gt;(m_logicalSize.height()));
-
</del><span class="cx">     RefPtr&lt;BitmapImage&gt; bitmapImage = BitmapImage::create(image.get());
</span><span class="cx">     bitmapImage-&gt;setSpaceSize(spaceSize());
</span><span class="cx"> 
</span><span class="lines">@@ -480,8 +478,8 @@
</span><span class="cx">         RetainPtr&lt;CGContextRef&gt; context = adoptCF(CGBitmapContextCreate(0, logicalSize().width(), logicalSize().height(), 8, 4 * logicalSize().width(), deviceRGBColorSpaceRef(), kCGImageAlphaPremultipliedLast));
</span><span class="cx">         CGContextSetBlendMode(context.get(), kCGBlendModeCopy);
</span><span class="cx">         CGContextClipToRect(context.get(), CGRectMake(0, 0, logicalSize().width(), logicalSize().height()));
</span><del>-        FloatSize imageRectInUserBounds = scaleSizeToUserSpace(logicalSize(), m_data.m_backingStoreSize, internalSize());
-        CGContextDrawImage(context.get(), CGRectMake(0, 0, imageRectInUserBounds.width(), imageRectInUserBounds.height()), image.get());
</del><ins>+        FloatSize imageSizeInUserSpace = scaleSizeToUserSpace(logicalSize(), m_data.m_backingStoreSize, internalSize());
+        CGContextDrawImage(context.get(), CGRectMake(0, 0, imageSizeInUserSpace.width(), imageSizeInUserSpace.height()), image.get());
</ins><span class="cx">         image = adoptCF(CGBitmapContextCreateImage(context.get()));
</span><span class="cx">     }
</span><span class="cx"> 
</span></span></pre></div>
<a id="trunkSourceWebKit2ChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/ChangeLog (161576 => 161577)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/ChangeLog        2014-01-09 21:19:26 UTC (rev 161576)
+++ trunk/Source/WebKit2/ChangeLog        2014-01-09 21:21:53 UTC (rev 161577)
</span><span class="lines">@@ -1,3 +1,16 @@
</span><ins>+2014-01-09  Brian Burg  &lt;bburg@apple.com&gt;
+
+        REGRESSION (r160152): Selection drag snapshot doesn't appear or has the wrong content on Retina
+        https://bugs.webkit.org/show_bug.cgi?id=125375
+
+        Reviewed by Darin Adler.
+
+        Remove scaling from WebDragClient because it is now selectively
+        performed by WebCore according to the drag image source.
+
+        * WebProcess/WebCoreSupport/mac/WebDragClientMac.mm:
+        (WebKit::WebDragClient::startDrag): Don't scale the provided drag image.
+
</ins><span class="cx"> 2014-01-09  Tim Horton  &lt;timothy_horton@apple.com&gt;
</span><span class="cx"> 
</span><span class="cx">         WebKit2 View Gestures: Support plugins that take over the page scale gesture
</span></span></pre></div>
<a id="trunkSourceWebKit2WebProcessWebCoreSupportmacWebDragClientMacmm"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/WebProcess/WebCoreSupport/mac/WebDragClientMac.mm (161576 => 161577)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/WebProcess/WebCoreSupport/mac/WebDragClientMac.mm        2014-01-09 21:19:26 UTC (rev 161576)
+++ trunk/Source/WebKit2/WebProcess/WebCoreSupport/mac/WebDragClientMac.mm        2014-01-09 21:21:53 UTC (rev 161577)
</span><span class="lines">@@ -79,7 +79,6 @@
</span><span class="cx"> void WebDragClient::startDrag(RetainPtr&lt;NSImage&gt; image, const IntPoint&amp; point, const IntPoint&amp;, Clipboard&amp;, Frame&amp; frame, bool linkDrag)
</span><span class="cx"> {
</span><span class="cx">     IntSize bitmapSize([image.get() size]);
</span><del>-    bitmapSize.scale(frame.page()-&gt;deviceScaleFactor());
</del><span class="cx">     RefPtr&lt;ShareableBitmap&gt; bitmap = convertImageToBitmap(image.get(), bitmapSize);
</span><span class="cx">     ShareableBitmap::Handle handle;
</span><span class="cx">     if (!bitmap || !bitmap-&gt;createHandle(handle))
</span></span></pre>
</div>
</div>

</body>
</html>