Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: Throttle ProgressControl ui updates, add document.hidden check to time-displays #6150

Closed
wants to merge 11 commits into from
Closed

perf: Throttle ProgressControl ui updates, add document.hidden check to time-displays #6150

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
1912049
perf: VisiblityMixin, SeekBar timer improvements
brandonocasey Jul 30, 2019
7c786c1
always round and add percent
brandonocasey Aug 1, 2019
a31af57
only change on change
brandonocasey Aug 1, 2019
6480e36
rework live
brandonocasey Aug 1, 2019
410fca0
add tests
brandonocasey Aug 2, 2019
a30c6c3
update after re-enable
brandonocasey Aug 2, 2019
4c82849
remove unused test
brandonocasey Aug 2, 2019
1a517cb
pass update, so we can update on pause
brandonocasey Aug 2, 2019
7e5c4df
better child checking
brandonocasey Aug 2, 2019
fbc12bd
do not expose update
brandonocasey Aug 2, 2019
20a307f
fix tests
brandonocasey Aug 2, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 64 additions & 41 deletions src/js/control-bar/progress-control/load-progress-bar.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,20 @@
* @file load-progress-bar.js
*/
import Component from '../../component.js';
import {bind} from '../../utils/fn.js';
import * as Dom from '../../utils/dom.js';
import activeElement from '../../mixins/active-element.js';

// get the percent width of a time compared to the total end
const percentify = function(time, end) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There isn't a good reason for this to define this every time the update function is run.

Copy link
Contributor Author

@brandonocasey brandonocasey Aug 1, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also css only takes into account the first two decimal places so we should just always round.

// no NaN
let percent = (time / end) || 0;

percent = (percent >= 1 ? 1 : percent) * 100;

// round to two digits
return percent.toFixed(2) + '%';
};

/**
* Shows loading progress
Expand All @@ -23,7 +36,16 @@ class LoadProgressBar extends Component {
constructor(player, options) {
super(player, options);
this.partEls_ = [];
this.on(player, 'progress', this.update);
this.update = bind(this, this.update);
activeElement(this, {
startUpdate: () => {
this.on(player, 'progress', this.update);
this.update();
},
stopUpdate: () => {
this.off(player, 'progress', this.update);
}
});
}

/**
Expand Down Expand Up @@ -54,54 +76,55 @@ class LoadProgressBar extends Component {
* @listens Player#progress
*/
update(event) {
const liveTracker = this.player_.liveTracker;
const buffered = this.player_.buffered();
const duration = (liveTracker && liveTracker.isLive()) ? liveTracker.seekableEnd() : this.player_.duration();
const bufferedEnd = this.player_.bufferedEnd();
const children = this.partEls_;
const controlTextPercentage = this.$('.vjs-control-text-loaded-percentage');

// get the percent width of a time compared to the total end
const percentify = function(time, end, rounded) {
// no NaN
let percent = (time / end) || 0;

percent = (percent >= 1 ? 1 : percent) * 100;

if (rounded) {
percent = percent.toFixed(2);
this.requestAnimationFrame(() => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without requestAnimationFrame this ran on the main thread, and it took a good chunk of time too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hiding whitespace changes makes this a lot easier to diff.

const liveTracker = this.player_.liveTracker;
const buffered = this.player_.buffered();
const duration = (liveTracker && liveTracker.isLive()) ? liveTracker.seekableEnd() : this.player_.duration();
const bufferedEnd = this.player_.bufferedEnd();
const children = this.partEls_;
const controlTextPercentage = this.$('.vjs-control-text-loaded-percentage');
const percent = percentify(bufferedEnd, duration);

if (this.el_.style.width !== percent) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only update the style if it changed

// update the width of the progress bar
this.el_.style.width = percent;
}

return percent + '%';
};
// update the control-text
if (controlTextPercentage.textContent !== percent) {
Dom.textContent(controlTextPercentage, percent);
}

// update the width of the progress bar
this.el_.style.width = percentify(bufferedEnd, duration);
// add child elements to represent the individual buffered time ranges
for (let i = 0; i < buffered.length; i++) {
const start = buffered.start(i);
const end = buffered.end(i);
let part = children[i];

// update the control-text
Dom.textContent(controlTextPercentage, percentify(bufferedEnd, duration, true));
if (!part) {
part = this.el_.appendChild(Dom.createEl());
children[i] = part;
}

// add child elements to represent the individual buffered time ranges
for (let i = 0; i < buffered.length; i++) {
const start = buffered.start(i);
const end = buffered.end(i);
let part = children[i];
// set the percent based on the width of the progress bar (bufferedEnd)
const leftPercent = percentify(start, bufferedEnd);
const widthPercent = percentify(end - start, bufferedEnd);

if (!part) {
part = this.el_.appendChild(Dom.createEl());
children[i] = part;
}
if (part.style.left !== leftPercent) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only update when changed.

part.style.left = leftPercent;
}

// set the percent based on the width of the progress bar (bufferedEnd)
part.style.left = percentify(start, bufferedEnd);
part.style.width = percentify(end - start, bufferedEnd);
}
if (part.style.width !== widthPercent) {
part.style.widh = widthPercent;
}
}

// remove unused buffered range elements
for (let i = children.length; i > buffered.length; i--) {
this.el_.removeChild(children[i - 1]);
}
children.length = buffered.length;
// remove unused buffered range elements
for (let i = children.length; i > buffered.length; i--) {
this.el_.removeChild(children[i - 1]);
}
children.length = buffered.length;
});
}

}
Expand Down
23 changes: 23 additions & 0 deletions src/js/control-bar/progress-control/progress-control.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import Component from '../../component.js';
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to do an update in progressControl rather then seekBar because progressControl is the component that is tabbed to and moused over.

