Add preferImageBitmap option that defaults to false

SilenceT · Apr 2, 2019 · 758cd16 · 758cd16
1 parent d900910
commit 758cd16
Show file tree

Hide file tree

Showing 18 changed files with 137 additions and 86 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -1,10 +1,14 @@
 Change Log
 ==========
 
-### 1.56.1 - 2019-04-01
+### 1.56.1 - 2019-04-02
+
+##### Additions :tada:
+* `Resource.fetchImage` now takes a `preferImageBitmap` option to use `createImageBitmap` when supported to move image decode off the main thread. This option defaults to `false`.
 
 ##### Breaking Changes :mega:
-* Changed the default image orientation on `Resource.fetchImage` to `flipY : false`. In general, if an image is going to be uploaded as a texture, `flipY` should be true. Textures in Cesium are flipped by default on upload. If the browser supports `ImageBitmap`, then it cannot be flipped during upload, so it should be flipped on fetch instead.
+* Changed `Resource.fetchImage` back to return an `Image` by default, instead of an `ImageBitmap` when supported. Note that an `ImageBitmap` cannot be flipped during texture upload. Instead, set `flipY : true` during fetch to flip it.
+* Changed the default `flipY` option in `Resource.fetchImage` to false. This only has an effect when ImageBitmap is used.
 
 ### 1.56 - 2019-04-01
 

