RESOLVED FIXED 22159
Repaint issue with outlines in child objects
https://bugs.webkit.org/show_bug.cgi?id=22159
Summary Repaint issue with outlines in child objects
Simon Fraser (smfr)
Reported 2008-11-10 10:42:21 PST
The attached testcase shows a bug where the focus ring is not correctly invalidated when a text input is moved by layout. It happens to use an animation on another element, but that's just incidental. There are no transforms here.
Attachments
Testcase (1.05 KB, text/html)
2008-11-10 10:42 PST, Simon Fraser (smfr)
no flags
Simple testcase with outline (749 bytes, text/html)
2008-11-20 21:15 PST, Simon Fraser (smfr)
no flags
Patch, testcase, changelog (10.91 KB, patch)
2008-11-20 22:18 PST, Simon Fraser (smfr)
darin: review+
Simon Fraser (smfr)
Comment 1 2008-11-10 10:42:37 PST
Created attachment 25021 [details] Testcase
Simon Fraser (smfr)
Comment 2 2008-11-20 21:15:32 PST
Created attachment 25332 [details] Simple testcase with outline This bug has nothing to do with focus, or animations. There's a basic repaint bug with outlines.
Simon Fraser (smfr)
Comment 3 2008-11-20 21:26:59 PST
The problem is that RenderBox::absoluteClippedOverflowRect() only looks at the outline size on the style of the element being laid out, but not any of its contents. We could either grovel through the child objects looking at outline size, or just use view->maximalOutlineSize().
mitz
Comment 4 2008-11-20 21:31:54 PST
I think it's going to have to be view->maximalOutlineSize() :-(
Simon Fraser (smfr)
Comment 5 2008-11-20 22:18:19 PST
Created attachment 25338 [details] Patch, testcase, changelog Patch includes some new bases for repaint tests whose repaint rects alter with this change (because of focus rings)
Darin Adler
Comment 6 2008-11-21 13:22:21 PST
Comment on attachment 25338 [details] Patch, testcase, changelog > + if (style()->outlineWidth() > 0 && style()->outlineSize() > maximalOutlineSize(PaintPhaseOutline)) > + static_cast<RenderView*>(document()->renderer())->setMaximalOutlineSize(style()->outlineSize()); Since you had to move this code, maybe there's a way to write a brief comment explaining the order dependency? I feel a little uncomfortable reviewing this with Hyatt on vacation. You should check with him afterwards just in case he thinks this will result in excessive painting. r=me
Simon Fraser (smfr)
Comment 7 2008-11-21 14:01:20 PST
Yes, the over-repaint is a worry, especially since any focus ring makes setMaximalOutlineSize() non-zero. The alternatives would be: 1. Store outline dimensions on RenderBox, and explicitly account for them whem computing repaint rects (but they don't affect overflow, unlike shadows, so we can't just put them in the overflow sizes), or 2. Set a bit on RenderObject to say that it has outlined children, and only inflate by maxOutlineSize when the object or children have outline.
Simon Fraser (smfr)
Comment 8 2008-11-21 14:49:35 PST
Committed r38678 M WebCore/ChangeLog M WebCore/rendering/RenderBox.cpp M LayoutTests/platform/mac/fast/repaint/outline-repaint-glitch-expected.checksum A LayoutTests/platform/mac/fast/repaint/outline-child-repaint-expected.checksum A LayoutTests/platform/mac/fast/repaint/outline-child-repaint-expected.png M LayoutTests/platform/mac/fast/repaint/4776765-expected.checksum A LayoutTests/platform/mac/fast/repaint/outline-child-repaint-expected.txt M LayoutTests/platform/mac/fast/repaint/delete-into-nested-block-expected.png M LayoutTests/platform/mac/fast/repaint/4776765-expected.png M LayoutTests/platform/mac/fast/repaint/delete-into-nested-block-expected.checksum M LayoutTests/platform/mac/fast/repaint/outline-repaint-glitch-expected.png M LayoutTests/ChangeLog A LayoutTests/fast/repaint/outline-child-repaint.html r38678 = 0ebe80da11f747b44e3ba9e89ae67bf891820c51 (trunk) I'll send mail to Hyatt to look at this change.
Simon Fraser (smfr)
Comment 9 2008-12-01 15:42:35 PST
Maybe this should have used getAbsoluteRepaintRectWithOutline()?
David Kilzer (:ddkilzer)
Comment 10 2009-06-02 09:56:04 PDT
*** Bug 14050 has been marked as a duplicate of this bug. ***
Note You need to log in before you can comment on or make changes to this bug.