import * as Dom from '../../utils/dom.js';
import {bind, throttle, UPDATE_REFRESH_INTERVAL} from '../../utils/fn.js';
import activeElement from '../../mixins/active-element.js';

import './seek-bar.js';

Expand All @@ -28,10 +29,32 @@ class ProgressControl extends Component {
super(player, options);
this.handleMouseMove = throttle(bind(this, this.handleMouseMove), UPDATE_REFRESH_INTERVAL);
this.throttledHandleMouseSeek = throttle(bind(this, this.handleMouseSeek), UPDATE_REFRESH_INTERVAL);
this.throttledUpdate = throttle(bind(this, this.update), UPDATE_REFRESH_INTERVAL);

activeElement(this, {
startUpdate: () => {
this.updateInterval_ = this.setInterval(this.throttledUpdate, UPDATE_REFRESH_INTERVAL);
this.throttledUpdate();
},
stopUpdate: () => {
this.clearInterval(this.updateInterval_);
this.updateInterval_ = null;
}
});

this.enable();
}

update() {
const seekBar = this.getChild('seekBar');

if (!seekBar) {
return;
}

seekBar.update();
}

/**
* Create the `Component`'s DOM element
*
Expand Down
135 changes: 29 additions & 106 deletions src/js/control-bar/progress-control/seek-bar.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,10 @@ import Slider from '../../slider/slider.js';
import Component from '../../component.js';
import {IS_IOS, IS_ANDROID} from '../../utils/browser.js';
import * as Dom from '../../utils/dom.js';
import * as Fn from '../../utils/fn.js';
import formatTime from '../../utils/format-time.js';
import {silencePromise} from '../../utils/promise';
import keycode from 'keycode';
import document from 'global/document';
import {bind, throttle, UPDATE_REFRESH_INTERVAL} from '../../utils/fn.js';

import './load-progress-bar.js';
import './play-progress-bar.js';
Expand All @@ -21,9 +20,6 @@ const STEP_SECONDS = 5;
// The multiplier of STEP_SECONDS that PgUp/PgDown move the timeline.
const PAGE_KEY_MULTIPLIER = 12;

// The interval at which the bar should update as it progresses.
const UPDATE_REFRESH_INTERVAL = 30;

/**
* Seek bar and container for the progress bars. Uses {@link PlayProgressBar}
* as its `bar`.
Expand All @@ -43,64 +39,7 @@ class SeekBar extends Slider {
*/
constructor(player, options) {
super(player, options);
this.setEventHandlers_();
}

