-
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
Update sample tilesets to version 1.0 #6857
Changes from all commits
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 |
---|---|---|
|
@@ -21,10 +21,13 @@ define([ | |
'../Core/RuntimeError', | ||
'../Core/Transforms', | ||
'../Core/WebGLConstants', | ||
'../ThirdParty/GltfPipeline/addDefaults', | ||
'../ThirdParty/GltfPipeline/ForEach', | ||
'../ThirdParty/GltfPipeline/getAccessorByteStride', | ||
'../ThirdParty/GltfPipeline/numberOfComponentsForType', | ||
'../ThirdParty/GltfPipeline/parseBinaryGltf', | ||
'../ThirdParty/GltfPipeline/processModelMaterialsCommon', | ||
'../ThirdParty/GltfPipeline/processPbrMetallicRoughness', | ||
'../ThirdParty/when', | ||
'./Axis', | ||
'./ClassificationType', | ||
|
@@ -56,10 +59,13 @@ define([ | |
RuntimeError, | ||
Transforms, | ||
WebGLConstants, | ||
addDefaults, | ||
ForEach, | ||
getAccessorByteStride, | ||
numberOfComponentsForType, | ||
parseBinaryGltf, | ||
processModelMaterialsCommon, | ||
processPbrMetallicRoughness, | ||
when, | ||
Axis, | ||
ClassificationType, | ||
|
@@ -124,7 +130,10 @@ define([ | |
|
||
if (gltf instanceof Uint8Array) { | ||
// Binary glTF | ||
gltf = parseBinaryGltf(gltf); | ||
gltf = parseBinaryGltf(gltf); // Updates to 2.0 and adds pipeline extras | ||
addDefaults(gltf); | ||
processModelMaterialsCommon(gltf); | ||
processPbrMetallicRoughness(gltf); | ||
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. 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. Noted in that PR. |
||
} else { | ||
throw new RuntimeError('Only binary glTF is supported as a classifier.'); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -508,8 +508,12 @@ define([ | |
options = defaultValue(options, {}); | ||
addDefaultsFromTemplate(gltf, gltfTemplate); | ||
addDefaultTransformToAnimatedNodes(gltf); | ||
addDefaultMaterial(gltf); | ||
addDefaultTechnique(gltf); | ||
|
||
if (gltf.asset.extras.gltf_pipeline_upgrade_10to20) { | ||
addDefaultMaterial(gltf); | ||
addDefaultTechnique(gltf); | ||
} | ||
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 is sort of a hack - this tells gltf-pipeline to only add default materials or techniques if the model was a 1.0 model. If the model has This is temporary until #6805. |
||
|
||
addDefaultByteOffsets(gltf); | ||
selectDefaultScene(gltf); | ||
inferBufferViewTargets(gltf); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -191,6 +191,13 @@ define([ | |
} | ||
} | ||
|
||
if (optimizeForCesium) { | ||
var baseColorParameter = defaultValue(techniqueParameters.baseColorTexture, techniqueParameters.baseColorFactor); | ||
if (defined(baseColorParameter)) { | ||
baseColorParameter.semantic = '_3DTILESDIFFUSE'; | ||
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 is required for |
||
} | ||
} | ||
|
||
// Generate uniforms object before attributes are added | ||
var techniqueUniforms = {}; | ||
for (var paramName in techniqueParameters) { | ||
|
This file was deleted.
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.
The regex was too fragile before. It failed for a GLSL line from
processPbrMetallicRoughness
. Updated to support any number of nested parentheses.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.
I didn't see a spec update for this. Can we add a test with the offending line since it sounds like a good test case?
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 code is tested in
Cesium3DTilesetSpec
with thecolorBlendMode
tests. I checked that this area of the code has 100% code coverage (the file is 99%).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.
sets colorBlendMode for textured tileset
passes in master, fails without thereplaceDiffuseTextureCalls
fix, and passes with thereplaceDiffuseTextureCalls
fix. So no update is required.