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

Support concave and/or n-vertex faces #85

Merged
merged 11 commits into from
Aug 9, 2017
298 changes: 249 additions & 49 deletions lib/loadObj.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,20 @@ var loadMtl = require('./loadMtl');
var readLines = require('./readLines');

var Axis = Cesium.Axis;
var Cartesian2 = Cesium.Cartesian2;
var Cartesian3 = Cesium.Cartesian3;
var ComponentDatatype = Cesium.ComponentDatatype;
var defaultValue = Cesium.defaultValue;
var defined = Cesium.defined;
var IntersectionTests = Cesium.IntersectionTests;
var Matrix3 = Cesium.Matrix3;
var Matrix4 = Cesium.Matrix4;
var OrientedBoundingBox = Cesium.OrientedBoundingBox;
var Plane = Cesium.Plane;
var PolygonPipeline = Cesium.PolygonPipeline;
var Ray = Cesium.Ray;
var RuntimeError = Cesium.RuntimeError;
var WindingOrder = Cesium.WindingOrder;

module.exports = loadObj;

Expand All @@ -41,13 +49,10 @@ function Primitive() {
}

// OBJ regex patterns are modified from ThreeJS (https://github.com/mrdoob/three.js/blob/master/examples/js/loaders/OBJLoader.js)
var vertexPattern = /v( +[\d|\.|\+|\-|e|E]+)( +[\d|\.|\+|\-|e|E]+)( +[\d|\.|\+|\-|e|E]+)/; // v float float float
var normalPattern = /vn( +[\d|\.|\+|\-|e|E]+)( +[\d|\.|\+|\-|e|E]+)( +[\d|\.|\+|\-|e|E]+)/; // vn float float float
var uvPattern = /vt( +[\d|\.|\+|\-|e|E]+)( +[\d|\.|\+|\-|e|E]+)/; // vt float float
var facePattern1 = /f( +-?\d+)\/?( +-?\d+)\/?( +-?\d+)\/?( +-?\d+)?\/?/; // f vertex vertex vertex ...
var facePattern2 = /f( +(-?\d+)\/(-?\d+)\/?)( +(-?\d+)\/(-?\d+)\/?)( +(-?\d+)\/(-?\d+)\/?)( +(-?\d+)\/(-?\d+)\/?)?/; // f vertex/uv vertex/uv vertex/uv ...
var facePattern3 = /f( +(-?\d+)\/(-?\d+)\/(-?\d+))( +(-?\d+)\/(-?\d+)\/(-?\d+))( +(-?\d+)\/(-?\d+)\/(-?\d+))( +(-?\d+)\/(-?\d+)\/(-?\d+))?/; // f vertex/uv/normal vertex/uv/normal vertex/uv/normal ...
var facePattern4 = /f( +(-?\d+)\/\/(-?\d+))( +(-?\d+)\/\/(-?\d+))( +(-?\d+)\/\/(-?\d+))( +(-?\d+)\/\/(-?\d+))?/; // f vertex//normal vertex//normal vertex//normal ...
var vertexPattern = /v( +[\d|\.|\+|\-|e|E]+)( +[\d|\.|\+|\-|e|E]+)( +[\d|\.|\+|\-|e|E]+)/; // v float float float
var normalPattern = /vn( +[\d|\.|\+|\-|e|E]+)( +[\d|\.|\+|\-|e|E]+)( +[\d|\.|\+|\-|e|E]+)/; // vn float float float
var uvPattern = /vt( +[\d|\.|\+|\-|e|E]+)( +[\d|\.|\+|\-|e|E]+)/; // vt float float
Copy link
Contributor

Choose a reason for hiding this comment

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

Can these three also benefit from the {3} syntax like below?

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 don't think they do, because unlike the face line parsing, there are a set number of them them, each with a capture group. The face lines are different because there are n items. The {3,} just specifies at least 3.

var facePattern = /(-?\d+)\/?(-?\d*)\/?(-?\d*)/g; // for any face format "f v", "f v/v", "f v//v", "f v/v/v"

var scratchCartesian = new Cartesian3();

