Bug 103955

Summary:

Make caret repainting container-aware

Product:

WebKit

Reporter:

Tien-Ren Chen <trchen>

Component:

New Bugs

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

aelias, dglazkov, eric, jamesr, mifenton, mitz, ojan.autocc, ojan, rniwa, simon.fraser, thorton, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

104200, 119122

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none

Tien-Ren Chen

Reported 2012-12-03 18:16:04 PST

Make caret repainting container aware

Attachments
Patch (15.21 KB, patch) 2012-12-03 18:22 PST, Tien-Ren Chen	no flags	Details Formatted Diff Diff
Patch (16.16 KB, patch) 2012-12-03 19:16 PST, Tien-Ren Chen	no flags	Details Formatted Diff Diff
Patch (9.91 KB, patch) 2012-12-05 17:59 PST, Tien-Ren Chen	no flags	Details Formatted Diff Diff
Patch (11.57 KB, patch) 2012-12-06 18:21 PST, Tien-Ren Chen	no flags	Details Formatted Diff Diff
Patch (11.52 KB, patch) 2012-12-06 18:43 PST, Tien-Ren Chen	no flags	Details Formatted Diff Diff
Patch (14.12 KB, patch) 2012-12-07 15:41 PST, Tien-Ren Chen	no flags	Details Formatted Diff Diff
Patch (14.24 KB, patch) 2013-01-08 22:06 PST, Tien-Ren Chen	no flags	Details Formatted Diff Diff
Patch (14.60 KB, patch) 2013-01-09 17:46 PST, Tien-Ren Chen	no flags	Details Formatted Diff Diff
Show Obsolete (7) View All Add attachment proposed patch, testcase, etc.

Tien-Ren Chen

Comment 1 2012-12-03 18:22:37 PST

Created attachment 177390 [details] Patch

Tien-Ren Chen

Comment 2 2012-12-03 19:16:09 PST

Created attachment 177394 [details] Patch

Tien-Ren Chen

Comment 3 2012-12-05 17:59:00 PST

Created attachment 177891 [details] Patch

Tien-Ren Chen

Comment 4 2012-12-05 18:00:35 PST

I'm moving the removal of RenderView::repaintRectangleInViewAndCompositedLayers() to a new patch, so this patch will be focused on refactoring the caret.

WebKit Review Bot

Comment 5 2012-12-05 21:43:01 PST

Comment on attachment 177891 [details] Patch Attachment 177891 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/15157641 New failing tests: fast/repaint/japanese-rl-selection-repaint.html fast/repaint/selection-rl.html fast/repaint/japanese-rl-selection-repaint-in-regions.html fast/repaint/4774354.html fast/repaint/repaint-across-writing-mode-boundary.html fast/repaint/caret-outside-block.html fast/repaint/4776765.html

Tien-Ren Chen

Comment 6 2012-12-06 18:21:09 PST

Created attachment 178127 [details] Patch

Tien-Ren Chen

Comment 7 2012-12-06 18:34:00 PST

I think the patch is almost ready for production. The layout test failures are expected and should be safe to rebaseline. PTAL. Changes from last patch: 1. Corrected the layout test fail with RTL text. We no longer need to manually call flipForWritingMode(). repaintRectangle() will do the right thing for us. 2. Corrected the caret ghosting bug. We need to retain the previous Node* that rendered the caret for the last frame because we no longer store an absolute rect. 3. Added a layout test for transformation. There is actually an improvement over repaintRectangleInViewAndCompositedLayers() such that we now handle transformation correctly. Layout test difference: The 1-pixel inflation is removed from the repaint rect. I examined the layout test result and didn't observe any problems with that. The inflation is probably no longer needed with the container-aware approach.

Tien-Ren Chen

Comment 8 2012-12-06 18:43:24 PST

Created attachment 178133 [details] Patch

Tien-Ren Chen

Comment 9 2012-12-06 18:43:49 PST

Change from last patch: Add ChangeLog comments.

WebKit Review Bot

Comment 10 2012-12-06 22:23:46 PST

Comment on attachment 178133 [details] Patch Attachment 178133 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/15187165 New failing tests: fast/repaint/japanese-rl-selection-repaint.html fast/repaint/japanese-rl-selection-repaint-in-regions.html fast/repaint/selection-rl.html fast/repaint/caret-with-transformation.html fast/repaint/4774354.html fast/repaint/repaint-across-writing-mode-boundary.html fast/repaint/caret-outside-block.html fast/repaint/4776765.html

Build Bot

Comment 11 2012-12-06 22:45:20 PST

Comment on attachment 178133 [details] Patch Attachment 178133 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/15160978 New failing tests: fast/repaint/caret-with-transformation.html

Tien-Ren Chen

Comment 12 2012-12-07 15:41:32 PST

Created attachment 178295 [details] Patch

Tien-Ren Chen

Comment 13 2012-12-07 15:42:40 PST

Change from last patch: 1. Rebased to latest tree 2. Update TestExpectations PTAL. Thanks!

Tien-Ren Chen

Comment 14 2013-01-08 22:06:45 PST

Created attachment 181845 [details] Patch

Tien-Ren Chen

Comment 15 2013-01-08 22:07:13 PST

Comment on attachment 181845 [details] Patch Rebased

Simon Fraser (smfr)

Comment 16 2013-01-08 22:54:42 PST

Comment on attachment 181845 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=181845&action=review > Source/WebCore/rendering/RenderView.cpp:548 > +void RenderView::repaintSelection() const Who calls this?

Tien-Ren Chen

Comment 17 2013-01-09 17:46:12 PST

Created attachment 182030 [details] Patch

Tien-Ren Chen

Comment 18 2013-01-09 17:47:24 PST

(In reply to comment #16) > (From update of attachment 181845 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=181845&action=review > > > Source/WebCore/rendering/RenderView.cpp:548 > > +void RenderView::repaintSelection() const > > Who calls this? Sorry, one hunk was dropped during the rebase. The new patch corrected that.

WebKit Review Bot

Comment 19 2013-01-09 22:24:55 PST

Comment on attachment 182030 [details] Patch Clearing flags on attachment: 182030 Committed r139282: <http://trac.webkit.org/changeset/139282>

WebKit Review Bot

Comment 20 2013-01-09 22:25:00 PST

All reviewed patches have been landed. Closing bug.

Tim Horton

Comment 21 2013-01-29 18:55:32 PST

This caused https://bugs.webkit.org/show_bug.cgi?id=108283.

Simon Fraser (smfr)

Comment 22 2013-01-30 12:56:51 PST

*** Bug 23629 has been marked as a duplicate of this bug. ***

Ryosuke Niwa

Comment 23 2013-03-20 15:25:11 PDT

This never got rebaselined :(

Tien-Ren Chen

Comment 24 2013-03-20 15:29:48 PDT

Thanks for reminding! Give me one day. I'll do it now.

Ryosuke Niwa

Comment 25 2013-03-20 15:56:20 PDT

(In reply to comment #24) > Thanks for reminding! Give me one day. I'll do it now. Done that in http://trac.webkit.org/changeset/146403.

Tien-Ren Chen

Comment 26 2013-03-20 16:38:12 PDT

(In reply to comment #25) > (In reply to comment #24) > > Thanks for reminding! Give me one day. I'll do it now. > > Done that in http://trac.webkit.org/changeset/146403. LGTM. Thank you so much! I owe you one. :)

Note You need to log in before you can comment on or make changes to this bug.