Changeset 134874 in webkit

Timestamp:

Nov 15, 2012 6:28:00 PM (11 years ago)

Author:

commit-queue@webkit.org

Message:

custom CSS cursors ignore hotspot values embedded in CUR files
​https://bugs.webkit.org/show_bug.cgi?id=100059

Patch by Rick Byers <​rbyers@chromium.org> on 2012-11-15
Reviewed by Kenneth Russell.

Source/WebCore:

Add reading the hotspot values to the ICOImageDecoder (for CUR files only),
and plumb it through so that the existing calls to ImageSource::getHotSpot
actually return the hot spot value when there is one.

Tests: fast/events/mouse-cursor.html, fast/events/mouse-cursor-multiframecur.html

• platform/graphics/ImageSource.cpp:

(WebCore::ImageSource::getHotSpot):

• platform/graphics/chromium/DeferredImageDecoder.cpp:

(WebCore::DeferredImageDecoder::hotSpot):
(WebCore::DeferredImageDecoder::hotSpotAtIndex):

• platform/graphics/chromium/DeferredImageDecoder.h:

(DeferredImageDecoder):

• platform/image-decoders/ImageDecoder.h:

(WebCore::ImageDecoder::hotSpot):
(WebCore::ImageDecoder::hotSpotAtIndex):
(ImageDecoder):

• platform/image-decoders/ico/ICOImageDecoder.cpp:

(WebCore::ICOImageDecoder::hotSpot):
(WebCore::ICOImageDecoder::hotSpotAtIndex):
(WebCore::ICOImageDecoder::processDirectory):
(WebCore::ICOImageDecoder::readDirectoryEntry):

• platform/image-decoders/ico/ICOImageDecoder.h:

(ICOImageDecoder):
(IconDirectoryEntry):

LayoutTests:

Add mouse cursor test cases that use CUR files that have embedded hotspot values.

