Bug 57130 - Relative mouse coordinates recalculated for each target
Summary: Relative mouse coordinates recalculated for each target
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Emil A Eklund
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-03-25 16:22 PDT by Emil A Eklund
Modified: 2011-03-29 04:02 PDT (History)
4 users (show)

See Also:


Attachments
Patch, work in progress (9.45 KB, patch)
2011-03-25 16:24 PDT, Emil A Eklund
no flags Details | Formatted Diff | Diff
Patch (13.06 KB, patch)
2011-03-25 19:01 PDT, Emil A Eklund
dglazkov: review+
Details | Formatted Diff | Diff
Patch (14.60 KB, patch)
2011-03-25 19:52 PDT, Emil A Eklund
dglazkov: review+
Details | Formatted Diff | Diff
Patch (17.10 KB, patch)
2011-03-28 13:48 PDT, Emil A Eklund
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Emil A Eklund 2011-03-25 16:22:37 PDT
Mouse events recalculate the relative coordinates (offsetX/Y and layerX/Y) for each target which can be quite expensive. 
See MouseRelatedEvent::receivedTarget.
Comment 1 Emil A Eklund 2011-03-25 16:24:09 PDT
Created attachment 86993 [details]
Patch, work in progress

Draft patch, comments on the general approach welcome.
Comment 2 Emil A Eklund 2011-03-25 19:01:38 PDT
Created attachment 87003 [details]
Patch

Added tests for offset and layer position.
Comment 3 Early Warning System Bot 2011-03-25 19:10:38 PDT
Attachment 87003 [details] did not build on qt:
Build output: http://queues.webkit.org/results/8256019
Comment 4 Dimitri Glazkov (Google) 2011-03-25 19:11:44 PDT
Comment on attachment 87003 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=87003&action=review

Looks great with nits. Love the test.

> Source/WebCore/ChangeLog:10
> +        structures significantly.

Probably should mention that this is changes O(N^2) to O(N). Don't be shy :)

> Source/WebCore/dom/MouseRelatedEvent.cpp:141
> +    m_hasCachedRelativePosition = false;

You can optimize this even further by checking (just outside receivedTarget callsite) to see if target is actually changing.

> Source/WebCore/dom/MouseRelatedEvent.h:44
> +        int layerX() const;
> +        int layerY() const;
> +        int offsetX() const;
> +        int offsetY() const;

These aren't const anymore, right? They will change the object.

> Source/WebCore/dom/MouseRelatedEvent.h:67
> +        void computeRelativePosition() const;

Ditto.

> Source/WebCore/dom/MouseRelatedEvent.h:84
> +        mutable int m_layerX;
> +        mutable int m_layerY;
> +        mutable int m_offsetX;
> +        mutable int m_offsetY;
> +        mutable bool m_hasCachedRelativePosition;

And once you remove consts, these don't need to be mutables.
Comment 5 Emil A Eklund 2011-03-25 19:17:26 PDT
This patch speeds up mouse event dispatching for deep DOM trees significantly. 

For a tree of a depth of 100 firing an event for the innermost element takes between 1300ms and 1320ms on my machine, with the patch that number is between 64 and 68ms. A ~20x improvement.

More realistically, with a depth of 10 there is a 2x improvement.

Some performance numbers for different depths:

Depth | Before | After
------+--------+------
  100 | 1310ms |  66ms
   50 |  394ms |  45ms
   25 |  148ms |  36ms
   10 |   62ms |  29ms
Comment 6 Emil A Eklund 2011-03-25 19:52:01 PDT
Created attachment 87004 [details]
Patch

Thanks Dimitri. Made the changes you suggested.
Comment 7 Dimitri Glazkov (Google) 2011-03-25 19:58:31 PDT
Comment on attachment 87004 [details]
Patch

yay Emil!
Comment 8 WebKit Review Bot 2011-03-27 14:11:20 PDT
Attachment 87004 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/8267480
Comment 9 Emil A Eklund 2011-03-28 13:48:59 PDT
Created attachment 87193 [details]
Patch

PTAL

Updated chromium's WebDOMMouseEvent to match the updated event object and updated the new mouse-relative-position test to account for platform differences.
Comment 10 Dimitri Glazkov (Google) 2011-03-28 14:45:10 PDT
Comment on attachment 87193 [details]
Patch

gorgeous.
Comment 11 WebKit Commit Bot 2011-03-29 04:02:52 PDT
Comment on attachment 87193 [details]
Patch

Clearing flags on attachment: 87193

Committed r82225: <http://trac.webkit.org/changeset/82225>
Comment 12 WebKit Commit Bot 2011-03-29 04:02:57 PDT
All reviewed patches have been landed.  Closing bug.