Changeset 161577 in webkit

Timestamp:

Jan 9, 2014 1:21:53 PM (10 years ago)

Author:

BJ Burg

Message:

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.

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.

Location:

trunk/Source

Files:

• WebCore/ChangeLog (modified) (1 diff)
• WebCore/page/DragController.cpp (modified) (2 diffs)
• WebCore/page/FrameSnapshotting.cpp (modified) (1 diff)
• WebCore/platform/DragImage.cpp (modified) (1 diff)
• WebCore/platform/graphics/cg/ImageBufferCG.cpp (modified) (2 diffs)
• WebKit2/ChangeLog (modified) (1 diff)
• WebKit2/WebProcess/WebCoreSupport/mac/WebDragClientMac.mm (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2014-01-09  Brian Burg  <bburg@apple.com>
+
+        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.
+
 2014-01-09  Myles C. Maxfield  <mmaxfield@apple.com>
 

trunk/Source/WebCore/page/DragController.cpp

@@ -829 +829 @@
             m_dragOffset = IntPoint(-size.width() / 2, -LinkDragBorderInset);
             dragLoc = IntPoint(mouseDraggedPoint.x() + m_dragOffset.x(), mouseDraggedPoint.y() + m_dragOffset.y());
+            // Later code expects the drag image to be scaled by device's scale factor.
+            dragImage = scaleDragImage(dragImage, FloatSize(m_page.deviceScaleFactor(), m_page.deviceScaleFactor()));
         }
         doSystemDrag(dragImage, dragLoc, mouseDraggedPoint, clipboard, src, true);
@@ -870 +872 @@
         IntSize fittedSize = dragImageSize(dragImage);
 
+        dragImage = scaleDragImage(dragImage, FloatSize(m_page.deviceScaleFactor(), m_page.deviceScaleFactor()));
         dragImage = dissolveDragImageToFraction(dragImage, DragImageAlpha);
 

trunk/Source/WebCore/page/FrameSnapshotting.cpp

@@ -91 +91 @@
     frame.view()->setPaintBehavior(paintBehavior);
 
-    float deviceScaleFactor = frame.page()->deviceScaleFactor();
-    IntRect usedRect(imageRect);
-    usedRect.scale(deviceScaleFactor);
-
-    std::unique_ptr<ImageBuffer> buffer = ImageBuffer::create(usedRect.size(), deviceScaleFactor, ColorSpaceDeviceRGB);
+    std::unique_ptr<ImageBuffer> buffer = ImageBuffer::create(imageRect.size(), frame.page()->deviceScaleFactor(), ColorSpaceDeviceRGB);
     if (!buffer)
         return nullptr;
-    buffer->context()->translate(-usedRect.x(), -usedRect.y());
-    buffer->context()->clip(FloatRect(0, 0, usedRect.maxX(), usedRect.maxY()));
+    buffer->context()->translate(-imageRect.x(), -imageRect.y());
 
-    frame.view()->paintContentsForSnapshot(buffer->context(), usedRect, shouldIncludeSelection, coordinateSpace);
+    frame.view()->paintContentsForSnapshot(buffer->context(), imageRect, shouldIncludeSelection, coordinateSpace);
     return buffer;
 }

trunk/Source/WebCore/platform/DragImage.cpp

@@ -109 +109 @@
     UNUSED_PARAM(node);
 #endif
-    RefPtr<Image> image = snapshot->copyImage(ImageBuffer::fastCopyImageMode());
+    RefPtr<Image> image = snapshot->copyImage(ImageBuffer::fastCopyImageMode(), Unscaled);
     if (!image)
         return nullptr;

trunk/Source/WebCore/platform/graphics/cg/ImageBufferCG.cpp

@@ -213 +213 @@
 {
     RetainPtr<CGImageRef> image;
-    if (m_resolutionScale == 1 || scaleBehavior == Unscaled)
+    if (m_resolutionScale == 1 || scaleBehavior == Unscaled) {
         image = copyNativeImage(copyBehavior);
-    else {
+        image = createCroppedImageIfNecessary(image.get(), internalSize());
+    } else {
         image = copyNativeImage(DontCopyBackingStore);
         RetainPtr<CGContextRef> context = adoptCF(CGBitmapContextCreate(0, logicalSize().width(), logicalSize().height(), 8, 4 * logicalSize().width(), deviceRGBColorSpaceRef(), kCGImageAlphaPremultipliedLast));
         CGContextSetBlendMode(context.get(), kCGBlendModeCopy);
-        CGContextDrawImage(context.get(), CGRectMake(0, 0, m_data.m_backingStoreSize.width(), m_data.m_backingStoreSize.height()), image.get());
+        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());
         image = adoptCF(CGBitmapContextCreateImage(context.get()));
     }
-    
-    image = createCroppedImageIfNecessary(image.get(), internalSize());
 
     if (!image)
-        return 0;
-
-    ASSERT(CGImageGetWidth(image.get()) == static_cast<size_t>(m_logicalSize.width()));
-    ASSERT(CGImageGetHeight(image.get()) == static_cast<size_t>(m_logicalSize.height()));
+        return nullptr;
 
     RefPtr<BitmapImage> bitmapImage = BitmapImage::create(image.get());
@@ -481 +479 @@
         CGContextSetBlendMode(context.get(), kCGBlendModeCopy);
         CGContextClipToRect(context.get(), CGRectMake(0, 0, logicalSize().width(), logicalSize().height()));
-        FloatSize imageRectInUserBounds = scaleSizeToUserSpace(logicalSize(), m_data.m_backingStoreSize, internalSize());
-        CGContextDrawImage(context.get(), CGRectMake(0, 0, imageRectInUserBounds.width(), imageRectInUserBounds.height()), image.get());
+        FloatSize imageSizeInUserSpace = scaleSizeToUserSpace(logicalSize(), m_data.m_backingStoreSize, internalSize());
+        CGContextDrawImage(context.get(), CGRectMake(0, 0, imageSizeInUserSpace.width(), imageSizeInUserSpace.height()), image.get());
         image = adoptCF(CGBitmapContextCreateImage(context.get()));
     }

trunk/Source/WebKit2/ChangeLog

@@ +1 @@
+2014-01-09  Brian Burg  <bburg@apple.com>
+
+        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.
+
 2014-01-09  Tim Horton  <timothy_horton@apple.com>
 

trunk/Source/WebKit2/WebProcess/WebCoreSupport/mac/WebDragClientMac.mm

@@ -80 +80 @@
 {
     IntSize bitmapSize([image.get() size]);
-    bitmapSize.scale(frame.page()->deviceScaleFactor());
     RefPtr<ShareableBitmap> bitmap = convertImageToBitmap(image.get(), bitmapSize);
     ShareableBitmap::Handle handle;