diff --git a/Source/Core/Resource.js b/Source/Core/Resource.js
@@ -872,11 +872,12 @@ define([
 
     /**
      * Asynchronously loads the given image resource.  Returns a promise that will resolve to
-     * an {@link https://developer.mozilla.org/en-US/docs/Web/API/ImageBitmap|ImageBitmap} if the browser supports `createImageBitmap` or otherwise an
+     * an {@link https://developer.mozilla.org/en-US/docs/Web/API/ImageBitmap|ImageBitmap} if `preferImageBitmap` is true and the browser supports `createImageBitmap` or otherwise an
      * {@link https://developer.mozilla.org/en-US/docs/Web/API/HTMLImageElement|Image} once loaded, or reject if the image failed to load.
      *
      * @param {Object} [options] An object with the following properties.
      * @param {Boolean} [options.preferBlob=false] If true, we will load the image via a blob.
+     * @param {Boolean} [options.preferImageBitmap=false] If true, image will be decoded during fetch and an `ImageBitmap` is returned.
      * @param {Boolean} [options.flipY=false] If true, image will be vertially flipped during decode. Only applies if the browser supports `createImageBitmap`.
      * @returns {Promise.<ImageBitmap>|Promise.<Image>|undefined} a promise that will resolve to the requested data when loaded. Returns undefined if <code>request.throttle</code> is true and the request does not have high enough priority.
      *
@@ -905,6 +906,7 @@ define([
             };
         }
         options = defaultValue(options, defaultValue.EMPTY_OBJECT);
+        var preferImageBitmap = defaultValue(options.preferImageBitmap, false);
         var preferBlob = defaultValue(options.preferBlob, false);
         var flipY = defaultValue(options.flipY, false);
 
@@ -916,7 +918,11 @@ define([
         // 3. It's a blob URI
         // 4. It doesn't have request headers and we preferBlob is false
         if (!xhrBlobSupported || this.isDataUri || this.isBlobUri || (!this.hasHeaders && !preferBlob)) {
-            return fetchImage(this, flipY);
+            return fetchImage({
+                resource: this,
+                flipY: flipY,
+                preferImageBitmap: preferImageBitmap
+            });
         }
 
         var blobPromise = this.fetchBlob();
@@ -925,18 +931,20 @@ define([
         }
 
         var supportsImageBitmap;
+        var useImageBitmap;
         var generatedBlobResource;
         var generatedBlob;
         return Resource.supportsImageBitmapOptions()
             .then(function(result) {
                 supportsImageBitmap = result;
+                useImageBitmap = supportsImageBitmap && preferImageBitmap;
                 return blobPromise;
             })
             .then(function(blob) {
                 if (!defined(blob)) {
                     return;
                 }
-                if (supportsImageBitmap) {
+                if (useImageBitmap) {
                     return Resource._Implementations.createImageBitmapFromBlob(blob, flipY);
                 }
                 generatedBlob = blob;
@@ -945,13 +953,17 @@ define([
                     url: blobUrl
                 });
 
-                return fetchImage(generatedBlobResource, flipY);
+                return fetchImage({
+                    resource: generatedBlobResource,
+                    flipY: flipY,
+                    preferImageBitmap: false
+                });
             })
             .then(function(image) {
                 if (!defined(image)) {
                     return;
                 }
-                if (supportsImageBitmap) {
+                if (useImageBitmap) {
                     return image;
                 }
                 window.URL.revokeObjectURL(generatedBlobResource.url);
@@ -970,7 +982,21 @@ define([
             });
     };
 
-    function fetchImage(resource, flipY) {
+    /**
+     * Fetches an image and returns a promise to it.
+     *
+     * @param {Object} [options] An object with the following properties.
+     * @param {Resource} [options.resource] Resource object that points to an image to fetch.
+     * @param {Boolean} [options.preferImageBitmap] If true, image will be decoded during fetch and an `ImageBitmap` is returned.
+     * @param {Boolean} [options.flipY] If true, image will be vertially flipped during decode. Only applies if the browser supports `createImageBitmap`.
+     *
+     * @private
+     */
+    function fetchImage(options) {
+        var resource = options.resource;
+        var flipY = options.flipY;
+        var preferImageBitmap = options.preferImageBitmap;
+
         var request = resource.request;
         request.url = resource.url;
         request.requestFunction = function() {
@@ -984,7 +1010,7 @@ define([
 
             var deferred = when.defer();
 
-            Resource._Implementations.createImage(url, crossOrigin, deferred, flipY);
+            Resource._Implementations.createImage(url, crossOrigin, deferred, flipY, preferImageBitmap);
 
             return deferred.promise;
         };
@@ -1008,7 +1034,11 @@ define([
                             request.state = RequestState.UNISSUED;
                             request.deferred = undefined;
 
-                            return fetchImage(resource, flipY);
+                            return fetchImage({
+                                resource: resource,
+                                flipY: flipY,
+                                preferImageBitmap: preferImageBitmap
+                            });
                         }
 
                         return when.reject(e);
@@ -1030,13 +1060,15 @@ define([
      * @param {Number} [options.retryAttempts=0] The number of times the retryCallback should be called before giving up.
      * @param {Request} [options.request] A Request object that will be used. Intended for internal use only.
      * @param {Boolean} [options.preferBlob = false]  If true, we will load the image via a blob.
+     * @param {Boolean} [options.preferImageBitmap = false] If true, image will be decoded during fetch and an `ImageBitmap` is returned.
      * @returns {Promise.<ImageBitmap>|Promise.<Image>|undefined} a promise that will resolve to the requested data when loaded. Returns undefined if <code>request.throttle</code> is true and the request does not have high enough priority.
      */
     Resource.fetchImage = function (options) {
         var resource = new Resource(options);
         return resource.fetchImage({
             flipY: options.flipY,
-            preferBlob: options.preferBlob
+            preferBlob: options.preferBlob,
+            preferImageBitmap: options.preferImageBitmap
         });
     };
 
@@ -1848,17 +1880,17 @@ define([
         image.src = url;
     }
 
-    Resource._Implementations.createImage = function(url, crossOrigin, deferred, flipY) {
+    Resource._Implementations.createImage = function(url, crossOrigin, deferred, flipY, preferImageBitmap) {
         // Passing an Image to createImageBitmap will force it to run on the main thread
         // since DOM elements don't exist on workers. We convert it to a blob so it's non-blocking.
         // See:
         //    https://bugzilla.mozilla.org/show_bug.cgi?id=1044102#c38
         //    https://bugs.chromium.org/p/chromium/issues/detail?id=580202#c10
         Resource.supportsImageBitmapOptions()
-            .then(function(result) {
+            .then(function(supportsImageBitmap) {
                 // We can only use ImageBitmap if we can flip on decode.
                 // See: https://github.com/AnalyticalGraphicsInc/cesium/pull/7579#issuecomment-466146898
-                if (!result) {
+                if (!(supportsImageBitmap && preferImageBitmap)) {
                     loadImageElement(url, crossOrigin, deferred);
                     return;
                 }
@@ -1885,16 +1917,9 @@ define([
     };
 
     Resource._Implementations.createImageBitmapFromBlob = function(blob, flipY) {
-        return Resource.supportsImageBitmapOptions()
-            .then(function(result) {
-                if (!result) {
-                    return createImageBitmap(blob);
-                }
-
-                return createImageBitmap(blob, {
-                    imageOrientation: flipY ? 'flipY' : 'none'
-                });
-            });
+        return createImageBitmap(blob, {
+            imageOrientation: flipY ? 'flipY' : 'none'
+        });
     };
 
     function decodeResponse(loadWithHttpResponse, responseType) {

diff --git a/Source/Core/VRTheWorldTerrainProvider.js b/Source/Core/VRTheWorldTerrainProvider.js
@@ -267,7 +267,9 @@ define([
             },
             request : request
         });
-        var promise = resource.fetchImage();
+        var promise = resource.fetchImage({
+            preferImageBitmap: true
+        });
         if (!defined(promise)) {
             return undefined;
         }

diff --git a/Source/Core/loadImageFromTypedArray.js b/Source/Core/loadImageFromTypedArray.js
@@ -37,7 +37,8 @@ define([
             request: request
         });
         return resource.fetchImage({
-            flipY : flipY
+            flipY : flipY,
+            preferImageBitmap : true
         })
             .then(function(image) {
                 window.URL.revokeObjectURL(blobUrl);

diff --git a/Source/Renderer/loadCubeMap.js b/Source/Renderer/loadCubeMap.js
@@ -67,7 +67,8 @@ define([
         // Also, it is perhaps acceptable to use the context here in the callbacks, but
         // ideally, we would do it in the primitive's update function.
         var flipOptions = {
-            flipY : true
+            flipY : true,
+            preferImageBitmap: true
         };
 
         var facePromises = [

diff --git a/Source/Scene/DiscardMissingTileImagePolicy.js b/Source/Scene/DiscardMissingTileImagePolicy.js
@@ -91,7 +91,7 @@ define([
 
         resource.fetchImage({
             preferBlob : true,
-            flipY : true
+            preferImageBitmap : true
         }).then(success).otherwise(failure);
     }
 

diff --git a/Source/Scene/ImageryProvider.js b/Source/Scene/ImageryProvider.js
@@ -346,11 +346,13 @@ define([
         } else if (defined(imageryProvider.tileDiscardPolicy)) {
             return resource.fetchImage({
                 preferBlob : true,
+                preferImageBitmap : true,
                 flipY : true
             });
         }
 
         return resource.fetchImage({
+            preferImageBitmap : true,
             flipY : true
         });
     };

diff --git a/Source/Scene/Material.js b/Source/Scene/Material.js
@@ -817,9 +817,7 @@ define([
                     } else if (crnRegex.test(uniformValue)) {
                         promise = loadCRN(resource);
                     } else {
-                        promise = resource.fetchImage({
-                            flipY : true
-                        });
+                        promise = resource.fetchImage();
                     }
                     when(promise, function(image) {
                         material._loadedImages.push({
@@ -868,17 +866,13 @@ define([
                 uniformValue.positiveZ + uniformValue.negativeZ;
 
             if (path !== material._texturePaths[uniformId]) {
-                var fetchOptions = {
-                    flipY : true
-                };
-
                 var promises = [
-                    Resource.createIfNeeded(uniformValue.positiveX).fetchImage(fetchOptions),
-                    Resource.createIfNeeded(uniformValue.negativeX).fetchImage(fetchOptions),
-                    Resource.createIfNeeded(uniformValue.positiveY).fetchImage(fetchOptions),
-                    Resource.createIfNeeded(uniformValue.negativeY).fetchImage(fetchOptions),
-                    Resource.createIfNeeded(uniformValue.positiveZ).fetchImage(fetchOptions),
-                    Resource.createIfNeeded(uniformValue.negativeZ).fetchImage(fetchOptions)
+                    Resource.createIfNeeded(uniformValue.positiveX).fetchImage(),
+                    Resource.createIfNeeded(uniformValue.negativeX).fetchImage(),
+                    Resource.createIfNeeded(uniformValue.positiveY).fetchImage(),
+                    Resource.createIfNeeded(uniformValue.negativeY).fetchImage(),
+                    Resource.createIfNeeded(uniformValue.positiveZ).fetchImage(),
+                    Resource.createIfNeeded(uniformValue.negativeZ).fetchImage()
                 ];
 
                 when.all(promises).then(function(images) {

diff --git a/Source/Scene/PostProcessStage.js b/Source/Scene/PostProcessStage.js
@@ -671,9 +671,7 @@ define([
                     url : stageNameUrlOrImage
                 });
 
-                promises.push(resource.fetchImage({
-                    flipY : true
-                }).then(createLoadImageFunction(stage, name)));
+                promises.push(resource.fetchImage().then(createLoadImageFunction(stage, name)));
             } else {
                 stage._texturesToCreate.push({
                     name : name,

diff --git a/Source/Scene/SingleTileImageryProvider.js b/Source/Scene/SingleTileImageryProvider.js
@@ -109,7 +109,7 @@ define([
         }
 
         function doRequest() {
-            when(resource.fetchImage({ flipY: true }), success, failure);
+            resource.fetchImage().then(success).otherwise(failure);
         }
 
         doRequest();

diff --git a/Source/Scene/TextureAtlas.js b/Source/Scene/TextureAtlas.js
@@ -360,9 +360,7 @@ define([
         } else if ((typeof image === 'string') || (image instanceof Resource)) {
             // Get a resource
             var resource = Resource.createIfNeeded(image);
-            image = resource.fetchImage({
-                flipY : true
-            });
+            image = resource.fetchImage();
         }
 
         var that = this;