Expand Down Expand Up @@ -91,6 +96,18 @@ function loadObj(objPath, options) {
// All mtl paths seen in the obj
var mtlPaths = [];

// Buffers for face data that spans multiple lines
var lineBuffer = '';

// Used for parsing face data
var faceVertices = []; // names of vertices for caching
var facePositions = []; // indices into position array
var faceUvs = []; // indices into uv array
var faceNormals = []; // indices into normal array
Copy link
Contributor

Choose a reason for hiding this comment

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

The inline comments probably aren't needed.


var positions3D = [];
var vertexIndices = [];

function getName(name) {
return (name === '' ? undefined : name);
}
Expand Down Expand Up @@ -135,6 +152,94 @@ function loadObj(objPath, options) {
primitive.material = getName(name);
}

var intPoint = new Cartesian3();
Copy link
Contributor

Choose a reason for hiding this comment

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

Rename to scratchIntersectionPoint. Also prepend scratch to the other variables.

var xAxis = Cesium.Cartesian3.UNIT_X.clone();
var yAxis = Cesium.Cartesian3.UNIT_Y.clone();
var zAxis = Cesium.Cartesian3.UNIT_Z.clone();
Copy link
Contributor

Choose a reason for hiding this comment

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

Do these need be cloned from UNIT_X and others? I think they can just be new Cesium.Cartesian3().

var origin = new Cartesian3();
var normal = new Cartesian3();
var ray = new Ray();
var plane = new Plane(Cesium.Cartesian3.UNIT_X, 0);
function projectTo2D(positions) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Move this function down below a bit, maybe right above get3DPoint.

var positions2D = new Array(positions.length);
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be best to use a scratch array for positions2D too. There should also be some bank of scratch Cartesian2 that can be used in place of the allocations below. The same applies to the cartesians that are allocated for positions3D.

var obb = OrientedBoundingBox.fromPoints(positions);
var halfAxes = obb.halfAxes;
Matrix3.getColumn(halfAxes, 0, xAxis);
Matrix3.getColumn(halfAxes, 1, yAxis);
Matrix3.getColumn(halfAxes, 2, zAxis);

var xMag = Cartesian3.magnitude(xAxis);
var yMag = Cartesian3.magnitude(yAxis);
var zMag = Cartesian3.magnitude(zAxis);
var min = Math.min(xMag, yMag, zMag);

// If all the points are on a line, just remove one of the zero dimensions
if (xMag === 0 && (yMag === 0 || zMag === 0)) {
var i;
for (i = 0; i < positions.length; i++) {
positions2D[i] = new Cartesian2(positions[i].y, positions[i].z);
}
return positions2D;
} else if (yMag === 0 && zMag === 0) {
for (i = 0; i < positions.length; i++) {
positions2D[i] = new Cartesian2(positions[i].x, positions[i].y);
}
return positions2D;
}

var center = obb.center;
var planeXAxis;
var planeYAxis;
if (min === xMag) {
if (!xAxis.equals(Cartesian3.ZERO)) {
Cartesian3.add(center, xAxis, origin);
Cartesian3.normalize(xAxis, normal);
}
planeXAxis = Cartesian3.normalize(yAxis, yAxis);
planeYAxis = Cartesian3.normalize(zAxis, zAxis);
} else if (min === yMag) {
if (!yAxis.equals(Cartesian3.ZERO)) {
Cartesian3.add(center, yAxis, origin);
Cartesian3.normalize(yAxis, normal);
}
planeXAxis = Cartesian3.normalize(xAxis, xAxis);
planeYAxis = Cartesian3.normalize(zAxis, zAxis);
} else {
if (!zAxis.equals(Cartesian3.ZERO)) {
Cartesian3.add(center, zAxis, origin);
Cartesian3.normalize(zAxis, normal);
}
planeXAxis = Cartesian3.normalize(xAxis, xAxis);
planeYAxis = Cartesian3.normalize(yAxis, yAxis);
}

if (min === 0) {
normal = Cartesian3.cross(planeXAxis, planeYAxis, normal);
normal = Cartesian3.normalize(normal, normal);
}

Plane.fromPointNormal(origin, normal, plane);
ray.direction = normal;

for (i = 0; i < positions.length; i++) {
ray.origin = positions[i];

var intersectionPoint = IntersectionTests.rayPlane(ray, plane, intPoint);

if (!defined(intersectionPoint)) {
Cartesian3.negate(ray.direction, ray.direction);
intersectionPoint = IntersectionTests.rayPlane(ray, plane, intPoint);
}
var v = Cartesian3.subtract(intersectionPoint, origin, intersectionPoint);
var x = Cartesian3.dot(planeXAxis, v);
var y = Cartesian3.dot(planeYAxis, v);

positions2D[i] = new Cartesian2(x, y);
}

return positions2D;
}

