We should not repaint the entirety of a layer every time it is dirtied.
Created attachment 214611 [details] patch
Attachment 214611 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/WebCore.exp.in', u'Source/WebCore/WebCore.xcodeproj/project.pbxproj', u'Source/WebCore/platform/graphics/mac/WebLayer.h', u'Source/WebCore/platform/graphics/mac/WebLayer.mm', u'Source/WebKit2/ChangeLog', u'Source/WebKit2/WebProcess/WebPage/mac/PlatformCALayerRemote.cpp', u'Source/WebKit2/WebProcess/WebPage/mac/RemoteLayerBackingStore.h', u'Source/WebKit2/WebProcess/WebPage/mac/RemoteLayerBackingStore.mm']" exit_code: 1 Source/WebCore/platform/graphics/mac/WebLayer.h:47: The parameter name "context" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 9 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 214611 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=214611&action=review > Source/WebCore/platform/graphics/mac/WebLayer.mm:64 > + bool isTiledLayer = [layer isKindOfClass:[CATiledLayer class]]; Move this down. > Source/WebCore/platform/graphics/mac/WebLayer.mm:117 > + // If we have no dirty rects, repaint the whole layer. Comment is the reverse of the immediately following statement which is confusing. > Source/WebKit2/WebProcess/WebPage/mac/RemoteLayerBackingStore.mm:124 > + Region cleanRegion(layerBounds); > + cleanRegion.subtract(m_dirtyRegion); > + > + RetainPtr<CGImageRef> frontImage = m_frontBuffer->makeCGImage(); > + > + for (const auto& rect : cleanRegion.rects()) > + context->drawNativeImage(frontImage.get(), m_frontBuffer->size(), ColorSpaceDeviceRGB, rect, rect); Seems like this could be much slower than copying over the entire layer in many cases.
(In reply to comment #3) > (From update of attachment 214611 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=214611&action=review > > > Source/WebCore/platform/graphics/mac/WebLayer.mm:64 > > + bool isTiledLayer = [layer isKindOfClass:[CATiledLayer class]]; > > Move this down. > > > Source/WebCore/platform/graphics/mac/WebLayer.mm:117 > > + // If we have no dirty rects, repaint the whole layer. > > Comment is the reverse of the immediately following statement which is confusing. OK, I'll flip it around. > > Source/WebKit2/WebProcess/WebPage/mac/RemoteLayerBackingStore.mm:124 > > + Region cleanRegion(layerBounds); > > + cleanRegion.subtract(m_dirtyRegion); > > + > > + RetainPtr<CGImageRef> frontImage = m_frontBuffer->makeCGImage(); > > + > > + for (const auto& rect : cleanRegion.rects()) > > + context->drawNativeImage(frontImage.get(), m_frontBuffer->size(), ColorSpaceDeviceRGB, rect, rect); > > Seems like this could be much slower than copying over the entire layer in many cases. Yeah. I meant to use the five-or-less rect set, not the raw set.
Created attachment 214622 [details] revised
Comment on attachment 214622 [details] revised View in context: https://bugs.webkit.org/attachment.cgi?id=214622&action=review > Source/WebKit2/WebProcess/WebPage/mac/RemoteLayerBackingStore.mm:99 > + setNeedsDisplay(); It's odd to call setNeedsDisplay() inside of display(). > Source/WebKit2/WebProcess/WebPage/mac/RemoteLayerBackingStore.mm:108 > + setNeedsDisplay(indicatorRect); Ditto.
http://trac.webkit.org/changeset/157658