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 missing textures in atlas. #3011

Merged
merged 10 commits into from
Sep 16, 2015
17 changes: 17 additions & 0 deletions Source/Renderer/RenderState.js
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,23 @@ define([
return cachedState;
};

/**
* Use sparingly until we have a real solution for removing render states from the cache, e.g. TextureAtlas
* polutting the cache on resize.
*
* @private
*/
RenderState.removeFromCache = function(renderState) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to reference count these, right? What if someone else created the same render state?

// remove full key if it exists
var states = new RenderState(renderState);
var fullKey = JSON.stringify(states);
delete renderStateCache[fullKey];

// remove partial key if it exists
var partialKey = JSON.stringify(renderState);
delete renderStateCache[partialKey];
};

function enableOrDisable(gl, glEnum, enable) {
if (enable) {
gl.enable(glEnum);
Expand Down
37 changes: 34 additions & 3 deletions Source/Scene/TextureAtlas.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ define([
'../Core/PixelFormat',
'../Core/RuntimeError',
'../Renderer/Framebuffer',
'../Renderer/RenderState',
'../Renderer/Texture',
'../ThirdParty/when'
], function(
Expand All @@ -27,6 +28,7 @@ define([
PixelFormat,
RuntimeError,
Framebuffer,
RenderState,
Texture,
when) {
"use strict";
Expand Down Expand Up @@ -95,6 +97,26 @@ define([
pixelFormat : this._pixelFormat
});
this._root = new TextureAtlasNode(new Cartesian2(), new Cartesian2(initialSize.x, initialSize.y));

var that = this;
var uniformMap = {
u_texture : function() {
return that._texture;
}
};

var fs =
'uniform sampler2D u_texture;\n' +
'varying vec2 v_textureCoordinates;\n' +
'void main()\n' +
'{\n' +
' gl_FragColor = texture2D(u_texture, v_textureCoordinates);\n' +
'}\n';


this._copyCommand = this._context.createViewportQuadCommand(fs, {
uniformMap : uniformMap
});
};

defineProperties(TextureAtlas.prototype, {
Expand Down Expand Up @@ -201,16 +223,25 @@ define([
pixelFormat : textureAtlas._pixelFormat
});

// Copy old texture into new using an fbo.
var framebuffer = new Framebuffer({
context : context,
colorTextures : [textureAtlas._texture]
colorTextures : [newTexture],
destroyAttachments : false
});

var command = textureAtlas._copyCommand;
command.renderState = RenderState.fromCache({
viewport : new BoundingRectangle(0, 0, oldAtlasWidth, oldAtlasHeight)
});

framebuffer._bind();
newTexture.copyFromFramebuffer(0, 0, 0, 0, oldAtlasWidth, oldAtlasHeight);
command.execute(textureAtlas._context);
Copy link
Contributor

Choose a reason for hiding this comment

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

TextureAtlas is some of the oldest code in Cesium - it predates commands by at least a year.

I am not sure that TextureAtlas should keep a reference to context. No one else does that. This allows TextureAtlas to make WebGL calls whenever it wants regardless of the global state. This is not good for scheduling requestAnimFrame and for interop for other WebGL engines (#648). I wonder if it is even the source of this issue.

Perhaps we could replace _context with an update function and use ComputeCommands? How hard is that refactor? Is it too architect astronaut-ish?

framebuffer._unBind();
framebuffer.destroy();
textureAtlas._texture = newTexture;

RenderState.removeFromCache(command.renderState);
command.renderState = undefined;
} else {
// First image exceeds initialSize
var initialWidth = scalingFactor * (image.width + textureAtlas._borderWidthInPixels);
Expand Down