Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Firefox console warnings. #5939

Merged
merged 12 commits into from
Mar 1, 2018
1 change: 1 addition & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ Change Log
* Fixed an issue causing the Bing Maps key to be sent unnecessarily with every tile request. [#6250](https://github.com/AnalyticalGraphicsInc/cesium/pull/6250)
* Fixed bug with zooming to dynamic geometry. [#6269](https://github.com/AnalyticalGraphicsInc/cesium/issues/6269)
* Fixed documentation issue for the `Cesium.Math` class. [#6233](https://github.com/AnalyticalGraphicsInc/cesium/issues/6233)
* Fix Firefox WebGL console warnings. [#5912](https://github.com/AnalyticalGraphicsInc/cesium/issues/5912)

### 1.42.1 - 2018-02-01
_This is an npm-only release to fix an issue with using Cesium in Node.js.__
Expand Down
20 changes: 20 additions & 0 deletions Source/Core/PixelFormat.js
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,26 @@ define([
componentsLength = 1;
}
return componentsLength * PixelDatatype.sizeInBytes(pixelDatatype) * width * height;
},

/**
* @private
*/
createTypedArray : function(pixelFormat, pixelDatatype, width, height) {
var constructor;
var sizeInBytes = PixelDatatype.sizeInBytes(pixelDatatype);
if (sizeInBytes === Uint8Array.BYTES_PER_ELEMENT) {
constructor = Uint8Array;
} else if (sizeInBytes === Uint16Array.BYTES_PER_ELEMENT) {
constructor = Uint16Array;
} else if (sizeInBytes === Float32Array.BYTES_PER_ELEMENT && pixelDatatype === PixelDatatype.FLOAT) {
constructor = Float32Array;
} else {
constructor = Uint32Array;
}

var size = PixelFormat.componentsLength(pixelFormat) * width * height;
return new constructor(size);
}
};

Expand Down
40 changes: 22 additions & 18 deletions Source/Renderer/CubeMap.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ define([
'use strict';

function CubeMap(options) {

options = defaultValue(options, defaultValue.EMPTY_OBJECT);

//>>includeStart('debug', pragmas.debug);
Expand Down Expand Up @@ -121,25 +120,29 @@ define([
gl.activeTexture(gl.TEXTURE0);
gl.bindTexture(textureTarget, texture);

function createFace(target, sourceFace) {
function createFace(target, sourceFace, preMultiplyAlpha, flipY) {
// TODO: gl.pixelStorei(gl._UNPACK_ALIGNMENT, 4);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you mean to leave this TODO in?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that was in the original code. I just moved it.

if (sourceFace.arrayBufferView) {
gl.pixelStorei(gl.UNPACK_PREMULTIPLY_ALPHA_WEBGL, false);
gl.pixelStorei(gl.UNPACK_FLIP_Y_WEBGL, false);
gl.texImage2D(target, 0, pixelFormat, size, size, 0, pixelFormat, pixelDatatype, sourceFace.arrayBufferView);
} else {
// Only valid for DOM-Element uploads
gl.pixelStorei(gl.UNPACK_PREMULTIPLY_ALPHA_WEBGL, preMultiplyAlpha);
gl.pixelStorei(gl.UNPACK_FLIP_Y_WEBGL, flipY);

// Source: ImageData, HTMLImageElement, HTMLCanvasElement, or HTMLVideoElement
gl.texImage2D(target, 0, pixelFormat, pixelFormat, pixelDatatype, sourceFace);
}
}

if (defined(source)) {
// TODO: _gl.pixelStorei(_gl._UNPACK_ALIGNMENT, 4);
gl.pixelStorei(gl.UNPACK_PREMULTIPLY_ALPHA_WEBGL, preMultiplyAlpha);
gl.pixelStorei(gl.UNPACK_FLIP_Y_WEBGL, flipY);

createFace(gl.TEXTURE_CUBE_MAP_POSITIVE_X, source.positiveX);
createFace(gl.TEXTURE_CUBE_MAP_NEGATIVE_X, source.negativeX);
createFace(gl.TEXTURE_CUBE_MAP_POSITIVE_Y, source.positiveY);
createFace(gl.TEXTURE_CUBE_MAP_NEGATIVE_Y, source.negativeY);
createFace(gl.TEXTURE_CUBE_MAP_POSITIVE_Z, source.positiveZ);
createFace(gl.TEXTURE_CUBE_MAP_NEGATIVE_Z, source.negativeZ);
createFace(gl.TEXTURE_CUBE_MAP_POSITIVE_X, source.positiveX, preMultiplyAlpha, flipY);
createFace(gl.TEXTURE_CUBE_MAP_NEGATIVE_X, source.negativeX, preMultiplyAlpha, flipY);
createFace(gl.TEXTURE_CUBE_MAP_POSITIVE_Y, source.positiveY, preMultiplyAlpha, flipY);
createFace(gl.TEXTURE_CUBE_MAP_NEGATIVE_Y, source.negativeY, preMultiplyAlpha, flipY);
createFace(gl.TEXTURE_CUBE_MAP_POSITIVE_Z, source.positiveZ, preMultiplyAlpha, flipY);
createFace(gl.TEXTURE_CUBE_MAP_NEGATIVE_Z, source.negativeZ, preMultiplyAlpha, flipY);
} else {
gl.texImage2D(gl.TEXTURE_CUBE_MAP_POSITIVE_X, 0, pixelFormat, size, size, 0, pixelFormat, pixelDatatype, null);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it valid to pass in null? If the value is null does the texture still require another texImage2D call with an empty typed array? Could that step go here instead? If so, then I think all the initialized checks in CubeMapFace may not be needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did it this way for the case where you create a texture and upload the entire texture later. This avoids creating an array buffer of all zeroes. The only time the array buffer of zeroes would be created would be when a texture is created and then a bunch of texSubImage2D's are called to fill out an atlas. If we don't care about that case or optimization, then Ill change this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, a texture that is going to be rendered to cannot be set.

gl.texImage2D(gl.TEXTURE_CUBE_MAP_NEGATIVE_X, 0, pixelFormat, size, size, 0, pixelFormat, pixelDatatype, null);
Expand All @@ -163,12 +166,13 @@ define([
this._flipY = flipY;
this._sampler = undefined;

this._positiveX = new CubeMapFace(gl, texture, textureTarget, gl.TEXTURE_CUBE_MAP_POSITIVE_X, pixelFormat, pixelDatatype, size, preMultiplyAlpha, flipY);
this._negativeX = new CubeMapFace(gl, texture, textureTarget, gl.TEXTURE_CUBE_MAP_NEGATIVE_X, pixelFormat, pixelDatatype, size, preMultiplyAlpha, flipY);
this._positiveY = new CubeMapFace(gl, texture, textureTarget, gl.TEXTURE_CUBE_MAP_POSITIVE_Y, pixelFormat, pixelDatatype, size, preMultiplyAlpha, flipY);
this._negativeY = new CubeMapFace(gl, texture, textureTarget, gl.TEXTURE_CUBE_MAP_NEGATIVE_Y, pixelFormat, pixelDatatype, size, preMultiplyAlpha, flipY);
this._positiveZ = new CubeMapFace(gl, texture, textureTarget, gl.TEXTURE_CUBE_MAP_POSITIVE_Z, pixelFormat, pixelDatatype, size, preMultiplyAlpha, flipY);
this._negativeZ = new CubeMapFace(gl, texture, textureTarget, gl.TEXTURE_CUBE_MAP_NEGATIVE_Z, pixelFormat, pixelDatatype, size, preMultiplyAlpha, flipY);
var initialized = defined(source);
this._positiveX = new CubeMapFace(gl, texture, textureTarget, gl.TEXTURE_CUBE_MAP_POSITIVE_X, pixelFormat, pixelDatatype, size, preMultiplyAlpha, flipY, initialized);
this._negativeX = new CubeMapFace(gl, texture, textureTarget, gl.TEXTURE_CUBE_MAP_NEGATIVE_X, pixelFormat, pixelDatatype, size, preMultiplyAlpha, flipY, initialized);
this._positiveY = new CubeMapFace(gl, texture, textureTarget, gl.TEXTURE_CUBE_MAP_POSITIVE_Y, pixelFormat, pixelDatatype, size, preMultiplyAlpha, flipY, initialized);
this._negativeY = new CubeMapFace(gl, texture, textureTarget, gl.TEXTURE_CUBE_MAP_NEGATIVE_Y, pixelFormat, pixelDatatype, size, preMultiplyAlpha, flipY, initialized);
this._positiveZ = new CubeMapFace(gl, texture, textureTarget, gl.TEXTURE_CUBE_MAP_POSITIVE_Z, pixelFormat, pixelDatatype, size, preMultiplyAlpha, flipY, initialized);
this._negativeZ = new CubeMapFace(gl, texture, textureTarget, gl.TEXTURE_CUBE_MAP_NEGATIVE_Z, pixelFormat, pixelDatatype, size, preMultiplyAlpha, flipY, initialized);

this.sampler = defined(options.sampler) ? options.sampler : new Sampler();
}
Expand Down
57 changes: 50 additions & 7 deletions Source/Renderer/CubeMapFace.js
Original file line number Diff line number Diff line change
@@ -1,21 +1,25 @@
define([
'../Core/Check',
'../Core/defaultValue',
'../Core/defined',
'../Core/defineProperties',
'../Core/DeveloperError',
'../Core/PixelFormat',
'./PixelDatatype'
], function(
Check,
defaultValue,
defined,
defineProperties,
DeveloperError,
PixelFormat,
PixelDatatype) {
'use strict';

/**
* @private
*/
function CubeMapFace(gl, texture, textureTarget, targetFace, pixelFormat, pixelDatatype, size, preMultiplyAlpha, flipY) {
function CubeMapFace(gl, texture, textureTarget, targetFace, pixelFormat, pixelDatatype, size, preMultiplyAlpha, flipY, initialized) {
this._gl = gl;
this._texture = texture;
this._textureTarget = textureTarget;
Expand All @@ -25,6 +29,7 @@ define([
this._size = size;
this._preMultiplyAlpha = preMultiplyAlpha;
this._flipY = flipY;
this._initialized = initialized;
}

defineProperties(CubeMapFace.prototype, {
Expand Down Expand Up @@ -91,15 +96,52 @@ define([
var target = this._textureTarget;

// TODO: gl.pixelStorei(gl._UNPACK_ALIGNMENT, 4);
gl.pixelStorei(gl.UNPACK_PREMULTIPLY_ALPHA_WEBGL, this._preMultiplyAlpha);
gl.pixelStorei(gl.UNPACK_FLIP_Y_WEBGL, this._flipY);

gl.activeTexture(gl.TEXTURE0);
gl.bindTexture(target, this._texture);

if (source.arrayBufferView) {
gl.texSubImage2D(this._targetFace, 0, xOffset, yOffset, source.width, source.height, this._pixelFormat, this._pixelDatatype, source.arrayBufferView);
} else {
gl.texSubImage2D(this._targetFace, 0, xOffset, yOffset, this._pixelFormat, this._pixelDatatype, source);
var uploaded = false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here and below seems more complicated than it needs to be; I will not have the time to dive deep for awhile.

@lilleyse can you take a look?

if (!this._initialized) {
if (xOffset === 0 && yOffset === 0 && source.width === this._size && source.height === this._size) {
// initialize the entire texture
if (defined(source.arrayBufferView)) {
gl.pixelStorei(gl.UNPACK_PREMULTIPLY_ALPHA_WEBGL, false);
gl.pixelStorei(gl.UNPACK_FLIP_Y_WEBGL, false);

gl.texImage2D(this._targetFace, 0, this._pixelFormat, this._size, this._size, 0, this._pixelFormat, this._pixelDatatype, source.arrayBufferView);
} else {
// Only valid for DOM-Element uploads
gl.pixelStorei(gl.UNPACK_PREMULTIPLY_ALPHA_WEBGL, this._preMultiplyAlpha);
gl.pixelStorei(gl.UNPACK_FLIP_Y_WEBGL, this._flipY);

gl.texImage2D(this._targetFace, 0, this._pixelFormat, this._pixelFormat, this._pixelDatatype, source);
}
uploaded = true;
} else {
gl.pixelStorei(gl.UNPACK_PREMULTIPLY_ALPHA_WEBGL, false);
gl.pixelStorei(gl.UNPACK_FLIP_Y_WEBGL, false);

// initialize the entire texture to zero
var bufferView = PixelFormat.createTypedArray(this._pixelFormat, this._pixelDatatype, this._size, this._size);
gl.texImage2D(this._targetFace, 0, this._pixelFormat, this._size, this._size, 0, this._pixelFormat, this._pixelDatatype, bufferView);
}
this._initialized = true;
}

if (!uploaded) {
if (source.arrayBufferView) {
gl.pixelStorei(gl.UNPACK_PREMULTIPLY_ALPHA_WEBGL, false);
gl.pixelStorei(gl.UNPACK_FLIP_Y_WEBGL, false);

gl.texSubImage2D(this._targetFace, 0, xOffset, yOffset, source.width, source.height, this._pixelFormat, this._pixelDatatype, source.arrayBufferView);
} else {
// Only valid for DOM-Element uploads
gl.pixelStorei(gl.UNPACK_PREMULTIPLY_ALPHA_WEBGL, this._preMultiplyAlpha);
gl.pixelStorei(gl.UNPACK_FLIP_Y_WEBGL, this._flipY);

// Source: ImageData, HTMLImageElement, HTMLCanvasElement, or HTMLVideoElement
gl.texSubImage2D(this._targetFace, 0, xOffset, yOffset, this._pixelFormat, this._pixelDatatype, source);
}
}

gl.bindTexture(target, null);
Expand Down Expand Up @@ -160,6 +202,7 @@ define([
gl.bindTexture(target, this._texture);
gl.copyTexSubImage2D(this._targetFace, 0, xOffset, yOffset, framebufferXOffset, framebufferYOffset, width, height);
gl.bindTexture(target, null);
this._initialized = true;
};

return CubeMapFace;
Expand Down
58 changes: 48 additions & 10 deletions Source/Renderer/Texture.js
Original file line number Diff line number Diff line change
Expand Up @@ -164,18 +164,20 @@ define([
var preMultiplyAlpha = options.preMultiplyAlpha || pixelFormat === PixelFormat.RGB || pixelFormat === PixelFormat.LUMINANCE;
var flipY = defaultValue(options.flipY, true);

var initialized = true;

var gl = context._gl;
var textureTarget = gl.TEXTURE_2D;
var texture = gl.createTexture();

// TODO: gl.pixelStorei(gl._UNPACK_ALIGNMENT, 4);
gl.pixelStorei(gl.UNPACK_PREMULTIPLY_ALPHA_WEBGL, false);
gl.pixelStorei(gl.UNPACK_FLIP_Y_WEBGL, false);

gl.activeTexture(gl.TEXTURE0);
gl.bindTexture(textureTarget, texture);

if (defined(source)) {
// TODO: _gl.pixelStorei(_gl._UNPACK_ALIGNMENT, 4);
gl.pixelStorei(gl.UNPACK_PREMULTIPLY_ALPHA_WEBGL, preMultiplyAlpha);
gl.pixelStorei(gl.UNPACK_FLIP_Y_WEBGL, flipY);

if (defined(source.arrayBufferView)) {
// Source: typed array
if (isCompressed) {
Expand All @@ -195,11 +197,16 @@ define([
source.framebuffer._unBind();
}
} else {
// Only valid for DOM-Element uploads
gl.pixelStorei(gl.UNPACK_PREMULTIPLY_ALPHA_WEBGL, preMultiplyAlpha);
gl.pixelStorei(gl.UNPACK_FLIP_Y_WEBGL, flipY);

// Source: ImageData, HTMLImageElement, HTMLCanvasElement, or HTMLVideoElement
gl.texImage2D(textureTarget, 0, internalFormat, pixelFormat, pixelDatatype, source);
}
} else {
gl.texImage2D(textureTarget, 0, internalFormat, width, height, 0, pixelFormat, pixelDatatype, null);
initialized = false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Basically the same note as above, can we initialize the texture with an empty typed array now to avoid the initialized check in copyFrom?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment here.

}
gl.bindTexture(textureTarget, null);

Expand All @@ -224,6 +231,7 @@ define([
this._sizeInBytes = sizeInBytes;
this._preMultiplyAlpha = preMultiplyAlpha;
this._flipY = flipY;
this._initialized = initialized;
this._sampler = undefined;

this.sampler = defined(options.sampler) ? options.sampler : new Sampler();
Expand Down Expand Up @@ -469,15 +477,44 @@ define([
var target = this._textureTarget;

// TODO: gl.pixelStorei(gl._UNPACK_ALIGNMENT, 4);
gl.pixelStorei(gl.UNPACK_PREMULTIPLY_ALPHA_WEBGL, this._preMultiplyAlpha);
gl.pixelStorei(gl.UNPACK_FLIP_Y_WEBGL, this._flipY);
gl.pixelStorei(gl.UNPACK_PREMULTIPLY_ALPHA_WEBGL, false);
gl.pixelStorei(gl.UNPACK_FLIP_Y_WEBGL, false);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make the same changes as CubeMapFace so that pixelStorei isn't called multiple times in some cases

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.


gl.activeTexture(gl.TEXTURE0);
gl.bindTexture(target, this._texture);

if (source.arrayBufferView) {
gl.texSubImage2D(target, 0, xOffset, yOffset, source.width, source.height, this._pixelFormat, this._pixelDatatype, source.arrayBufferView);
} else {
gl.texSubImage2D(target, 0, xOffset, yOffset, this._pixelFormat, this._pixelDatatype, source);
var uploaded = false;
if (!this._initialized) {
if (xOffset === 0 && yOffset === 0 && source.width === this._width && source.height === this._height) {
// initialize the entire texture
if (defined(source.arrayBufferView)) {
gl.texImage2D(target, 0, this._pixelFormat, this._width, this._height, 0, this._pixelFormat, this._pixelDatatype, source.arrayBufferView);
} else {
// Only valid for DOM-Element uploads
gl.pixelStorei(gl.UNPACK_PREMULTIPLY_ALPHA_WEBGL, this._preMultiplyAlpha);
gl.pixelStorei(gl.UNPACK_FLIP_Y_WEBGL, this._flipY);

gl.texImage2D(target, 0, this._pixelFormat, this._pixelFormat, this._pixelDatatype, source);
}
uploaded = true;
} else {
// initialize the entire texture to zero
var bufferView = PixelFormat.createTypedArray(this._pixelFormat, this._pixelDatatype, this._width, this._height);
gl.texImage2D(target, 0, this._pixelFormat, this._width, this._height, 0, this._pixelFormat, this._pixelDatatype, bufferView);
}
this._initialized = true;
}

if (!uploaded) {
if (source.arrayBufferView) {
gl.texSubImage2D(target, 0, xOffset, yOffset, source.width, source.height, this._pixelFormat, this._pixelDatatype, source.arrayBufferView);
} else {
// Only valid for DOM-Element uploads
gl.pixelStorei(gl.UNPACK_PREMULTIPLY_ALPHA_WEBGL, this._preMultiplyAlpha);
gl.pixelStorei(gl.UNPACK_FLIP_Y_WEBGL, this._flipY);

gl.texSubImage2D(target, 0, xOffset, yOffset, this._pixelFormat, this._pixelDatatype, source);
}
}

gl.bindTexture(target, null);
Expand Down Expand Up @@ -536,6 +573,7 @@ define([
gl.bindTexture(target, this._texture);
gl.copyTexSubImage2D(target, 0, xOffset, yOffset, framebufferXOffset, framebufferYOffset, width, height);
gl.bindTexture(target, null);
this._initialized = true;
};

/**
Expand Down