Compositing and transforms break caret rendering

[Gregg]

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:

http://jsfiddle.net/galtschul/StbKg/

One thing to point out is that when I scale using scale3d(1.01,1.01,1) the issue does not seem to occur.

[Gregg]

Created attachment 160527 [details]
html file with the scale3d issue

[Eric Seidel (no email)]

Wow.  Not only do we not repaint the caret correctly, but we also repaint the whole screen each blink. :(

Thanks for the bug!

[Gregg]

No problem. Glad I could help!

[Gregg]

I just found that the same problem happens with the translate3d function. See attached file.

[Gregg]

Created attachment 160530 [details]
html file with the translate3d issue

[Simon Fraser (smfr)]

*** This bug has been marked as a duplicate of bug 15671 ***

[Eric Seidel (no email)]

Does that mean this is fixed?  The issue reproduces for me on Chromium-dev channel (as noted in comment #2).

[Gregg]

How is this a duplicate? The original one is marked as resolved, but based on my examples it is very clearly not resolved.

[Simon Fraser (smfr)]

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

[Gregg]

But, with respect to the caret issues, it looks like 18751 ultimately links back to 15671 in the end anyway.

[Simon Fraser (smfr)]

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.

[Radar WebKit Bug Importer]

<rdar://problem/12181912>

[Simon Fraser (smfr)]

void RenderLayerCompositor::recursiveRepaintLayerRect(RenderLayer* layer, const IntRect& rect)
{
    // FIXME: This method does not work correctly with transforms.

[Simon Fraser (smfr)]

Caret repainting is pretty sucking; it invalidates the window and all compositing layers that intersect the caret rectangle. We should try harder.

[Arvid Nilsson]

Created attachment 169506 [details]
Patch

[Arvid Nilsson]

Comment on attachment 169506 [details]
Patch

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.

[Simon Fraser (smfr)]

Comment on attachment 169506 [details]
Patch

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);

Ditto.

[Arvid Nilsson]

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.

[Arvid Nilsson]

(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)

Benefits:
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.

Drawbacks:
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.

[Simon Fraser (smfr)]

*** This bug has been marked as a duplicate of bug 15671 ***

[Simon Fraser (smfr)]

Bug 103955 also fixed some caret issues.