/**
* Sets the event handlers
*
* @private
*/
setEventHandlers_() {
this.update = Fn.throttle(Fn.bind(this, this.update), UPDATE_REFRESH_INTERVAL);

this.on(this.player_, 'timeupdate', this.update);
this.on(this.player_, 'ended', this.handleEnded);
this.on(this.player_, 'durationchange', this.update);
if (this.player_.liveTracker) {
this.on(this.player_.liveTracker, 'liveedgechange', this.update);
}

// when playing, let's ensure we smoothly update the play progress bar
// via an interval
this.updateInterval = null;

this.on(this.player_, ['playing'], this.enableInterval_);

this.on(this.player_, ['ended', 'pause', 'waiting'], this.disableInterval_);

// we don't need to update the play progress if the document is hidden,
// also, this causes the CPU to spike and eventually crash the page on IE11.
if ('hidden' in document && 'visibilityState' in document) {
this.on(document, 'visibilitychange', this.toggleVisibility_);
}
}

toggleVisibility_(e) {
if (document.hidden) {
this.disableInterval_(e);
} else {
this.enableInterval_();

// we just switched back to the page and someone may be looking, so, update ASAP
this.requestAnimationFrame(this.update);
}
}

enableInterval_() {
this.clearInterval(this.updateInterval);

this.updateInterval = this.setInterval(() =>{
this.requestAnimationFrame(this.update);
}, UPDATE_REFRESH_INTERVAL);
}

disableInterval_(e) {
if (this.player_.liveTracker && this.player_.liveTracker.isLive() && e.type !== 'ended') {
return;
}

this.clearInterval(this.updateInterval);
this.update = throttle(bind(this, this.update), UPDATE_REFRESH_INTERVAL);
}

/**
Expand Down Expand Up @@ -130,31 +69,33 @@ class SeekBar extends Slider {
* @private
*/
update_(currentTime, percent) {
const liveTracker = this.player_.liveTracker;
let duration = this.player_.duration();
this.requestAnimationFrame(() => {
const liveTracker = this.player_.liveTracker;
let duration = this.player_.duration();

if (liveTracker && liveTracker.isLive()) {
duration = this.player_.liveTracker.liveCurrentTime();
}
if (liveTracker && liveTracker.isLive()) {
duration = this.player_.liveTracker.liveCurrentTime();
}

// machine readable value of progress bar (percentage complete)
this.el_.setAttribute('aria-valuenow', (percent * 100).toFixed(2));

// human readable value of progress bar (time complete)
this.el_.setAttribute(
'aria-valuetext',
this.localize(
'progress bar timing: currentTime={1} duration={2}',
[formatTime(currentTime, duration),
formatTime(duration, duration)],
'{1} of {2}'
)
);

// Update the `PlayProgressBar`.
if (this.bar) {
this.bar.update(Dom.getBoundingClientRect(this.el_), percent);
}
// machine readable value of progress bar (percentage complete)
this.el_.setAttribute('aria-valuenow', (percent * 100).toFixed(2));

// human readable value of progress bar (time complete)
this.el_.setAttribute(
'aria-valuetext',
this.localize(
'progress bar timing: currentTime={1} duration={2}',
[formatTime(currentTime, duration),
formatTime(duration, duration)],
'{1} of {2}'
)
);

// Update the `PlayProgressBar` when we are in mouse focus
if (this.bar && (this.player_.ended() || (!this.parentComponent_ || this.parentComponent_.mouseFocus_))) {
this.bar.update(Dom.getBoundingClientRect(this.el_), percent);
}
});
}

/**
Expand All @@ -169,15 +110,10 @@ class SeekBar extends Slider {
* The current percent at a number from 0-1
*/
update(event) {
// if the offsetParent is null, then this element is hidden, in which case
// we don't need to update it.
if (this.el().offsetParent === null) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not needed as this component is always visible for screen readers.

return;
}

const percent = super.update();
const currentTime = !this.player_.ended() ? this.getCurrentTime_() : this.player_.duration();

this.update_(this.getCurrentTime_(), percent);
this.update_(currentTime, percent);
return percent;
}

Expand All @@ -196,19 +132,6 @@ class SeekBar extends Slider {
this.player_.currentTime();
}

/**
* We want the seek bar to be full on ended
* no matter what the actual internal values are. so we force it.
*
* @param {EventTarget~Event} [event]
* The `timeupdate` or `ended` event that caused this to run.
*
* @listens Player#ended
*/
handleEnded(event) {
this.update_(this.player_.duration(), 1);
}

/**
* Get the percentage of media played so far.
*
Expand Down
Loading