Bug 103955 - Make caret repainting container-aware
Summary: Make caret repainting container-aware
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
: 23629 (view as bug list)
Depends on:
Blocks: 104200 119122
  Show dependency treegraph
 
Reported: 2012-12-03 18:16 PST by Tien-Ren Chen
Modified: 2013-07-25 20:56 PDT (History)
12 users (show)

See Also:


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

Note You need to log in before you can comment on or make changes to this bug.
Description Tien-Ren Chen 2012-12-03 18:16:04 PST
Make caret repainting container aware
Comment 1 Tien-Ren Chen 2012-12-03 18:22:37 PST
Created attachment 177390 [details]
Patch
Comment 2 Tien-Ren Chen 2012-12-03 19:16:09 PST
Created attachment 177394 [details]
Patch
Comment 3 Tien-Ren Chen 2012-12-05 17:59:00 PST
Created attachment 177891 [details]
Patch
Comment 4 Tien-Ren Chen 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.
Comment 5 WebKit Review Bot 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
Comment 6 Tien-Ren Chen 2012-12-06 18:21:09 PST
Created attachment 178127 [details]
Patch
Comment 7 Tien-Ren Chen 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.
Comment 8 Tien-Ren Chen 2012-12-06 18:43:24 PST
Created attachment 178133 [details]
Patch
Comment 9 Tien-Ren Chen 2012-12-06 18:43:49 PST
Change from last patch: Add ChangeLog comments.
Comment 10 WebKit Review Bot 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
Comment 11 Build Bot 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
Comment 12 Tien-Ren Chen 2012-12-07 15:41:32 PST
Created attachment 178295 [details]
Patch
Comment 13 Tien-Ren Chen 2012-12-07 15:42:40 PST
Change from last patch:
1. Rebased to latest tree
2. Update TestExpectations

PTAL. Thanks!
Comment 14 Tien-Ren Chen 2013-01-08 22:06:45 PST
Created attachment 181845 [details]
Patch
Comment 15 Tien-Ren Chen 2013-01-08 22:07:13 PST
Comment on attachment 181845 [details]
Patch

Rebased
Comment 16 Simon Fraser (smfr) 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?
Comment 17 Tien-Ren Chen 2013-01-09 17:46:12 PST
Created attachment 182030 [details]
Patch
Comment 18 Tien-Ren Chen 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.
Comment 19 WebKit Review Bot 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>
Comment 20 WebKit Review Bot 2013-01-09 22:25:00 PST
All reviewed patches have been landed.  Closing bug.
Comment 21 Tim Horton 2013-01-29 18:55:32 PST
This caused https://bugs.webkit.org/show_bug.cgi?id=108283.
Comment 22 Simon Fraser (smfr) 2013-01-30 12:56:51 PST
*** Bug 23629 has been marked as a duplicate of this bug. ***
Comment 23 Ryosuke Niwa 2013-03-20 15:25:11 PDT
This never got rebaselined :(
Comment 24 Tien-Ren Chen 2013-03-20 15:29:48 PDT
Thanks for reminding! Give me one day. I'll do it now.
Comment 25 Ryosuke Niwa 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.
Comment 26 Tien-Ren Chen 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. :)