-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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 oct-encoded normal upsampling. #1961
Changes from 9 commits
a678cd9
a59d9ee
393df80
e670ac7
7129514
93fdefa
95ef5bb
fc60338
fae0d2f
c2d8b85
05058ad
9630474
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,102 @@ | ||
/*global define*/ | ||
define([ | ||
'./Cartesian3', | ||
'./Math', | ||
'./defined', | ||
'./DeveloperError' | ||
], function( | ||
Cartesian3, | ||
CesiumMath, | ||
defined, | ||
DeveloperError) { | ||
"use strict"; | ||
|
||
/** | ||
* Oct encoding and decoding functions. | ||
* | ||
* Oct encoding is a compact representation of unit length vectors. The encoding and decoding functions are low cost, and represent the normalized vector within 1 degree of error. | ||
* The 'oct' encoding is described in "A Survey of Efficient Representations of Independent Unit Vectors", | ||
* Cigolle et al 2014: {@link http://jcgt.org/published/0003/02/01/} | ||
* | ||
* @namespace | ||
* @alias Oct | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this need to be public? Seems like it would be a private class to me. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, seems like it probably should be private. Easy enough to make it public in the future if there's a need for it. |
||
*/ | ||
var Oct = {}; | ||
|
||
/** | ||
* Encodes a normalized vector into 2 SNORM values in the range of [0-255] following the 'oct' encoding. | ||
* | ||
* @param {Cartesian3} vector The normalized vector to be compressed into 2 byte 'oct' encoding. | ||
* @param {Cartesian2} result The 2 byte oct-encoded unit length vector. | ||
* @returns {Cartesian2} The 2 byte oct-encoded unit length vector. | ||
* | ||
* @exception {DeveloperError} vector must be defined. | ||
* @exception {DeveloperError} result must be defined. | ||
* @exception {DeveloperError} vector must be normalized. | ||
*/ | ||
Oct.encode = function(vector, result) { | ||
//>>includeStart('debug', pragmas.debug); | ||
if (!defined(vector)) { | ||
throw new DeveloperError('vector is required.'); | ||
} | ||
if (!defined(result)) { | ||
throw new DeveloperError('result is required.'); | ||
} | ||
var magSquared = Cartesian3.magnitudeSquared(vector); | ||
if (Math.abs(magSquared - 1.0) > CesiumMath.EPSILON6) { | ||
throw new DeveloperError('vector must be normalized.'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be documented. |
||
} | ||
//>>includeEnd('debug'); | ||
|
||
result.x = vector.x / (Math.abs(vector.x) + Math.abs(vector.y) + Math.abs(vector.z)); | ||
result.y = vector.y / (Math.abs(vector.x) + Math.abs(vector.y) + Math.abs(vector.z)); | ||
if (vector.z < 0) { | ||
var x = result.x; | ||
var y = result.y; | ||
result.x = (1.0 - Math.abs(y)) * CesiumMath.signNotZero(x); | ||
result.y = (1.0 - Math.abs(x)) * CesiumMath.signNotZero(y); | ||
} | ||
|
||
result.x = CesiumMath.toSNorm(result.x); | ||
result.y = CesiumMath.toSNorm(result.y); | ||
|
||
return result; | ||
}; | ||
|
||
/** | ||
* Decodes a unit-length vector in 'oct' encoding to a normalized 3-component vector. | ||
* | ||
* @param {Number} x The x component of the oct-encoded unit length vector. | ||
* @param {Number} y The y component of the oct-encoded unit length vector. | ||
* @param {Cartesian3} result The decoded and normalized vector | ||
* @returns {Cartesian3} The decoded and normalized vector. | ||
* | ||
* @exception {DeveloperError} result must be defined. | ||
* @exception {DeveloperError} x and y must be a signed normalized integer between 0 and 255. | ||
*/ | ||
Oct.decode = function(x, y, result) { | ||
//>>includeStart('debug', pragmas.debug); | ||
if (!defined(result)) { | ||
throw new DeveloperError('result is required.'); | ||
} | ||
if (x < 0 || x > 255 || y < 0 || y > 255) { | ||
throw new DeveloperError('x and y must be a signed normalized integer between 0 and 255'); | ||
} | ||
//>>includeEnd('debug'); | ||
|
||
result.x = CesiumMath.fromSNorm(x); | ||
result.y = CesiumMath.fromSNorm(y); | ||
result.z = 1.0 - (Math.abs(result.x) + Math.abs(result.y)); | ||
|
||
if (result.z < 0.0) | ||
{ | ||
var oldVX = result.x; | ||
result.x = (1.0 - Math.abs(result.y)) * CesiumMath.signNotZero(oldVX); | ||
result.y = (1.0 - Math.abs(oldVX)) * CesiumMath.signNotZero(result.y); | ||
} | ||
|
||
return Cartesian3.normalize(result, result); | ||
}; | ||
|
||
return Oct; | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,23 +1,27 @@ | ||
/*global define*/ | ||
define([ | ||
'../Core/BoundingSphere', | ||
'../Core/Cartesian2', | ||
'../Core/Cartesian3', | ||
'../Core/Cartographic', | ||
'../Core/defined', | ||
'../Core/Ellipsoid', | ||
'../Core/EllipsoidalOccluder', | ||
'../Core/Intersections2D', | ||
'../Core/Math', | ||
'../Core/Oct', | ||
'./createTaskProcessorWorker' | ||
], function( | ||
BoundingSphere, | ||
Cartesian2, | ||
Cartesian3, | ||
Cartographic, | ||
defined, | ||
Ellipsoid, | ||
EllipsoidalOccluder, | ||
Intersections2D, | ||
CesiumMath, | ||
Oct, | ||
createTaskProcessorWorker) { | ||
"use strict"; | ||
|
||
|
@@ -372,18 +376,46 @@ define([ | |
return CesiumMath.lerp(this.first.getV(), this.second.getV(), this.ratio); | ||
}; | ||
|
||
var encodedScratch = new Cartesian2(); | ||
// An upsampled triangle may be clipped twice before it is assigned an index | ||
// In this case, we need a buffer to handle the recursion of getNormalX() and getNormalY(). | ||
var depth = -1; | ||
var cartesianScratch1 = [new Cartesian3(), new Cartesian3()]; | ||
var cartesianScratch2 = [new Cartesian3(), new Cartesian3()]; | ||
function lerpOctEncodedNormal(vertex, result) { | ||
depth += 1; | ||
|
||
var first = cartesianScratch1[depth]; | ||
var second = cartesianScratch2[depth]; | ||
|
||
first = Oct.decode(vertex.first.getNormalX(), vertex.first.getNormalY(), first); | ||
second = Oct.decode(vertex.second.getNormalX(), vertex.second.getNormalY(), second); | ||
cartesian3Scratch = Cartesian3.lerp(first, second, vertex.ratio, cartesian3Scratch); | ||
Cartesian3.normalize(cartesian3Scratch, cartesian3Scratch); | ||
|
||
Oct.encode(cartesian3Scratch, result); | ||
|
||
depth -= 1; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nitpicky, but why not |
||
|
||
return result; | ||
} | ||
|
||
Vertex.prototype.getNormalX = function() { | ||
if (defined(this.index)) { | ||
return this.normalBuffer[this.index * 2]; | ||
} | ||
return Math.round(CesiumMath.lerp(this.first.getNormalX(), this.second.getNormalX(), this.ratio)); | ||
|
||
encodedScratch = lerpOctEncodedNormal(this, encodedScratch); | ||
return encodedScratch.x; | ||
}; | ||
|
||
Vertex.prototype.getNormalY = function() { | ||
if (defined(this.index)) { | ||
return this.normalBuffer[this.index * 2 + 1]; | ||
} | ||
return Math.round(CesiumMath.lerp(this.first.getNormalY(), this.second.getNormalY(), this.ratio)); | ||
|
||
encodedScratch = lerpOctEncodedNormal(this, encodedScratch); | ||
return encodedScratch.y; | ||
}; | ||
|
||
var polygonVertices = []; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,101 @@ | ||
/*global defineSuite*/ | ||
defineSuite([ | ||
'Core/Oct', | ||
'Core/Cartesian2', | ||
'Core/Cartesian3' | ||
], function( | ||
Oct, | ||
Cartesian2, | ||
Cartesian3) { | ||
"use strict"; | ||
/*global jasmine,describe,xdescribe,it,xit,expect,beforeEach,afterEach,beforeAll,afterAll,spyOn,runs,waits,waitsFor*/ | ||
|
||
var negativeUnitZ = new Cartesian3(0.0, 0.0, -1.0); | ||
it('oct decode(0, 0)', function() { | ||
var result = new Cartesian3(); | ||
Oct.decode(0, 0, result); | ||
expect(result).toEqual(negativeUnitZ); | ||
}); | ||
|
||
it('oct encode(0, 0, -1)', function() { | ||
var result = new Cartesian2(); | ||
Oct.encode(negativeUnitZ, result); | ||
expect(result).toEqual(new Cartesian2(255, 255)); | ||
}); | ||
|
||
it('oct encode(0, 0, 1)', function() { | ||
var result = new Cartesian2(); | ||
Oct.encode(Cartesian3.UNIT_Z, result); | ||
expect(result).toEqual(new Cartesian2(128, 128)); | ||
}); | ||
|
||
it('oct extents are equal', function() { | ||
var result = new Cartesian3(); | ||
// lower left | ||
Oct.decode(0, 0, result); | ||
expect(result).toEqual(negativeUnitZ); | ||
// lower right | ||
Oct.decode(255, 0, result); | ||
expect(result).toEqual(negativeUnitZ); | ||
// upper right | ||
Oct.decode(255, 255, result); | ||
expect(result).toEqual(negativeUnitZ); | ||
// upper left | ||
Oct.decode(255, 0, result); | ||
expect(result).toEqual(negativeUnitZ); | ||
}); | ||
|
||
it('throws oct encode vector undefined', function() { | ||
var vector; | ||
var result = new Cartesian3(); | ||
expect(function() { | ||
Oct.encode(vector, result); | ||
}).toThrowDeveloperError(); | ||
}); | ||
|
||
it('throws oct encode result undefined', function() { | ||
var result; | ||
expect(function() { | ||
Oct.encode(Cartesian3.UNIT_Z, result); | ||
}).toThrowDeveloperError(); | ||
}); | ||
|
||
it('throws oct encode non unit vector', function() { | ||
var nonUnitLengthVector = new Cartesian3(2.0, 0.0, 0.0); | ||
var result = new Cartesian2(); | ||
expect(function() { | ||
Oct.encode(nonUnitLengthVector, result); | ||
}).toThrowDeveloperError(); | ||
}); | ||
|
||
it('throws oct encode zero length vector', function() { | ||
var result = new Cartesian2(); | ||
expect(function() { | ||
Oct.encode(Cartesian3.ZERO, result); | ||
}).toThrowDeveloperError(); | ||
}); | ||
|
||
it('throws oct decode result undefined', function() { | ||
var result; | ||
expect(function() { | ||
Oct.decode(Cartesian2.ZERO, result); | ||
}).toThrowDeveloperError(); | ||
}); | ||
|
||
it('throws oct decode x out of bounds', function() { | ||
var result = new Cartesian3(); | ||
var invalidSNorm = new Cartesian2(256, 0); | ||
expect(function() { | ||
Oct.decode(invalidSNorm, result); | ||
}).toThrowDeveloperError(); | ||
}); | ||
|
||
it('throws oct decode y out of bounds', function() { | ||
var result = new Cartesian3(); | ||
var invalidSNorm = new Cartesian2(0, 256); | ||
expect(function() { | ||
Oct.decode(invalidSNorm, result); | ||
}).toThrowDeveloperError(); | ||
}); | ||
|
||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs to go away now that
Oct
is private.