| 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: | P2 | ||||||||||||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||||||||||
| Hardware: | Unspecified | ||||||||||||||||||||
| OS: | Unspecified | ||||||||||||||||||||
| Bug Depends on: | |||||||||||||||||||||
| Bug Blocks: | 104200, 119122 | ||||||||||||||||||||
| Attachments: |
|
||||||||||||||||||||
|
Description
Tien-Ren Chen
2012-12-03 18:16:04 PST
Created attachment 177390 [details]
Patch
Created attachment 177394 [details]
Patch
Created attachment 177891 [details]
Patch
I'm moving the removal of RenderView::repaintRectangleInViewAndCompositedLayers() to a new patch, so this patch will be focused on refactoring the caret. 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 Created attachment 178127 [details]
Patch
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. Created attachment 178133 [details]
Patch
Change from last patch: Add ChangeLog comments. 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 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 Created attachment 178295 [details]
Patch
Change from last patch: 1. Rebased to latest tree 2. Update TestExpectations PTAL. Thanks! Created attachment 181845 [details]
Patch
Comment on attachment 181845 [details]
Patch
Rebased
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? Created attachment 182030 [details]
Patch
(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 on attachment 182030 [details] Patch Clearing flags on attachment: 182030 Committed r139282: <http://trac.webkit.org/changeset/139282> All reviewed patches have been landed. Closing bug. This caused https://bugs.webkit.org/show_bug.cgi?id=108283. *** Bug 23629 has been marked as a duplicate of this bug. *** This never got rebaselined :( Thanks for reminding! Give me one day. I'll do it now. (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. (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. :) |