From a3a6aebf0af94ccfa63182eb6c9799b3a5853508 Mon Sep 17 00:00:00 2001 From: Scott Hunter Date: Fri, 4 Apr 2014 19:46:55 -0400 Subject: [PATCH] Improve detection of transferring array buffers. Remove FeatureDetection.supportsTransferringArrayBuffers which incorrectly used window.postMessage. Detection is now done internally inside TaskProcessor using a real web worker, which round-trips a typed array to detect a related bug in older Firefox versions. TaskProcessor sends the result of the detection to the worker along with the task object, instead of having the worker detect itself. Instead of detecting whether or not ArrayBuffers can be transferred everywhere in Cesium code, we now assume they can, and the TaskProcessor/createTaskProcessorWorker system will internally truncate the array if it's not supported. --- Source/Core/FeatureDetection.js | 29 +--- Source/Core/TaskProcessor.js | 109 ++++++++++----- Source/Scene/Primitive.js | 3 - Source/Scene/PrimitivePipeline.js | 126 ++---------------- Source/Workers/combineGeometry.js | 1 - Source/Workers/createTaskProcessorWorker.js | 4 + Source/Workers/createVerticesFromHeightmap.js | 5 +- .../createVerticesFromQuantizedTerrainMesh.js | 5 +- Source/Workers/transferTypedArrayTest.js | 18 +++ .../Workers/upsampleQuantizedTerrainMesh.js | 6 +- Specs/Core/TaskProcessorSpec.js | 26 ++-- 11 files changed, 122 insertions(+), 210 deletions(-) create mode 100644 Source/Workers/transferTypedArrayTest.js diff --git a/Source/Core/FeatureDetection.js b/Source/Core/FeatureDetection.js index bf068a4ca005..8e527e6b3613 100644 --- a/Source/Core/FeatureDetection.js +++ b/Source/Core/FeatureDetection.js @@ -9,7 +9,7 @@ define([ function extractVersion(versionString) { var parts = versionString.split('.'); - for ( var i = 0, len = parts.length; i < len; ++i) { + for (var i = 0, len = parts.length; i < len; ++i) { parts[i] = parseInt(parts[i], 10); } return parts; @@ -149,32 +149,5 @@ define([ return typeof ArrayBuffer !== 'undefined'; }; - var supportsTransferringArrayBuffersResult; - - /** - * Detects whether the current browser can transfer an ArrayBuffer - * to / from a web worker. - * - * @returns true if the browser can transfer ArrayBuffers; otherwise, false. - */ - FeatureDetection.supportsTransferringArrayBuffers = function() { - if (!defined(supportsTransferringArrayBuffersResult)) { - if (!FeatureDetection.supportsTypedArrays()) { - supportsTransferringArrayBuffersResult = false; - return; - } - - var arrayBuffer = new ArrayBuffer(1); - try { - /*global postMessage*/ - postMessage({ value : arrayBuffer }, [arrayBuffer]); - supportsTransferringArrayBuffersResult = true; - } catch(e) { - supportsTransferringArrayBuffersResult = false; - } - } - return supportsTransferringArrayBuffersResult; - }; - return FeatureDetection; }); \ No newline at end of file diff --git a/Source/Core/TaskProcessor.js b/Source/Core/TaskProcessor.js index e30d00e59f7c..c481094f99bf 100644 --- a/Source/Core/TaskProcessor.js +++ b/Source/Core/TaskProcessor.js @@ -19,12 +19,51 @@ define([ Uri) { "use strict"; - function completeTask(processor, event) { + function canTransferArrayBuffer() { + if (!defined(TaskProcessor._canTransferArrayBuffer)) { + var worker = new Worker(getWorkerUrl('Workers/transferTypedArrayTest.js')); + worker.postMessage = defaultValue(worker.webkitPostMessage, worker.postMessage); + + var value = 99; + var array = new Int8Array([value]); + + try { + // postMessage might fail with a DataCloneError + // if transferring array buffers is not supported. + worker.postMessage({ + array : array + }, [array.buffer]); + } catch (e) { + TaskProcessor._canTransferArrayBuffer = false; + return TaskProcessor._canTransferArrayBuffer; + } + + var deferred = when.defer(); + + worker.onmessage = function(event) { + var array = event.data.array; + + // some versions of Firefox silently fail to transfer typed arrays. + // https://bugzilla.mozilla.org/show_bug.cgi?id=841904 + // Check to make sure the value round-trips successfully. + var result = defined(array) && array[0] === value; + deferred.resolve(result); + + worker.terminate(); + + TaskProcessor._canTransferArrayBuffer = result; + }; + + TaskProcessor._canTransferArrayBuffer = deferred.promise; + } + + return TaskProcessor._canTransferArrayBuffer; + } + + function completeTask(processor, data) { --processor._activeTasks; - var data = event.data; var id = data.id; - if (!defined(id)) { // This is not one of ours. return; @@ -42,17 +81,12 @@ define([ delete deferreds[id]; } - var _bootstrapperUrl; - function getBootstrapperUrl() { - if (defined(_bootstrapperUrl)) { - return _bootstrapperUrl; - } - - _bootstrapperUrl = buildModuleUrl('Workers/cesiumWorkerBootstrapper.js'); + function getWorkerUrl(moduleID) { + var url = buildModuleUrl(moduleID); - if (isCrossOriginUrl(_bootstrapperUrl)) { + if (isCrossOriginUrl(url)) { //to load cross-origin, create a shim worker from a blob URL - var script = 'importScripts("' + _bootstrapperUrl + '");'; + var script = 'importScripts("' + url + '");'; var blob; try { @@ -67,18 +101,24 @@ define([ } var URL = window.URL || window.webkitURL; - _bootstrapperUrl = URL.createObjectURL(blob); + url = URL.createObjectURL(blob); } - return _bootstrapperUrl; + return url; + } + + var bootstrapperUrlResult; + function getBootstrapperUrl() { + if (!defined(bootstrapperUrlResult)) { + bootstrapperUrlResult = getWorkerUrl('Workers/cesiumWorkerBootstrapper.js'); + } + return bootstrapperUrlResult; } function createWorker(processor) { - var bootstrapperUrl = getBootstrapperUrl(); - var worker = new Worker(bootstrapperUrl); + var worker = new Worker(getBootstrapperUrl()); worker.postMessage = defaultValue(worker.webkitPostMessage, worker.postMessage); - //bootstrap var bootstrapMessage = { loaderConfig : {}, workerModule : TaskProcessor._workerModulePrefix + processor._workerName @@ -98,10 +138,10 @@ define([ worker.postMessage(bootstrapMessage); worker.onmessage = function(event) { - completeTask(processor, event); + completeTask(processor, event.data); }; - processor._worker = worker; + return worker; } /** @@ -157,7 +197,7 @@ define([ */ TaskProcessor.prototype.scheduleTask = function(parameters, transferableObjects) { if (!defined(this._worker)) { - createWorker(this); + this._worker = createWorker(this); } if (this._activeTasks >= this._maximumActiveTasks) { @@ -166,20 +206,26 @@ define([ ++this._activeTasks; - if (!defined(transferableObjects)) { - transferableObjects = emptyTransferableObjectArray; - } + var processor = this; + return when(canTransferArrayBuffer(), function(canTransferArrayBuffer) { + if (!defined(transferableObjects)) { + transferableObjects = emptyTransferableObjectArray; + } else if (!canTransferArrayBuffer) { + transferableObjects.length = 0; + } - var id = this._nextID++; - var deferred = when.defer(); - this._deferreds[id] = deferred; + var id = processor._nextID++; + var deferred = when.defer(); + processor._deferreds[id] = deferred; - this._worker.postMessage({ - id : id, - parameters : parameters - }, transferableObjects); + processor._worker.postMessage({ + id : id, + parameters : parameters, + canTransferArrayBuffer : canTransferArrayBuffer + }, transferableObjects); - return deferred.promise; + return deferred.promise; + }); }; /** @@ -219,6 +265,7 @@ define([ TaskProcessor._defaultWorkerModulePrefix = 'Workers/'; TaskProcessor._workerModulePrefix = TaskProcessor._defaultWorkerModulePrefix; TaskProcessor._loaderConfig = undefined; + TaskProcessor._canTransferArrayBuffer = undefined; return TaskProcessor; }); diff --git a/Source/Scene/Primitive.js b/Source/Scene/Primitive.js index 292d00c013f9..2651debf4477 100644 --- a/Source/Scene/Primitive.js +++ b/Source/Scene/Primitive.js @@ -622,9 +622,6 @@ define([ this._state = PrimitiveState.COMBINING; when(promise, function(result) { - PrimitivePipeline.receiveGeometries(result.geometries); - PrimitivePipeline.receivePerInstanceAttributes(result.vaAttributes); - that._geometries = result.geometries; that._attributeLocations = result.attributeLocations; that._vaAttributes = result.vaAttributes; diff --git a/Source/Scene/PrimitivePipeline.js b/Source/Scene/PrimitivePipeline.js index ee042d3ac768..00ea68162b27 100644 --- a/Source/Scene/PrimitivePipeline.js +++ b/Source/Scene/PrimitivePipeline.js @@ -386,75 +386,23 @@ define([ }; }; - /* - * The below functions are needed when transferring typed arrays to/from web - * workers. This is a workaround for: - * - * https://bugzilla.mozilla.org/show_bug.cgi?id=841904 - */ - - function stupefyTypedArray(typedArray) { - if (defined(typedArray.constructor.name)) { - return { - type : typedArray.constructor.name, - buffer : typedArray.buffer - }; - } else { - return typedArray; - } - } - - var typedArrayMap = { - Int8Array : Int8Array, - Uint8Array : Uint8Array, - Int16Array : Int16Array, - Uint16Array : Uint16Array, - Int32Array : Int32Array, - Uint32Array : Uint32Array, - Float32Array : Float32Array, - Float64Array : Float64Array - }; - - function unStupefyTypedArray(typedArray) { - if (defined(typedArray.type)) { - return new typedArrayMap[typedArray.type](typedArray.buffer); - } else { - return typedArray; - } - } - /** * @private */ PrimitivePipeline.transferGeometry = function(geometry, transferableObjects) { - var typedArray; var attributes = geometry.attributes; - for (var name in attributes) { - if (attributes.hasOwnProperty(name) && - defined(attributes[name]) && - defined(attributes[name].values)) { - typedArray = attributes[name].values; - - if (FeatureDetection.supportsTransferringArrayBuffers() && transferableObjects.indexOf(attributes[name].values.buffer) < 0) { - transferableObjects.push(typedArray.buffer); - } + for ( var name in attributes) { + if (attributes.hasOwnProperty(name)) { + var attribute = attributes[name]; - if (!defined(typedArray.type)) { - attributes[name].values = stupefyTypedArray(typedArray); + if (defined(attribute) && defined(attribute.values)) { + transferableObjects.push(attribute.values.buffer); } } } if (defined(geometry.indices)) { - typedArray = geometry.indices; - - if (FeatureDetection.supportsTransferringArrayBuffers()) { - transferableObjects.push(typedArray.buffer); - } - - if (!defined(typedArray.type)) { - geometry.indices = stupefyTypedArray(geometry.indices); - } + transferableObjects.push(geometry.indices.buffer); } }; @@ -477,11 +425,7 @@ define([ var vaAttributes = perInstanceAttributes[i]; var vaLength = vaAttributes.length; for (var j = 0; j < vaLength; ++j) { - var typedArray = vaAttributes[j].values; - if (FeatureDetection.supportsTransferringArrayBuffers()) { - transferableObjects.push(typedArray.buffer); - } - vaAttributes[j].values = stupefyTypedArray(typedArray); + transferableObjects.push(vaAttributes[j].values.buffer); } } }; @@ -492,61 +436,7 @@ define([ PrimitivePipeline.transferInstances = function(instances, transferableObjects) { var length = instances.length; for (var i = 0; i < length; ++i) { - var instance = instances[i]; - PrimitivePipeline.transferGeometry(instance.geometry, transferableObjects); - } - }; - - /** - * @private - */ - PrimitivePipeline.receiveGeometry = function(geometry) { - var attributes = geometry.attributes; - for (var name in attributes) { - if (attributes.hasOwnProperty(name) && - defined(attributes[name]) && - defined(attributes[name].values)) { - attributes[name].values = unStupefyTypedArray(attributes[name].values); - } - } - - if (defined(geometry.indices)) { - geometry.indices = unStupefyTypedArray(geometry.indices); - } - }; - - /** - * @private - */ - PrimitivePipeline.receiveGeometries = function(geometries) { - var length = geometries.length; - for (var i = 0; i < length; ++i) { - PrimitivePipeline.receiveGeometry(geometries[i]); - } - }; - - /** - * @private - */ - PrimitivePipeline.receivePerInstanceAttributes = function(perInstanceAttributes) { - var length = perInstanceAttributes.length; - for (var i = 0; i < length; ++i) { - var vaAttributes = perInstanceAttributes[i]; - var vaLength = vaAttributes.length; - for (var j = 0; j < vaLength; ++j) { - vaAttributes[j].values = unStupefyTypedArray(vaAttributes[j].values); - } - } - }; - - /** - * @private - */ - PrimitivePipeline.receiveInstances = function(instances) { - var length = instances.length; - for (var i = 0; i < length; ++i) { - var instance = instances[i]; - PrimitivePipeline.receiveGeometry(instance.geometry); + PrimitivePipeline.transferGeometry(instances[i].geometry, transferableObjects); } }; diff --git a/Source/Workers/combineGeometry.js b/Source/Workers/combineGeometry.js index 66dce61eb148..3c470b698fa2 100644 --- a/Source/Workers/combineGeometry.js +++ b/Source/Workers/combineGeometry.js @@ -20,7 +20,6 @@ define([ parameters.projection = parameters.isGeographic ? new GeographicProjection(parameters.ellipsoid) : new WebMercatorProjection(parameters.ellipsoid); parameters.modelMatrix = Matrix4.clone(parameters.modelMatrix); - PrimitivePipeline.receiveInstances(parameters.instances); var result = PrimitivePipeline.combineGeometry(parameters); PrimitivePipeline.transferGeometries(result.geometries, transferableObjects); PrimitivePipeline.transferPerInstanceAttributes(result.vaAttributes, transferableObjects); diff --git a/Source/Workers/createTaskProcessorWorker.js b/Source/Workers/createTaskProcessorWorker.js index 29aed69c15bd..9b22b4b0f940 100644 --- a/Source/Workers/createTaskProcessorWorker.js +++ b/Source/Workers/createTaskProcessorWorker.js @@ -60,6 +60,10 @@ define([ postMessage = defaultValue(self.webkitPostMessage, self.postMessage); } + if (!data.canTransferArrayBuffer) { + transferableObjects.length = 0; + } + try { postMessage(responseMessage, transferableObjects); } catch (e) { diff --git a/Source/Workers/createVerticesFromHeightmap.js b/Source/Workers/createVerticesFromHeightmap.js index 3893c53c227d..b8669441d06b 100644 --- a/Source/Workers/createVerticesFromHeightmap.js +++ b/Source/Workers/createVerticesFromHeightmap.js @@ -29,10 +29,7 @@ define([ } var vertices = new Float32Array(arrayWidth * arrayHeight * numberOfAttributes); - - if (FeatureDetection.supportsTransferringArrayBuffers()) { - transferableObjects.push(vertices.buffer); - } + transferableObjects.push(vertices.buffer); parameters.ellipsoid = Ellipsoid.clone(parameters.ellipsoid); parameters.extent = Extent.clone(parameters.extent); diff --git a/Source/Workers/createVerticesFromQuantizedTerrainMesh.js b/Source/Workers/createVerticesFromQuantizedTerrainMesh.js index 7bf6ee934cb9..596e40040c1d 100644 --- a/Source/Workers/createVerticesFromQuantizedTerrainMesh.js +++ b/Source/Workers/createVerticesFromQuantizedTerrainMesh.js @@ -86,10 +86,7 @@ define([ indexBufferIndex = addSkirt(vertexBuffer, vertexBufferIndex, indexBuffer, indexBufferIndex, parameters.northIndices, center, ellipsoid, extent, parameters.northSkirtHeight, true); vertexBufferIndex += parameters.northIndices.length * vertexStride; - if (FeatureDetection.supportsTransferringArrayBuffers()) { - transferableObjects.push(vertexBuffer.buffer); - transferableObjects.push(indexBuffer.buffer); - } + transferableObjects.push(vertexBuffer.buffer, indexBuffer.buffer); return { vertices : vertexBuffer.buffer, diff --git a/Source/Workers/transferTypedArrayTest.js b/Source/Workers/transferTypedArrayTest.js new file mode 100644 index 000000000000..afc53c0d7aa0 --- /dev/null +++ b/Source/Workers/transferTypedArrayTest.js @@ -0,0 +1,18 @@ +/*global self:true*/ +// make sure self is defined so that the Dojo build can evaluate this file without crashing. +self = self || {}; + +self.onmessage = function(event) { + "use strict"; + var array = event.data.array; + var postMessage = self.webkitPostMessage || self.postMessage; + + try { + // transfer the test array back to the caller + postMessage({ + array : array + }, [array.buffer]); + } catch (e) { + postMessage({}); + } +}; \ No newline at end of file diff --git a/Source/Workers/upsampleQuantizedTerrainMesh.js b/Source/Workers/upsampleQuantizedTerrainMesh.js index 8e1983bb7712..bd4f0e4dd7d0 100644 --- a/Source/Workers/upsampleQuantizedTerrainMesh.js +++ b/Source/Workers/upsampleQuantizedTerrainMesh.js @@ -240,11 +240,7 @@ define([ } var indicesTypedArray = new Uint16Array(indices); - - if (FeatureDetection.supportsTransferringArrayBuffers()) { - transferableObjects.push(vertices.buffer); - transferableObjects.push(indicesTypedArray.buffer); - } + transferableObjects.push(vertices.buffer, indicesTypedArray.buffer); return { vertices : vertices.buffer, diff --git a/Specs/Core/TaskProcessorSpec.js b/Specs/Core/TaskProcessorSpec.js index 39886862bea4..da7e7309bb35 100644 --- a/Specs/Core/TaskProcessorSpec.js +++ b/Specs/Core/TaskProcessorSpec.js @@ -65,34 +65,28 @@ defineSuite([ }); it('can transfer array buffer', function() { - if (!FeatureDetection.supportsTransferringArrayBuffers()) { - // This browser cannot transer array buffers at all. - return; - } - taskProcessor = new TaskProcessor('returnByteLength'); var byteLength = 100; var parameters = new ArrayBuffer(byteLength); expect(parameters.byteLength).toEqual(byteLength); - var promise = taskProcessor.scheduleTask(parameters, [parameters]); + waitsForPromise(TaskProcessor._canTransferArrayBuffer).then(function(canTransferArrayBuffer) { + var promise = taskProcessor.scheduleTask(parameters, [parameters]); - // array buffer should be neutered when transferred - expect(parameters.byteLength).toEqual(0); + if (canTransferArrayBuffer) { + // array buffer should be neutered when transferred + expect(parameters.byteLength).toEqual(0); + } - // the worker should see the array with proper byte length - waitsForPromise(promise).then(function(result) { - expect(result).toEqual(byteLength); + // the worker should see the array with proper byte length + waitsForPromise(promise).then(function(result) { + expect(result).toEqual(byteLength); + }); }); }); it('can transfer array buffer back from worker', function() { - if (!FeatureDetection.supportsTransferringArrayBuffers()) { - // This browser cannot transer array buffers at all. - return; - } - taskProcessor = new TaskProcessor('transferArrayBuffer'); var byteLength = 100;