Bug 57130 – Relative mouse coordinates recalculated for each target

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Emil A Eklund]

Mouse events recalculate the relative coordinates (offsetX/Y and layerX/Y) for each target which can be quite expensive. 
See MouseRelatedEvent::receivedTarget.

[Emil A Eklund]

Created attachment 86993 [details]
Patch, work in progress

Draft patch, comments on the general approach welcome.

[Emil A Eklund]

Created attachment 87003 [details]
Patch

Added tests for offset and layer position.

[Early Warning System Bot]

Attachment 87003 [details] did not build on qt:
Build output: http://queues.webkit.org/results/8256019

[Dimitri Glazkov (Google)]

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.

[Emil A Eklund]

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

[Emil A Eklund]

Created attachment 87004 [details]
Patch

Thanks Dimitri. Made the changes you suggested.

[Dimitri Glazkov (Google)]

Comment on attachment 87004 [details]
Patch

yay Emil!

[WebKit Review Bot]

Attachment 87004 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/8267480

[Emil A Eklund]

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.

[Dimitri Glazkov (Google)]

Comment on attachment 87193 [details]
Patch

gorgeous.

[WebKit Commit Bot]

Comment on attachment 87193 [details]
Patch

Clearing flags on attachment: 87193

Committed r82225: <http://trac.webkit.org/changeset/82225>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.