From 22608011f215957e6f095f94761d6b92d1aac02c Mon Sep 17 00:00:00 2001 From: Hristo Hristov Date: Wed, 8 Jun 2016 15:41:30 +0300 Subject: [PATCH] Avoid excessive requestLayout calls (#2270) Added View.isLayoutRequired - to check if Layout call is needed after a measure call. Use in list-view. Measure pass now get all its properties from nativeLayoutParams property instead of using instance dependency properties (e.g. perf improvement). List-view now layouts cells only if there is need to. List-view now measure rows with the specified rowHeight. --- tns-core-modules/ui/core/view-common.ts | 37 +++++++------------ tns-core-modules/ui/core/view.android.ts | 4 ++ tns-core-modules/ui/core/view.d.ts | 7 ++-- tns-core-modules/ui/core/view.ios.ts | 7 +++- .../ui/list-view/list-view-common.ts | 3 +- .../ui/list-view/list-view.ios.ts | 9 +++-- 6 files changed, 33 insertions(+), 34 deletions(-) diff --git a/tns-core-modules/ui/core/view-common.ts b/tns-core-modules/ui/core/view-common.ts index 0ff1d431e..af94abba2 100644 --- a/tns-core-modules/ui/core/view-common.ts +++ b/tns-core-modules/ui/core/view-common.ts @@ -15,18 +15,10 @@ import {PropertyMetadataSettings, PropertyChangeData, Property, ValueSource, Pro import {registerSpecialProperty} from "ui/builder/special-properties"; import {CommonLayoutParams, nativeLayoutParamsProperty} from "ui/styling/style"; import * as visualStateConstants from "ui/styling/visual-state-constants"; -import * as bindableModule from "ui/core/bindable"; import * as visualStateModule from "../styling/visual-state"; import * as animModule from "ui/animation"; import { Source } from "utils/debug"; -var bindable: typeof bindableModule; -function ensureBindable() { - if (!bindable) { - bindable = require("ui/core/bindable"); - } -} - var visualState: typeof visualStateModule; function ensureVisualState() { if (!visualState) { @@ -175,21 +167,18 @@ export class View extends ProxyObject implements definition.View { private _isLoaded: boolean; private _isLayoutValid: boolean = false; + private _updatingInheritedProperties: boolean; private _registeredAnimations: Array; public _domId: number; public _isAddedToNativeVisualTree = false; - public _cssClasses: Array = []; - public _gestureObservers = {}; public getGestureObservers(type: gestures.GestureTypes): Array { return this._gestureObservers[type]; } - private _updatingInheritedProperties: boolean; - constructor() { super({}); @@ -208,7 +197,6 @@ export class View extends ProxyObject implements definition.View { public addEventListener(arg: string | gestures.GestureTypes, callback: (data: observable.EventData) => void, thisArg?: any) { if (types.isString(arg)) { - arg = getEventOrGestureName(arg); var gesture = gestures.fromString(arg); @@ -521,6 +509,10 @@ export class View extends ProxyObject implements definition.View { throw new Error("View.style property is read-only."); } + get isLayoutRequired(): boolean { + return true; + } + get isLayoutValid(): boolean { return this._isLayoutValid; } @@ -687,7 +679,6 @@ export class View extends ProxyObject implements definition.View { } public static layoutChild(parent: View, child: View, left: number, top: number, right: number, bottom: number): void { - if (!child || !child._isVisible) { return; } @@ -702,11 +693,11 @@ export class View extends ProxyObject implements definition.View { var childHeight = child.getMeasuredHeight(); var vAlignment: string; - if (lp.height >= 0 && child.verticalAlignment === enums.VerticalAlignment.stretch) { + if (lp.height >= 0 && lp.verticalAlignment === enums.VerticalAlignment.stretch) { vAlignment = enums.VerticalAlignment.center; } else { - vAlignment = child.verticalAlignment; + vAlignment = lp.verticalAlignment; } let marginTop = lp.topMargin; @@ -736,11 +727,11 @@ export class View extends ProxyObject implements definition.View { } var hAlignment: string; - if (lp.width >= 0 && child.horizontalAlignment === enums.HorizontalAlignment.stretch) { + if (lp.width >= 0 && lp.horizontalAlignment === enums.HorizontalAlignment.stretch) { hAlignment = enums.HorizontalAlignment.center; } else { - hAlignment = child.horizontalAlignment; + hAlignment = lp.horizontalAlignment; } switch (hAlignment) { @@ -771,6 +762,7 @@ export class View extends ProxyObject implements definition.View { if (trace.enabled) { trace.write(child.parent + " :layoutChild: " + child + " " + childLeft + ", " + childTop + ", " + childRight + ", " + childBottom, trace.categories.Layout); } + child.layout(childLeft, childTop, childRight, childBottom); } @@ -779,7 +771,6 @@ export class View extends ProxyObject implements definition.View { var measureHeight = 0; if (child && child._isVisible) { - var width = utils.layout.getMeasureSpecSize(widthMeasureSpec); var widthMode = utils.layout.getMeasureSpecMode(widthMeasureSpec); @@ -840,7 +831,7 @@ export class View extends ProxyObject implements definition.View { // Parent has imposed an exact size on us case utils.layout.EXACTLY: resultSize = measureLength; - var stretched = horizontal ? view.horizontalAlignment === enums.HorizontalAlignment.stretch : view.verticalAlignment === enums.VerticalAlignment.stretch; + var stretched = horizontal ? lp.horizontalAlignment === enums.HorizontalAlignment.stretch : lp.verticalAlignment === enums.VerticalAlignment.stretch; // if stretched - nativeView wants to be our size. So be it. // else - nativeView wants to determine its own size. It can't be bigger than us. @@ -1044,9 +1035,7 @@ export class View extends ProxyObject implements definition.View { view.onUnloaded(); } - ensureBindable(); - - view._setValue(bindable.Bindable.bindingContextProperty, undefined, ValueSource.Inherited); + view._setValue(ProxyObject.bindingContextProperty, undefined, ValueSource.Inherited); view._eachSetProperty((property) => { if (!(property instanceof styling.Property) && property.inheritable) { view._resetValue(property, ValueSource.Inherited); @@ -1224,4 +1213,4 @@ export class View extends ProxyObject implements definition.View { // Check for a valid _nativeView instance return !!this._nativeView; } -} +} \ No newline at end of file diff --git a/tns-core-modules/ui/core/view.android.ts b/tns-core-modules/ui/core/view.android.ts index 8055476e1..1c1f7982a 100644 --- a/tns-core-modules/ui/core/view.android.ts +++ b/tns-core-modules/ui/core/view.android.ts @@ -280,6 +280,10 @@ export class View extends viewCommon.View { return this.android; } + get isLayoutRequired(): boolean { + return !this.isLayoutValid; + } + get isLayoutValid(): boolean { if (this._nativeView) { return !this._nativeView.isLayoutRequested(); diff --git a/tns-core-modules/ui/core/view.d.ts b/tns-core-modules/ui/core/view.d.ts index 42c1e876c..0fe721c5c 100644 --- a/tns-core-modules/ui/core/view.d.ts +++ b/tns-core-modules/ui/core/view.d.ts @@ -484,8 +484,8 @@ declare module "ui/core/view" { isLoaded: boolean; _addView(view: View, atIndex?: number); - _propagateInheritableProperties(view: View) - _inheritProperties(parentView: View) + _propagateInheritableProperties(view: View); + _inheritProperties(parentView: View); _removeView(view: View); _context: any /* android.content.Context */; @@ -499,8 +499,8 @@ declare module "ui/core/view" { public _applyXmlAttribute(attribute: string, value: any): boolean; - // TODO: Implement logic for stripping these lines out //@private + isLayoutRequired: boolean; _parentChanged(oldParent: View): void; _gestureObservers: any; _isInheritedChange(): boolean; @@ -610,5 +610,4 @@ declare module "ui/core/view" { */ _applyXmlAttribute(attributeName: string, attrValue: any): boolean; } - } diff --git a/tns-core-modules/ui/core/view.ios.ts b/tns-core-modules/ui/core/view.ios.ts index c19bb386f..6a7f377ee 100644 --- a/tns-core-modules/ui/core/view.ios.ts +++ b/tns-core-modules/ui/core/view.ios.ts @@ -94,6 +94,10 @@ export class View extends viewCommon.View { return this.ios; } + get isLayoutRequired(): boolean { + return (this._privateFlags & PFLAG_LAYOUT_REQUIRED) === PFLAG_LAYOUT_REQUIRED; + } + get isLayoutRequested(): boolean { return (this._privateFlags & PFLAG_FORCE_LAYOUT) === PFLAG_FORCE_LAYOUT; } @@ -109,7 +113,6 @@ export class View extends viewCommon.View { } public measure(widthMeasureSpec: number, heightMeasureSpec: number): void { - var measureSpecsChanged = this._setCurrentMeasureSpecs(widthMeasureSpec, heightMeasureSpec); var forceLayout = (this._privateFlags & PFLAG_FORCE_LAYOUT) === PFLAG_FORCE_LAYOUT; if (forceLayout || measureSpecsChanged) { @@ -136,9 +139,11 @@ export class View extends viewCommon.View { this.onLayout(left, top, right, bottom); this._privateFlags &= ~PFLAG_LAYOUT_REQUIRED; } + if (sizeChanged) { this._onSizeChanged(); } + this._privateFlags &= ~PFLAG_FORCE_LAYOUT; } diff --git a/tns-core-modules/ui/list-view/list-view-common.ts b/tns-core-modules/ui/list-view/list-view-common.ts index 7bc7473c8..722f31c87 100644 --- a/tns-core-modules/ui/list-view/list-view-common.ts +++ b/tns-core-modules/ui/list-view/list-view-common.ts @@ -174,7 +174,8 @@ export class ListView extends view.View implements definition.ListView { } private _getDataItem(index: number): any { - return this.items.getItem ? this.items.getItem(index) : this.items[index]; + let thisItems = this.items; + return thisItems.getItem ? thisItems.getItem(index) : thisItems[index]; } public _getDefaultItemContent(index: number): view.View { diff --git a/tns-core-modules/ui/list-view/list-view.ios.ts b/tns-core-modules/ui/list-view/list-view.ios.ts index ea5841231..b0fdedc8d 100644 --- a/tns-core-modules/ui/list-view/list-view.ios.ts +++ b/tns-core-modules/ui/list-view/list-view.ios.ts @@ -74,7 +74,7 @@ class DataSource extends NSObject implements UITableViewDataSource { owner._prepareCell(cell, indexPath); let cellView: view.View = cell.view; - if (cellView) { + if (cellView && cellView.isLayoutRequired) { // Arrange cell views. We do it here instead of _layoutCell because _layoutCell is called // from 'tableViewHeightForRowAtIndexPath' method too (in iOS 7.1) and we don't want to arrange the fake cell. let width = utils.layout.getMeasureSpecSize(owner.widthMeasureSpec); @@ -180,7 +180,7 @@ class UITableViewRowHeightDelegateImpl extends NSObject implements UITableViewDe return DEFAULT_HEIGHT; } - return owner.rowHeight; + return owner._rowHeight; } } @@ -208,11 +208,11 @@ export class ListView extends common.ListView { private _preparingCell: boolean = false; private _isDataDirty: boolean = false; private _map: Map; + public _rowHeight: number = -1; widthMeasureSpec: number = 0; constructor() { super(); - this._ios = new UITableView(); this._ios.registerClassForCellReuseIdentifier(ListViewCell.class(), CELLIDENTIFIER); this._ios.autoresizingMask = UIViewAutoresizing.UIViewAutoresizingNone; @@ -284,6 +284,7 @@ export class ListView extends common.ListView { } public _onRowHeightPropertyChanged(data: dependencyObservable.PropertyChangeData) { + this._rowHeight = data.newValue; if (data.newValue < 0) { this._nativeView.rowHeight = UITableViewAutomaticDimension; this._nativeView.estimatedRowHeight = DEFAULT_HEIGHT; @@ -320,7 +321,7 @@ export class ListView extends common.ListView { private _layoutCell(cellView: view.View, indexPath: NSIndexPath): number { if (cellView) { - let measuredSize = view.View.measureChild(this, cellView, this.widthMeasureSpec, infinity); + let measuredSize = view.View.measureChild(this, cellView, this.widthMeasureSpec, this._rowHeight < 0 ? infinity : this._rowHeight); let height = measuredSize.measuredHeight; this.setHeight(indexPath.row, height); return height;