Bug 22159 - Repaint issue with outlines in child objects
Summary: Repaint issue with outlines in child objects
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P2 Normal
Assignee: Simon Fraser (smfr)
URL:
Keywords:
: 14050 (view as bug list)
Depends on:
Blocks:
 
Reported: 2008-11-10 10:42 PST by Simon Fraser (smfr)
Modified: 2009-06-02 09:56 PDT (History)
2 users (show)

See Also:


Attachments
Testcase (1.05 KB, text/html)
2008-11-10 10:42 PST, Simon Fraser (smfr)
no flags Details
Simple testcase with outline (749 bytes, text/html)
2008-11-20 21:15 PST, Simon Fraser (smfr)
no flags Details
Patch, testcase, changelog (10.91 KB, patch)
2008-11-20 22:18 PST, Simon Fraser (smfr)
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 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.
Comment 1 Simon Fraser (smfr) 2008-11-10 10:42:37 PST
Created attachment 25021 [details]
Testcase
Comment 2 Simon Fraser (smfr) 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.
Comment 3 Simon Fraser (smfr) 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().
Comment 4 mitz 2008-11-20 21:31:54 PST
I think it's going to have to be view->maximalOutlineSize() :-(
Comment 5 Simon Fraser (smfr) 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)
Comment 6 Darin Adler 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
Comment 7 Simon Fraser (smfr) 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.
Comment 8 Simon Fraser (smfr) 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.
Comment 9 Simon Fraser (smfr) 2008-12-01 15:42:35 PST
Maybe this should have used getAbsoluteRepaintRectWithOutline()?
Comment 10 David Kilzer (:ddkilzer) 2009-06-02 09:56:04 PDT
*** Bug 14050 has been marked as a duplicate of this bug. ***