• fast/events/mouse-cursor-expected.txt:
• fast/events/mouse-cursor-multiframecur-expected.txt: Added.
• fast/events/mouse-cursor-multiframecur.html: Copied from LayoutTests/fast/events/mouse-cursor.html.
• fast/events/mouse-cursor.html:
• fast/events/resources/greenbox-3frames.cur: Added.
• fast/events/resources/greenbox-hotspot35-4.cur: Added.
• fast/events/resources/greenbox-hotspot5-4.cur: Added.
• platform/mac/TestExpectations: Skip the multi-frame case on mac because it causes a hang

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/events/mouse-cursor-expected.txt (modified) (1 diff)
• LayoutTests/fast/events/mouse-cursor-multiframecur-expected.txt (added)
• LayoutTests/fast/events/mouse-cursor-multiframecur.html (added)
• LayoutTests/fast/events/mouse-cursor.html (modified) (3 diffs)
• LayoutTests/fast/events/resources/greenbox-3frames.cur (added)
• LayoutTests/fast/events/resources/greenbox-hotspot35-4.cur (added)
• LayoutTests/fast/events/resources/greenbox-hotspot5-4.cur (added)
• LayoutTests/platform/mac/TestExpectations (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/platform/graphics/ImageSource.cpp (modified) (1 diff)
• Source/WebCore/platform/graphics/chromium/DeferredImageDecoder.cpp (modified) (1 diff)
• Source/WebCore/platform/graphics/chromium/DeferredImageDecoder.h (modified) (1 diff)
• Source/WebCore/platform/image-decoders/ImageDecoder.h (modified) (1 diff)
• Source/WebCore/platform/image-decoders/ico/ICOImageDecoder.cpp (modified) (3 diffs)
• Source/WebCore/platform/image-decoders/ico/ICOImageDecoder.h (modified) (4 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2012-11-15  Rick Byers  <rbyers@chromium.org>
+
+        custom CSS cursors ignore hotspot values embedded in CUR files
+        https://bugs.webkit.org/show_bug.cgi?id=100059
+
+        Reviewed by Kenneth Russell.
+
+        Add mouse cursor test cases that use CUR files that have embedded hotspot values.
+
+        * fast/events/mouse-cursor-expected.txt:
+        * fast/events/mouse-cursor-multiframecur-expected.txt: Added.
+        * fast/events/mouse-cursor-multiframecur.html: Copied from LayoutTests/fast/events/mouse-cursor.html.
+        * fast/events/mouse-cursor.html:
+        * fast/events/resources/greenbox-3frames.cur: Added.
+        * fast/events/resources/greenbox-hotspot35-4.cur: Added.
+        * fast/events/resources/greenbox-hotspot5-4.cur: Added.
+        * platform/mac/TestExpectations: Skip the multi-frame case on mac because it causes a hang
+
 2012-11-15  Kenichi Ishibashi  <bashi@chromium.org>
 

trunk/LayoutTests/fast/events/mouse-cursor-expected.txt

@@ -46 +46 @@
 Cursor Info: type=Custom hotSpot=0,0 image=25x25
 
+TEST CASE: Image with implicit hot spot at (5,4)
+Cursor Info: type=Custom hotSpot=5,4 image=25x25
+
+TEST CASE: Image with explicit hot spot at (20,10) overriding implicit hot spot
+Cursor Info: type=Custom hotSpot=20,10 image=25x25
+
+TEST CASE: Image with implicit hot spot outside image at (35,4)
+Cursor Info: type=Custom hotSpot=0,0 image=25x25
+
 TEST CASE: Over large image with fallback to pointer
 Cursor Info: type=Hand hotSpot=0,0

trunk/LayoutTests/fast/events/mouse-cursor.html

@@ -29 +29 @@
   <div style='cursor: url(resources/greenbox.png) -1 -1, pointer'>Image with explicit hot spot at (-1,-1)</div>
   <div style='cursor: url(resources/greenbox.png) 30 30, pointer'>Image with explicit hot spot outside image at (30,30)</div>
+  <div style='cursor: url(resources/greenbox-hotspot5-4.cur), pointer'>Image with implicit hot spot at (5,4)</div>
+  <div style='cursor: url(resources/greenbox-hotspot5-4.cur) 20 10, pointer'>Image with explicit hot spot at (20,10) overriding implicit hot spot</div>
+  <div style='cursor: url(resources/greenbox-hotspot35-4.cur), pointer'>Image with implicit hot spot outside image at (35,4)</div>
   <div style='cursor: url(resources/onload-image.png), pointer'>Over large image with fallback to pointer</div>
   <div style='cursor: url(#greenbox), pointer'>SVG cursor</div>
@@ -68 +71 @@
             var node = nodesToTest[i];
             debug('TEST CASE: ' + node.textContent);
-            eventSender.mouseMoveTo(node.offsetLeft + 3, node.offsetTop + 3);
+
+            // Make sure the node is visible and move the mouse over top of it.
+            document.body.scrollTop = node.offsetTop - 50;
+            eventSender.mouseMoveTo(node.offsetLeft + 3, node.offsetTop - document.body.scrollTop + 3);
+
+            // Get details of the current mouse cursor.
             // Note that we could return structured data which we then validate, but that's a lot more
             // work and is redundant with relying on the expected output anyway.  Better to just dump
@@ -99 +107 @@
     { url: 'resources/greenbox.png' },
     { url: 'resources/onload-image.png' },
+    { url: 'resources/greenbox-hotspot5-4.cur' },
+    { url: 'resources/greenbox-hotspot35-4.cur' },
     { url: 'doesntexist.png', error: true },
     { url: 'unknown-scheme:cursor.png', error: true },

trunk/LayoutTests/platform/mac/TestExpectations

@@ -1186 +1186 @@
 # Mountain Lion and prior do not support custom media data loading
 Bug(jernoble) [ MountainLion Lion SnowLeopard ] media/video-src-blob.html
+
+# Hangs safari apparently due to an issue with weird multi-frame CUR image files
+webkit.org/b/101811 fast/events/mouse-cursor-multiframecur.html [ Skip ]

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-11-15  Rick Byers  <rbyers@chromium.org>
+
+        custom CSS cursors ignore hotspot values embedded in CUR files
+        https://bugs.webkit.org/show_bug.cgi?id=100059
+
+        Reviewed by Kenneth Russell.
+
+        Add reading the hotspot values to the ICOImageDecoder (for CUR files only),
+        and plumb it through so that the existing calls to ImageSource::getHotSpot
+        actually return the hot spot value when there is one.
+
+        Tests: fast/events/mouse-cursor.html, fast/events/mouse-cursor-multiframecur.html
+
+        * platform/graphics/ImageSource.cpp:
+        (WebCore::ImageSource::getHotSpot):
+        * platform/graphics/chromium/DeferredImageDecoder.cpp:
+        (WebCore::DeferredImageDecoder::hotSpot):
+        (WebCore::DeferredImageDecoder::hotSpotAtIndex):
+        * platform/graphics/chromium/DeferredImageDecoder.h:
+        (DeferredImageDecoder):
+        * platform/image-decoders/ImageDecoder.h:
+        (WebCore::ImageDecoder::hotSpot):
+        (WebCore::ImageDecoder::hotSpotAtIndex):
+        (ImageDecoder):
+        * platform/image-decoders/ico/ICOImageDecoder.cpp:
+        (WebCore::ICOImageDecoder::hotSpot):
+        (WebCore::ICOImageDecoder::hotSpotAtIndex):
+        (WebCore::ICOImageDecoder::processDirectory):
+        (WebCore::ICOImageDecoder::readDirectoryEntry):
+        * platform/image-decoders/ico/ICOImageDecoder.h:
+        (ICOImageDecoder):
+        (IconDirectoryEntry):
+
 2012-11-15  Kenichi Ishibashi  <bashi@chromium.org>
 

trunk/Source/WebCore/platform/graphics/ImageSource.cpp

@@ -122 +122 @@
 }
 
-bool ImageSource::getHotSpot(IntPoint&) const
-{
-    return false;
+bool ImageSource::getHotSpot(IntPoint& hotSpot) const
+{
+    return m_decoder ? m_decoder->hotSpot(hotSpot) : false;
 }
 

trunk/Source/WebCore/platform/graphics/chromium/DeferredImageDecoder.cpp

@@ -220 +220 @@
 }
 
+bool DeferredImageDecoder::hotSpot(IntPoint& hotSpot) const
+{
+    return m_actualDecoder ? m_actualDecoder->hotSpot(hotSpot) : false;
+}
+
 } // namespace WebCore

