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
1 change: 1 addition & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ Change Log
* Fixed an issue with drill picking at low frame rates that would cause a crash. [#3010](https://github.com/AnalyticalGraphicsInc/cesium/pull/3010)
* Fixed a bug that prevented `setView` from working across all scene modes.
* Fixed a bug that caused `camera.positionWC` to occasionally return the incorrect value.
* Added a workaround for Chrome 45, where the first character in a label with a small font size would not appear. [#3011](https://github.com/AnalyticalGraphicsInc/cesium/pull/3011)

### 1.13 - 2015-09-01

Expand Down
64 changes: 60 additions & 4 deletions Source/Renderer/RenderState.js
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,8 @@ define([
var partialKey = JSON.stringify(renderState);
var cachedState = renderStateCache[partialKey];
if (defined(cachedState)) {
return cachedState;
++cachedState.referenceCount;
return cachedState.state;
}

// Cache miss. Fully define render state and try again.
Expand All @@ -411,16 +412,71 @@ define([
if (!defined(cachedState)) {
states.id = nextRenderStateId++;

cachedState = states;
cachedState = {
referenceCount : 0,
state : states
};

// Cache full render state. Multiple partially defined render states may map to this.
renderStateCache[fullKey] = cachedState;
}

++cachedState.referenceCount;

// Cache partial render state so we can skip validation on a cache hit for a partially defined render state
renderStateCache[partialKey] = cachedState;
renderStateCache[partialKey] = {
referenceCount : 1,
state : cachedState.state
};

return cachedState.state;
};

/**
* @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?

var states = new RenderState(renderState);
var fullKey = JSON.stringify(states);
var fullCachedState = renderStateCache[fullKey];

// decrement partial key reference count
var partialKey = JSON.stringify(renderState);
var cachedState = renderStateCache[partialKey];
if (defined(cachedState)) {
--cachedState.referenceCount;

if (cachedState.referenceCount === 0) {
// remove partial key
delete renderStateCache[partialKey];

return cachedState;
// decrement full key reference count
if (defined(fullCachedState)) {
--fullCachedState.referenceCount;
}
}
}

// remove full key if reference count is zero
if (defined(fullCachedState) && (fullCachedState.referenceCount === 0)) {
delete renderStateCache[fullKey];
}
};

/**
* This function is for testing purposes only.
* @private
*/
RenderState.getCache = function() {
return renderStateCache;
};

/**
* This function is for testing purposes only.
* @private
*/
RenderState.clearCache = function() {
renderStateCache = {};
};

function enableOrDisable(gl, glEnum, enable) {
Expand Down
40 changes: 37 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,28 @@ 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;
var renderState = {
viewport : new BoundingRectangle(0, 0, oldAtlasWidth, oldAtlasHeight)
};
command.renderState = RenderState.fromCache(renderState);

// Copy by rendering a viewport quad, instead of using Texture.copyFromFramebuffer,
// to workaround a Chrome 45 issue, https://github.com/AnalyticalGraphicsInc/cesium/issues/2997
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(renderState);
command.renderState = undefined;
} else {
// First image exceeds initialSize
var initialWidth = scalingFactor * (image.width + textureAtlas._borderWidthInPixels);
Expand Down
46 changes: 46 additions & 0 deletions Specs/Renderer/RenderStateSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,52 @@ defineSuite([
expect(rs4).not.toBe(rs);
});

it('removes from render cache', function() {
RenderState.clearCache();
var cache = RenderState.getCache();

var rs = RenderState.fromCache();
var undefinedKey = JSON.stringify(undefined);
var fullKey = JSON.stringify(new RenderState());

expect(cache[fullKey].referenceCount).toEqual(1);
expect(cache[undefinedKey].referenceCount).toEqual(1);

var rs2 = RenderState.fromCache();

expect(cache[fullKey].referenceCount).toEqual(1);
expect(cache[undefinedKey].referenceCount).toEqual(2);

// rs3 is still the same state as rs and rs2, but with a partial definition
var param = {
depthTest : {
enabled : false,
func : WebGLConstants.LESS
}
};
var rs3 = RenderState.fromCache(param);
var paramKey = JSON.stringify(param);

expect(rs2).toBe(rs);
expect(rs3).toBe(rs);

expect(cache[fullKey].referenceCount).toEqual(2);
expect(cache[undefinedKey].referenceCount).toEqual(2);
expect(cache[paramKey].referenceCount).toEqual(1);

RenderState.removeFromCache(param);

expect(cache[fullKey].referenceCount).toEqual(1);
expect(cache[undefinedKey].referenceCount).toEqual(2);
expect(cache[paramKey]).not.toBeDefined();

RenderState.removeFromCache();
RenderState.removeFromCache();

expect(cache[undefinedKey]).not.toBeDefined();
expect(cache[fullKey]).not.toBeDefined();
});

it('fails to create (frontFace)', function() {
expect(function() {
RenderState.fromCache({
Expand Down