[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 #10 from W. James MacLean <wjmaclean at chromium.org>  2012-04-30 08:53:53 PST ---
(In reply to comment #9)
> (In reply to comment #8)
> > (From update of attachment 138954 [details] [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.

That patch has changed a lot. First, using the existing code gave very poor targets in my opinion. Your code worked quite nicely for me though. I may try again with the touch highlight region from my patch, and your node selector.

> 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.

Great, thanks!

> > > 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

OK, so we need to figure out how to work around this then. Is there anything that points to the graphics layer that is ref-counted, that we could store instead?

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