Skip to content

Commit 4a81b82

Browse files
authored
Merge pull request #10111 from CesiumGS/require-color-space-conversion-for-image-bitmap-support
Don't use ImageBitmap unless colorSpaceConversion option is supported
2 parents 0ae728c + df541b6 commit 4a81b82

File tree

4 files changed

+47
-155
lines changed

4 files changed

+47
-155
lines changed

CHANGES.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,8 @@
1515
##### Fixes :wrench:
1616

1717
- Fixed a bug where updating `ModelExperimental`'s model matrix would not update its bounding sphere. [#10078](https://github.com/CesiumGS/cesium/pull/10078)
18-
- Fixed a bug where a translucent shader applied to a `ModeleExperimental` with opaque features was not being rendered. [#10110](https://github.com/CesiumGS/cesium/pull/10110)
18+
- Fixed feature ID texture artifacts on Safari. [#10111](https://github.com/CesiumGS/cesium/pull/10111)
19+
- Fixed a bug where a translucent shader applied to a `ModelExperimental` with opaque features was not being rendered. [#10110](https://github.com/CesiumGS/cesium/pull/10110)
1920

2021
### 1.90 - 2022-02-01
2122

Source/Core/Resource.js

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import DeveloperError from "./DeveloperError.js";
1010
import getAbsoluteUri from "./getAbsoluteUri.js";
1111
import getBaseUri from "./getBaseUri.js";
1212
import getExtensionFromUri from "./getExtensionFromUri.js";
13+
import getImagePixels from "./getImagePixels.js";
1314
import isBlobUri from "./isBlobUri.js";
1415
import isCrossOriginUrl from "./isCrossOriginUrl.js";
1516
import isDataUri from "./isDataUri.js";
@@ -361,6 +362,13 @@ Resource.supportsImageBitmapOptions = function () {
361362
// Until the HTML folks figure out what to do about this, we need to actually try loading an image to
362363
// know if this browser supports passing options to the createImageBitmap function.
363364
// https://github.com/whatwg/html/pull/4248
365+
//
366+
// We also need to check whether the colorSpaceConversion option is supported.
367+
// We do this by loading a PNG with an embedded color profile, first with
368+
// colorSpaceConversion: "none" and then with colorSpaceConversion: "default".
369+
// If the pixel color is different then we know the option is working.
370+
// As of Webkit 17612.3.6.1.6 the createImageBitmap promise resolves but the
371+
// option is not actually supported.
364372
if (defined(supportsImageBitmapOptionsPromise)) {
365373
return supportsImageBitmapOptionsPromise;
366374
}
@@ -371,20 +379,27 @@ Resource.supportsImageBitmapOptions = function () {
371379
}
372380

373381
const imageDataUri =
374-
"data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVQImWP4////fwAJ+wP9CNHoHgAAAABJRU5ErkJggg==";
382+
"data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAIAAACQd1PeAAAABGdBTUEAAE4g3rEiDgAAACBjSFJNAAB6JgAAgIQAAPoAAACA6AAAdTAAAOpgAAA6mAAAF3CculE8AAAADElEQVQI12Ng6GAAAAEUAIngE3ZiAAAAAElFTkSuQmCC";
375383

376384
supportsImageBitmapOptionsPromise = Resource.fetchBlob({
377385
url: imageDataUri,
378386
})
379387
.then(function (blob) {
380-
return createImageBitmap(blob, {
381-
imageOrientation: "flipY",
382-
premultiplyAlpha: "none",
383-
colorSpaceConversion: "none",
384-
});
388+
const imageBitmapOptions = {
389+
imageOrientation: "flipY", // default is "none"
390+
premultiplyAlpha: "none", // default is "default"
391+
colorSpaceConversion: "none", // default is "default"
392+
};
393+
return when.all([
394+
createImageBitmap(blob, imageBitmapOptions),
395+
createImageBitmap(blob),
396+
]);
385397
})
386-
.then(function (imageBitmap) {
387-
return true;
398+
.then(function (imageBitmaps) {
399+
// Check whether the colorSpaceConversion option had any effect on the green channel
400+
const colorWithOptions = getImagePixels(imageBitmaps[0]);
401+
const colorWithDefaults = getImagePixels(imageBitmaps[1]);
402+
return colorWithOptions[1] !== colorWithDefaults[1];
388403
})
389404
.otherwise(function () {
390405
return false;

Source/Core/getImagePixels.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ const context2DsByWidthAndHeight = {};
88
*
99
* @function getImagePixels
1010
*
11-
* @param {HTMLImageElement} image The image to extract pixels from.
11+
* @param {HTMLImageElement|ImageBitmap} image The image to extract pixels from.
1212
* @param {Number} width The width of the image. If not defined, then image.width is assigned.
1313
* @param {Number} height The height of the image. If not defined, then image.height is assigned.
1414
* @returns {ImageData} The pixels of the image.

Specs/Core/ResourceSpec.js

Lines changed: 21 additions & 145 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import { DefaultProxy } from "../../Source/Cesium.js";
22
import { defaultValue } from "../../Source/Cesium.js";
3-
import { FeatureDetection } from "../../Source/Cesium.js";
43
import { queryToObject } from "../../Source/Cesium.js";
54
import { Request } from "../../Source/Cesium.js";
65
import { RequestErrorEvent } from "../../Source/Cesium.js";
@@ -1390,210 +1389,87 @@ describe("Core/Resource", function () {
13901389
return;
13911390
}
13921391

1393-
let loadedImage;
1394-
13951392
return Resource.fetchImage({
13961393
url: "./Data/Images/BlueOverRed.png",
13971394
flipY: true,
13981395
preferImageBitmap: true,
1399-
})
1400-
.then(function (image) {
1401-
loadedImage = image;
1402-
return Resource.supportsImageBitmapOptions();
1403-
})
1404-
.then(function (supportsImageBitmapOptions) {
1405-
if (supportsImageBitmapOptions) {
1406-
expect(getColorAtPixel(loadedImage, 0, 0)).toEqual([
1407-
255,
1408-
0,
1409-
0,
1410-
255,
1411-
]);
1412-
} else {
1413-
expect(getColorAtPixel(loadedImage, 0, 0)).toEqual([
1414-
0,
1415-
0,
1416-
255,
1417-
255,
1418-
]);
1419-
}
1420-
});
1396+
}).then(function (loadedImage) {
1397+
expect(getColorAtPixel(loadedImage, 0, 0)).toEqual([255, 0, 0, 255]);
1398+
});
14211399
});
14221400

14231401
it("correctly loads image without flip when ImageBitmapOptions are supported", function () {
14241402
if (!supportsImageBitmapOptions) {
14251403
return;
14261404
}
14271405

1428-
let loadedImage;
1429-
14301406
return Resource.fetchImage({
14311407
url: "./Data/Images/BlueOverRed.png",
14321408
flipY: false,
14331409
preferImageBitmap: true,
1434-
})
1435-
.then(function (image) {
1436-
loadedImage = image;
1437-
return Resource.supportsImageBitmapOptions();
1438-
})
1439-
.then(function (supportsImageBitmapOptions) {
1440-
if (supportsImageBitmapOptions) {
1441-
expect(getColorAtPixel(loadedImage, 0, 0)).toEqual([
1442-
0,
1443-
0,
1444-
255,
1445-
255,
1446-
]);
1447-
} else {
1448-
expect(getColorAtPixel(loadedImage, 0, 0)).toEqual([
1449-
0,
1450-
0,
1451-
255,
1452-
255,
1453-
]);
1454-
}
1455-
});
1410+
}).then(function (loadedImage) {
1411+
expect(getColorAtPixel(loadedImage, 0, 0)).toEqual([0, 0, 255, 255]);
1412+
});
14561413
});
14571414

14581415
it("correctly ignores gamma color profile when ImageBitmapOptions are supported", function () {
1459-
// On newer versions of Safari and Firefox, the colorSpaceConversion option for createImageBitmap()
1460-
// is unsupported. See https://github.com/CesiumGS/cesium/issues/9875 for more information.
1461-
if (
1462-
FeatureDetection.isFirefox() ||
1463-
FeatureDetection.isSafari() ||
1464-
!supportsImageBitmapOptions
1465-
) {
1416+
if (!supportsImageBitmapOptions) {
14661417
return;
14671418
}
14681419

1469-
let loadedImage;
1470-
14711420
return Resource.fetchImage({
14721421
url: "./Data/Images/Gamma.png",
14731422
flipY: false,
14741423
skipColorSpaceConversion: true,
14751424
preferImageBitmap: true,
1476-
})
1477-
.then(function (image) {
1478-
loadedImage = image;
1479-
return Resource.supportsImageBitmapOptions();
1480-
})
1481-
.then(function (supportsImageBitmapOptions) {
1482-
if (supportsImageBitmapOptions) {
1483-
expect(getColorAtPixel(loadedImage, 0, 0)).toEqual([
1484-
0,
1485-
136,
1486-
0,
1487-
255,
1488-
]);
1489-
} else {
1490-
expect(getColorAtPixel(loadedImage, 0, 0)).toEqual([0, 59, 0, 255]);
1491-
}
1492-
});
1425+
}).then(function (loadedImage) {
1426+
expect(getColorAtPixel(loadedImage, 0, 0)).toEqual([0, 136, 0, 255]);
1427+
});
14931428
});
14941429

14951430
it("correctly allows gamma color profile when ImageBitmapOptions are supported", function () {
14961431
if (!supportsImageBitmapOptions) {
14971432
return;
14981433
}
14991434

1500-
let loadedImage;
1501-
15021435
return Resource.fetchImage({
15031436
url: "./Data/Images/Gamma.png",
15041437
flipY: false,
15051438
skipColorSpaceConversion: false,
15061439
preferImageBitmap: true,
1507-
})
1508-
.then(function (image) {
1509-
loadedImage = image;
1510-
return Resource.supportsImageBitmapOptions();
1511-
})
1512-
.then(function (supportsImageBitmapOptions) {
1513-
if (supportsImageBitmapOptions) {
1514-
expect(getColorAtPixel(loadedImage, 0, 0)).toEqual([0, 59, 0, 255]);
1515-
} else {
1516-
expect(getColorAtPixel(loadedImage, 0, 0)).toEqual([0, 59, 0, 255]);
1517-
}
1518-
});
1440+
}).then(function (loadedImage) {
1441+
expect(getColorAtPixel(loadedImage, 0, 0)).toEqual([0, 59, 0, 255]);
1442+
});
15191443
});
15201444