function getOffset(a, attributeData, components) {
var i = parseInt(a);
if (i < 0) {
Expand All @@ -146,7 +251,7 @@ function loadObj(objPath, options) {

function createVertex(p, u, n) {
// Positions
if (defined(p)) {
if (p.length > 0) {
var pi = getOffset(p, positions, 3);
var px = positions.get(pi + 0);
var py = positions.get(pi + 1);
Expand All @@ -157,7 +262,7 @@ function loadObj(objPath, options) {
}

// Normals
if (defined(n)) {
if (n.length > 0) {
var ni = getOffset(n, normals, 3);
var nx = normals.get(ni + 0);
var ny = normals.get(ni + 1);
Expand All @@ -168,7 +273,7 @@ function loadObj(objPath, options) {
}

// UVs
if (defined(u)) {
if (u.length > 0) {
var ui = getOffset(u, uvs, 2);
var ux = uvs.get(ui + 0);
var uy = uvs.get(ui + 1);
Expand All @@ -195,21 +300,121 @@ function loadObj(objPath, options) {
return index;
}

function addFace(v1, p1, u1, n1, v2, p2, u2, n2, v3, p3, u3, n3, v4, p4, u4, n4) {
var index1 = addVertex(v1, p1, u1, n1);
var index2 = addVertex(v2, p2, u2, n2);
var index3 = addVertex(v3, p3, u3, n3);
function get3DPoint(index, result) {
var pi = getOffset(index, positions, 3);
var px = positions.get(pi + 0);
var py = positions.get(pi + 1);
var pz = positions.get(pi + 2);
return Cartesian3.fromElements(px, py, pz, result);
}

function get3DNormal(index, result) {
var ni = getOffset(index, normals, 3);
var nx = normals.get(ni + 0);
var ny = normals.get(ni + 1);
var nz = normals.get(ni + 2);
return Cartesian3.fromElements(nx, ny, nz, result);
}

// Given a sequence of three points A B C, determine whether vector BC
// "turns" clockwise (positive) or counter-clockwise (negative) from vector AB
var scratch1 = new Cartesian3();
var scratch2 = new Cartesian3();
function getTurnDirection(pointA, pointB, pointC) {
var vector1 = Cartesian2.subtract(pointA, pointB, scratch1);
var vector2 = Cartesian2.subtract(pointC, pointB, scratch2);
return vector1.x * vector2.y - vector1.y * vector2.x;
}

// Given the cartesian 2 vertices of a polygon, determine if convex
function isConvex(positions2D) {
var i;
var turnDirection = getTurnDirection(positions2D[0], positions2D[1], positions2D[2]);
for (i=1; i < positions2D.length-2; ++i) {
var currentTurnDirection = getTurnDirection(positions2D[i], positions2D[i+1], positions2D[i+2]);
if (turnDirection * currentTurnDirection < 0) {
return false;
}
}
return true;
}

primitive.indices.push(index1);
primitive.indices.push(index2);
primitive.indices.push(index3);
var scratch3 = new Cartesian3();
// Checks if winding order matches the given normal.
function checkWindingCorrect(positionIndex1, positionIndex2, positionIndex3, normal) {
var A = get3DPoint(positionIndex1, scratch1);
var B = get3DPoint(positionIndex2, scratch2);
var C = get3DPoint(positionIndex3, scratch3);

// Triangulate if the face is a quad
if (defined(v4)) {
var index4 = addVertex(v4, p4, u4, n4);
Cartesian3.subtract(B, A, B);
Cartesian3.subtract(C, A, C);
var cross = Cartesian3.cross(A, C, scratch1);

return (Cartesian3.dot(normal, cross) >= 0);
}

function addTriangle(index1, index2, index3, correctWinding) {
if (correctWinding) {
primitive.indices.push(index1);
primitive.indices.push(index2);
primitive.indices.push(index3);
} else {
primitive.indices.push(index1);
primitive.indices.push(index3);
primitive.indices.push(index4);
primitive.indices.push(index2);
}
}

var scratchNormal = new Cartesian3();
function addFace(vertices, positions, uvs, normals) {
Copy link
Contributor

Choose a reason for hiding this comment

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

How easy would it be to move winding order correct / triangulation code into a separate file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Definitely doable, but the file logic will be pretty tangled though, because it will essentially be all the logic from addFace, which uses addVertex, the global normals / position buffers and the face position and normal buffers. Don't know if it's worth it?

var isWindingCorrect = true;
var faceNormal;

// If normals are defined, find a face normal to use in winding order sanitization.
// If no face normal, we have to assume the winding is correct.
if (normals[0].length > 0) {
faceNormal = get3DNormal(normals[0], scratchNormal);
isWindingCorrect = checkWindingCorrect(positions[0], positions[1], positions[2], faceNormal);
}

if (vertices.length === 3) {
var index1 = addVertex(vertices[0], positions[0], uvs[0], normals[0]);
var index2 = addVertex(vertices[1], positions[1], uvs[1], normals[1]);
var index3 = addVertex(vertices[2], positions[2], uvs[2], normals[2]);
addTriangle(index1, index2, index3, isWindingCorrect);
} else { // Triangulate if the face is not a triangle
positions3D.length = 0;
vertexIndices.length = 0;

var i;
for (i = 0; i < vertices.length; ++i) {
var index = addVertex(vertices[i], positions[i], uvs[i], normals[i]);
vertexIndices.push(index);

// Collect the vertex positions as 3D points
positions3D.push(get3DPoint(positions[i], new Cartesian3()));
}

var positions2D = projectTo2D(positions3D);

if (isConvex(positions2D)) {
for (i=1; i < vertices.length-1; ++i) {
addTriangle(vertexIndices[0], vertexIndices[i], vertexIndices[i+1], isWindingCorrect);
}
} else {
// Since the projection doesn't preserve winding order, reverse the order of
// the vertices before triangulating to enforce counter clockwise.
var projectedWindingOrder = PolygonPipeline.computeWindingOrder2D(positions2D);
if (projectedWindingOrder === WindingOrder.CLOCKWISE) {
positions2D.reverse();
}

// Use an ear-clipping algorithm to triangulate
var positionIndices = PolygonPipeline.triangulate(positions2D);
for (i = 0; i < positionIndices.length-2; i += 3) {
addTriangle(vertexIndices[positionIndices[i]], vertexIndices[positionIndices[i+1]], vertexIndices[positionIndices[i+2]], isWindingCorrect);
}
}
}
}

Expand Down Expand Up @@ -256,34 +461,29 @@ function loadObj(objPath, options) {
} else if ((result = uvPattern.exec(line)) !== null) {
uvs.push(parseFloat(result[1]));
uvs.push(1.0 - parseFloat(result[2])); // Flip y so 0.0 is the bottom of the image
} else if ((result = facePattern1.exec(line)) !== null) {
addFace(
result[1], result[1], undefined, undefined,
result[2], result[2], undefined, undefined,
result[3], result[3], undefined, undefined,
result[4], result[4], undefined, undefined
);
} else if ((result = facePattern2.exec(line)) !== null) {
addFace(
result[1], result[2], result[3], undefined,
result[4], result[5], result[6], undefined,
result[7], result[8], result[9], undefined,
result[10], result[11], result[12], undefined
);
} else if ((result = facePattern3.exec(line)) !== null) {
addFace(
result[1], result[2], result[3], result[4],
result[5], result[6], result[7], result[8],
result[9], result[10], result[11], result[12],
result[13], result[14], result[15], result[16]
);
} else if ((result = facePattern4.exec(line)) !== null) {
addFace(
result[1], result[2], undefined, result[3],
result[4], result[5], undefined, result[6],
result[7], result[8], undefined, result[9],
result[10], result[11], undefined, result[12]
);
} else { // face line or invalid line
// Because face lines can contain n vertices, we use a line buffer in case the face data spans multiple lines.
// If there's a line continuation don't create face yet
if (line.slice(-1) === '\\') {
lineBuffer += line.substring(0, line.length-1);
return;
}
lineBuffer += line;
if (lineBuffer.substring(0, 2) === 'f ') {
while ( (result = facePattern.exec(lineBuffer)) !== null ) {
faceVertices.push(result[0]);
facePositions.push(result[1]);
faceUvs.push(result[2]);
faceNormals.push(result[3]);
}
addFace(faceVertices, facePositions, faceUvs, faceNormals);

faceVertices.length = 0;
facePositions.length = 0;
faceNormals.length = 0;
faceUvs.length = 0;
}
Copy link
Contributor

@lilleyse lilleyse Jun 26, 2017

Choose a reason for hiding this comment

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

After looking at the multi-line parsing a bit, I think it could be simplified by buffering the string rather than buffering the parsed data. The face parsing code should take a single string regardless of whether it originally spanned multiple lines or not.

Then with some modifications to the first four regexes it should be possible to capture all the attributes like before, with addFace just handling them in a more generic fashion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, that would definitely be neater -- I'll modify this to use line buffering now.

lineBuffer = '';
}
}

Expand Down
14 changes: 6 additions & 8 deletions specs/data/box-triangles/box-triangles.mtl
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
# Blender MTL File: 'box.blend'
# Blender MTL File: 'None'
# Material Count: 1

newmtl Material
Ns 96.078431
newmtl None
Ns 0
Ka 0.000000 0.000000 0.000000
Kd 0.640000 0.640000 0.640000
Ks 0.500000 0.500000 0.500000
Ke 0.000000 0.000000 0.000000
Ni 1.000000
d 1.000000
Kd 0.8 0.8 0.8
Ks 0.8 0.8 0.8
d 1
illum 2