Bug 79724 - Stop doubling maximalOutlineSize during painting
Summary: Stop doubling maximalOutlineSize during painting
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Julien Chaffraix
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-02-27 17:32 PST by Julien Chaffraix
Modified: 2012-02-29 12:50 PST (History)
3 users (show)

See Also:


Attachments
Proposed refactoring: remove the doubling and introduce some localRepaintRect to simplify the logic. (4.79 KB, patch)
2012-02-28 11:55 PST, Julien Chaffraix
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Julien Chaffraix 2012-02-27 17:32:40 PST
Several call sites are wrongly doubling the maximal outline when manipulating the repaint rectangle:

* RenderReplaced
* RenderTableSection
* RenderTableCell

There is no historical explanation for the doubling. Moreover the rest of the code handle the maximalOutlineSize by just inflating the repaint rectangle once so it looks like a bug!
Comment 1 Julien Chaffraix 2012-02-28 11:55:43 PST
Created attachment 129303 [details]
Proposed refactoring: remove the doubling and introduce some localRepaintRect to simplify the logic.
Comment 2 Tony Chang 2012-02-29 10:27:07 PST
Comment on attachment 129303 [details]
Proposed refactoring: remove the doubling and introduce some localRepaintRect to simplify the logic.

View in context: https://bugs.webkit.org/attachment.cgi?id=129303&action=review

> Source/WebCore/ChangeLog:8
> +        Refactoring only, covered by existing tests (mostly repaint ones).

Can you name the pixel tests that need a rebaseline from this?  Maybe we can pre-emptively mark them in test_expectations.txt so the bots don't go red.
Comment 3 Julien Chaffraix 2012-02-29 10:32:47 PST
Comment on attachment 129303 [details]
Proposed refactoring: remove the doubling and introduce some localRepaintRect to simplify the logic.

View in context: https://bugs.webkit.org/attachment.cgi?id=129303&action=review

>> Source/WebCore/ChangeLog:8
>> +        Refactoring only, covered by existing tests (mostly repaint ones).
> 
> Can you name the pixel tests that need a rebaseline from this?  Maybe we can pre-emptively mark them in test_expectations.txt so the bots don't go red.

There is none that needs to be rebaselined. However there are several tests involving tables and replaced elements in our tree. The repaint tests are the one that checks that I did not break our repainting by "shrinking" the outline. I can change the wording if you want.
Comment 4 Tony Chang 2012-02-29 11:04:53 PST
Comment on attachment 129303 [details]
Proposed refactoring: remove the doubling and introduce some localRepaintRect to simplify the logic.

View in context: https://bugs.webkit.org/attachment.cgi?id=129303&action=review

>>> Source/WebCore/ChangeLog:8
>>> +        Refactoring only, covered by existing tests (mostly repaint ones).
>> 
>> Can you name the pixel tests that need a rebaseline from this?  Maybe we can pre-emptively mark them in test_expectations.txt so the bots don't go red.
> 
> There is none that needs to be rebaselined. However there are several tests involving tables and replaced elements in our tree. The repaint tests are the one that checks that I did not break our repainting by "shrinking" the outline. I can change the wording if you want.

I see, I was confused by "Stop doubling maximalOutlineSize during painting", which sounds like a different behavior.  I think the current working is OK.  Feel free to cq+.
Comment 5 Julien Chaffraix 2012-02-29 11:32:08 PST
Comment on attachment 129303 [details]
Proposed refactoring: remove the doubling and introduce some localRepaintRect to simplify the logic.

View in context: https://bugs.webkit.org/attachment.cgi?id=129303&action=review

>>>> Source/WebCore/ChangeLog:8
>>>> +        Refactoring only, covered by existing tests (mostly repaint ones).
>>> 
>>> Can you name the pixel tests that need a rebaseline from this?  Maybe we can pre-emptively mark them in test_expectations.txt so the bots don't go red.
>> 
>> There is none that needs to be rebaselined. However there are several tests involving tables and replaced elements in our tree. The repaint tests are the one that checks that I did not break our repainting by "shrinking" the outline. I can change the wording if you want.
> 
> I see, I was confused by "Stop doubling maximalOutlineSize during painting", which sounds like a different behavior.  I think the current working is OK.  Feel free to cq+.

OK , cqinating :-)
Comment 6 WebKit Review Bot 2012-02-29 12:50:00 PST
Comment on attachment 129303 [details]
Proposed refactoring: remove the doubling and introduce some localRepaintRect to simplify the logic.

Clearing flags on attachment: 129303

Committed r109246: <http://trac.webkit.org/changeset/109246>
Comment 7 WebKit Review Bot 2012-02-29 12:50:06 PST
All reviewed patches have been landed.  Closing bug.