Bug 103955

Summary: Make caret repainting container-aware
Product: WebKit Reporter: Tien-Ren Chen <trchen>
Component: New BugsAssignee: 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: P2    
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
DetailsFormatted DiffDiff
Patch (16.16 KB, patch)
2012-12-03 19:16 PST, Tien-Ren Chen 		no flags
DetailsFormatted DiffDiff
Patch (9.91 KB, patch)
2012-12-05 17:59 PST, Tien-Ren Chen 		no flags
DetailsFormatted DiffDiff
Patch (11.57 KB, patch)
2012-12-06 18:21 PST, Tien-Ren Chen 		no flags
DetailsFormatted DiffDiff
Patch (11.52 KB, patch)
2012-12-06 18:43 PST, Tien-Ren Chen 		no flags
DetailsFormatted DiffDiff
Patch (14.12 KB, patch)
2012-12-07 15:41 PST, Tien-Ren Chen 		no flags
DetailsFormatted DiffDiff
Patch (14.24 KB, patch)
2013-01-08 22:06 PST, Tien-Ren Chen 		no flags
DetailsFormatted DiffDiff
Patch (14.60 KB, patch)
2013-01-09 17:46 PST, Tien-Ren Chen 		no flags
DetailsFormatted DiffDiff
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.