15211445
it("correctly ignores custom color profile when ImageBitmapOptions are supported", function () {
1522-
// On newer versions of Safari and Firefox, the colorSpaceConversion option for createImageBitmap()
1523-
// is unsupported. See https://github.com/CesiumGS/cesium/issues/9875 for more information.
1524-
if (
1525-
FeatureDetection.isFirefox() ||
1526-
FeatureDetection.isSafari() ||
1527-
!supportsImageBitmapOptions
1528-
) {
1446+
if (!supportsImageBitmapOptions) {
15291447
return;
15301448
}
15311449

1532-
let loadedImage;
1533-
15341450
return Resource.fetchImage({
15351451
url: "./Data/Images/CustomColorProfile.png",
15361452
flipY: false,
15371453
skipColorSpaceConversion: true,
15381454
preferImageBitmap: true,
1539-
})
1540-
.then(function (image) {
1541-
loadedImage = image;
1542-
return Resource.supportsImageBitmapOptions();
1543-
})
1544-
.then(function (supportsImageBitmapOptions) {
1545-
if (supportsImageBitmapOptions) {
1546-
expect(getColorAtPixel(loadedImage, 0, 0)).toEqual([
1547-
0,
1548-
136,
1549-
0,
1550-
255,
1551-
]);
1552-
} else {
1553-
expect(getColorAtPixel(loadedImage, 0, 0)).toEqual([
1554-
193,
1555-
0,
1556-
0,
1557-
255,
1558-
]);
1559-
}
1560-
});
1455+
}).then(function (loadedImage) {
1456+
expect(getColorAtPixel(loadedImage, 0, 0)).toEqual([0, 136, 0, 255]);
1457+
});
15611458
});
15621459

