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

Fix remove normals #287

Merged
merged 2 commits into from
Jun 6, 2017
Merged

Fix remove normals #287

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
df94719
Fix remove normals
lilleyse Jun 5, 2017
8484f9b
Fix options
lilleyse Jun 6, 2017
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
3 changes: 2 additions & 1 deletion lib/Pipeline.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,13 +107,14 @@ Pipeline.processJSONWithExtras = function(gltfWithExtras, options) {
removeNormals(gltfWithExtras);
}

RemoveUnusedProperties.removeAll(gltfWithExtras);
var smoothNormals = defaultValue(options.smoothNormals, false);
var faceNormals = defaultValue(options.faceNormals, false);
if (smoothNormals || faceNormals) {
generateNormals(gltfWithExtras, options);
}

RemoveUnusedProperties.removeAll(gltfWithExtras);

var mergeVertices = defaultValue(options.mergeVertices, false);
if (!shouldPreserve) {
if (mergeVertices) {
Expand Down
61 changes: 49 additions & 12 deletions lib/removeNormals.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,25 +1,30 @@
'use strict';
var Cesium = require('cesium');
var clone = require('clone');
var addExtensionsUsed = require('./addExtensionsUsed');
var getPrimitiveAttributeSemantics = require('./getPrimitiveAttributeSemantics');
var getUniqueId = require('./getUniqueId');
var processModelMaterialsCommon = require('./processModelMaterialsCommon');

var defaultValue = Cesium.defaultValue;
var defined = Cesium.defined;

module.exports = removeNormals;

/**
* Removes normals from primitives. The technique is stripped off of the primitive's material so that
* it can be regenerated.
* Removes normals from primitives. The material is regenerated to use constant lighting.
*
* The glTF asset must be initialized for the pipeline.
*
* @param {Object} gltf A javascript object containing a glTF asset.
* @param {Object} [options] Options passed to processModelMaterialsCommon.
* @returns {Object} The glTF asset with removed normals.
*
* @see addPipelineExtras
* @see loadGltfUris
*/
function removeNormals(gltf) {
var materials = gltf.materials;
function removeNormals(gltf, options) {
Copy link
Member

Choose a reason for hiding this comment

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

@lilleyse CI is failing because options is unused.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed now.

var generatedMaterials = {};
var meshes = gltf.meshes;
for (var meshId in meshes) {
if (meshes.hasOwnProperty(meshId)) {
Expand All @@ -29,17 +34,14 @@ function removeNormals(gltf) {
for (var i = 0; i < primitivesLength; i++) {
var primitive = primitives[i];
if (removePrimitiveNormals(primitive)) {
var materialId = primitive.material;
if (defined(materialId)) {
var material = materials[materialId];
if (defined(material.technique)) {
delete material.technique;
}
}
generateMaterial(gltf, primitive, generatedMaterials, options);
}
}
}
}
if (Object.keys(generatedMaterials).length > 0) {
processModelMaterialsCommon(gltf, options);
}
}

function removePrimitiveNormals(primitive) {
Expand All @@ -54,4 +56,39 @@ function removePrimitiveNormals(primitive) {
}
}
return removedNormals;
}
}

function generateMaterial(gltf, primitive, generatedMaterials, options) {
gltf.materials = defaultValue(gltf.materials, {});
var materials = gltf.materials;
var materialId = primitive.material;
var material = materials[materialId];

var generatedMaterialId = generatedMaterials[materialId];
if (defined(generatedMaterialId)) {
primitive.material = generatedMaterialId;
return;
}

addExtensionsUsed(gltf, 'KHR_materials_common');
generatedMaterialId = getUniqueId(gltf, materialId);

var values = [];
if (defined(material)) {
values = defaultValue(clone(material.values), []);
}

gltf.materials[generatedMaterialId] = {
extensions : {
KHR_materials_common : {
technique : 'CONSTANT',
values : values,
extras: {
_pipeline: {}
}
}
}
};
primitive.material = generatedMaterialId;
generatedMaterials[materialId] = generatedMaterialId;
}
38 changes: 23 additions & 15 deletions specs/lib/removeNormalsSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,19 @@
'use strict';

var removeNormals = require('../../lib/removeNormals');
var techniqueParameterForSemantic = require('../../lib/techniqueParameterForSemantic');

describe('removeNormals', function() {
it('removes normals and strips the technique from the material', function() {
it('removes normals and generates constant technique', function() {
var gltf = {
asset : {},
meshes : {
mesh: {
primitives: [
{
attributes: {
NORMAL: 'some_accessor'
POSITION : 'position_accessor',
NORMAL: 'normal_accessor'
},
material: 'material'
}
Expand All @@ -27,12 +30,18 @@ describe('removeNormals', function() {
}
};
removeNormals(gltf);
expect(gltf.meshes.mesh.primitives[0].attributes.NORMAL).not.toBeDefined();
expect(gltf.materials.material.technique).not.toBeDefined();
var primitive = gltf.meshes.mesh.primitives[0];
var material = gltf.materials[primitive.material];
var technique = gltf.techniques[material.technique];
expect(primitive.attributes.NORMAL).toBeUndefined();
expect(technique).toBeDefined();
var parameter = techniqueParameterForSemantic(technique, 'NORMAL');
expect(parameter).toBeUndefined(); // Technique should not reference normals
});

it('handles undefined technique', function() {
var gltf = {
asset : {},
meshes : {
mesh: {
primitives: [
Expand All @@ -50,11 +59,12 @@ describe('removeNormals', function() {
}
};
removeNormals(gltf);
expect(gltf.meshes.mesh.primitives[0].attributes.NORMAL).not.toBeDefined();
expect(gltf.meshes.mesh.primitives[0].attributes.NORMAL).toBeUndefined();
});

it('handles undefined material', function() {
var gltf = {
asset : {},
meshes : {
mesh: {
primitives: [
Expand All @@ -68,11 +78,12 @@ describe('removeNormals', function() {
}
};
removeNormals(gltf);
expect(gltf.meshes.mesh.primitives[0].attributes.NORMAL).not.toBeDefined();
expect(gltf.meshes.mesh.primitives[0].attributes.NORMAL).toBeUndefined();
});

it('only deletes normals and handles subscripts', function() {
var gltf = {
asset : {},
meshes : {
mesh: {
primitives: [{
Expand Down Expand Up @@ -100,19 +111,16 @@ describe('removeNormals', function() {
}
},
techniques : {
technique : {}
technique : {},
technique2 : {}
}
};
removeNormals(gltf);
var primitives = gltf.meshes.mesh.primitives;
expect(primitives[0].attributes.NORMAL).not.toBeDefined();
expect(primitives[0].attributes.NORMAL).toBeUndefined();
expect(primitives[0].attributes.POSITION).toBeDefined();
expect(primitives[0].material).toBeDefined();
expect(gltf.materials.material.technique).not.toBeDefined();
expect(primitives[1].attributes.NORMAL).not.toBeDefined();
expect(primitives[1].attributes.NORMAL_0).not.toBeDefined();
expect(primitives[1].attributes.NORMAL).toBeUndefined();
expect(primitives[1].attributes.NORMAL_0).toBeUndefined();
expect(primitives[1].attributes.POSITION).toBeDefined();
expect(primitives[1].material).toBeDefined();
expect(gltf.materials.material2.technique).not.toBeDefined();
});
});
});