trunk/Source/WebCore/platform/graphics/chromium/DeferredImageDecoder.h

@@ -63 +63 @@
     unsigned frameBytesAtIndex(size_t index) const;
     ImageOrientation orientation() const;
+    bool hotSpot(IntPoint&) const;
 
 private:

trunk/Source/WebCore/platform/image-decoders/ImageDecoder.h

@@ -377 +377 @@
 #endif
 
+        // If the image has a cursor hot-spot, stores it in the argument
+        // and returns true. Otherwise returns false.
+        virtual bool hotSpot(IntPoint&) const { return false; }
+
         virtual void reportMemoryUsage(MemoryObjectInfo*) const;
 

trunk/Source/WebCore/platform/image-decoders/ico/ICOImageDecoder.cpp

@@ -134 +134 @@
 }
 
+bool ICOImageDecoder::hotSpot(IntPoint& hotSpot) const
+{
+    // When unspecified, the default frame is always frame 0. This is consistent with
+    // BitmapImage where currentFrame() starts at 0 and only increases when animation is
+    // requested.
+    return hotSpotAtIndex(0, hotSpot);
+}
+
+bool ICOImageDecoder::hotSpotAtIndex(size_t index, IntPoint& hotSpot) const
+{
+    if (index >= m_dirEntries.size() || m_fileType != CURSOR)
+        return false;
+
+    hotSpot = m_dirEntries[index].m_hotSpot;
+    return true;
+}
+
+
 // static
 bool ICOImageDecoder::compareEntries(const IconDirectoryEntry& a, const IconDirectoryEntry& b)
@@ -233 +251 @@
     // See if this is an icon filetype we understand, and make sure we have at
     // least one entry in the directory.
-    enum {
-        ICON = 1,
-        CURSOR = 2,
-    };
     if (((fileType != ICON) && (fileType != CURSOR)) || (!idCount))
         return setFailed();
+
+    m_fileType = static_cast<FileType>(fileType);
 
     // Enlarge member vectors to hold all the entries.
@@ -288 +304 @@
     IconDirectoryEntry entry;
     entry.m_size = IntSize(width, height);
-    entry.m_bitCount = readUint16(6);
+    if (m_fileType == CURSOR) {
+        entry.m_bitCount = 0;
+        entry.m_hotSpot = IntPoint(readUint16(4), readUint16(6));
+    } else {
+        entry.m_bitCount = readUint16(6);
+        entry.m_hotSpot = IntPoint();
+    }
     entry.m_imageOffset = readUint32(12);
 

trunk/Source/WebCore/platform/image-decoders/ico/ICOImageDecoder.h

@@ -57 +57 @@
         // inside BMPImageReader!
         virtual bool setFailed();
+        virtual bool hotSpot(IntPoint&) const;
 
     private:
@@ -65 +66 @@
         };
 
+        enum FileType {
+            ICON = 1,
+            CURSOR = 2,
+        };
+
         struct IconDirectoryEntry {
             IntSize m_size;
             uint16_t m_bitCount;
+            IntPoint m_hotSpot;
             uint32_t m_imageOffset;
         };
@@ -110 +117 @@
         bool processDirectoryEntries();
 
+        // Stores the hot-spot for |index| in |hotSpot| and returns true,
+        // or returns false if there is none.
+        bool hotSpotAtIndex(size_t index, IntPoint& hotSpot) const;
+
         // Reads and returns a directory entry from the current offset into
         // |data|.
@@ -122 +133 @@
         // BMPImageReader takes over this will not be updated further.
         size_t m_decodedOffset;
+
+        // Which type of file (ICO/CUR) this is.
+        FileType m_fileType;
 
         // The headers for the ICO.