Bug 94985 - Compositing and transforms break caret rendering
Summary: Compositing and transforms break caret rendering
Status: RESOLVED DUPLICATE of bug 15671
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Major
Assignee: Arvid Nilsson
Keywords: InRadar
Depends on:
Reported: 2012-08-24 17:15 PDT by Gregg
Modified: 2013-01-22 15:46 PST (History)
10 users (show)

See Also:

html file with the scale3d issue (469 bytes, text/html)
2012-08-24 17:15 PDT, Gregg
no flags Details
html file with the translate3d issue (479 bytes, text/html)
2012-08-24 17:40 PDT, Gregg
no flags Details
Another testcase (686 bytes, text/html)
2012-08-27 13:04 PDT, Simon Fraser (smfr)
no flags Details
Patch (4.13 KB, patch)
2012-10-18 16:42 PDT, Arvid Nilsson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Gregg 2012-08-24 17:15:07 PDT
I am seeing repaint issues with transforming elements that have carets. This includes textareas, inputs, and divs (possibly more, but that's all I've tested). Adding the scale3d css property to an element the caret no longer blinks and no longer moves when using the arrow keys. When the arrow keys are moved, the caret DOES actually move, just not visually, since when you type the text appears in the correct spot.

I've attached an HTML file that shows the issue. Also, here is a jsfiddle showing the same:


One thing to point out is that when I scale using scale3d(1.01,1.01,1) the issue does not seem to occur.
Comment 1 Gregg 2012-08-24 17:15:51 PDT
Created attachment 160527 [details]
html file with the scale3d issue
Comment 2 Eric Seidel (no email) 2012-08-24 17:18:46 PDT
Wow.  Not only do we not repaint the caret correctly, but we also repaint the whole screen each blink. :(

Thanks for the bug!
Comment 3 Gregg 2012-08-24 17:19:56 PDT
No problem. Glad I could help!
Comment 4 Gregg 2012-08-24 17:39:01 PDT
I just found that the same problem happens with the translate3d function. See attached file.
Comment 5 Gregg 2012-08-24 17:40:17 PDT
Created attachment 160530 [details]
html file with the translate3d issue
Comment 6 Simon Fraser (smfr) 2012-08-27 12:44:13 PDT

*** This bug has been marked as a duplicate of bug 15671 ***
Comment 7 Eric Seidel (no email) 2012-08-27 12:48:43 PDT
Does that mean this is fixed?  The issue reproduces for me on Chromium-dev channel (as noted in comment #2).
Comment 8 Gregg 2012-08-27 12:49:27 PDT
How is this a duplicate? The original one is marked as resolved, but based on my examples it is very clearly not resolved.
Comment 9 Simon Fraser (smfr) 2012-08-27 12:49:59 PDT
Oh, sorry, no. I meant to dup to 18751, but there could be two different issues here:
a) caret under software transform
b) caret in compositing layers
Comment 10 Gregg 2012-08-27 12:54:40 PDT
But, with respect to the caret issues, it looks like 18751 ultimately links back to 15671 in the end anyway.
Comment 11 Simon Fraser (smfr) 2012-08-27 13:04:32 PDT
Created attachment 160786 [details]
Another testcase

Issue appears to be when a composited layer has a non-identity transform. Note the half-caret in the lower pair of form controls.
Comment 12 Radar WebKit Bug Importer 2012-08-27 13:05:28 PDT
Comment 13 Simon Fraser (smfr) 2012-08-27 13:09:56 PDT
void RenderLayerCompositor::recursiveRepaintLayerRect(RenderLayer* layer, const IntRect& rect)
    // FIXME: This method does not work correctly with transforms.
Comment 14 Simon Fraser (smfr) 2012-08-27 13:11:34 PDT
Caret repainting is pretty sucking; it invalidates the window and all compositing layers that intersect the caret rectangle. We should try harder.
Comment 15 Arvid Nilsson 2012-10-18 16:42:51 PDT
Created attachment 169506 [details]
Comment 16 Arvid Nilsson 2012-10-18 16:45:25 PDT
Comment on attachment 169506 [details]

This patch doesn't have test cases, proper commit msg etc, so the r? isn't really appropriate. I'm happy to hear your feedback though.
Comment 17 Simon Fraser (smfr) 2012-10-18 16:54:04 PDT
Comment on attachment 169506 [details]

View in context: https://bugs.webkit.org/attachment.cgi?id=169506&action=review

Approach seems OK. Would be nice to eliminate repaintRectangleInViewAndCompositedLayers() if we can.

> Source/WebCore/editing/FrameSelection.cpp:1331
> +        container = caretPainter->enclosingLayer()->enclosingCompositingLayer();

You should use RenderObject::containerForRepaint().

> Source/WebCore/editing/FrameSelection.cpp:1350
> +            container->setBackingNeedsRepaintInRect(oldContainerCaretRepaintBounds);

This should call container->repaintUsingContainer(container, oldContainerCaretRepaintBounds).

> Source/WebCore/editing/FrameSelection.cpp:1354
> +                container->setBackingNeedsRepaintInRect(m_containerCaretRepaintBounds);

Comment 18 Arvid Nilsson 2012-10-18 17:05:00 PDT
Thanks for your feedback! Getting rid of repaintRectangleInViewAndCompositedLayers should be possible, thanks to containerForRepaint/repaintUsingContainer which I just learned about, it generalizes to the non-composited case.

I'll be back another day with a better patch and some layouttest.
Comment 19 Arvid Nilsson 2012-11-11 09:09:38 PST
(In reply to comment #14)
> Caret repainting is pretty sucking; it invalidates the window and all compositing layers that intersect the caret rectangle. We should try harder.

Hi there! I've been thinking about this for some time now, and I would like to ask for your feedback on the idea to make the caret a separate AC layer on ports that support AC (which are the only ones that are going to run into this bug)

1. Faster painting (well, because there is no painting, only compositing) - not even the containing layer needs to be repainted. This can be used to add the blinking effect (in BlackBerry's case, even with fading) without having to pay a penalty in continuous repaints.
2. When the caret moves, no need to repaint the container where the caret was, just move the caret AC layer to the new container.

1. The caret could be painted in front of content it was really supposed to be behind. I don't have enough insight into when layers are created to say if this is a big problem - hopefully the thing in front of it will be a separate layer.

I didn't intend for the caret to force the creation of layers by itself - it would only be added to the container's containing AC layer (which might be the root layer actually). This could augment the effect of Drawback 2 because content in front of the caret would not be forced to have a layer.

This could be done similar to the scrollbar layers (every RenderLayerCompositor has one), or it could be a layer that the FrameSelection class owns, and that it moves from layer to layer.
Comment 20 Simon Fraser (smfr) 2013-01-22 15:45:00 PST

*** This bug has been marked as a duplicate of bug 15671 ***
Comment 21 Simon Fraser (smfr) 2013-01-22 15:46:46 PST
Bug 103955 also fixed some caret issues.