Summary: | [Qt][WK2] Accelerated compositing example with links being animated crashes tap highlight | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jesus Sanchez-Palencia <jesus> | ||||||||
Component: | WebKit Qt | Assignee: | Allan Sandfeld Jensen <allan.jensen> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | abecsi, allan.jensen, cmarcelo, jturcotte, luciano.wolf, marcelo.lira, ossy | ||||||||
Priority: | P2 | Keywords: | Qt, QtTriaged | ||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Bug Depends on: | |||||||||||
Bug Blocks: | 79668, 110211 | ||||||||||
Attachments: |
|
Description
Jesus Sanchez-Palencia
2012-08-17 06:57:03 PDT
This looks very much like what I fixed in bug #93845, do you have a recent checkout? (In reply to comment #1) > This looks very much like what I fixed in bug #93845, do you have a recent checkout? I've just tested it with today's trunk (3e50844462d4997e76300743ac5109868e233e5e) and the ASSERT is still reached: ASSERTION FAILED: !currContainer->hasTransform() /home/jeez/code/webkit/Source/WebCore/rendering/RenderObject.cpp(2167) : WebCore::LayoutSize WebCore::RenderObject::offsetFromAncestorContainer(WebCore::RenderObject*) const Also, if you try it in release mode to avoid the assert, you can see Highlight being drawn as a rectangle. I know this is a different bug, but it's a bug. :P (In reply to comment #2) > (In reply to comment #1) > > This looks very much like what I fixed in bug #93845, do you have a recent checkout? > > I've just tested it with today's trunk (3e50844462d4997e76300743ac5109868e233e5e) and the ASSERT is still reached: > > ASSERTION FAILED: !currContainer->hasTransform() > /home/jeez/code/webkit/Source/WebCore/rendering/RenderObject.cpp(2167) : WebCore::LayoutSize WebCore::RenderObject::offsetFromAncestorContainer(WebCore::RenderObject*) const > Ahh, that makes sense, so the problem is offsetFromAncestorContainer can not handle transformation. > > Also, if you try it in release mode to avoid the assert, you can see Highlight being drawn as a rectangle. I know this is a different bug, but it's a bug. :P Don't be too sure it is a different bug, it could very well be a consequence of this one. (In reply to comment #3) > Don't be too sure it is a different bug, it could very well be a consequence of this one. Can you reproduce the reported bugs, Allan? (In reply to comment #4) > (In reply to comment #3) > > Don't be too sure it is a different bug, it could very well be a consequence of this one. > > Can you reproduce the reported bugs, Allan? Yes, though I don't get any highlight when using the release version. I also know what the problem is better now, and there is no easy way to fix it. This is a design flaw in how we draw the tap-highlight. At best we can just ignore clipping layers when transforms are involved. See bug #84936 for more details on how this could be solved with a different design for calculating tap highlights. Reported as QTBUG-32454 now. https://bugreports.qt-project.org/browse/QTBUG-32454 Created attachment 208344 [details]
Patch
Comment on attachment 208344 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=208344&action=review > Source/WebCore/page/GestureTapHighlighter.cpp:224 > + // Use bounding box because we can only highlight rects currently. > + ringRect = ringQuad.enclosingBoundingBox(); This will look bogus with rotation, perspective, etc. If you could check ringQuad.isRectilinear() first that would make this patch an improvement in every case I can think of. Created attachment 208418 [details]
Patch
Comment on attachment 208418 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=208418&action=review > Source/WebCore/ChangeLog:3 > + [Qt][WK2] Accelerated compositing example with links being animated crashes tap highlight The Qt style bot will complain downstream, "Accelerated compositing" -> "AC" would help :P > Source/WebCore/page/GestureTapHighlighter.cpp:227 > + if (ringQuad.isRectilinear()) > + ringRect = ringQuad.enclosingBoundingBox(); > + else > + ringRect = LayoutRect(); Nit: Could you just break directly here? if (!isRectilinear()) break; (In reply to comment #11) > (From update of attachment 208418 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=208418&action=review > > > Source/WebCore/ChangeLog:3 > > + [Qt][WK2] Accelerated compositing example with links being animated crashes tap highlight > > The Qt style bot will complain downstream, "Accelerated compositing" -> "AC" would help :P > Good idea. > > Source/WebCore/page/GestureTapHighlighter.cpp:227 > > + if (ringQuad.isRectilinear()) > > + ringRect = ringQuad.enclosingBoundingBox(); > > + else > > + ringRect = LayoutRect(); > > Nit: Could you just break directly here? > if (!isRectilinear()) > break; No, then ringRect would have its untransformed value. The rects are painted in a later iteration. Thanks for the review :) Committed r153893: <http://trac.webkit.org/changeset/153893> |