[Webkit-unassigned] [Bug 84936] [chromium] Compute highlight for touch targets

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=84936





--- Comment #9 from Tien-Ren Chen <trchen at chromium.org>  2012-04-27 13:28:08 PST ---
(In reply to comment #8)
> (From update of attachment 138954 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=138954&action=review
> 
> Overall the code seems to provide good targets, and does a good job generating the outline paths. Let's keep at this!

I saw your patch that uses GestureTapHighlighter, that is actually a very interesting piece of code. I think we'd rather try to extend it instead of invent our own stuff.
Be more specific these are the 2 functionality we want to add:

1. To support transformed elements, using the Quads family functions. IMO all the Rects family should be deprecated sooner or later...
2. Support compositor side scrolling/animation. That is, the highlights should be assigned to individual GraphicsLayers with layer-relative cords. Example:
<a href="">
<div style="width:100px;height:100px"/>
<div style="width:100px;height:100px;-webkit-transform:translateZ(0);-webkit-animation-name:someanimation"/>
</a>

> > Source/WebKit/chromium/src/WebViewImpl.cpp:3561
> > +        SkPath* path = highlight->get(graphicsLayer);
> 
> I would prefer if we can use Path here, as it's more general and is used by the proposed patch for drawing the highlight layers.

Sure I will upload a new patch uses WebCore::Path.

> > Source/WebKit/chromium/src/WebViewImpl.cpp:3586
> > +            path->moveTo(quad.p1().x(), quad.p1().y());
> 
> Can use Path::addRect(const FloatRect&) here, it's simpler. Can we add an option to call Path::addRoundedRect(), perhaps as an argument to this function?

It won't work for transformed touch target. For example: <div style="width:100px;height:100px;-webkit-transform:rotated(30deg)"></div>
Only 3D-transformed render objects will create its own GraphicsLayer.

> > Source/WebKit/chromium/src/WebViewImpl.h:590
> > +    typedef HashMap<WebCore::GraphicsLayer*, OwnPtr<SkPath> > HighlightMap;
> 
> Any reason we can't store RefPtr<GraphicsLayer> instead? This way the timer that controls the highlight animation (and which is the ultimate consumer of this data), doesn't have to worry about pointers going stale before the data is used.

I wanted to, but GraphicsLayer is not ref-counted, and modifying all existing code to use RefPtr will be a pain. :X

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.