[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 #14 from Tien-Ren Chen <trchen at chromium.org>  2012-05-01 03:44:36 PST ---
I won't be in the office until Friday. I will try to keep in touch by email but I won't have too much time to work on code.

I changed the patch a little bit to make the highlight generation code a separate class. It is very likely I will have to improve the implementation later, but the interface will stay the same.

(In reply to comment #10)
> (In reply to comment #9)
> > (In reply to comment #8)
> > > (From update of attachment 138954 [details] [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.

The node selector from my patch is very minimal. We have some disambiguation logic in downstream code that I don't 100% understand yet. I'll try to upstream those incrementally in later patches.

As for the highlight region, I think addFocusRingRects is actually very similar to our implementation, except that it uses absolute rects (thus can behave very wrong when transformation is used).

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

I don't think anything in the render tree is ref-counted.

LayerChromium is ref-counted but it doesn't back-reference to GraphicsLayer. One possibility is to do everything on LayerChromium. That is what I did with the compositor scrollbar layers.

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