From b10add14f36923afc0dea590f2fd1248217c7a35 Mon Sep 17 00:00:00 2001 From: Tim van der Meij Date: Fri, 2 Sep 2016 17:51:13 +0200 Subject: [PATCH 1/3] Refactor `text_layer.js` to pass the task as a parameter We pass many parameters to `appendText` while we might as well pass the `task` object that contains them. This saves a few lines of code and makes the signature of `appendText` more clear. We do the same for `expand`, which is useful for the next commit in which we replace `div.dataset` with a `WeakMap`. Furthermore, this patch adds a missing parameter to a comment block to make it clear which parameters remain. --- src/display/text_layer.js | 40 ++++++++++++++++++--------------------- 1 file changed, 18 insertions(+), 22 deletions(-) diff --git a/src/display/text_layer.js b/src/display/text_layer.js index 9787183424290..62a4cfc17c4d1 100644 --- a/src/display/text_layer.js +++ b/src/display/text_layer.js @@ -46,6 +46,8 @@ var getDefaultSetting = displayDOMUtils.getDefaultSetting; * initially be set to empty array. * @property {number} timeout - (optional) Delay in milliseconds before * rendering of the text runs occurs. + * @property {boolean} enhanceTextSelection - (optional) Whether to turn on the + * text selection enhancement. */ var renderTextLayer = (function renderTextLayerClosure() { var MAX_TEXT_DIVS_TO_RENDER = 100000; @@ -56,16 +58,15 @@ var renderTextLayer = (function renderTextLayerClosure() { return !NonWhitespaceRegexp.test(str); } - function appendText(textDivs, viewport, geom, styles, bounds, - enhanceTextSelection) { - var style = styles[geom.fontName]; + function appendText(task, geom) { + var style = task._textContent.styles[geom.fontName]; var textDiv = document.createElement('div'); - textDivs.push(textDiv); + task._textDivs.push(textDiv); if (isAllWhitespace(geom.str)) { textDiv.dataset.isWhitespace = true; return; } - var tx = Util.transform(viewport.transform, geom.transform); + var tx = Util.transform(task._viewport.transform, geom.transform); var angle = Math.atan2(tx[1], tx[0]); if (style.vertical) { angle += Math.PI / 2; @@ -108,19 +109,19 @@ var renderTextLayer = (function renderTextLayerClosure() { // lots of such divs a lot faster. if (geom.str.length > 1) { if (style.vertical) { - textDiv.dataset.canvasWidth = geom.height * viewport.scale; + textDiv.dataset.canvasWidth = geom.height * task._viewport.scale; } else { - textDiv.dataset.canvasWidth = geom.width * viewport.scale; + textDiv.dataset.canvasWidth = geom.width * task._viewport.scale; } } - if (enhanceTextSelection) { + if (task._enhanceTextSelection) { var angleCos = 1, angleSin = 0; if (angle !== 0) { angleCos = Math.cos(angle); angleSin = Math.sin(angle); } var divWidth = (style.vertical ? geom.height : geom.width) * - viewport.scale; + task._viewport.scale; var divHeight = fontHeight; var m, b; @@ -131,7 +132,7 @@ var renderTextLayer = (function renderTextLayerClosure() { b = [left, top, left + divWidth, top + divHeight]; } - bounds.push({ + task._bounds.push({ left: b[0], top: b[1], right: b[2], @@ -209,7 +210,10 @@ var renderTextLayer = (function renderTextLayerClosure() { capability.resolve(); } - function expand(bounds, viewport) { + function expand(task) { + var bounds = task._bounds; + var viewport = task._viewport; + var expanded = expandBounds(viewport.width, viewport.height, bounds); for (var i = 0; i < expanded.length; i++) { var div = bounds[i].div; @@ -487,8 +491,7 @@ var renderTextLayer = (function renderTextLayerClosure() { this._textContent = textContent; this._container = container; this._viewport = viewport; - textDivs = textDivs || []; - this._textDivs = textDivs; + this._textDivs = textDivs || []; this._renderingDone = false; this._canceled = false; this._capability = createPromiseCapability(); @@ -513,15 +516,8 @@ var renderTextLayer = (function renderTextLayerClosure() { _render: function TextLayer_render(timeout) { var textItems = this._textContent.items; - var styles = this._textContent.styles; - var textDivs = this._textDivs; - var viewport = this._viewport; - var bounds = this._bounds; - var enhanceTextSelection = this._enhanceTextSelection; - for (var i = 0, len = textItems.length; i < len; i++) { - appendText(textDivs, viewport, textItems[i], styles, bounds, - enhanceTextSelection); + appendText(this, textItems[i]); } if (!timeout) { // Render right away @@ -540,7 +536,7 @@ var renderTextLayer = (function renderTextLayerClosure() { return; } if (!this._expanded) { - expand(this._bounds, this._viewport); + expand(this); this._expanded = true; this._bounds.length = 0; } From b3818d5c36696e0f24e04893416f968125fab26e Mon Sep 17 00:00:00 2001 From: Tim van der Meij Date: Fri, 2 Sep 2016 18:10:37 +0200 Subject: [PATCH 2/3] Replace `div.dataset` with a `WeakMap` in `text_layer.js` This patch improves performance by avoiding unnecessary type conversions, which also help the JIT for optimizations. Moreover, this patch fixes issues with the div expansion code where `textScale` would be undefined in a division. Because of the `dataset` usage, other comparisons evaluated to `true` while `false` would have been correct. This makes the expansion mode now work correctly for cases with, for example, each glyph in one div. The polyfill for `WeakMap` has been provided by @yurydelendik. --- src/display/text_layer.js | 149 +++++++++++++++++++++----------------- src/shared/util.js | 33 +++++++++ 2 files changed, 117 insertions(+), 65 deletions(-) diff --git a/src/display/text_layer.js b/src/display/text_layer.js index 62a4cfc17c4d1..f1aa9b8298fc6 100644 --- a/src/display/text_layer.js +++ b/src/display/text_layer.js @@ -12,6 +12,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ +/* globals WeakMap */ 'use strict'; @@ -58,16 +59,31 @@ var renderTextLayer = (function renderTextLayerClosure() { return !NonWhitespaceRegexp.test(str); } - function appendText(task, geom) { - var style = task._textContent.styles[geom.fontName]; + function appendText(task, geom, styles) { + // Initialize all used properties to keep the caches monomorphic. var textDiv = document.createElement('div'); + var textDivProperties = { + angle: 0, + canvasWidth: 0, + isWhitespace: false, + originalTransform: '', + originalWidth: 0, + paddingBottom: 0, + paddingLeft: 0, + paddingRight: 0, + paddingTop: 0, + }; + task._textDivs.push(textDiv); if (isAllWhitespace(geom.str)) { - textDiv.dataset.isWhitespace = true; + textDivProperties.isWhitespace = true; + task._textDivProperties.set(textDiv, textDivProperties); return; } + var tx = Util.transform(task._viewport.transform, geom.transform); var angle = Math.atan2(tx[1], tx[0]); + var style = styles[geom.fontName]; if (style.vertical) { angle += Math.PI / 2; } @@ -96,24 +112,26 @@ var renderTextLayer = (function renderTextLayerClosure() { textDiv.textContent = geom.str; // |fontName| is only used by the Font Inspector. This test will succeed // when e.g. the Font Inspector is off but the Stepper is on, but it's - // not worth the effort to do a more accurate test. + // not worth the effort to do a more accurate test. We only use `dataset` + // here to make the font name available for the debugger. if (getDefaultSetting('pdfBug')) { textDiv.dataset.fontName = geom.fontName; } - // Storing into dataset will convert number into string. if (angle !== 0) { - textDiv.dataset.angle = angle * (180 / Math.PI); + textDivProperties.angle = angle * (180 / Math.PI); } // We don't bother scaling single-char text divs, because it has very // little effect on text highlighting. This makes scrolling on docs with // lots of such divs a lot faster. if (geom.str.length > 1) { if (style.vertical) { - textDiv.dataset.canvasWidth = geom.height * task._viewport.scale; + textDivProperties.canvasWidth = geom.height * task._viewport.scale; } else { - textDiv.dataset.canvasWidth = geom.width * task._viewport.scale; + textDivProperties.canvasWidth = geom.width * task._viewport.scale; } } + task._textDivProperties.set(textDiv, textDivProperties); + if (task._enhanceTextSelection) { var angleCos = 1, angleSin = 0; if (angle !== 0) { @@ -171,7 +189,8 @@ var renderTextLayer = (function renderTextLayerClosure() { var lastFontFamily; for (var i = 0; i < textDivsLength; i++) { var textDiv = textDivs[i]; - if (textDiv.dataset.isWhitespace !== undefined) { + var textDivProperties = task._textDivProperties.get(textDiv); + if (textDivProperties.isWhitespace) { continue; } @@ -186,25 +205,23 @@ var renderTextLayer = (function renderTextLayerClosure() { } var width = ctx.measureText(textDiv.textContent).width; - textDiv.dataset.originalWidth = width; + textDivProperties.originalWidth = width; textLayerFrag.appendChild(textDiv); - var transform; - if (textDiv.dataset.canvasWidth !== undefined && width > 0) { - // Dataset values are of type string. - var textScale = textDiv.dataset.canvasWidth / width; - transform = 'scaleX(' + textScale + ')'; - } else { - transform = ''; + var transform = ''; + if (textDivProperties.canvasWidth !== 0 && width > 0) { + var scale = textDivProperties.canvasWidth / width; + transform = 'scaleX(' + scale + ')'; } - var rotation = textDiv.dataset.angle; - if (rotation) { + var rotation = textDivProperties.angle; + if (rotation !== 0) { transform = 'rotate(' + rotation + 'deg) ' + transform; } - if (transform) { - textDiv.dataset.originalTransform = transform; - CustomStyle.setProp('transform' , textDiv, transform); + if (transform !== '') { + textDivProperties.originalTransform = transform; + CustomStyle.setProp('transform', textDiv, transform); } + task._textDivProperties.set(textDiv, textDivProperties); } task._renderingDone = true; capability.resolve(); @@ -217,11 +234,13 @@ var renderTextLayer = (function renderTextLayerClosure() { var expanded = expandBounds(viewport.width, viewport.height, bounds); for (var i = 0; i < expanded.length; i++) { var div = bounds[i].div; - if (!div.dataset.angle) { - div.dataset.paddingLeft = bounds[i].left - expanded[i].left; - div.dataset.paddingTop = bounds[i].top - expanded[i].top; - div.dataset.paddingRight = expanded[i].right - bounds[i].right; - div.dataset.paddingBottom = expanded[i].bottom - bounds[i].bottom; + var divProperties = task._textDivProperties.get(div); + if (divProperties.angle === 0) { + divProperties.paddingLeft = bounds[i].left - expanded[i].left; + divProperties.paddingTop = bounds[i].top - expanded[i].top; + divProperties.paddingRight = expanded[i].right - bounds[i].right; + divProperties.paddingBottom = expanded[i].bottom - bounds[i].bottom; + task._textDivProperties.set(div, divProperties); continue; } // Box is rotated -- trying to find padding so rotated div will not @@ -266,10 +285,11 @@ var renderTextLayer = (function renderTextLayerClosure() { // Not based on math, but to simplify calculations, using cos and sin // absolute values to not exceed the box (it can but insignificantly). var boxScale = 1 + Math.min(Math.abs(c), Math.abs(s)); - div.dataset.paddingLeft = findPositiveMin(ts, 32, 16) / boxScale; - div.dataset.paddingTop = findPositiveMin(ts, 48, 16) / boxScale; - div.dataset.paddingRight = findPositiveMin(ts, 0, 16) / boxScale; - div.dataset.paddingBottom = findPositiveMin(ts, 16, 16) / boxScale; + divProperties.paddingLeft = findPositiveMin(ts, 32, 16) / boxScale; + divProperties.paddingTop = findPositiveMin(ts, 48, 16) / boxScale; + divProperties.paddingRight = findPositiveMin(ts, 0, 16) / boxScale; + divProperties.paddingBottom = findPositiveMin(ts, 16, 16) / boxScale; + task._textDivProperties.set(div, divProperties); } } @@ -492,6 +512,7 @@ var renderTextLayer = (function renderTextLayerClosure() { this._container = container; this._viewport = viewport; this._textDivs = textDivs || []; + this._textDivProperties = new WeakMap(); this._renderingDone = false; this._canceled = false; this._capability = createPromiseCapability(); @@ -516,8 +537,9 @@ var renderTextLayer = (function renderTextLayerClosure() { _render: function TextLayer_render(timeout) { var textItems = this._textContent.items; + var textStyles = this._textContent.styles; for (var i = 0, len = textItems.length; i < len; i++) { - appendText(this, textItems[i]); + appendText(this, textItems[i], textStyles); } if (!timeout) { // Render right away @@ -540,55 +562,52 @@ var renderTextLayer = (function renderTextLayerClosure() { this._expanded = true; this._bounds.length = 0; } - if (expandDivs) { - for (var i = 0, ii = this._textDivs.length; i < ii; i++) { - var div = this._textDivs[i]; - var transform; - var width = div.dataset.originalWidth; - if (div.dataset.canvasWidth !== undefined && width > 0) { - // Dataset values are of type string. - var textScale = div.dataset.canvasWidth / width; - transform = 'scaleX(' + textScale + ')'; - } else { - transform = ''; + + for (var i = 0, ii = this._textDivs.length; i < ii; i++) { + var div = this._textDivs[i]; + var divProperties = this._textDivProperties.get(div); + + if (expandDivs) { + var transform = ''; + var scale = 1; + + if (divProperties.canvasWidth !== 0 && + divProperties.originalWidth > 0) { + scale = divProperties.canvasWidth / divProperties.originalWidth; + transform = 'scaleX(' + scale + ')'; } - var rotation = div.dataset.angle; - if (rotation) { - transform = 'rotate(' + rotation + 'deg) ' + transform; + if (divProperties.angle !== 0) { + transform = 'rotate(' + divProperties.angle + 'deg) ' + transform; } - if (div.dataset.paddingLeft) { + if (divProperties.paddingLeft !== 0) { div.style.paddingLeft = - (div.dataset.paddingLeft / textScale) + 'px'; + (divProperties.paddingLeft / scale) + 'px'; transform += ' translateX(' + - (-div.dataset.paddingLeft / textScale) + 'px)'; + (-divProperties.paddingLeft / scale) + 'px)'; } - if (div.dataset.paddingTop) { - div.style.paddingTop = div.dataset.paddingTop + 'px'; - transform += ' translateY(' + (-div.dataset.paddingTop) + 'px)'; + if (divProperties.paddingTop !== 0) { + div.style.paddingTop = divProperties.paddingTop + 'px'; + transform += ' translateY(' + (-divProperties.paddingTop) + 'px)'; } - if (div.dataset.paddingRight) { + if (divProperties.paddingRight !== 0) { div.style.paddingRight = - div.dataset.paddingRight / textScale + 'px'; + divProperties.paddingRight / scale + 'px'; } - if (div.dataset.paddingBottom) { - div.style.paddingBottom = div.dataset.paddingBottom + 'px'; + if (divProperties.paddingBottom !== 0) { + div.style.paddingBottom = divProperties.paddingBottom + 'px'; } - if (transform) { - CustomStyle.setProp('transform' , div, transform); + if (transform !== '') { + CustomStyle.setProp('transform', div, transform); } - } - } else { - for (i = 0, ii = this._textDivs.length; i < ii; i++) { - div = this._textDivs[i]; + } else { div.style.padding = 0; - transform = div.dataset.originalTransform || ''; - CustomStyle.setProp('transform', div, transform); + CustomStyle.setProp('transform', div, + divProperties.originalTransform); } } }, }; - /** * Starts rendering of the text layer. * diff --git a/src/shared/util.js b/src/shared/util.js index 32492b78dc10f..1633e3588abc1 100644 --- a/src/shared/util.js +++ b/src/shared/util.js @@ -1442,6 +1442,39 @@ function createPromiseCapability() { //#endif })(); +//#if !MOZCENTRAL +(function WeakMapClosure() { + if (globalScope.WeakMap) { + return; + } + + var id = 0; + function WeakMap() { + this.id = '$weakmap' + (id++); + } + WeakMap.prototype = { + has: function(obj) { + return !!Object.getOwnPropertyDescriptor(obj, this.id); + }, + get: function(obj, defaultValue) { + return this.has(obj) ? obj[this.id] : defaultValue; + }, + set: function(obj, value) { + Object.defineProperty(obj, this.id, { + value: value, + enumerable: false, + configurable: true + }); + }, + delete: function(obj) { + delete obj[this.id]; + } + }; + + globalScope.WeakMap = WeakMap; +})(); +//#endif + var StatTimer = (function StatTimerClosure() { function rpad(str, pad, length) { while (str.length < length) { From 96593571eb25ab3f55bb09c8717244a74d771ec1 Mon Sep 17 00:00:00 2001 From: Tim van der Meij Date: Sun, 4 Sep 2016 20:19:31 +0200 Subject: [PATCH 3/3] Optimize scale calculation in `text_layer.js` This patch avoids having to calculate the scale twice by saving it in the properties object. Moreover, we remove a temporary variable and place parentheses around a calculation inside a string concatenation. --- src/display/text_layer.js | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/src/display/text_layer.js b/src/display/text_layer.js index f1aa9b8298fc6..a1e82e0b2483d 100644 --- a/src/display/text_layer.js +++ b/src/display/text_layer.js @@ -67,11 +67,11 @@ var renderTextLayer = (function renderTextLayerClosure() { canvasWidth: 0, isWhitespace: false, originalTransform: '', - originalWidth: 0, paddingBottom: 0, paddingLeft: 0, paddingRight: 0, paddingTop: 0, + scale: 1, }; task._textDivs.push(textDiv); @@ -205,17 +205,15 @@ var renderTextLayer = (function renderTextLayerClosure() { } var width = ctx.measureText(textDiv.textContent).width; - textDivProperties.originalWidth = width; textLayerFrag.appendChild(textDiv); var transform = ''; if (textDivProperties.canvasWidth !== 0 && width > 0) { - var scale = textDivProperties.canvasWidth / width; - transform = 'scaleX(' + scale + ')'; + textDivProperties.scale = textDivProperties.canvasWidth / width; + transform = 'scaleX(' + textDivProperties.scale + ')'; } - var rotation = textDivProperties.angle; - if (rotation !== 0) { - transform = 'rotate(' + rotation + 'deg) ' + transform; + if (textDivProperties.angle !== 0) { + transform = 'rotate(' + textDivProperties.angle + 'deg) ' + transform; } if (transform !== '') { textDivProperties.originalTransform = transform; @@ -569,21 +567,18 @@ var renderTextLayer = (function renderTextLayerClosure() { if (expandDivs) { var transform = ''; - var scale = 1; - if (divProperties.canvasWidth !== 0 && - divProperties.originalWidth > 0) { - scale = divProperties.canvasWidth / divProperties.originalWidth; - transform = 'scaleX(' + scale + ')'; + if (divProperties.scale !== 1) { + transform = 'scaleX(' + divProperties.scale + ')'; } if (divProperties.angle !== 0) { transform = 'rotate(' + divProperties.angle + 'deg) ' + transform; } if (divProperties.paddingLeft !== 0) { div.style.paddingLeft = - (divProperties.paddingLeft / scale) + 'px'; + (divProperties.paddingLeft / divProperties.scale) + 'px'; transform += ' translateX(' + - (-divProperties.paddingLeft / scale) + 'px)'; + (-divProperties.paddingLeft / divProperties.scale) + 'px)'; } if (divProperties.paddingTop !== 0) { div.style.paddingTop = divProperties.paddingTop + 'px'; @@ -591,7 +586,7 @@ var renderTextLayer = (function renderTextLayerClosure() { } if (divProperties.paddingRight !== 0) { div.style.paddingRight = - divProperties.paddingRight / scale + 'px'; + (divProperties.paddingRight / divProperties.scale) + 'px'; } if (divProperties.paddingBottom !== 0) { div.style.paddingBottom = divProperties.paddingBottom + 'px';