From c35bbd11b01f1965f3c5976fe3e0e2fff79d20f8 Mon Sep 17 00:00:00 2001 From: Tim van der Meij Date: Sat, 9 Dec 2017 17:24:31 +0100 Subject: [PATCH] Use native `Math` functions in the custom `log2` function It is quite confusing that the custom function is called `log2` while it actually returns the ceiling value and handles zero and negative values differently than the native function. To resolve this, we add a comment that explains these differences and make the function use the native `Math` functions internally instead of using our own custom logic. To verify that the function does what we expect, we add unit tests. All browsers except for IE support `Math.log2` for quite a long time already (see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Math/log2). For IE, we use the core-js polyfill. According to the microbenchmark at https://jsperf.com/log2-pdfjs/1, using the native functions should also be faster, in my testing almost six times as fast. --- src/shared/compatibility.js | 8 ++++++++ src/shared/util.js | 11 ++++++----- test/unit/util_spec.js | 16 +++++++++++++++- 3 files changed, 29 insertions(+), 6 deletions(-) diff --git a/src/shared/compatibility.js b/src/shared/compatibility.js index 2bfc74c0f..1a9fcdc30 100644 --- a/src/shared/compatibility.js +++ b/src/shared/compatibility.js @@ -348,6 +348,14 @@ PDFJS.compatibilityChecked = true; Array.prototype.includes = require('core-js/fn/array/includes'); })(); +// Provides support for Math.log2 in legacy browsers. +// Support: IE. +(function checkMathLog2() { + if (Math.log2) { + return; + } + Math.log2 = require('core-js/fn/math/log2'); +})(); // Provides support for Number.isNaN in legacy browsers. // Support: IE. diff --git a/src/shared/util.js b/src/shared/util.js index 0dd597e2c..93d6f1583 100644 --- a/src/shared/util.js +++ b/src/shared/util.js @@ -600,13 +600,14 @@ function string32(value) { (value >> 8) & 0xff, value & 0xff); } +// Calculate the base 2 logarithm of the number `x`. This differs from the +// native function in the sense that it returns the ceiling value and that it +// returns 0 instead of `Infinity`/`NaN` for `x` values smaller than/equal to 0. function log2(x) { - var n = 1, i = 0; - while (x > n) { - n <<= 1; - i++; + if (x <= 0) { + return 0; } - return i; + return Math.ceil(Math.log2(x)); } function readInt8(data, start) { diff --git a/test/unit/util_spec.js b/test/unit/util_spec.js index 798404bd0..c5e18d564 100644 --- a/test/unit/util_spec.js +++ b/test/unit/util_spec.js @@ -15,7 +15,7 @@ import { bytesToString, isArrayBuffer, isBool, isEmptyObj, isNum, isSpace, isString, - ReadableStream, removeNullCharacters, stringToBytes, stringToPDFString + log2, ReadableStream, removeNullCharacters, stringToBytes, stringToPDFString } from '../../src/shared/util'; describe('util', function() { @@ -136,6 +136,20 @@ describe('util', function() { }); }); + describe('log2', function() { + it('handles values smaller than/equal to zero', function() { + expect(log2(0)).toEqual(0); + expect(log2(-1)).toEqual(0); + }); + + it('handles values larger than zero', function() { + expect(log2(1)).toEqual(0); + expect(log2(2)).toEqual(1); + expect(log2(3)).toEqual(2); + expect(log2(3.14)).toEqual(2); + }); + }); + describe('stringToBytes', function() { it('handles non-string arguments', function() { expect(function() {