Changeset 206945 in webkit

Timestamp:

Oct 7, 2016, 5:20:05 PM (9 years ago)

Author:

Simon Fraser

Message:

[WK1 Mac] Fix repaints of fixed-background elements in layer-backed WebViews
​https://bugs.webkit.org/show_bug.cgi?id=163154
rdar://problem/28674216

Reviewed by Tim Horton.

r55159 added code to counteract AppKit's adjustment of dirty regions during scrolling,
by offsetting the dirty rect in -setNeedsDisplayInRect: if the call happens during
the NSViewBoundsDidChangeNotification handling.

However, AppKit only does dirty region adjustment in the code path that blits ("copy
on scroll"), so r55159 produces incorrect behavior in, for example, layer-backed views.

Fix by overriding -[NSClipView _canCopyOnScrollForDeltaX:deltaY:] to know if a single
scroll is going to blit, and only do adjustments in -setNeedsDisplayInRect: if it is.

• WebView/WebClipView.h:
• WebView/WebClipView.mm:

(-[WebClipView _immediateScrollToPoint:]):
(-[WebClipView _canCopyOnScrollForDeltaX:deltaY:]):
(-[WebClipView currentScrollIsBlit]):

• WebView/WebHTMLView.mm:

(-[WebHTMLView setNeedsDisplayInRect:]):
(-[WebHTMLView drawRect:]):

Location:

trunk/Source/WebKit/mac

Files:

• ChangeLog (modified) (1 diff)
• WebView/WebClipView.h (modified) (1 diff)
• WebView/WebClipView.mm (modified) (3 diffs)
• WebView/WebHTMLView.mm (modified) (2 diffs)

trunk/Source/WebKit/mac/ChangeLog

@@ +1 @@
+2016-10-07  Simon Fraser  <simon.fraser@apple.com>
+
+        [WK1 Mac] Fix repaints of fixed-background elements in layer-backed WebViews
+        https://bugs.webkit.org/show_bug.cgi?id=163154
+        rdar://problem/28674216
+
+        Reviewed by Tim Horton.
+        
+        r55159 added code to counteract AppKit's adjustment of dirty regions during scrolling,
+        by offsetting the dirty rect in -setNeedsDisplayInRect: if the call happens during
+        the NSViewBoundsDidChangeNotification handling.
+        
+        However, AppKit only does dirty region adjustment in the code path that blits ("copy
+        on scroll"), so r55159 produces incorrect behavior in, for example, layer-backed views.
+        
+        Fix by overriding -[NSClipView _canCopyOnScrollForDeltaX:deltaY:] to know if a single
+        scroll is going to blit, and only do adjustments in -setNeedsDisplayInRect: if it is.
+
+        * WebView/WebClipView.h:
+        * WebView/WebClipView.mm:
+        (-[WebClipView _immediateScrollToPoint:]):
+        (-[WebClipView _canCopyOnScrollForDeltaX:deltaY:]):
+        (-[WebClipView currentScrollIsBlit]):
+        * WebView/WebHTMLView.mm:
+        (-[WebHTMLView setNeedsDisplayInRect:]):
+        (-[WebHTMLView drawRect:]):
+
 2016-10-06  Youenn Fablet  <youenn@apple.com>
 

trunk/Source/WebKit/mac/WebView/WebClipView.h

@@ -35 +35 @@
     BOOL _haveAdditionalClip;
     BOOL _isScrolling;
+    BOOL _currentScrollIsBlit;
     NSRect _additionalClip;
 }
 
+- (BOOL)currentScrollIsBlit;
 - (void)setAdditionalClip:(NSRect)additionalClip;
 - (void)resetAdditionalClip;

trunk/Source/WebKit/mac/WebView/WebClipView.mm

@@ -51 +51 @@
 @interface NSClipView (WebNSClipViewDetails)
 - (void)_immediateScrollToPoint:(NSPoint)newOrigin;
+- (BOOL)_canCopyOnScrollForDeltaX:(CGFloat)deltaX deltaY:(CGFloat)deltaY;
 @end
 
@@ -105 +106 @@
 {
     _isScrolling = YES;
+    _currentScrollIsBlit = NO;
 
     [[self window] _disableDelayedWindowDisplay];
@@ -125 +127 @@
 
     _isScrolling = NO;
+}
+
+- (BOOL)_canCopyOnScrollForDeltaX:(CGFloat)deltaX deltaY:(CGFloat)deltaY
+{
+    _currentScrollIsBlit = [super _canCopyOnScrollForDeltaX:deltaX deltaY:deltaY];
+    return _currentScrollIsBlit;
+}
+
+- (BOOL)currentScrollIsBlit
+{
+    return _currentScrollIsBlit;
 }
 

trunk/Source/WebKit/mac/WebView/WebHTMLView.mm

@@ -4111 +4111 @@
 #endif
 
+static BOOL currentScrollIsBlit(NSView *clipView)
+{
+#if PLATFORM(MAC)
+    return [clipView isKindOfClass:[WebClipView class]] && [(WebClipView *)clipView currentScrollIsBlit];
+#else
+    return NO;
+#endif
+}
+
+// FIXME: this entire function could be #ifdeffed out on iOS. The below workaround is AppKit-specific.
 - (void)setNeedsDisplayInRect:(NSRect)invalidRect
 {
-    if (_private->inScrollPositionChanged) {
+    if (_private->inScrollPositionChanged && currentScrollIsBlit([self superview])) {
         // When scrolling, the dirty regions are adjusted for the scroll only
         // after NSViewBoundsDidChangeNotification is sent. Translate the invalid
@@ -4228 +4238 @@
     const float cWastedSpaceThreshold = 0.75f; 
     BOOL useUnionedRect = (count <= 1) || (count > cRectThreshold); 
-    if (!useUnionedRect) { 
+    if (!useUnionedRect) {
         // Attempt to guess whether or not we should use the unioned rect or the individual rects. 
         // We do this by computing the percentage of "wasted space" in the union.  If that wasted space