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

KHR_materials_common and KHR_binary_glTF compatibility #3294

Closed
wants to merge 19 commits into from
Closed

KHR_materials_common and KHR_binary_glTF compatibility #3294

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
fdaec2c
Some adaptions to match KHR_materials_common spec
Dec 4, 2015
ee5362e
gitignore update for Webstorm
Dec 4, 2015
45dfb73
Alignment of binary body changed according to binary glTF spec
Dec 4, 2015
776b2ab
Removed special handling of non-4-byte-aligned binary bodies (should …
Dec 8, 2015
948b1ed
Changed binary buffer name to binary_glTF (like in the spec)
Dec 8, 2015
c566172
Adapted interpretation of shininess value according to KHR_materials_…
Dec 8, 2015
503f087
Backwards compatibility with Cesium 1.15
Dec 9, 2015
09a7fae
Update CONTRIBUTORS.md
Dec 14, 2015
164bca9
Rollback of dynamic shininess handling (will be changed in spec)
Dec 17, 2015
b25fea7
Merge branch 'KHR_common_materials-compatibility' of https://github.c…
Dec 17, 2015
15cbbef
Fixed merge conflict, added reference to dragon glb (only for testing)
Dec 28, 2015
61677e6
Removed references to temporary test models, added compatibility with…
Dec 28, 2015
3f962e0
reset content of .gitignore to master
Dec 28, 2015
3dfe370
Reset content of .idea directory to master
Dec 28, 2015
93c7b64
Reset gallery 3D models example to master
Dec 28, 2015
6709e57
Added comment on boolean parameters
Dec 28, 2015
b79010f
Used cesium defined function, adapted comment about KHR_binary_glTF
Jan 4, 2016
9d34c12
Fixed merge conflict in CONTRIBUTORS.md
Jan 11, 2016
1d22e74
Updated change log
Jan 11, 2016
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: 5 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,8 @@ npm-debug.log
# WebStorm user-specific
.idea/workspace.xml
.idea/tasks.xml
.idea/cesium.iml
.idea/encodings.xml
.idea/inspectionProfiles/Project_Default.xml
.idea/jsLinters/jshint.xml
.idea/misc.xml
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we want to ignore any of these files (and they are also already committed in master, so ignoring them would product odd behavior). What is your reasoning for adding them here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the hint!
My Webstorm seemed to have changed some of them automatically, and I didn't want to commit those changes (but then I don't need to add the changes to .gitignore, of course). I wanted to revert this inside the pull request, but didn't do it so far.

Copy link
Contributor

Choose a reason for hiding this comment

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

No problem, I'm not WebStorm expert so I wanted to make sure we didn't do something non-standard on our side. Most of us our running WebStorm 11, so if you're on an earlier version, that could be the source of it changing files locally. Out of curiosity, what did it change? Maybe we can prevent that from happening with a different config tweak.

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 am using an older WebStorm version - that's probably the problem.

Here's a diff:

diff --git a/.idea/cesium.iml b/.idea/cesium.iml
index 9d1caef..1f7737b 100644
--- a/.idea/cesium.iml
+++ b/.idea/cesium.iml
@@ -3,6 +3,7 @@
   <component name="NewModuleRootManager">
     <content url="file://$MODULE_DIR$">
       <sourceFolder url="file://$MODULE_DIR$/Specs" isTestSource="true" />
+      <excludeFolder url="file://$MODULE_DIR$/Apps/SampleData/models/BloodhoundSSC" />
       <excludeFolder url="file://$MODULE_DIR$/Build" />
       <excludeFolder url="file://$MODULE_DIR$/Instrumented" />
     </content>
diff --git a/.idea/encodings.xml b/.idea/encodings.xml
index 97626ba..f758959 100644
--- a/.idea/encodings.xml
+++ b/.idea/encodings.xml
@@ -1,6 +1,6 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <project version="4">
-  <component name="Encoding">
+  <component name="Encoding" useUTFGuessing="true" native2AsciiForPropertiesFiles="false">
     <file url="PROJECT" charset="UTF-8" />
   </component>
 </project>
