Bug 23042 - Remove reflection repaint hack in computeAbsoluteRepaintRect()
Summary: Remove reflection repaint hack in computeAbsoluteRepaintRect()
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:
Depends on:
Blocks:
 
Reported: 2008-12-30 13:20 PST by Simon Fraser (smfr)
Modified: 2008-12-30 16:12 PST (History)
2 users (show)

See Also:


Attachments
Patch, changelog, testcase (9.68 KB, patch)
2008-12-30 14:02 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-12-30 13:20:58 PST
RenderBox::computeAbsoluteRepaintRect() has a horrible hack that does a repaint if the object has a reflection.

This is a horrible hack, and ugly for the reasons explained in the comments.
Comment 1 Simon Fraser (smfr) 2008-12-30 14:02:28 PST
Created attachment 26322 [details]
Patch, changelog, testcase
Comment 2 Darin Adler 2008-12-30 14:25:53 PST
Comment on attachment 26322 [details]
Patch, changelog, testcase

> +    if (hasReflection()) {
> +        IntRect rectInReflection = reflectedRect(rect);
> +        rect.unite(rectInReflection);
>      }

I think this would read better without the local variable:

    if (hasReflection())
        rect.unite(reflectedRect(rect));

> +    IntRect result;
> +    if (!m_style->boxReflect())
> +        return result;
> +
> +    IntRect box = borderBox();
> +    result = r;

I don't think it's so great to use that empty result in this way. I'd write it like this:

    if (!m_style->boxReflect())
        return IntRect();

    IntRect box = borderBox();
    IntRect result = r;

r=me
Comment 3 Simon Fraser (smfr) 2008-12-30 16:12:54 PST
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	LayoutTests/ChangeLog
	A	LayoutTests/fast/repaint/reflection-redraw.html
	A	LayoutTests/platform/mac/fast/repaint/reflection-redraw-expected.checksum
	A	LayoutTests/platform/mac/fast/repaint/reflection-redraw-expected.png
	A	LayoutTests/platform/mac/fast/repaint/reflection-redraw-expected.txt
	M	WebCore/ChangeLog
	M	WebCore/rendering/RenderBox.cpp
	M	WebCore/rendering/RenderObject.cpp
	M	WebCore/rendering/RenderObject.h
Committed r39522