15631460
it("correctly allows custom color profile when ImageBitmapOptions are supported", function () {
15641461
if (!supportsImageBitmapOptions) {
15651462
return;
15661463
}
15671464

1568-
let loadedImage;
1569-
15701465
return Resource.fetchImage({
15711466
url: "./Data/Images/CustomColorProfile.png",
15721467
flipY: false,
15731468
skipColorSpaceConversion: false,
15741469
preferImageBitmap: true,
1575-
})
1576-
.then(function (image) {
1577-
loadedImage = image;
1578-
return Resource.supportsImageBitmapOptions();
1579-
})
1580-
.then(function (supportsImageBitmapOptions) {
1581-
if (supportsImageBitmapOptions) {
1582-
expect(getColorAtPixel(loadedImage, 0, 0)).toEqual([
1583-
193,
1584-
0,
1585-
0,
1586-
255,
1587-
]);
1588-
} else {
1589-
expect(getColorAtPixel(loadedImage, 0, 0)).toEqual([
1590-
193,
1591-
0,
1592-
0,
1593-
255,
1594-
]);
1595-
}
1596-
});
1470+
}).then(function (loadedImage) {
1471+
expect(getColorAtPixel(loadedImage, 0, 0)).toEqual([193, 0, 0, 255]);
1472+
});
15971473
});
15981474

15991475
it("does not use ImageBitmap when ImageBitmapOptions are not supported", function () {
@@ -1609,7 +1485,7 @@ describe("Core/Resource", function () {
16091485
return Resource.fetchImage({
16101486
url: "./Data/Images/Green.png",
16111487
preferImageBitmap: true,
1612-
}).then(function (loadedImage) {
1488+
}).then(function () {
16131489
expect(window.createImageBitmap).not.toHaveBeenCalledWith();
16141490
});
16151491
});

0 commit comments

Comments
 (0)