[iOS][WK2] Support speculative tiling
Created attachment 230736 [details] Patch
Comment on attachment 230736 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=230736&action=review > Source/WebCore/ChangeLog:9 > + Move ScrollView::setScrollVelocity() and ScrollView::computeCoverageRect() to FrameView. > + When speculative tiling is disabled, return an unmodified exposed rect. You could leave it in ScrollView, override in FrameView and check m_speculativeTilingEnabled before calling to the superclass. But, was there a reason it was in ScrollView itself in the first place? > Source/WebCore/page/FrameView.cpp:4125 > +FloatRect FrameView::computeCoverageRect(double horizontalMargin, double verticalMargin) const It still makes me sad that this is separate from the one in TileController (that this logic is spread out).
Comment on attachment 230736 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=230736&action=review > Source/WebCore/page/FrameView.h:694 > + double m_horizontalVelocity; > + double m_verticalVelocity; > + double m_scaleChangeRate; > + double m_lastVelocityUpdateTime; Shame to bloat all FrameViews with this, when only the main one will care. Maybe move it into a unique_ptr'd object?
(In reply to comment #3) > (From update of attachment 230736 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=230736&action=review > > > Source/WebCore/page/FrameView.h:694 > > + double m_horizontalVelocity; > > + double m_verticalVelocity; > > + double m_scaleChangeRate; > > + double m_lastVelocityUpdateTime; > > Shame to bloat all FrameViews with this, when only the main one will care. Maybe move it into a unique_ptr'd object? It is not worth optimizing memory for document-level classes. There are never enough objects that it causes issues.
(In reply to comment #2) > (From update of attachment 230736 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=230736&action=review > > > Source/WebCore/ChangeLog:9 > > + Move ScrollView::setScrollVelocity() and ScrollView::computeCoverageRect() to FrameView. > > + When speculative tiling is disabled, return an unmodified exposed rect. > > You could leave it in ScrollView, override in FrameView and check m_speculativeTilingEnabled before calling to the superclass. But, was there a reason it was in ScrollView itself in the first place? It was on ScrollView because it is related to the exposedRect() concept. > > Source/WebCore/page/FrameView.cpp:4125 > > +FloatRect FrameView::computeCoverageRect(double horizontalMargin, double verticalMargin) const > > It still makes me sad that this is separate from the one in TileController (that this logic is spread out). Maybe I am misunderstanding something here: isn't TileController generic for any tiled graphic layer? I agree the code does not really belong on ScrollView/FrameView, but I was under the impression the final home for this would be the compositor or something like that.
(In reply to comment #5) > (In reply to comment #2) > > (From update of attachment 230736 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=230736&action=review > > > > > Source/WebCore/ChangeLog:9 > > > + Move ScrollView::setScrollVelocity() and ScrollView::computeCoverageRect() to FrameView. > > > + When speculative tiling is disabled, return an unmodified exposed rect. > > > > You could leave it in ScrollView, override in FrameView and check m_speculativeTilingEnabled before calling to the superclass. But, was there a reason it was in ScrollView itself in the first place? > > It was on ScrollView because it is related to the exposedRect() concept. Right. But no actual reason to keep it there. OK! > > > Source/WebCore/page/FrameView.cpp:4125 > > > +FloatRect FrameView::computeCoverageRect(double horizontalMargin, double verticalMargin) const > > > > It still makes me sad that this is separate from the one in TileController (that this logic is spread out). > > Maybe I am misunderstanding something here: isn't TileController generic for any tiled graphic layer? Sure, but e.g. TileController::computeTileCoverageRect does math that is special for scrolling tiled layers (etc.), based on information pushed down to it. > I agree the code does not really belong on ScrollView/FrameView, but I was under the impression the final home for this would be the compositor or something like that. Maybe! I think you should land this as-is, in any case.
Committed r168233: <http://trac.webkit.org/changeset/168233>