\ No newline at end of file
diff --git a/.idea/inspectionProfiles/Project_Default.xml b/.idea/inspectionProfiles/Project_Default.xml
index eff7139..76d98c8 100644
--- a/.idea/inspectionProfiles/Project_Default.xml
+++ b/.idea/inspectionProfiles/Project_Default.xml
@@ -1,6 +1,7 @@
 <component name="InspectionProjectProfileManager">
-  <profile version="1.0">
+  <profile version="1.0" is_locked="false">
     <option name="myName" value="Project Default" />
+    <option name="myLocal" value="true" />
     <inspection_tool class="JSHint" enabled="true" level="ERROR" enabled_by_default="true" />
   </profile>
 </component>
\ No newline at end of file
diff --git a/.idea/jsLinters/jshint.xml b/.idea/jsLinters/jshint.xml
index aed467c..bf9607e 100644
--- a/.idea/jsLinters/jshint.xml
+++ b/.idea/jsLinters/jshint.xml
@@ -5,31 +5,25 @@
     <option bitwise="true" />
     <option boss="false" />
     <option browser="true" />
-    <option browserify="false" />
     <option camelcase="false" />
     <option couch="false" />
     <option curly="true" />
     <option debug="false" />
     <option devel="false" />
     <option dojo="false" />
-    <option elision="false" />
-    <option enforceall="false" />
     <option eqeqeq="true" />
     <option eqnull="false" />
     <option es3="false" />
-    <option es5="false" />
     <option esnext="false" />
     <option evil="false" />
     <option expr="false" />
     <option forin="true" />
     <option freeze="false" />
     <option funcscope="false" />
-    <option futurehostile="false" />
     <option gcl="false" />
     <option globalstrict="false" />
     <option immed="false" />
     <option iterator="false" />
-    <option jasmine="false" />
     <option jquery="false" />
     <option lastsemic="false" />
     <option latedef="false" />
@@ -37,13 +31,11 @@
     <option laxcomma="false" />
     <option loopfunc="false" />
     <option maxerr="50" />
-    <option mocha="false" />
     <option mootools="false" />
     <option moz="false" />
     <option multistr="false" />
     <option newcap="false" />
     <option noarg="true" />
-    <option nocomma="false" />
     <option node="false" />
     <option noempty="true" />
     <option nomen="false" />
@@ -58,24 +50,19 @@
     <option plusplus="false" />
     <option proto="false" />
     <option prototypejs="false" />
-    <option qunit="false" />
     <option quotmark="false" />
     <option rhino="false" />
     <option scripturl="false" />
     <option shadow="false" />
-    <option shelljs="false" />
-    <option singleGroups="false" />
     <option smarttabs="false" />
     <option strict="true" />
     <option sub="false" />
     <option supernew="false" />
     <option trailing="false" />
-    <option typed="false" />
     <option undef="true" />
     <option unused="false" />
     <option validthis="false" />
     <option white="false" />
-    <option withstmt="false" />
     <option worker="false" />
     <option wsh="false" />
     <option yui="false" />
diff --git a/.idea/misc.xml b/.idea/misc.xml
index 72abef0..19f74da 100644
--- a/.idea/misc.xml
+++ b/.idea/misc.xml
@@ -10,4 +10,5 @@
     <ConfirmationsSetting value="0" id="Add" />
     <ConfirmationsSetting value="0" id="Remove" />
   </component>
+  <component name="ProjectRootManager" version="2" />
 </project>
\ No newline at end of file

