From bf198ba02239d207b7211ea7e48d3dc75ff64938 Mon Sep 17 00:00:00 2001 From: Daisuke Akatsuka Date: Mon, 29 Mar 2021 08:21:30 +0000 Subject: [PATCH] Bug 1699040: Avoid rendering items that are out of displayed area. r=jdescottes So far, we had rendered all of the animation graphs amenably. Thus, since it took time if the number of animations is huge, it had affected the performance. This bug was caused by that reason. In order to resolve it, we make it restrict to render graphs that are out of the displaying area. Differential Revision: https://phabricator.services.mozilla.com/D109845 --- .../animation/components/AnimationItem.js | 39 +++++++----- .../animation/components/AnimationList.js | 7 ++- .../components/AnimationListContainer.js | 60 +++++++++++++++++-- 3 files changed, 83 insertions(+), 23 deletions(-) diff --git a/devtools/client/inspector/animation/components/AnimationItem.js b/devtools/client/inspector/animation/components/AnimationItem.js index aca9d89a7c71..2e44dc404d78 100644 --- a/devtools/client/inspector/animation/components/AnimationItem.js +++ b/devtools/client/inspector/animation/components/AnimationItem.js @@ -27,6 +27,7 @@ class AnimationItem extends Component { emitEventForTest: PropTypes.func.isRequired, getAnimatedPropertyMap: PropTypes.func.isRequired, getNodeFromActor: PropTypes.func.isRequired, + isDisplayable: PropTypes.bool.isRequired, selectAnimation: PropTypes.func.isRequired, selectedAnimation: PropTypes.object.isRequired, setHighlightedNode: PropTypes.func.isRequired, @@ -52,6 +53,7 @@ class AnimationItem extends Component { shouldComponentUpdate(nextProps, nextState) { return ( + this.props.isDisplayable !== nextProps.isDisplayable || this.state.isSelected !== nextState.isSelected || this.props.animation !== nextProps.animation || this.props.timeScale !== nextProps.timeScale @@ -72,6 +74,7 @@ class AnimationItem extends Component { emitEventForTest, getAnimatedPropertyMap, getNodeFromActor, + isDisplayable, selectAnimation, setHighlightedNode, setSelectedNode, @@ -86,22 +89,26 @@ class AnimationItem extends Component { `animation-item ${animation.state.type} ` + (isSelected ? "selected" : ""), }, - AnimationTarget({ - animation, - dispatch, - emitEventForTest, - getNodeFromActor, - setHighlightedNode, - setSelectedNode, - }), - SummaryGraph({ - animation, - emitEventForTest, - getAnimatedPropertyMap, - selectAnimation, - simulateAnimation, - timeScale, - }) + isDisplayable + ? [ + AnimationTarget({ + animation, + dispatch, + emitEventForTest, + getNodeFromActor, + setHighlightedNode, + setSelectedNode, + }), + SummaryGraph({ + animation, + emitEventForTest, + getAnimatedPropertyMap, + selectAnimation, + simulateAnimation, + timeScale, + }), + ] + : null ); } } diff --git a/devtools/client/inspector/animation/components/AnimationList.js b/devtools/client/inspector/animation/components/AnimationList.js index 6e222014a364..d07de37df5ec 100644 --- a/devtools/client/inspector/animation/components/AnimationList.js +++ b/devtools/client/inspector/animation/components/AnimationList.js @@ -20,6 +20,7 @@ class AnimationList extends PureComponent { return { animations: PropTypes.arrayOf(PropTypes.object).isRequired, dispatch: PropTypes.func.isRequired, + displayableRange: PropTypes.object.isRequired, emitEventForTest: PropTypes.func.isRequired, getAnimatedPropertyMap: PropTypes.func.isRequired, getNodeFromActor: PropTypes.func.isRequired, @@ -35,6 +36,7 @@ class AnimationList extends PureComponent { const { animations, dispatch, + displayableRange, emitEventForTest, getAnimatedPropertyMap, getNodeFromActor, @@ -45,17 +47,20 @@ class AnimationList extends PureComponent { timeScale, } = this.props; + const { startIndex, endIndex } = displayableRange; + return dom.ul( { className: "animation-list", }, - animations.map(animation => + animations.map((animation, index) => AnimationItem({ animation, dispatch, emitEventForTest, getAnimatedPropertyMap, getNodeFromActor, + isDisplayable: startIndex <= index && index <= endIndex, selectAnimation, setHighlightedNode, setSelectedNode, diff --git a/devtools/client/inspector/animation/components/AnimationListContainer.js b/devtools/client/inspector/animation/components/AnimationListContainer.js index bda1647086ac..5de0b414e3bd 100644 --- a/devtools/client/inspector/animation/components/AnimationListContainer.js +++ b/devtools/client/inspector/animation/components/AnimationListContainer.js @@ -6,12 +6,12 @@ const { createFactory, + createRef, PureComponent, } = require("devtools/client/shared/vendor/react"); const { connect } = require("devtools/client/shared/vendor/react-redux"); const dom = require("devtools/client/shared/vendor/react-dom-factories"); const PropTypes = require("devtools/client/shared/vendor/react-prop-types"); -const ReactDOM = require("devtools/client/shared/vendor/react-dom"); const AnimationList = createFactory( require("devtools/client/inspector/animation/components/AnimationList") @@ -27,6 +27,7 @@ const { findOptimalTimeInterval, } = require("devtools/client/inspector/animation/utils/utils"); const { getStr } = require("devtools/client/inspector/animation/utils/l10n"); +const { throttle } = require("devtools/shared/throttle"); // The minimum spacing between 2 time graduation headers in the timeline (px). const TIME_GRADUATION_MIN_SPACING = 40; @@ -55,14 +56,43 @@ class AnimationListContainer extends PureComponent { constructor(props) { super(props); + this._ref = createRef(); + + this.updateDisplayableRange = throttle( + this.updateDisplayableRange, + 100, + this + ); + this.state = { // tick labels and lines on the progress inspection panel ticks: [], + // Displayable range. + displayableRange: { startIndex: 0, endIndex: 0 }, }; } componentDidMount() { - this.updateState(this.props); + this.updateTicks(this.props); + + const current = this._ref.current; + this._inspectionPanelEl = current.querySelector( + ".progress-inspection-panel" + ); + this._inspectionPanelEl.addEventListener("scroll", () => { + this.updateDisplayableRange(); + }); + + this._animationListEl = current.querySelector(".animation-list"); + const resizeObserver = new current.ownerGlobal.ResizeObserver(() => { + this.updateDisplayableRange(); + }); + resizeObserver.observe(this._animationListEl); + + const animationItemEl = current.querySelector(".animation-item"); + this._itemHeight = animationItemEl.offsetHeight; + + this.updateDisplayableRange(); } componentDidUpdate(prevProps) { @@ -73,13 +103,29 @@ class AnimationListContainer extends PureComponent { timeScale.zeroPositionTime !== prevProps.timeScale.zeroPositionTime || sidebarWidth !== prevProps.sidebarWidth ) { - this.updateState(this.props); + this.updateTicks(this.props); } } - updateState(props) { + /** + * Since it takes too much time if we render all of animation graphs, + * we restrict to render the items that are not in displaying area. + * This function calculates the displayable item range. + */ + updateDisplayableRange() { + const count = + Math.floor(this._animationListEl.offsetHeight / this._itemHeight) + 1; + const index = Math.floor( + this._inspectionPanelEl.scrollTop / this._itemHeight + ); + this.setState({ + displayableRange: { startIndex: index, endIndex: index + count }, + }); + } + + updateTicks(props) { const { animations, timeScale } = props; - const tickLinesEl = ReactDOM.findDOMNode(this).querySelector(".tick-lines"); + const tickLinesEl = this._ref.current.querySelector(".tick-lines"); const width = tickLinesEl.offsetWidth; const animationDuration = timeScale.getDuration(); const minTimeInterval = @@ -132,11 +178,12 @@ class AnimationListContainer extends PureComponent { simulateAnimation, timeScale, } = this.props; - const { ticks } = this.state; + const { displayableRange, ticks } = this.state; return dom.div( { className: "animation-list-container", + ref: this._ref, }, ProgressInspectionPanel({ indicator: CurrentTimeScrubber({ @@ -149,6 +196,7 @@ class AnimationListContainer extends PureComponent { list: AnimationList({ animations, dispatch, + displayableRange, emitEventForTest, getAnimatedPropertyMap, getNodeFromActor,