Skip to content

Commit 49bdead

Browse files
lilleyselilleyse
authored
Merge pull request #8630 from CesiumGS/faster
Greatly improve performance of clamped Entity geometry with dynamic colors
2 parents a63992b + 14e91be commit 49bdead

12 files changed

+18
-102
lines changed

‎CHANGES.md

+1
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ Change Log
1010

1111
* Added `Entity.tileset` for loading a 3D Tiles tileset via the Entity API using the new `Cesium3DTilesetGraphics` class.
1212
* Added `tileset.uri`, `tileset.show`, and `tileset.maximumScreenSpaceError` properties to CZML processing for loading 3D Tiles.
13+
* Massively improved performance of clamped Entity ground geometry with dynamic colors. [#8630](https://github.com/CesiumGS/cesium/pull/8630)
1314
* Added `Color.lerp` for linearly interpolating between two RGB colors. [#8607](https://github.com/CesiumGS/cesium/pull/8607)
1415
* `CesiumTerrainProvider` now supports terrain tiles using a `WebMercatorTilingScheme` by specifying `"projection": "EPSG:3857"` in `layer.json`. It also now supports numbering tiles from the North instead of the South by specifying `"scheme": "slippyMap"` in `layer.json`. [#8563](https://github.com/CesiumGS/cesium/pull/8563)
1516
* Added basic support for `isNaN`, `isFinite`, `null`, and `undefined` in the 3D Tiles styling GLSL backend for point clouds. [#8621](https://github.com/CesiumGS/cesium/pull/8621)

‎Source/DataSources/CorridorGeometryUpdater.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ import Property from './Property.js';
174174
!Property.isConstant(corridor.outlineWidth) || //
175175
!Property.isConstant(corridor.cornerType) || //
176176
!Property.isConstant(corridor.zIndex) || //
177-
(this._onTerrain && !Property.isConstant(this._materialProperty));
177+
(this._onTerrain && !Property.isConstant(this._materialProperty) && !(this._materialProperty instanceof ColorMaterialProperty));
178178
};
179179

180180
CorridorGeometryUpdater.prototype._setStaticOptions = function(entity, corridor) {

‎Source/DataSources/EllipseGeometryUpdater.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ import Property from './Property.js';
179179
!Property.isConstant(ellipse.outlineWidth) || //
180180
!Property.isConstant(ellipse.numberOfVerticalLines) || //
181181
!Property.isConstant(ellipse.zIndex) || //
182-
(this._onTerrain && !Property.isConstant(this._materialProperty));
182+
(this._onTerrain && !Property.isConstant(this._materialProperty) && !(this._materialProperty instanceof ColorMaterialProperty));
183183
};
184184

185185
EllipseGeometryUpdater.prototype._setStaticOptions = function(entity, ellipse) {

‎Source/DataSources/GeometryVisualizer.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ import WallGeometryUpdater from './WallGeometryUpdater.js';
141141
if (supportsMaterialsforEntitiesOnTerrain) {
142142
for (i = 0; i < numberOfClassificationTypes; ++i) {
143143
groundMaterialBatches.push(new StaticGroundGeometryPerMaterialBatch(groundPrimitives, i, MaterialAppearance));
144-
groundColorBatches[i] = new StaticGroundGeometryPerMaterialBatch(groundPrimitives, i, PerInstanceColorAppearance);
144+
groundColorBatches[i] = new StaticGroundGeometryColorBatch(groundPrimitives, i);
145145
}
146146
} else {
147147
for (i = 0; i < numberOfClassificationTypes; ++i) {

‎Source/DataSources/PolygonGeometryUpdater.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,7 @@ import Property from './Property.js';
240240
!Property.isConstant(polygon.closeBottom) || //
241241
!Property.isConstant(polygon.zIndex) || //
242242
!Property.isConstant(polygon.arcType) || //
243-
(this._onTerrain && !Property.isConstant(this._materialProperty));
243+
(this._onTerrain && !Property.isConstant(this._materialProperty) && !(this._materialProperty instanceof ColorMaterialProperty));
244244
};
245245

246246
PolygonGeometryUpdater.prototype._setStaticOptions = function(entity, polygon) {

‎Source/DataSources/RectangleGeometryUpdater.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ import Property from './Property.js';
179179
!Property.isConstant(rectangle.rotation) || //
180180
!Property.isConstant(rectangle.outlineWidth) || //
181181
!Property.isConstant(rectangle.zIndex) || //
182-
(this._onTerrain && !Property.isConstant(this._materialProperty));
182+
(this._onTerrain && !Property.isConstant(this._materialProperty) && !(this._materialProperty instanceof ColorMaterialProperty));
183183
};
184184

185185
RectangleGeometryUpdater.prototype._setStaticOptions = function(entity, rectangle) {

‎Source/DataSources/StaticGroundGeometryColorBatch.js

+4-10
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import AssociativeArray from '../Core/AssociativeArray.js';
22
import Color from '../Core/Color.js';
3+
import ColorGeometryInstanceAttribute from '../Core/ColorGeometryInstanceAttribute.js';
34
import defined from '../Core/defined.js';
45
import DistanceDisplayCondition from '../Core/DistanceDisplayCondition.js';
56
import DistanceDisplayConditionGeometryInstanceAttribute from '../Core/DistanceDisplayConditionGeometryInstanceAttribute.js';
@@ -65,8 +66,6 @@ import Property from './Property.js';
6566
return false;
6667
};
6768

68-
var scratchArray = new Array(4);
69-
7069
Batch.prototype.update = function(time) {
7170
var isUpdated = true;
7271
var removedCount = 0;
@@ -135,12 +134,7 @@ import Property from './Property.js';
135134

136135
if (!Color.equals(attributes._lastColor, fillColor)) {
137136
attributes._lastColor = Color.clone(fillColor, attributes._lastColor);
138-
var color = this.color;
139-
var newColor = fillColor.toBytes(scratchArray);
140-
if (color[0] !== newColor[0] || color[1] !== newColor[1] ||
141-
color[2] !== newColor[2] || color[3] !== newColor[3]) {
142-
this.itemsToRemove[removedCount++] = updater;
143-
}
137+
attributes.color = ColorGeometryInstanceAttribute.toValue(fillColor, attributes.color);
144138
}
145139
}
146140

@@ -241,9 +235,9 @@ import Property from './Property.js';
241235
StaticGroundGeometryColorBatch.prototype.add = function(time, updater) {
242236
var instance = updater.createFillGeometryInstance(time);
243237
var batches = this._batches;
244-
// color and zIndex are batch breakers, so we'll use that for the key
238+
// zIndex is a batch breaker, so we'll use that for the key
245239
var zIndex = Property.getValueOrDefault(updater.zIndex, 0);
246-
var batchKey = new Uint32Array(instance.attributes.color.value.buffer)[0] + ':' + zIndex;
240+
var batchKey = zIndex;
247241
var batch;
248242
if (batches.contains(batchKey)) {
249243
batch = batches.get(batchKey);

‎Source/Scene/ClassificationPrimitive.js

-8
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,6 @@ import StencilConstants from './StencilConstants.js';
2626
import StencilFunction from './StencilFunction.js';
2727
import StencilOperation from './StencilOperation.js';
2828

29-
var ClassificationPrimitiveReadOnlyInstanceAttributes = ['color'];
30-
3129
/**
3230
* A classification primitive represents a volume enclosing geometry in the {@link Scene} to be highlighted.
3331
* <p>
@@ -172,11 +170,6 @@ import StencilOperation from './StencilOperation.js';
172170

173171
this.appearance = options.appearance;
174172

175-
var readOnlyAttributes;
176-
if (defined(geometryInstances) && isArray(geometryInstances) && geometryInstances.length > 1) {
177-
readOnlyAttributes = ClassificationPrimitiveReadOnlyInstanceAttributes;
178-
}
179-
180173
this._createBoundingVolumeFunction = options._createBoundingVolumeFunction;
181174
this._updateAndQueueCommandsFunction = options._updateAndQueueCommandsFunction;
182175

@@ -191,7 +184,6 @@ import StencilOperation from './StencilOperation.js';
191184
allowPicking : defaultValue(options.allowPicking, true),
192185
asynchronous : defaultValue(options.asynchronous, true),
193186
compressVertices : defaultValue(options.compressVertices, true),
194-
_readOnlyInstanceAttributes : readOnlyAttributes,
195187
_createBoundingVolumeFunction : undefined,
196188
_createRenderStatesFunction : undefined,
197189
_createShaderProgramFunction : undefined,

‎Source/Scene/Primitive.js

+2-19
Original file line numberDiff line numberDiff line change
@@ -323,8 +323,6 @@ import ShadowMode from './ShadowMode.js';
323323
this._colorCommands = [];
324324
this._pickCommands = [];
325325

326-
this._readOnlyInstanceAttributes = options._readOnlyInstanceAttributes;
327-
328326
this._createBoundingVolumeFunction = options._createBoundingVolumeFunction;
329327
this._createRenderStatesFunction = options._createRenderStatesFunction;
330328
this._createShaderProgramFunction = options._createShaderProgramFunction;
@@ -2072,24 +2070,9 @@ import ShadowMode from './ShadowMode.js';
20722070
if (perInstanceAttributeIndices.hasOwnProperty(name)) {
20732071
var attributeIndex = perInstanceAttributeIndices[name];
20742072
properties[name] = {
2075-
get : createGetFunction(batchTable, index, attributeIndex)
2073+
get : createGetFunction(batchTable, index, attributeIndex),
2074+
set : createSetFunction(batchTable, index, attributeIndex, this, name)
20762075
};
2077-
2078-
var createSetter = true;
2079-
var readOnlyAttributes = this._readOnlyInstanceAttributes;
2080-
if (createSetter && defined(readOnlyAttributes)) {
2081-
length = readOnlyAttributes.length;
2082-
for (var k = 0; k < length; ++k) {
2083-
if (name === readOnlyAttributes[k]) {
2084-
createSetter = false;
2085-
break;
2086-
}
2087-
}
2088-
}
2089-
2090-
if (createSetter) {
2091-
properties[name].set = createSetFunction(batchTable, index, attributeIndex, this, name);
2092-
}
20932076
}
20942077
}
20952078

‎Specs/DataSources/GeometryVisualizerSpec.js

-51
Original file line numberDiff line numberDiff line change
@@ -708,57 +708,6 @@ describe('DataSources/GeometryVisualizer', function() {
708708
});
709709
});
710710

711-
it('batches ground entities by identical color if ground entity materials are not supported', function() {
712-
spyOn(GroundPrimitive, 'supportsMaterials').and.callFake(function() {
713-
return false;
714-
});
715-
var entities = new EntityCollection();
716-
var visualizer = new GeometryVisualizer(scene, entities, scene.primitives, scene.groundPrimitives);
717-
718-
var blueColor = Color.BLUE.withAlpha(0.5);
719-
entities.add({
720-
position : new Cartesian3(1, 2, 3),
721-
ellipse : {
722-
semiMajorAxis : 2,
723-
semiMinorAxis : 1,
724-
material : blueColor
725-
}
726-
});
727-
728-
return visualizerUpdated(visualizer).then(function() {
729-
expect(scene.groundPrimitives.length).toEqual(1);
730-
731-
entities.add({
732-
position : new Cartesian3(12, 34, 45),
733-
ellipse : {
734-
semiMajorAxis : 2,
735-
semiMinorAxis : 1,
736-
material : blueColor
737-
}
738-
});
739-
740-
return visualizerUpdated(visualizer);
741-
}).then(function() {
742-
expect(scene.groundPrimitives.length).toEqual(1);
743-
744-
entities.add({
745-
position : new Cartesian3(123, 456, 789),
746-
ellipse : {
747-
semiMajorAxis : 2,
748-
semiMinorAxis : 1,
749-
material : Color.BLUE.withAlpha(0.6)
750-
}
751-
});
752-
753-
return visualizerUpdated(visualizer);
754-
}).then(function() {
755-
expect(scene.groundPrimitives.length).toEqual(2);
756-
757-
entities.removeAll();
758-
visualizer.destroy();
759-
});
760-
});
761-
762711
it('batches ground entities by material if ground entity materials is supported', function() {
763712
if (!GroundPrimitive.isSupported(scene) || !GroundPrimitive.supportsMaterials(scene)) {
764713
return;

‎Specs/DataSources/StaticGroundGeometryColorBatchSpec.js

+4-7
Original file line numberDiff line numberDiff line change
@@ -37,11 +37,8 @@ describe('DataSources/StaticGroundGeometryColorBatch', function() {
3737
ApproximateTerrainHeights._terrainHeights = undefined;
3838
});
3939

40-
function computeKey(color, zIndex) {
41-
var ui8 = new Uint8Array(color);
42-
var ui32 = new Uint32Array(ui8.buffer);
43-
zIndex = defaultValue(zIndex, 0);
44-
return ui32[0] + ':' + zIndex;
40+
function computeKey(zIndex) {
41+
return defaultValue(zIndex, 0);
4542
}
4643

4744
it('updates color attribute after rebuilding primitive', function() {
@@ -75,7 +72,7 @@ describe('DataSources/StaticGroundGeometryColorBatch', function() {
7572
var primitive = scene.groundPrimitives.get(0);
7673
var attributes = primitive.getGeometryInstanceAttributes(entity);
7774
var red = [255, 0, 0, 255];
78-
var redKey = computeKey(red);
75+
var redKey = computeKey();
7976
expect(attributes.color).toEqual(red);
8077

8178
// Verify we have 1 batch with the key for red
@@ -97,7 +94,7 @@ describe('DataSources/StaticGroundGeometryColorBatch', function() {
9794
var primitive = scene.groundPrimitives.get(0);
9895
var attributes = primitive.getGeometryInstanceAttributes(entity);
9996
var green = [0, 128, 0, 255];
100-
var greenKey = computeKey(green);
97+
var greenKey = computeKey();
10198
expect(attributes.color).toEqual(green);
10299

103100
// Verify we have 1 batch with the key for green

‎Specs/createGeometryUpdaterGroundGeometrySpecs.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -27,14 +27,14 @@ import { PrimitiveCollection } from '../Source/Cesium.js';
2727
expect(updater.zIndex.getValue()).toBe(22);
2828
});
2929

30-
it('A time-varying color causes ground geometry to be dynamic', function() {
30+
it('A time-varying color does not cause ground geometry to be dynamic', function() {
3131
var entity = createEntity();
3232
var color = new SampledProperty(Color);
3333
color.addSample(time, Color.WHITE);
3434
entity[geometryPropertyName].material = new ColorMaterialProperty(color);
3535
var updater = new Updater(entity, getScene());
3636

37-
expect(updater.isDynamic).toBe(true);
37+
expect(updater.isDynamic).toBe(false);
3838
});
3939

4040
it('Checks that an entity without height and extrudedHeight is on terrain', function() {

0 commit comments

Comments
 (0)