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.

Sign up for GitHub

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

Interpolate custom vertex attributes on triangles crossing the IDL #6644

Merged
merged 3 commits into from
Jun 5, 2018
Merged

Interpolate custom vertex attributes on triangles crossing the IDL #6644

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a76957d
interpolate custom vertex attributes on triangles crossing the IDL
likangning93 May 31, 2018
da39c4e
merge master into customVertexAttribsTrianglesIDL
likangning93 Jun 5, 2018
65b6a4e
PR comments
likangning93 Jun 5, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion CHANGES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ Change Log
* `PostProcessStage` has a `selectedFeatures` property which is an array of primitives used for selectively applying a post-process stage. In the fragment shader, use the function `bool czm_selected(vec2 textureCoordinates` to determine whether or not the stage should be applied at that fragment.
* The black-and-white and silhouette stages have per-feature support.

##### Fixes :wrench:
* Fixed a bug causing crashes with custom vertex attributes on `Geometry` crossing the IDL. Attributes will be barycentrically interpolated. [#6644](https://github.com/AnalyticalGraphicsInc/cesium/pull/6644)

### 1.46.1 - 2018-06-01

* This is an npm only release to fix the improperly published 1.46.0. There were no code changes.
Expand Down Expand Up @@ -44,7 +47,7 @@ Change Log
* Added a new Sandcastle label `Post Processing` to showcase the different built-in post-process stages.
* Added `zIndex` for ground geometry, including corridor, ellipse, polygon and rectangle entities. [#6362](https://github.com/AnalyticalGraphicsInc/cesium/pull/6362)
* Added `Rectangle.equalsEpsilon` for comparing the equality of two rectangles [#6533](https://github.com/AnalyticalGraphicsInc/cesium/pull/6533)

##### Fixes :wrench:
* Fixed a bug causing custom TilingScheme classes to not be able to use a GeographicProjection. [#6524](https://github.com/AnalyticalGraphicsInc/cesium/pull/6524)
* Fixed incorrect 3D Tiles statistics when a tile fails during processing. [#6558](https://github.com/AnalyticalGraphicsInc/cesium/pull/6558)
Expand Down
138 changes: 79 additions & 59 deletions Source/Core/GeometryPipeline.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1916,16 +1916,41 @@ define([
}
}

function generateBarycentricInterpolateFunction(CartesianType, numberOfComponents) {
var v0Scratch = new CartesianType();
var v1Scratch = new CartesianType();
var v2Scratch = new CartesianType();

return function(i0, i1, i2, coords, sourceValues, currentValues, insertedIndex, normalize) {
var v0 = CartesianType.fromArray(sourceValues, i0 * numberOfComponents, v0Scratch);
var v1 = CartesianType.fromArray(sourceValues, i1 * numberOfComponents, v1Scratch);
var v2 = CartesianType.fromArray(sourceValues, i2 * numberOfComponents, v2Scratch);

CartesianType.multiplyByScalar(v0, coords.x, v0);
CartesianType.multiplyByScalar(v1, coords.y, v1);
CartesianType.multiplyByScalar(v2, coords.z, v2);

var value = CartesianType.add(v0, v1, v0);
CartesianType.add(value, v2, value);

if (normalize) {
CartesianType.normalize(value, value);
}

CartesianType.pack(value, currentValues, insertedIndex * numberOfComponents);
};
}

var interpolateAndPackCartesian4 = generateBarycentricInterpolateFunction(Cartesian4, 4);
var interpolateAndPackCartesian3 = generateBarycentricInterpolateFunction(Cartesian3, 3);
var interpolateAndPackCartesian2 = generateBarycentricInterpolateFunction(Cartesian2, 2);

var p0Scratch = new Cartesian3();
var p1Scratch = new Cartesian3();
var p2Scratch = new Cartesian3();
var barycentricScratch = new Cartesian3();
var s0Scratch = new Cartesian2();
var s1Scratch = new Cartesian2();
var s2Scratch = new Cartesian2();

function computeTriangleAttributes(i0, i1, i2, point, positions, normals, tangents, bitangents, texCoords, extrudeDirections, currentAttributes, insertedIndex) {
if (!defined(normals) && !defined(tangents) && !defined(bitangents) && !defined(texCoords) && !defined(extrudeDirections)) {
function computeTriangleAttributes(i0, i1, i2, point, positions, normals, tangents, bitangents, texCoords, extrudeDirections, currentAttributes, customAttributeNames, customAttributesLength, allAttributes, insertedIndex) {
if (!defined(normals) && !defined(tangents) && !defined(bitangents) && !defined(texCoords) && !defined(extrudeDirections) && customAttributesLength === 0) {
return;
}

Expand All @@ -1935,19 +1960,7 @@ define([
var coords = barycentricCoordinates(point, p0, p1, p2, barycentricScratch);

if (defined(normals)) {
var n0 = Cartesian3.fromArray(normals, i0 * 3, p0Scratch);
var n1 = Cartesian3.fromArray(normals, i1 * 3, p1Scratch);
var n2 = Cartesian3.fromArray(normals, i2 * 3, p2Scratch);

Cartesian3.multiplyByScalar(n0, coords.x, n0);
Cartesian3.multiplyByScalar(n1, coords.y, n1);
Cartesian3.multiplyByScalar(n2, coords.z, n2);

var normal = Cartesian3.add(n0, n1, n0);
Cartesian3.add(normal, n2, normal);
Cartesian3.normalize(normal, normal);

Cartesian3.pack(normal, currentAttributes.normal.values, insertedIndex * 3);
interpolateAndPackCartesian3(i0, i1, i2, coords, normals, currentAttributes.normal.values, insertedIndex, true);
}

if (defined(extrudeDirections)) {
Expand All @@ -1974,50 +1987,41 @@ define([
}

if (defined(tangents)) {
var t0 = Cartesian3.fromArray(tangents, i0 * 3, p0Scratch);
var t1 = Cartesian3.fromArray(tangents, i1 * 3, p1Scratch);
var t2 = Cartesian3.fromArray(tangents, i2 * 3, p2Scratch);

Cartesian3.multiplyByScalar(t0, coords.x, t0);
Cartesian3.multiplyByScalar(t1, coords.y, t1);
Cartesian3.multiplyByScalar(t2, coords.z, t2);

var tangent = Cartesian3.add(t0, t1, t0);
Cartesian3.add(tangent, t2, tangent);
Cartesian3.normalize(tangent, tangent);

Cartesian3.pack(tangent, currentAttributes.tangent.values, insertedIndex * 3);
interpolateAndPackCartesian3(i0, i1, i2, coords, tangents, currentAttributes.tangent.values, insertedIndex, true);
}

if (defined(bitangents)) {
var b0 = Cartesian3.fromArray(bitangents, i0 * 3, p0Scratch);
var b1 = Cartesian3.fromArray(bitangents, i1 * 3, p1Scratch);
var b2 = Cartesian3.fromArray(bitangents, i2 * 3, p2Scratch);

Cartesian3.multiplyByScalar(b0, coords.x, b0);
Cartesian3.multiplyByScalar(b1, coords.y, b1);
Cartesian3.multiplyByScalar(b2, coords.z, b2);

var bitangent = Cartesian3.add(b0, b1, b0);
Cartesian3.add(bitangent, b2, bitangent);
Cartesian3.normalize(bitangent, bitangent);

Cartesian3.pack(bitangent, currentAttributes.bitangent.values, insertedIndex * 3);
interpolateAndPackCartesian3(i0, i1, i2, coords, bitangents, currentAttributes.bitangent.values, insertedIndex, true);
}

if (defined(texCoords)) {
var s0 = Cartesian2.fromArray(texCoords, i0 * 2, s0Scratch);
var s1 = Cartesian2.fromArray(texCoords, i1 * 2, s1Scratch);
var s2 = Cartesian2.fromArray(texCoords, i2 * 2, s2Scratch);

Cartesian2.multiplyByScalar(s0, coords.x, s0);
Cartesian2.multiplyByScalar(s1, coords.y, s1);
Cartesian2.multiplyByScalar(s2, coords.z, s2);
interpolateAndPackCartesian2(i0, i1, i2, coords, texCoords, currentAttributes.st.values, insertedIndex);
}

var texCoord = Cartesian2.add(s0, s1, s0);
Cartesian2.add(texCoord, s2, texCoord);
if (customAttributesLength > 0) {
for (var i = 0; i < customAttributesLength; i++) {
var attributeName = customAttributeNames[i];
genericInterpolate(i0, i1, i2, coords, insertedIndex, allAttributes[attributeName], currentAttributes[attributeName]);
}
}
}

Cartesian2.pack(texCoord, currentAttributes.st.values, insertedIndex * 2);
function genericInterpolate(i0, i1, i2, coords, insertedIndex, sourceAttribute, currentAttribute) {
var componentsPerAttribute = sourceAttribute.componentsPerAttribute;
var sourceValues = sourceAttribute.values;
var currentValues = currentAttribute.values;
switch(componentsPerAttribute) {
case 4:
interpolateAndPackCartesian4(i0, i1, i2, coords, sourceValues, currentValues, insertedIndex, false);
break;
case 3:
interpolateAndPackCartesian3(i0, i1, i2, coords, sourceValues, currentValues, insertedIndex, false);
break;
case 2:
interpolateAndPackCartesian2(i0, i1, i2, coords, sourceValues, currentValues, insertedIndex, false);
break;
default:
currentValues[insertedIndex] = sourceValues[i0] * coords.x + sourceValues[i1] * coords.y + sourceValues[i2] * coords.z;
}
}

Expand All @@ -2044,6 +2048,14 @@ define([
return insertIndex;
}

var NAMED_ATTRIBUTES = {
position : true,
normal : true,
bitangent : true,
tangent : true,
st : true,
extrudeDirection : true
};
function splitLongitudeTriangles(instance) {
var geometry = instance.geometry;
var attributes = geometry.attributes;
Expand All @@ -2055,6 +2067,14 @@ define([
var extrudeDirections = (defined(attributes.extrudeDirection)) ? attributes.extrudeDirection.values : undefined;
var indices = geometry.indices;

var customAttributeNames = [];
for (var attributeName in attributes) {
if (attributes.hasOwnProperty(attributeName) && !NAMED_ATTRIBUTES[attributeName] && defined(attributes[attributeName])) {
customAttributeNames.push(attributeName);
}
}
var customAttributesLength = customAttributeNames.length;

var eastGeometry = copyGeometryForSplit(geometry);
var westGeometry = copyGeometryForSplit(geometry);

Expand Down Expand Up @@ -2106,7 +2126,7 @@ define([
}

insertedIndex = insertSplitPoint(currentAttributes, currentIndices, currentIndexMap, indices, resultIndex < 3 ? i + resultIndex : -1, point);
computeTriangleAttributes(i0, i1, i2, point, positions, normals, tangents, bitangents, texCoords, extrudeDirections, currentAttributes, insertedIndex);
computeTriangleAttributes(i0, i1, i2, point, positions, normals, tangents, bitangents, texCoords, extrudeDirections, currentAttributes, customAttributeNames, customAttributesLength, attributes, insertedIndex);
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we're changing this code around anyway, can we make this an options instead of having 50 parameters?

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 thought the gazillion parameters thing might have been an attempt at increasing performance for a hot area of code by avoiding dictionary access.

If that were true the custom attributes code would be "slow," but I think it's acceptable since the geometry that triggers this shouldn't have as much triangle density.

Copy link
Contributor

Choose a reason for hiding this comment

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

Normally I would agree with @hpinkos but since this a local private function only called in a few places; I'm not sure turning it into an options object buys us anything in terms of code clarity/easy of maintenance.

Copy link
Contributor

Choose a reason for hiding this comment

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

Gotcha, yeah this is fine then

}
} else {
if (defined(result)) {
Expand All @@ -2126,13 +2146,13 @@ define([
}

insertedIndex = insertSplitPoint(currentAttributes, currentIndices, currentIndexMap, indices, i, p0);
computeTriangleAttributes(i0, i1, i2, p0, positions, normals, tangents, bitangents, texCoords, extrudeDirections, currentAttributes, insertedIndex);
computeTriangleAttributes(i0, i1, i2, p0, positions, normals, tangents, bitangents, texCoords, extrudeDirections, currentAttributes, customAttributeNames, customAttributesLength, attributes, insertedIndex);

insertedIndex = insertSplitPoint(currentAttributes, currentIndices, currentIndexMap, indices, i + 1, p1);
computeTriangleAttributes(i0, i1, i2, p1, positions, normals, tangents, bitangents, texCoords, extrudeDirections, currentAttributes, insertedIndex);
computeTriangleAttributes(i0, i1, i2, p1, positions, normals, tangents, bitangents, texCoords, extrudeDirections, currentAttributes, customAttributeNames, customAttributesLength, attributes, insertedIndex);

insertedIndex = insertSplitPoint(currentAttributes, currentIndices, currentIndexMap, indices, i + 2, p2);
computeTriangleAttributes(i0, i1, i2, p2, positions, normals, tangents, bitangents, texCoords, extrudeDirections, currentAttributes, insertedIndex);
computeTriangleAttributes(i0, i1, i2, p2, positions, normals, tangents, bitangents, texCoords, extrudeDirections, currentAttributes, customAttributeNames, customAttributesLength, attributes, insertedIndex);
}
}

Expand Down
134 changes: 134 additions & 0 deletions Specs/Core/GeometryPipelineSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1871,6 +1871,140 @@ defineSuite([
expect(splitInstance).toBe(instance);
});

it('splitLongitude interpolates custom attributes for geometry split by the IDL', function() {
var p0 = Cartesian3.fromDegrees(-179.0, 0.0);
var p1 = Cartesian3.fromDegrees(179.0, 0.0);
var p2 = Cartesian3.fromDegrees(-179.0, 1.0);

var positions = new Float64Array([
p0.x, p0.y, p0.z,
p1.x, p1.y, p1.z,
p2.x, p2.y, p2.z
]);

var vec4s = new Uint8Array([
0, 0, 0, 0,
0, 0, 0, 255,
0, 0, 0, 0
]);

var vec3s = new Uint8Array([
0, 0, 0,
0, 0, 255,
0, 0, 0
]);

var vec2s = new Uint8Array([
0, 0,
0, 255,
0, 0
]);

var scalars = new Uint8Array([0, 255, 0]);

var instance = new GeometryInstance({
geometry : new Geometry({
attributes : {
position : new GeometryAttribute({
componentDatatype : ComponentDatatype.DOUBLE,
componentsPerAttribute : 3,
values : positions
}),
vec4s: new GeometryAttribute({
componentDatatype: ComponentDatatype.UNSIGNED_BYTE,
componentsPerAttribute: 4,
values: vec4s
}),
vec3s: new GeometryAttribute({
componentDatatype: ComponentDatatype.UNSIGNED_BYTE,
componentsPerAttribute: 3,
values: vec3s
}),
vec2s: new GeometryAttribute({
componentDatatype: ComponentDatatype.UNSIGNED_BYTE,
componentsPerAttribute: 2,
values: vec2s
}),
scalars: new GeometryAttribute({
componentDatatype: ComponentDatatype.UNSIGNED_BYTE,
componentsPerAttribute: 1,
values: scalars
})
},
indices : new Uint16Array([0, 1, 2]),
primitiveType : PrimitiveType.TRIANGLES,
boundingSphere : BoundingSphere.fromVertices(positions)
})
});

var splitInstance = GeometryPipeline.splitLongitude(instance);
var eastHemisphereGeometry = splitInstance.eastHemisphereGeometry;
expect(eastHemisphereGeometry.indices.length).toEqual(3);

var newVec4s = eastHemisphereGeometry.attributes.vec4s.values;
var newVec3s = eastHemisphereGeometry.attributes.vec3s.values;
var newVec2s = eastHemisphereGeometry.attributes.vec2s.values;
var newScalars = eastHemisphereGeometry.attributes.scalars.values;
var i;
var index;

// Expect eastern hemisphere vertices to all be 255 or 127 at the end of the value
expect(newScalars.indexOf(127)).not.toBe(-1);
expect(newVec4s.indexOf(127)).not.toBe(-1);
expect(newVec3s.indexOf(127)).not.toBe(-1);
expect(newVec2s.indexOf(127)).not.toBe(-1);
for (i = 0; i < 3; i++) {
expect(newScalars[i] === 255 || newScalars[i] === 127).toBe(true);

index = i * 2;
expect(newVec2s[index]).toBe(0);
expect(newVec2s[index + 1] === 255 || newVec2s[index + 1] === 127).toBe(true);

index = i * 3;
expect(newVec3s[index]).toBe(0);
expect(newVec3s[index + 1]).toBe(0);
expect(newVec3s[index + 2] === 255 || newVec3s[index + 2] === 127).toBe(true);

index = i * 4;
expect(newVec4s[index]).toBe(0);
expect(newVec4s[index + 1]).toBe(0);
expect(newVec4s[index + 2]).toBe(0);
expect(newVec4s[index + 3] === 255 || newVec4s[index + 3] === 127).toBe(true);
}

var westHemisphereGeometry = splitInstance.westHemisphereGeometry;
expect(westHemisphereGeometry.indices.length).toEqual(6);

newVec4s = westHemisphereGeometry.attributes.vec4s.values;
newVec3s = westHemisphereGeometry.attributes.vec3s.values;
newVec2s = westHemisphereGeometry.attributes.vec2s.values;
newScalars = westHemisphereGeometry.attributes.scalars.values;

// Expect eastern hemisphere vertices to all be 0 or 127 at the end of the value
expect(newScalars.indexOf(127)).not.toBe(-1);
expect(newVec4s.indexOf(127)).not.toBe(-1);
expect(newVec3s.indexOf(127)).not.toBe(-1);
expect(newVec2s.indexOf(127)).not.toBe(-1);
for (i = 0; i < 4; i++) {
expect(newScalars[i] === 0 || newScalars[i] === 127).toBe(true);

index = i * 2;
expect(newVec2s[index]).toBe(0);
expect(newVec2s[index + 1] === 0 || newVec2s[index + 1] === 127).toBe(true);

index = i * 3;
expect(newVec3s[index]).toBe(0);
expect(newVec3s[index + 1]).toBe(0);
expect(newVec3s[index + 2] === 0 || newVec3s[index + 2] === 127).toBe(true);

index = i * 4;
expect(newVec4s[index]).toBe(0);
expect(newVec4s[index + 1]).toBe(0);
expect(newVec4s[index + 2]).toBe(0);
expect(newVec4s[index + 3] === 0 || newVec4s[index + 3] === 127).toBe(true);
}
});

it('splitLongitude provides indices for an un-indexed triangle list', function() {
var instance = new GeometryInstance({
geometry : new Geometry({
Expand Down