Bug 1301673, use more specific coordinates for screen position in drag calculations, r=tn
authorNeil Deakin <neil@mozilla.com>
Wed, 19 Oct 2016 15:01:39 -0400
changeset 363749 0bef53dbf128cb0571c59cde502538f0a40d7587
parent 363748 73ad49f175814ebeb16b8fb7b09b3217540332a9
child 363750 05f152da0f3fe524d81001785eaf5f30fc3c5951
push id1369
push userjlorenzo@mozilla.com
push dateMon, 27 Feb 2017 14:59:41 +0000
treeherdermozilla-release@d75a1dba431f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstn
bugs1301673
milestone52.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1301673, use more specific coordinates for screen position in drag calculations, r=tn
widget/cocoa/nsDragService.mm
widget/gtk/nsDragService.cpp
widget/nsBaseDragService.cpp
widget/nsBaseDragService.h
widget/nsDragServiceProxy.cpp
widget/windows/nsDragService.cpp
--- a/widget/cocoa/nsDragService.mm
+++ b/widget/cocoa/nsDragService.mm
@@ -126,36 +126,27 @@ static nsresult SetUpDragClipboard(nsIAr
 
 NSImage*
 nsDragService::ConstructDragImage(nsIDOMNode* aDOMNode,
                                   nsIntRect* aDragRect,
                                   nsIScriptableRegion* aRegion)
 {
   NS_OBJC_BEGIN_TRY_ABORT_BLOCK_NIL;
 
-  NSPoint screenPoint =
-    nsCocoaUtils::ConvertPointToScreen([gLastDragView window],
-                                       [gLastDragMouseDownEvent locationInWindow]);
-  // Y coordinates are bottom to top, so reverse this
-  screenPoint.y = nsCocoaUtils::FlippedScreenY(screenPoint.y);
-
   CGFloat scaleFactor = nsCocoaUtils::GetBackingScaleFactor(gLastDragView);
 
   RefPtr<SourceSurface> surface;
   nsPresContext* pc;
-  nsresult rv = DrawDrag(aDOMNode, aRegion,
-                         NSToIntRound(screenPoint.x),
-                         NSToIntRound(screenPoint.y),
+  nsresult rv = DrawDrag(aDOMNode, aRegion, mScreenPosition,
                          aDragRect, &surface, &pc);
-  if (!aDragRect->width || !aDragRect->height) {
+  if (pc && (!aDragRect->width || !aDragRect->height)) {
     // just use some suitable defaults
     int32_t size = nsCocoaUtils::CocoaPointsToDevPixels(20, scaleFactor);
-    aDragRect->SetRect(nsCocoaUtils::CocoaPointsToDevPixels(screenPoint.x, scaleFactor),
-                       nsCocoaUtils::CocoaPointsToDevPixels(screenPoint.y, scaleFactor),
-                       size, size);
+    aDragRect->SetRect(pc->CSSPixelsToDevPixels(mScreenPosition.x),
+                       pc->CSSPixelsToDevPixels(mScreenPosition.y), size, size);
   }
 
   if (NS_FAILED(rv) || !surface)
     return nil;
 
   uint32_t width = aDragRect->width;
   uint32_t height = aDragRect->height;
 
--- a/widget/gtk/nsDragService.cpp
+++ b/widget/gtk/nsDragService.cpp
@@ -1626,26 +1626,25 @@ nsDragService::SourceDataGet(GtkWidget  
 void nsDragService::SetDragIcon(GdkDragContext* aContext)
 {
     if (!mHasImage && !mSelection)
         return;
 
     nsIntRect dragRect;
     nsPresContext* pc;
     RefPtr<SourceSurface> surface;
-    DrawDrag(mSourceNode, mSourceRegion, mScreenX, mScreenY,
+    DrawDrag(mSourceNode, mSourceRegion, mScreenPosition,
              &dragRect, &surface, &pc);
     if (!pc)
         return;
 
-    int32_t sx = mScreenX, sy = mScreenY;
-    ConvertToUnscaledDevPixels(pc, &sx, &sy);
-
-    int32_t offsetX = sx - dragRect.x;
-    int32_t offsetY = sy - dragRect.y;
+    LayoutDeviceIntPoint screenPoint =
+      ConvertToUnscaledDevPixels(pc, mScreenPosition);
+    int32_t offsetX = screenPoint.x - dragRect.x;
+    int32_t offsetY = screenPoint.y - dragRect.y;
 
     // If a popup is set as the drag image, use its widget. Otherwise, use
     // the surface that DrawDrag created.
     //
     // XXX: Disable drag popups on GTK 3.19.4 and above: see bug 1264454.
     //      Fix this once a new GTK version ships that does not destroy our
     //      widget in gtk_drag_set_icon_widget.
     if (mDragPopup && gtk_check_version(3, 19, 4)) {
--- a/widget/nsBaseDragService.cpp
+++ b/widget/nsBaseDragService.cpp
@@ -52,18 +52,17 @@ using namespace mozilla::image;
 
 nsBaseDragService::nsBaseDragService()
   : mCanDrop(false), mOnlyChromeDrop(false), mDoingDrag(false),
     mHasImage(false), mUserCancelled(false),
     mDragEventDispatchedToChildProcess(false),
     mDragAction(DRAGDROP_ACTION_NONE),
     mDragActionFromChildProcess(DRAGDROP_ACTION_UNINITIALIZED), mTargetSize(0,0),
     mContentPolicyType(nsIContentPolicy::TYPE_OTHER),
-    mScreenX(-1), mScreenY(-1), mSuppressLevel(0),
-    mInputSource(nsIDOMMouseEvent::MOZ_SOURCE_MOUSE)
+    mSuppressLevel(0), mInputSource(nsIDOMMouseEvent::MOZ_SOURCE_MOUSE)
 {
 }
 
 nsBaseDragService::~nsBaseDragService()
 {
 }
 
 NS_IMPL_ISUPPORTS(nsBaseDragService, nsIDragService, nsIDragSession)
@@ -257,18 +256,18 @@ nsBaseDragService::InvokeDragSessionWith
 
   mDataTransfer = aDataTransfer;
   mSelection = nullptr;
   mHasImage = true;
   mDragPopup = nullptr;
   mImage = aImage;
   mImageOffset = CSSIntPoint(aImageX, aImageY);
 
-  aDragEvent->GetScreenX(&mScreenX);
-  aDragEvent->GetScreenY(&mScreenY);
+  aDragEvent->GetScreenX(&mScreenPosition.x);
+  aDragEvent->GetScreenY(&mScreenPosition.y);
   aDragEvent->GetMozInputSource(&mInputSource);
 
   nsresult rv = InvokeDragSession(aDOMNode, aTransferableArray,
                                   aRegion, aActionType,
                                   nsIContentPolicy::TYPE_INTERNAL_IMAGE);
 
   if (NS_FAILED(rv)) {
     mImage = nullptr;
@@ -292,18 +291,18 @@ nsBaseDragService::InvokeDragSessionWith
 
   mDataTransfer = aDataTransfer;
   mSelection = aSelection;
   mHasImage = true;
   mDragPopup = nullptr;
   mImage = nullptr;
   mImageOffset = CSSIntPoint();
 
-  aDragEvent->GetScreenX(&mScreenX);
-  aDragEvent->GetScreenY(&mScreenY);
+  aDragEvent->GetScreenX(&mScreenPosition.x);
+  aDragEvent->GetScreenY(&mScreenPosition.y);
   aDragEvent->GetMozInputSource(&mInputSource);
 
   // just get the focused node from the selection
   // XXXndeakin this should actually be the deepest node that contains both
   // endpoints of the selection
   nsCOMPtr<nsIDOMNode> node;
   aSelection->GetFocusNode(getter_AddRefs(node));
 
@@ -354,18 +353,18 @@ nsBaseDragService::StartDragSession()
 }
 
 void
 nsBaseDragService::OpenDragPopup()
 {
   if (mDragPopup) {
     nsXULPopupManager* pm = nsXULPopupManager::GetInstance();
     if (pm) {
-      pm->ShowPopupAtScreen(mDragPopup, mScreenX - mImageOffset.x,
-                            mScreenY - mImageOffset.y, false, nullptr);
+      pm->ShowPopupAtScreen(mDragPopup, mScreenPosition.x - mImageOffset.x,
+                            mScreenPosition.y - mImageOffset.y, false, nullptr);
     }
   }
 }
 
 int32_t
 nsBaseDragService::TakeChildProcessDragAction()
 {
   // If the last event was dispatched to the child process, use the drag action
@@ -414,18 +413,17 @@ nsBaseDragService::EndDragSession(bool a
   mSourceNode = nullptr;
   mSelection = nullptr;
   mDataTransfer = nullptr;
   mHasImage = false;
   mUserCancelled = false;
   mDragPopup = nullptr;
   mImage = nullptr;
   mImageOffset = CSSIntPoint();
-  mScreenX = -1;
-  mScreenY = -1;
+  mScreenPosition = CSSIntPoint();
   mEndDragPoint = LayoutDeviceIntPoint(0, 0);
   mInputSource = nsIDOMMouseEvent::MOZ_SOURCE_MOUSE;
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsBaseDragService::FireDragEventAtSource(EventMessage aEventMessage)
@@ -494,27 +492,27 @@ GetPresShellForContent(nsIDOMNode* aDOMN
   }
 
   return nullptr;
 }
 
 nsresult
 nsBaseDragService::DrawDrag(nsIDOMNode* aDOMNode,
                             nsIScriptableRegion* aRegion,
-                            int32_t aScreenX, int32_t aScreenY,
+                            CSSIntPoint aScreenPosition,
                             nsIntRect* aScreenDragRect,
                             RefPtr<SourceSurface>* aSurface,
                             nsPresContext** aPresContext)
 {
   *aSurface = nullptr;
   *aPresContext = nullptr;
 
   // use a default size, in case of an error.
-  aScreenDragRect->x = aScreenX - mImageOffset.x;
-  aScreenDragRect->y = aScreenY - mImageOffset.y;
+  aScreenDragRect->x = aScreenPosition.x - mImageOffset.x;
+  aScreenDragRect->y = aScreenPosition.y - mImageOffset.y;
   aScreenDragRect->width = 1;
   aScreenDragRect->height = 1;
 
   // if a drag image was specified, use that, otherwise, use the source node
   nsCOMPtr<nsIDOMNode> dragNode = mImage ? mImage.get() : aDOMNode;
 
   // get the presshell for the node being dragged. If the drag image is not in
   // a document or has no frame, get the presshell from the source drag node
@@ -550,21 +548,19 @@ nsBaseDragService::DrawDrag(nsIDOMNode* 
             return NS_OK;
           }
         }
       }
     }
   }
 
   // convert mouse position to dev pixels of the prescontext
-  int32_t sx = aScreenX, sy = aScreenY;
-  ConvertToUnscaledDevPixels(*aPresContext, &sx, &sy);
-
-  aScreenDragRect->x = sx - mImageOffset.x;
-  aScreenDragRect->y = sy - mImageOffset.y;
+  LayoutDeviceIntPoint screenPoint = ConvertToUnscaledDevPixels(*aPresContext, aScreenPosition);
+  aScreenDragRect->x = screenPoint.x - mImageOffset.x;
+  aScreenDragRect->y = screenPoint.y - mImageOffset.y;
 
   // check if drag images are disabled
   bool enableDragImages = Preferences::GetBool(DRAGIMAGES_PREF, true);
 
   // didn't want an image, so just set the screen rectangle to the frame size
   if (!enableDragImages || !mHasImage) {
     // if a region was specified, set the screen rectangle to the area that
     // the region occupies
@@ -607,25 +603,23 @@ nsBaseDragService::DrawDrag(nsIDOMNode* 
 
   // if a custom image was specified, check if it is an image node and draw
   // using the source rather than the displayed image. But if mImage isn't
   // an image or canvas, fall through to RenderNode below.
   if (mImage) {
     nsCOMPtr<nsIContent> content = do_QueryInterface(dragNode);
     HTMLCanvasElement *canvas = HTMLCanvasElement::FromContentOrNull(content);
     if (canvas) {
-      return DrawDragForImage(nullptr, canvas, sx, sy,
-                              aScreenDragRect, aSurface);
+      return DrawDragForImage(nullptr, canvas, aScreenDragRect, aSurface);
     }
 
     nsCOMPtr<nsIImageLoadingContent> imageLoader = do_QueryInterface(dragNode);
     // for image nodes, create the drag image from the actual image data
     if (imageLoader) {
-      return DrawDragForImage(imageLoader, nullptr, sx, sy,
-                              aScreenDragRect, aSurface);
+      return DrawDragForImage(imageLoader, nullptr, aScreenDragRect, aSurface);
     }
 
     // If the image is a popup, use that as the image. This allows custom drag
     // images that can change during the drag, but means that any platform
     // default image handling won't occur.
     // XXXndeakin this should be chrome-only
 
     nsIFrame* frame = content->GetPrimaryFrame();
@@ -671,27 +665,26 @@ nsBaseDragService::DrawDrag(nsIDOMNode* 
     *aSurface = presShell->RenderNode(dragNode, aRegion ? &clipRegion : nullptr,
                                     pnt, aScreenDragRect,
                                     renderFlags);
   }
 
   // if an image was specified, reposition the drag rectangle to
   // the supplied offset in mImageX and mImageY.
   if (mImage) {
-    aScreenDragRect->x = sx - mImageOffset.x;
-    aScreenDragRect->y = sy - mImageOffset.y;
+    aScreenDragRect->x = screenPoint.x - mImageOffset.x;
+    aScreenDragRect->y = screenPoint.y - mImageOffset.y;
   }
 
   return NS_OK;
 }
 
 nsresult
 nsBaseDragService::DrawDragForImage(nsIImageLoadingContent* aImageLoader,
                                     HTMLCanvasElement* aCanvas,
-                                    int32_t aScreenX, int32_t aScreenY,
                                     nsIntRect* aScreenDragRect,
                                     RefPtr<SourceSurface>* aSurface)
 {
   nsCOMPtr<imgIContainer> imgContainer;
   if (aImageLoader) {
     nsCOMPtr<imgIRequest> imgRequest;
     nsresult rv = aImageLoader->GetRequest(nsIImageLoadingContent::CURRENT_REQUEST,
                                           getter_AddRefs(imgRequest));
@@ -745,23 +738,23 @@ nsBaseDragService::DrawDragForImage(nsII
     *aSurface = dt->Snapshot();
   } else {
     *aSurface = aCanvas->GetSurfaceSnapshot();
   }
 
   return result;
 }
 
-void
+LayoutDeviceIntPoint
 nsBaseDragService::ConvertToUnscaledDevPixels(nsPresContext* aPresContext,
-                                              int32_t* aScreenX, int32_t* aScreenY)
+                                              CSSIntPoint aScreenPosition)
 {
   int32_t adj = aPresContext->DeviceContext()->AppUnitsPerDevPixelAtUnitFullZoom();
-  *aScreenX = nsPresContext::CSSPixelsToAppUnits(*aScreenX) / adj;
-  *aScreenY = nsPresContext::CSSPixelsToAppUnits(*aScreenY) / adj;
+  return LayoutDeviceIntPoint(nsPresContext::CSSPixelsToAppUnits(aScreenPosition.x) / adj,
+                              nsPresContext::CSSPixelsToAppUnits(aScreenPosition.y) / adj);
 }
 
 NS_IMETHODIMP
 nsBaseDragService::Suppress()
 {
   EndDragSession(false);
   ++mSuppressLevel;
   return NS_OK;
--- a/widget/nsBaseDragService.h
+++ b/widget/nsBaseDragService.h
@@ -82,53 +82,52 @@ protected:
   /**
    * Draw the drag image, if any, to a surface and return it. The drag image
    * is constructed from mImage if specified, or aDOMNode if mImage is null.
    *
    * aRegion may be used to draw only a subset of the element. This region
    * should be supplied using x and y coordinates measured in css pixels
    * that are relative to the upper-left corner of the window.
    *
-   * aScreenX and aScreenY should be the screen coordinates of the mouse click
-   * for the drag. These are in desktop pixels.
+   * aScreenPosition should be the screen coordinates of the mouse click
+   * for the drag. These are in CSS pixels.
    *
    * On return, aScreenDragRect will contain the screen coordinates of the
    * area being dragged. This is used by the platform-specific part of the
    * drag service to determine the drag feedback. This rect will be in the
    * device pixels of the presContext.
    *
    * If there is no drag image, the returned surface will be null, but
    * aScreenDragRect will still be set to the drag area.
    *
    * aPresContext will be set to the nsPresContext used determined from
    * whichever of mImage or aDOMNode is used.
    */
   nsresult DrawDrag(nsIDOMNode* aDOMNode,
                     nsIScriptableRegion* aRegion,
-                    int32_t aScreenX, int32_t aScreenY,
+                    mozilla::CSSIntPoint aScreenPosition,
                     nsIntRect* aScreenDragRect,
                     RefPtr<SourceSurface>* aSurface,
                     nsPresContext **aPresContext);
 
   /**
    * Draw a drag image for an image node specified by aImageLoader or aCanvas.
    * This is called by DrawDrag.
    */
   nsresult DrawDragForImage(nsIImageLoadingContent* aImageLoader,
                             mozilla::dom::HTMLCanvasElement* aCanvas,
-                            int32_t aScreenX, int32_t aScreenY,
                             nsIntRect* aScreenDragRect,
                             RefPtr<SourceSurface>* aSurface);
 
   /**
-   * Convert aScreenX and aScreenY from CSS pixels into unscaled device pixels.
+   * Convert aScreenPosition from CSS pixels into unscaled device pixels.
    */
-  void
+  mozilla::LayoutDeviceIntPoint
   ConvertToUnscaledDevPixels(nsPresContext* aPresContext,
-                             int32_t* aScreenX, int32_t* aScreenY);
+                             mozilla::CSSIntPoint aScreenPosition);
 
   /**
    * If the drag image is a popup, open the popup when the drag begins.
    */
   void OpenDragPopup();
 
   // Returns true if a drag event was dispatched to a child process after
   // the previous TakeDragEventDispatchedToChildProcess() call.
@@ -168,20 +167,18 @@ protected:
   // set if a selection is being dragged
   nsCOMPtr<nsISelection> mSelection;
 
   // set if the image in mImage is a popup. If this case, the popup will be opened
   // and moved instead of using a drag image.
   nsCOMPtr<nsIContent> mDragPopup;
 
   // the screen position where drag gesture occurred, used for positioning the
-  // drag image when no image is specified. If a value is -1, no event was
-  // supplied so the screen position is not known
-  int32_t mScreenX;
-  int32_t mScreenY;
+  // drag image.
+  mozilla::CSSIntPoint mScreenPosition;
 
   // the screen position where the drag ended
   mozilla::LayoutDeviceIntPoint mEndDragPoint;
 
   uint32_t mSuppressLevel;
 
   // The input source of the drag event. Possible values are from nsIDOMMouseEvent.
   uint16_t mInputSource;
--- a/widget/nsDragServiceProxy.cpp
+++ b/widget/nsDragServiceProxy.cpp
@@ -42,18 +42,17 @@ nsDragServiceProxy::InvokeDragSessionImp
                                                   false,
                                                   child->Manager(),
                                                   nullptr);
 
   if (mHasImage || mSelection) {
     nsIntRect dragRect;
     nsPresContext* pc;
     RefPtr<mozilla::gfx::SourceSurface> surface;
-    DrawDrag(mSourceNode, aRegion, mScreenX, mScreenY,
-             &dragRect, &surface, &pc);
+    DrawDrag(mSourceNode, aRegion, mScreenPosition, &dragRect, &surface, &pc);
 
     if (surface) {
       RefPtr<mozilla::gfx::DataSourceSurface> dataSurface =
         surface->GetDataSurface();
       if (dataSurface) {
         size_t length;
         int32_t stride;
         Shmem surfaceData;
--- a/widget/windows/nsDragService.cpp
+++ b/widget/windows/nsDragService.cpp
@@ -76,19 +76,17 @@ nsDragService::CreateDragImage(nsIDOMNod
   memset(psdi, 0, sizeof(SHDRAGIMAGE));
   if (!aDOMNode)
     return false;
 
   // Prepare the drag image
   nsIntRect dragRect;
   RefPtr<SourceSurface> surface;
   nsPresContext* pc;
-  DrawDrag(aDOMNode, aRegion,
-           mScreenX, mScreenY,
-           &dragRect, &surface, &pc);
+  DrawDrag(aDOMNode, aRegion, mScreenPosition, &dragRect, &surface, &pc);
   if (!surface)
     return false;
 
   uint32_t bmWidth = dragRect.width, bmHeight = dragRect.height;
 
   if (bmWidth == 0 || bmHeight == 0)
     return false;
 
@@ -145,26 +143,20 @@ nsDragService::CreateDragImage(nsIDOMNod
       CopySurfaceDataToPackedArray(map.mData, static_cast<uint8_t*>(lpBits),
                                    dataSurface->GetSize(), map.mStride,
                                    BytesPerPixel(dataSurface->GetFormat()));
     }
 
     psdi->sizeDragImage.cx = bmWidth;
     psdi->sizeDragImage.cy = bmHeight;
 
-    // Mouse position in center
-    if (mScreenX == -1 || mScreenY == -1) {
-      psdi->ptOffset.x = (uint32_t)((float)bmWidth/2.0f);
-      psdi->ptOffset.y = (uint32_t)((float)bmHeight/2.0f);
-    } else {
-      int32_t sx = mScreenX, sy = mScreenY;
-      ConvertToUnscaledDevPixels(pc, &sx, &sy);
-      psdi->ptOffset.x = sx - dragRect.x;
-      psdi->ptOffset.y = sy - dragRect.y;
-    }
+    LayoutDeviceIntPoint screenPoint =
+      ConvertToUnscaledDevPixels(pc, mScreenPosition);
+    psdi->ptOffset.x = screenPoint.x - dragRect.x;
+    psdi->ptOffset.y = screenPoint.y - dragRect.y;
 
     DeleteDC(hdcSrc);
   }
 
   dataSurface->Unmap();
 
   return psdi->hbmpDragImage != nullptr;
 }