Sakurai/pdf.js
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

[EnhanceTextSelection] Make expandTextDivs more efficient by updating all styles at once instead of piecewise

Browse Source 
I intended to provide proper benchmarking results here, as outlined in https://github.com/mozilla/pdf.js/wiki/Benchmarking-your-changes, but after wasting a couple of hours over the weekend getting weird results I gave up.
It appears that there's a lot of, i.e. way too much, variance between subsequent runs of `text` tests for the results to be meaningful.
(Previously I've only benchmarked `eq` tests, so I don't know if the `text` tests has never worked well or if it's a newer problem. For reference, please see the results of back-to-back benchmark runs on the current `master` with a *very* simple manifest file: [link here].)

Instead I used `console.time/timeEnd` in `appendText` and `expandTextDivs` to be able to compare the performance with/without the patch. The entire viewer was (skip-cache) reloaded between measurements, and the result are available here: [link here].
Given the troubles I've had with benchmarking, I've not yet computed any statistics on the results (e.g. mean, variance, confidence intervals, and so on).
However, just by looking at the data I think it's safe to say that this patch first of all doesn't seem to regress the current performance. Secondly it certainly looks *very* likely that this patch actually improves the performance, especially for the one-glyph-per-text-div case (cf. issue 7224).

Re: issue 7584.
This commit is contained in:
Jonas Jenwald 2016-09-13 17:04:57 +02:00
parent a7c35025fe
commit cb5f9df0c8
1 changed files with 31 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

48
src/display/text_layer.js
View File

@ -59,14 +59,20 @@ var renderTextLayer = (function renderTextLayerClosure() {
return !NonWhitespaceRegexp.test(str);
}
// Text layers may contain many thousand div's, and using `styleBuf` avoids
// creating many intermediate strings when building their 'style' properties.
var styleBuf = ['left: ', 0, 'px; top: ', 0, 'px; font-size: ', 0,
'px; font-family: ', '', ';'];
function appendText(task, geom, styles) {
// Initialize all used properties to keep the caches monomorphic.
var textDiv = document.createElement('div');
var textDivProperties = {
style: null,
angle: 0,
canvasWidth: 0,
isWhitespace: false,
originalTransform: '',
originalTransform: null,
paddingBottom: 0,
paddingLeft: 0,
paddingRight: 0,
@ -104,10 +110,12 @@ var renderTextLayer = (function renderTextLayerClosure() {
left = tx[4] + (fontAscent * Math.sin(angle));
top = tx[5] - (fontAscent * Math.cos(angle));
}
textDiv.style.left = left + 'px';
textDiv.style.top = top + 'px';
textDiv.style.fontSize = fontHeight + 'px';
textDiv.style.fontFamily = style.fontFamily;
styleBuf[1] = left;
styleBuf[3] = top;
styleBuf[5] = fontHeight;
styleBuf[7] = style.fontFamily;
textDivProperties.style = styleBuf.join('');
textDiv.setAttribute('style', textDivProperties.style);
textDiv.textContent = geom.str;
// |fontName| is only used by the Font Inspector. This test will succeed
@ -517,7 +525,6 @@ var renderTextLayer = (function renderTextLayerClosure() {
this._renderTimer = null;
this._bounds = [];
this._enhanceTextSelection = !!enhanceTextSelection;
this._expanded = false;
}
TextLayerRenderTask.prototype = {
get promise() {
@ -555,18 +562,20 @@ var renderTextLayer = (function renderTextLayerClosure() {
if (!this._enhanceTextSelection || !this._renderingDone) {
return;
}
if (!this._expanded) {
if (this._bounds !== null) {
expand(this);
this._expanded = true;
this._bounds.length = 0;
this._bounds = null;
}
for (var i = 0, ii = this._textDivs.length; i < ii; i++) {
var div = this._textDivs[i];
var divProperties = this._textDivProperties.get(div);
if (divProperties.isWhitespace) {
continue;
}
if (expandDivs) {
var transform = '';
var transform = '', padding = '';
if (divProperties.scale !== 1) {
transform = 'scaleX(' + divProperties.scale + ')';
@ -575,21 +584,26 @@ var renderTextLayer = (function renderTextLayerClosure() {
transform = 'rotate(' + divProperties.angle + 'deg) ' + transform;
}
if (divProperties.paddingLeft !== 0) {
div.style.paddingLeft =
(divProperties.paddingLeft / divProperties.scale) + 'px';
padding += ' padding-left: ' +
(divProperties.paddingLeft / divProperties.scale) + 'px;';
transform += ' translateX(' +
(-divProperties.paddingLeft / divProperties.scale) + 'px)';
}
if (divProperties.paddingTop !== 0) {
div.style.paddingTop = divProperties.paddingTop + 'px';
padding += ' padding-top: ' + divProperties.paddingTop + 'px;';
transform += ' translateY(' + (-divProperties.paddingTop) + 'px)';
}
if (divProperties.paddingRight !== 0) {
div.style.paddingRight =
(divProperties.paddingRight / divProperties.scale) + 'px';
padding += ' padding-right: ' +
(divProperties.paddingRight / divProperties.scale) + 'px;';
}
if (divProperties.paddingBottom !== 0) {
div.style.paddingBottom = divProperties.paddingBottom + 'px';
padding += ' padding-bottom: ' +
divProperties.paddingBottom + 'px;';
}
if (padding !== '') {
div.setAttribute('style', divProperties.style + padding);
}
if (transform !== '') {
CustomStyle.setProp('transform', div, transform);
@ -597,7 +611,7 @@ var renderTextLayer = (function renderTextLayerClosure() {
} else {
div.style.padding = 0;
CustomStyle.setProp('transform', div,
divProperties.originalTransform);
divProperties.originalTransform || '');
}
}
},