webkit-backface-visibility on a parent element stops background-position from updating

[Nathan Stitt]

If a parent element in the DOM has -webkit-backface-visibility: none applied,  it's children no longer re-render when their background-position is updated.

http://jsfiddle.net/nathanstitt/JcX4g/3/ will illustrate the issue.   Observe how the magnifier image shows the high-resolution image when it's moved.  Uncommenting line 3 ( webkit-backface-visibility) and re-rerunning the jsfiddle and the image never redraws, even though the attributes in the DOM are updated.

I encountered this when using the flexslider library (http://flexslider.woothemes.com/), which applies the backface-visibility.

Hope this helps, contact me if you need any further details.

[Nathan Stitt]

Forgot to mention, removing the div & re-adding it does force an update to the background-position and it will re-render properly.  http://jsfiddle.net/JcX4g/5/ line 28 shows the work-around.

[Nathan Stitt]

I've also prepared a plain javascript version of the test case:  http://jsfiddle.net/nathanstitt/JcX4g/7/

[Simon Fraser (smfr)]

The bug occurs when there's both a position change and a background-position change.

[Radar WebKit Bug Importer]

<rdar://problem/13932329>

[Simon Fraser (smfr)]

Created attachment 202210 [details]
Testcase

[Simon Fraser (smfr)]

Not a recent regression.

[Simon Fraser (smfr)]

Probably regressed with http://trac.webkit.org/changeset/102952

[Simon Fraser (smfr)]

The optimization added in r102952 is basically incorrect. RenderStyle::diff() returns StyleDifferenceLayoutPositionedMovementOnly, but that doesn't mean that some other property changes don't require repainting.

[Simon Fraser (smfr)]

Created attachment 202216 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 202216 [details]
Patch

Wrong bug.

[Simon Fraser (smfr)]

Created attachment 204781 [details]
Patch

[Darin Adler]

Comment on attachment 204781 [details]
Patch

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

> Source/WebCore/rendering/RenderObject.h:1255
> +            if (oldStyle && m_style->diffRequiresRepaint(oldStyle))

Is there some other level of the code already doing a diff? Is it bad for performance to do this here too?

> Source/WebCore/rendering/style/RenderStyle.cpp:797
> +    unsigned contextSensitiveProperties = 0;

This should be named changedContextSensitiveProperties. Leaving out the word “changed” makes it hard to understand that this is an out argument and whether 0 is the correct value to initialize to.

[Simon Fraser (smfr)]

(In reply to comment #12)
> (From update of attachment 204781 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=204781&action=review
> 
> > Source/WebCore/rendering/RenderObject.h:1255
> > +            if (oldStyle && m_style->diffRequiresRepaint(oldStyle))
> 
> Is there some other level of the code already doing a diff? Is it bad for performance to do this here too?

There is, but the earlier diff bailed early, never testing the repaint properties. So this isn't another full diff.

> > Source/WebCore/rendering/style/RenderStyle.cpp:797
> > +    unsigned contextSensitiveProperties = 0;
> 
> This should be named changedContextSensitiveProperties. Leaving out the word “changed” makes it hard to understand that this is an out argument and whether 0 is the correct value to initialize to.

Agreedn.

[Simon Fraser (smfr)]

http://trac.webkit.org/changeset/151622