From 08830731c068d3571d2437245a017089bcee3a86 Mon Sep 17 00:00:00 2001 From: Yury Delendik Date: Mon, 16 Apr 2012 13:38:27 -0500 Subject: [PATCH 1/8] Fix the operator list deallocation --- src/api.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/api.js b/src/api.js index 18644ebe6..f4c42f48c 100644 --- a/src/api.js +++ b/src/api.js @@ -305,7 +305,7 @@ var PDFPageProxy = (function PDFPageProxyClosure() { gfx.executeOperatorList(operatorList, startIdx, next, stepper); if (startIdx == length) { gfx.endDrawing(); - delete this.operatorList; + delete self.operatorList; stats.timeEnd('Rendering'); stats.timeEnd('Overall'); if (callback) callback(); From 1e96c73207f3bd8a9aa1ba3443ec902f6171e27a Mon Sep 17 00:00:00 2001 From: Yury Delendik Date: Mon, 16 Apr 2012 14:13:41 -0500 Subject: [PATCH 2/8] Fixing concurent draw page requests for de-allocation --- src/api.js | 11 +++++++++-- web/viewer.js | 1 + 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/api.js b/src/api.js index f4c42f48c..1bebce28d 100644 --- a/src/api.js +++ b/src/api.js @@ -203,8 +203,9 @@ var PDFPageProxy = (function PDFPageProxyClosure() { stats.time('Overall'); // If there is no displayReadyPromise yet, then the operatorList was never // requested before. Make the request and create the promise. - if (!this.displayReadyPromise) { + if (!this.displayReadyPromise || this.destroyed) { this.displayReadyPromise = new Promise(); + this.destroyed = false; this.stats.time('Page Request'); this.transport.messageHandler.send('RenderPageRequest', { @@ -250,6 +251,7 @@ var PDFPageProxy = (function PDFPageProxyClosure() { // Always defer call to display() to work around bug in // Firefox error reporting from XHR callbacks. setTimeout(function pageSetTimeout() { + delete self.operatorList; self.displayReadyPromise.resolve(); }); }; @@ -305,7 +307,6 @@ var PDFPageProxy = (function PDFPageProxyClosure() { gfx.executeOperatorList(operatorList, startIdx, next, stepper); if (startIdx == length) { gfx.endDrawing(); - delete self.operatorList; stats.timeEnd('Rendering'); stats.timeEnd('Overall'); if (callback) callback(); @@ -333,6 +334,12 @@ var PDFPageProxy = (function PDFPageProxyClosure() { }; promise.resolve(operationList); return promise; + }, + /** + * Destroys allocated by page resources. + */ + destroy: function() { + this.destroyed = true; } }; return PDFPageProxy; diff --git a/web/viewer.js b/web/viewer.js index 68f0a6a33..a25a2a3ce 100644 --- a/web/viewer.js +++ b/web/viewer.js @@ -756,6 +756,7 @@ var PageView = function pageView(container, pdfPage, id, scale, div.removeAttribute('data-loaded'); delete this.canvas; + this.pdfPage.destroy(); this.loadingIconDiv = document.createElement('div'); this.loadingIconDiv.className = 'loadingIcon'; From 357805696b1f87f63dfd96026ce703887ff75027 Mon Sep 17 00:00:00 2001 From: Yury Delendik Date: Mon, 16 Apr 2012 14:49:55 -0500 Subject: [PATCH 3/8] Fixing concurrency and test driver --- src/api.js | 18 ++++++++++++++++-- test/driver.js | 2 ++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/src/api.js b/src/api.js index 1bebce28d..1dfafb9ee 100644 --- a/src/api.js +++ b/src/api.js @@ -133,6 +133,7 @@ var PDFPageProxy = (function PDFPageProxyClosure() { this.stats = new StatTimer(); this.stats.enabled = !!globalScope.PDFJS.enableStats; this.objs = transport.objs; + this.renderRequests = 0; } PDFPageProxy.prototype = { /** @@ -198,12 +199,14 @@ var PDFPageProxy = (function PDFPageProxyClosure() { * rendering. */ render: function(params) { + this.renderRequests++; + var promise = new Promise(); var stats = this.stats; stats.time('Overall'); // If there is no displayReadyPromise yet, then the operatorList was never // requested before. Make the request and create the promise. - if (!this.displayReadyPromise || this.destroyed) { + if (!this.displayReadyPromise) { this.displayReadyPromise = new Promise(); this.destroyed = false; @@ -213,7 +216,14 @@ var PDFPageProxy = (function PDFPageProxyClosure() { }); } + var self = this; function complete(error) { + self.renderRequests--; + if (self.destroyed && self.renderRequests == 0) { + delete self.operatorList; + delete self.displayReadyPromise; + } + if (error) promise.reject(error); else @@ -251,7 +261,6 @@ var PDFPageProxy = (function PDFPageProxyClosure() { // Always defer call to display() to work around bug in // Firefox error reporting from XHR callbacks. setTimeout(function pageSetTimeout() { - delete self.operatorList; self.displayReadyPromise.resolve(); }); }; @@ -340,6 +349,11 @@ var PDFPageProxy = (function PDFPageProxyClosure() { */ destroy: function() { this.destroyed = true; + + if (this.renderRequests == 0) { + delete self.operatorList; + delete self.displayReadyPromise; + } } }; return PDFPageProxy; diff --git a/test/driver.js b/test/driver.js index 26c5a156a..2f17fa43c 100644 --- a/test/driver.js +++ b/test/driver.js @@ -195,9 +195,11 @@ function nextPage(task, loadError) { viewport: viewport }; page.render(renderContext).then(function() { + page.destroy(); snapshotCurrentPage(task, false); }, function(error) { + page.destroy(); snapshotCurrentPage(task, 'render : ' + error); }); }, From f701a1427a4473f0e9196edbb5e987a699a0333f Mon Sep 17 00:00:00 2001 From: Yury Delendik Date: Mon, 16 Apr 2012 15:23:24 -0500 Subject: [PATCH 4/8] Remove operatorList cache from the backend --- src/core.js | 8 +------- web/viewer.js | 8 ++++++-- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/src/core.js b/src/core.js index 41f9a9c61..cbd2abf8d 100644 --- a/src/core.js +++ b/src/core.js @@ -132,11 +132,6 @@ var Page = (function PageClosure() { }, getOperatorList: function Page_getOperatorList(handler, dependency) { - if (this.operatorList) { - // content was compiled - return this.operatorList; - } - var xref = this.xref; var content = this.content; var resources = this.resources; @@ -154,8 +149,7 @@ var Page = (function PageClosure() { var pe = this.pe = new PartialEvaluator( xref, handler, 'p' + this.pageNumber + '_'); - this.operatorList = pe.getOperatorList(content, resources, dependency); - return this.operatorList; + return pe.getOperatorList(content, resources, dependency); }, getLinks: function Page_getLinks() { diff --git a/web/viewer.js b/web/viewer.js index a25a2a3ce..3233371cc 100644 --- a/web/viewer.js +++ b/web/viewer.js @@ -32,7 +32,7 @@ var Cache = function cacheCache(size) { data.splice(i); data.push(view); if (data.length > size) - data.shift().update(); + data.shift().destroy(); }; }; @@ -743,6 +743,11 @@ var PageView = function pageView(container, pdfPage, id, scale, container.appendChild(anchor); container.appendChild(div); + this.destroy = function pageViewDestroy() { + this.update(); + this.pdfPage.destroy(); + }; + this.update = function pageViewUpdate(scale) { this.scale = scale || this.scale; var viewport = this.pdfPage.getViewport(this.scale); @@ -756,7 +761,6 @@ var PageView = function pageView(container, pdfPage, id, scale, div.removeAttribute('data-loaded'); delete this.canvas; - this.pdfPage.destroy(); this.loadingIconDiv = document.createElement('div'); this.loadingIconDiv.className = 'loadingIcon'; From 0380d408e09f2c00489a8f309b732320e50f2b36 Mon Sep 17 00:00:00 2001 From: Yury Delendik Date: Mon, 16 Apr 2012 15:28:34 -0500 Subject: [PATCH 5/8] Fixes comment --- src/api.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/api.js b/src/api.js index 1dfafb9ee..633b1d493 100644 --- a/src/api.js +++ b/src/api.js @@ -345,7 +345,7 @@ var PDFPageProxy = (function PDFPageProxyClosure() { return promise; }, /** - * Destroys allocated by page resources. + * Destroys resources allocated by the page. */ destroy: function() { this.destroyed = true; From 32684fe324839c5d7e084888e6a9f28e251e777c Mon Sep 17 00:00:00 2001 From: Yury Delendik Date: Mon, 16 Apr 2012 16:19:45 -0500 Subject: [PATCH 6/8] Change render status logic --- src/api.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/api.js b/src/api.js index 633b1d493..189b94dbe 100644 --- a/src/api.js +++ b/src/api.js @@ -133,7 +133,7 @@ var PDFPageProxy = (function PDFPageProxyClosure() { this.stats = new StatTimer(); this.stats.enabled = !!globalScope.PDFJS.enableStats; this.objs = transport.objs; - this.renderRequests = 0; + this.renderInProgress = false; } PDFPageProxy.prototype = { /** @@ -199,7 +199,7 @@ var PDFPageProxy = (function PDFPageProxyClosure() { * rendering. */ render: function(params) { - this.renderRequests++; + this.renderInProgress = true; var promise = new Promise(); var stats = this.stats; @@ -218,8 +218,8 @@ var PDFPageProxy = (function PDFPageProxyClosure() { var self = this; function complete(error) { - self.renderRequests--; - if (self.destroyed && self.renderRequests == 0) { + self.renderInProgress = false; + if (self.destroyed) { delete self.operatorList; delete self.displayReadyPromise; } @@ -350,7 +350,7 @@ var PDFPageProxy = (function PDFPageProxyClosure() { destroy: function() { this.destroyed = true; - if (this.renderRequests == 0) { + if (!this.renderInProgress) { delete self.operatorList; delete self.displayReadyPromise; } From b6edbb38c1eafa35044408bd8eaa2e045d5378a0 Mon Sep 17 00:00:00 2001 From: Yury Delendik Date: Mon, 16 Apr 2012 16:46:26 -0500 Subject: [PATCH 7/8] Fixes content stream reset; terminating rendering when destroyed --- src/api.js | 9 +++++++-- src/core.js | 7 +++++-- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/api.js b/src/api.js index 189b94dbe..dfc245b03 100644 --- a/src/api.js +++ b/src/api.js @@ -233,6 +233,11 @@ var PDFPageProxy = (function PDFPageProxyClosure() { // Once the operatorList and fonts are loaded, do the actual rendering. this.displayReadyPromise.then( function pageDisplayReadyPromise() { + if (self.destroyed) { + complete(); + return; + } + var gfx = new CanvasGraphics(params.canvasContext, this.objs, params.textLayer); try { @@ -351,8 +356,8 @@ var PDFPageProxy = (function PDFPageProxyClosure() { this.destroyed = true; if (!this.renderInProgress) { - delete self.operatorList; - delete self.displayReadyPromise; + delete this.operatorList; + delete this.displayReadyPromise; } } }; diff --git a/src/core.js b/src/core.js index cbd2abf8d..2734d0eef 100644 --- a/src/core.js +++ b/src/core.js @@ -137,10 +137,13 @@ var Page = (function PageClosure() { var resources = this.resources; if (isArray(content)) { // fetching items + var streams = []; var i, n = content.length; for (i = 0; i < n; ++i) - content[i] = xref.fetchIfRef(content[i]); - content = new StreamsSequenceStream(content); + streams.push(xref.fetchIfRef(content[i])); + content = new StreamsSequenceStream(streams); + } else if (isStream(content)) { + content.reset(); } else if (!content) { // replacing non-existent page content with empty one content = new Stream(new Uint8Array(0)); From cff6c8db082dec1c1d8de7cee3f258af15456b6e Mon Sep 17 00:00:00 2001 From: Yury Delendik Date: Mon, 16 Apr 2012 18:44:51 -0500 Subject: [PATCH 8/8] skip cached objects and has consistent font ids --- src/api.js | 2 ++ src/evaluator.js | 3 ++- test/driver.js | 10 ++++++---- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/api.js b/src/api.js index dfc245b03..3d97dacd2 100644 --- a/src/api.js +++ b/src/api.js @@ -489,6 +489,8 @@ var WorkerTransport = (function WorkerTransportClosure() { messageHandler.on('obj', function transportObj(data) { var id = data[0]; var type = data[1]; + if (this.objs.hasData(id)) + return; switch (type) { case 'JpegStream': diff --git a/src/evaluator.js b/src/evaluator.js index c57e291c0..e07394201 100644 --- a/src/evaluator.js +++ b/src/evaluator.js @@ -153,13 +153,14 @@ var PartialEvaluator = (function PartialEvaluatorClosure() { font = xref.fetchIfRef(font) || fontRes.get(fontName); assertWellFormed(isDict(font)); + ++self.objIdCounter; if (!font.translated) { font.translated = self.translateFont(font, xref, resources, dependency); if (font.translated) { // keep track of each font we translated so the caller can // load them asynchronously before calling display on a page - loadedName = 'font_' + uniquePrefix + (++self.objIdCounter); + loadedName = 'font_' + uniquePrefix + self.objIdCounter; font.translated.properties.loadedName = loadedName; font.loadedName = loadedName; diff --git a/test/driver.js b/test/driver.js index 2f17fa43c..cd5ea49e7 100644 --- a/test/driver.js +++ b/test/driver.js @@ -194,13 +194,15 @@ function nextPage(task, loadError) { textLayer: textLayerBuilder, viewport: viewport }; - page.render(renderContext).then(function() { + var completeRender = (function(error) { page.destroy(); - snapshotCurrentPage(task, false); + snapshotCurrentPage(task, error); + }); + page.render(renderContext).then(function() { + completeRender(false); }, function(error) { - page.destroy(); - snapshotCurrentPage(task, 'render : ' + error); + completeRender('render : ' + error); }); }, function(error) {