6 changes: 3 additions & 3 deletions Source/Scene/Model.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ define([
var result = parseBinaryGltfHeader(gltf);

// CESIUM_binary_glTF is from the beginning of the file but
// KHR_binary_glTF is from the beginning of the binary section
// binary_glTF is from the beginning of the binary section
Copy link

Choose a reason for hiding this comment

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

Technically the extension is still called KHR_binary_glTF so there's no reason to change these comments.

if (result.binaryOffset !== 0) {
gltf = gltf.subarray(result.binaryOffset);
}
Expand Down Expand Up @@ -975,7 +975,7 @@ define([
// Load binary glTF
var result = parseBinaryGltfHeader(array);
// CESIUM_binary_glTF is from the beginning of the file but
// KHR_binary_glTF is from the beginning of the binary section
// binary_glTF is from the beginning of the binary section
if (result.binaryOffset !== 0) {
array = array.subarray(result.binaryOffset);
}
Expand Down Expand Up @@ -1148,7 +1148,7 @@ define([
if (buffers.hasOwnProperty(id)) {
var buffer = buffers[id];

if (id === 'CESIUM_binary_glTF' || id === 'KHR_binary_glTF') {
if (id === 'CESIUM_binary_glTF' || id === 'binary_glTF') {
// Buffer is the binary glTF file itself that is already loaded
var loadResources = model._loadResources;
loadResources.buffers[id] = model._cachedGltf.bgltf;
Expand Down
38 changes: 29 additions & 9 deletions Source/Scene/modelMaterialsCommon.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,14 +223,17 @@ define([
var lowerCase;
var hasTexCoords = false;
for(var name in parameterValues) {
if (parameterValues.hasOwnProperty(name)) {
var value = parameterValues[name];
//generate shader parameters for KHR_materials_common attributes
//(including a check, because some boolean flags should not be used as shader parameters)
if (parameterValues.hasOwnProperty(name) &&
name !== "transparent" && name !== "doubleSided") {
var valType = getKHRMaterialsCommonValueType(name);
lowerCase = name.toLowerCase();
if (!hasTexCoords && (value.type === WebGLConstants.SAMPLER_2D)) {
if (!hasTexCoords && (valType === WebGLConstants.SAMPLER_2D)) {
hasTexCoords = true;
}
techniqueParameters[lowerCase] = {
type: value.type
type: valType
};
}
}
Expand Down Expand Up @@ -609,18 +612,35 @@ define([
return techniqueId;
}

function getKHRMaterialsCommonValueType(paramName)
{
switch (paramName)
{
case "ambient" : return WebGLConstants.FLOAT_VEC4;
case "diffuse" : return WebGLConstants.FLOAT_VEC4;
case "doubleSided" : return WebGLConstants.BOOL;
case "emission" : return WebGLConstants.FLOAT_VEC4;
case "specular" : return WebGLConstants.FLOAT_VEC4;
case "shininess" : return WebGLConstants.FLOAT;
case "transparency" : return WebGLConstants.FLOAT;
case "transparent" : return WebGLConstants.BOOL;
}
}

function getTechniqueKey(khrMaterialsCommon) {
var techniqueKey = '';
techniqueKey += 'technique:' + khrMaterialsCommon.technique + ';';
techniqueKey += 'technique:' + khrMaterialsCommon.technique + ';';

var values = khrMaterialsCommon.values;
var keys = Object.keys(values).sort();
var keysCount = keys.length;
for (var i=0;i<keysCount;++i) {
var name = keys[i];
if (values.hasOwnProperty(name)) {
var value = values[name];
techniqueKey += name + ':' + value.type.toString();
//generate first part of key using shader parameters for KHR_materials_common attributes
//(including a check, because some boolean flags should not be used as shader parameters)
if (values.hasOwnProperty(name) &&
name !== "transparent" && name !== "doubleSided") {
techniqueKey += name + ':' + getKHRMaterialsCommonValueType(name).toString();
techniqueKey += ';';
}
}
Expand Down Expand Up @@ -692,7 +712,7 @@ define([
for (var valueName in values) {
if (values.hasOwnProperty(valueName)) {
var value = values[valueName];
material.values[valueName] = value.value;
material.values[valueName] = value;
}
}

Expand Down