[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 #8 from W. James MacLean <wjmaclean at chromium.org>  2012-04-27 11:33:42 PST ---
(From update of attachment 138954)
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!

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

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

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

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