Bug 79724 – Stop doubling maximalOutlineSize during painting

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Julien Chaffraix]

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!

[Julien Chaffraix]

Created attachment 129303 [details]
Proposed refactoring: remove the doubling and introduce some localRepaintRect to simplify the logic.

[Tony Chang]

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.

[Julien Chaffraix]

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.

[Tony Chang]

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+.

[Julien Chaffraix]

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 :-)

[WebKit Review Bot]

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>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.