Skip to content

Commit

Permalink
Fix draco decoding vertex colors that are UNSIGNED_BYTE or UNSIGNED_S…
Browse files Browse the repository at this point in the history
…HORT normalized
  • Loading branch information
lilleyse committed Jan 8, 2025
1 parent 268232a commit 99e3a9e
Show file tree
Hide file tree
Showing 11 changed files with 204 additions and 15 deletions.
1 change: 1 addition & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

- Fixed error when resetting `Cesium3DTileset.modelMatrix` to its initial value. [#12409](https://github.com/CesiumGS/cesium/pull/12409)
- Fixed type of `ImageryLayer.fromProviderAsync`, to correctly show that the param `options` is optional. [#12400](https://github.com/CesiumGS/cesium/pull/12400)
- Fixed Draco decoding for vertex colors that are normalized `UNSIGNED_BYTE` or `UNSIGNED_SHORT`.

### 1.125 - 2025-01-02

Expand Down
45 changes: 45 additions & 0 deletions packages/engine/Source/Scene/GltfDracoLoader.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import Check from "../Core/Check.js";
import ComponentDatatype from "../Core/ComponentDatatype.js";
import defaultValue from "../Core/defaultValue.js";
import defined from "../Core/defined.js";
import DracoLoader from "./DracoLoader.js";
import ResourceLoader from "./ResourceLoader.js";
import ResourceLoaderState from "./ResourceLoaderState.js";
import VertexAttributeSemantic from "./VertexAttributeSemantic.js";

/**
* Load a draco buffer from a glTF.
Expand All @@ -18,6 +20,7 @@ import ResourceLoaderState from "./ResourceLoaderState.js";
* @param {object} options Object with the following properties:
* @param {ResourceCache} options.resourceCache The {@link ResourceCache} (to avoid circular dependencies).
* @param {object} options.gltf The glTF JSON.
* @param {object} options.primitive The primitive containing the Draco extension.
* @param {object} options.draco The Draco extension object.
* @param {Resource} options.gltfResource The {@link Resource} containing the glTF.
* @param {Resource} options.baseResource The {@link Resource} that paths in the glTF JSON are relative to.
Expand All @@ -29,6 +32,7 @@ function GltfDracoLoader(options) {
options = defaultValue(options, defaultValue.EMPTY_OBJECT);
const resourceCache = options.resourceCache;
const gltf = options.gltf;
const primitive = options.primitive;
const draco = options.draco;
const gltfResource = options.gltfResource;
const baseResource = options.baseResource;
Expand All @@ -37,6 +41,7 @@ function GltfDracoLoader(options) {
//>>includeStart('debug', pragmas.debug);
Check.typeOf.func("options.resourceCache", resourceCache);
Check.typeOf.object("options.gltf", gltf);
Check.typeOf.object("options.primitive", primitive);
Check.typeOf.object("options.draco", draco);
Check.typeOf.object("options.gltfResource", gltfResource);
Check.typeOf.object("options.baseResource", baseResource);
Expand All @@ -46,6 +51,7 @@ function GltfDracoLoader(options) {
this._gltfResource = gltfResource;
this._baseResource = baseResource;
this._gltf = gltf;
this._primitive = primitive;
this._draco = draco;
this._cacheKey = cacheKey;
this._bufferViewLoader = undefined;
Expand Down Expand Up @@ -169,6 +175,24 @@ async function processDecode(loader, decodePromise) {
}
}

const SemanticToDracoAttributeType = {};
SemanticToDracoAttributeType[VertexAttributeSemantic.POSITION] = "POSITION";
SemanticToDracoAttributeType[VertexAttributeSemantic.NORMAL] = "NORMAL";
SemanticToDracoAttributeType[VertexAttributeSemantic.COLOR] = "COLOR";
SemanticToDracoAttributeType[VertexAttributeSemantic.TEXCOORD] = "TEX_COORD";

function getDracoAttributeType(attribute) {
for (const semantic in SemanticToDracoAttributeType) {
if (SemanticToDracoAttributeType.hasOwnProperty(semantic)) {
if (attribute.startsWith(semantic)) {
return SemanticToDracoAttributeType[semantic];
}
}
}

return undefined;
}

/**
* Processes the resource until it becomes ready.
*
Expand Down Expand Up @@ -203,12 +227,31 @@ GltfDracoLoader.prototype.process = function (frameState) {
}

const draco = this._draco;
const primitive = this._primitive;
const gltf = this._gltf;
const bufferViews = gltf.bufferViews;
const bufferViewId = draco.bufferView;
const bufferView = bufferViews[bufferViewId];
const compressedAttributes = draco.attributes;

// Skip de-quantization transform if present for floating point attributes.
// They will stay quantized in memory and be dequantized in the shader.
const attributesToSkipTransform = [];

for (const attribute in primitive.attributes) {
if (primitive.attributes.hasOwnProperty(attribute)) {
const dracoAttributeType = getDracoAttributeType(attribute);
if (defined(dracoAttributeType)) {
const accessor = gltf.accessors[primitive.attributes[attribute]];
if (accessor.componentType === ComponentDatatype.FLOAT) {
if (!attributesToSkipTransform.includes(dracoAttributeType)) {
attributesToSkipTransform.push(dracoAttributeType);
}
}
}
}
}

const decodeOptions = {
// Need to make a copy of the typed array otherwise the underlying
// ArrayBuffer may be accessed on both the worker and the main thread. This
Expand All @@ -218,6 +261,7 @@ GltfDracoLoader.prototype.process = function (frameState) {
bufferView: bufferView,
compressedAttributes: compressedAttributes,
dequantizeInShader: true,
attributesToSkipTransform: attributesToSkipTransform,
};

const decodePromise = DracoLoader.decodeBufferView(decodeOptions);
Expand All @@ -243,6 +287,7 @@ GltfDracoLoader.prototype.unload = function () {
this._bufferViewTypedArray = undefined;
this._decodedData = undefined;
this._gltf = undefined;
this._primitive = undefined;
};

export default GltfDracoLoader;
5 changes: 5 additions & 0 deletions packages/engine/Source/Scene/GltfIndexBufferLoader.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import ResourceLoaderState from "./ResourceLoaderState.js";
* @param {number} options.accessorId The accessor ID corresponding to the index buffer.
* @param {Resource} options.gltfResource The {@link Resource} containing the glTF.
* @param {Resource} options.baseResource The {@link Resource} that paths in the glTF JSON are relative to.
* @param {object} [options.primitive] The primitive containing the Draco extension.
* @param {object} [options.draco] The Draco extension object.
* @param {string} [options.cacheKey] The cache key of the resource.
* @param {boolean} [options.asynchronous=true] Determines if WebGL resource creation will be spread out over several frames or block until all WebGL resources are created.
Expand All @@ -41,6 +42,7 @@ function GltfIndexBufferLoader(options) {
const accessorId = options.accessorId;
const gltfResource = options.gltfResource;
const baseResource = options.baseResource;
const primitive = options.primitive;
const draco = options.draco;
const cacheKey = options.cacheKey;
const asynchronous = defaultValue(options.asynchronous, true);
Expand Down Expand Up @@ -68,6 +70,7 @@ function GltfIndexBufferLoader(options) {
this._gltf = gltf;
this._accessorId = accessorId;
this._indexDatatype = indexDatatype;
this._primitive = primitive;
this._draco = draco;
this._cacheKey = cacheKey;
this._asynchronous = asynchronous;
Expand Down Expand Up @@ -174,6 +177,7 @@ async function loadFromDraco(indexBufferLoader) {
try {
const dracoLoader = resourceCache.getDracoLoader({
gltf: indexBufferLoader._gltf,
primitive: indexBufferLoader._primitive,
draco: indexBufferLoader._draco,
gltfResource: indexBufferLoader._gltfResource,
baseResource: indexBufferLoader._baseResource,
Expand Down Expand Up @@ -411,6 +415,7 @@ GltfIndexBufferLoader.prototype.unload = function () {
this._typedArray = undefined;
this._buffer = undefined;
this._gltf = undefined;
this._primitive = undefined;
};

export default GltfIndexBufferLoader;
19 changes: 17 additions & 2 deletions packages/engine/Source/Scene/GltfLoader.js
Original file line number Diff line number Diff line change
Expand Up @@ -685,6 +685,7 @@ function getVertexBufferLoader(
loader,
accessorId,
semantic,
primitive,
draco,
loadBuffer,
loadTypedArray,
Expand All @@ -700,6 +701,7 @@ function getVertexBufferLoader(
baseResource: loader._baseResource,
frameState: frameState,
bufferViewId: bufferViewId,
primitive: primitive,
draco: draco,
attributeSemantic: semantic,
accessorId: accessorId,
Expand All @@ -714,6 +716,7 @@ function getVertexBufferLoader(
function getIndexBufferLoader(
loader,
accessorId,
primitive,
draco,
loadBuffer,
loadTypedArray,
Expand All @@ -725,6 +728,7 @@ function getIndexBufferLoader(
gltfResource: loader._gltfResource,
baseResource: loader._baseResource,
frameState: frameState,
primitive: primitive,
draco: draco,
asynchronous: loader._asynchronous,
loadBuffer: loadBuffer,
Expand Down Expand Up @@ -1154,6 +1158,7 @@ function loadAttribute(
loader,
accessorId,
semanticInfo,
primitive,
draco,
loadBuffer,
loadTypedArray,
Expand Down Expand Up @@ -1188,6 +1193,7 @@ function loadAttribute(
loader,
accessorId,
gltfSemantic,
primitive,
draco,
loadBuffer,
loadTypedArray,
Expand Down Expand Up @@ -1232,6 +1238,7 @@ function loadVertexAttribute(
loader,
accessorId,
semanticInfo,
primitive,
draco,
hasInstances,
needsPostProcessing,
Expand Down Expand Up @@ -1278,6 +1285,7 @@ function loadVertexAttribute(
loader,
accessorId,
semanticInfo,
primitive,
draco,
loadBuffer,
loadTypedArray,
Expand Down Expand Up @@ -1345,12 +1353,13 @@ function loadInstancedAttribute(

const loadTypedArray = loadAsTypedArrayOnly || loadTranslationAsTypedArray;

// Don't pass in draco object since instanced attributes can't be draco compressed
// Don't pass in primitive or draco object since instanced attributes can't be draco compressed
return loadAttribute(
loader,
accessorId,
semanticInfo,
undefined,
undefined,
loadBuffer,
loadTypedArray,
frameState,
Expand All @@ -1360,6 +1369,7 @@ function loadInstancedAttribute(
function loadIndices(
loader,
accessorId,
primitive,
draco,
hasFeatureIds,
needsPostProcessing,
Expand Down Expand Up @@ -1403,6 +1413,7 @@ function loadIndices(
const indexBufferLoader = getIndexBufferLoader(
loader,
accessorId,
primitive,
draco,
loadBuffer,
loadTypedArray,
Expand Down Expand Up @@ -1865,7 +1876,8 @@ function loadMorphTarget(
) {
const morphTarget = new MorphTarget();

// Don't pass in draco object since morph targets can't be draco compressed
// Don't pass in primitive or draco object since morph targets can't be draco compressed
const primitive = undefined;
const draco = undefined;
const hasInstances = false;

Expand All @@ -1885,6 +1897,7 @@ function loadMorphTarget(
loader,
accessorId,
semanticInfo,
primitive,
draco,
hasInstances,
needsPostProcessing,
Expand Down Expand Up @@ -1970,6 +1983,7 @@ function loadPrimitive(loader, gltfPrimitive, hasInstances, frameState) {
loader,
accessorId,
semanticInfo,
gltfPrimitive,
draco,
hasInstances,
needsPostProcessing,
Expand Down Expand Up @@ -2002,6 +2016,7 @@ function loadPrimitive(loader, gltfPrimitive, hasInstances, frameState) {
const indicesPlan = loadIndices(
loader,
indices,
gltfPrimitive,
draco,
hasFeatureIds,
needsPostProcessing,
Expand Down
13 changes: 13 additions & 0 deletions packages/engine/Source/Scene/GltfVertexBufferLoader.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import ResourceLoaderState from "./ResourceLoaderState.js";
* @param {Resource} options.gltfResource The {@link Resource} containing the glTF.
* @param {Resource} options.baseResource The {@link Resource} that paths in the glTF JSON are relative to.
* @param {number} [options.bufferViewId] The bufferView ID corresponding to the vertex buffer.
* @param {object} [options.primitive] The primitive containing the Draco extension.
* @param {object} [options.draco] The Draco extension object.
* @param {string} [options.attributeSemantic] The attribute semantic, e.g. POSITION or NORMAL.
* @param {number} [options.accessorId] The accessor id.
Expand All @@ -47,6 +48,7 @@ function GltfVertexBufferLoader(options) {
const gltfResource = options.gltfResource;
const baseResource = options.baseResource;
const bufferViewId = options.bufferViewId;
const primitive = options.primitive;
const draco = options.draco;
const attributeSemantic = options.attributeSemantic;
const accessorId = options.accessorId;
Expand All @@ -67,6 +69,7 @@ function GltfVertexBufferLoader(options) {
}

const hasBufferViewId = defined(bufferViewId);
const hasPrimitive = defined(primitive);
const hasDraco = hasDracoCompression(draco, attributeSemantic);
const hasAttributeSemantic = defined(attributeSemantic);
const hasAccessorId = defined(accessorId);
Expand All @@ -89,7 +92,14 @@ function GltfVertexBufferLoader(options) {
);
}

if (hasDraco && !hasPrimitive) {
throw new DeveloperError(
"When options.draco is defined options.primitive must also be defined.",
);
}

if (hasDraco) {
Check.typeOf.object("options.primitive", primitive);
Check.typeOf.object("options.draco", draco);
Check.typeOf.string("options.attributeSemantic", attributeSemantic);
Check.typeOf.number("options.accessorId", accessorId);
Expand All @@ -101,6 +111,7 @@ function GltfVertexBufferLoader(options) {
this._baseResource = baseResource;
this._gltf = gltf;
this._bufferViewId = bufferViewId;
this._primitive = primitive;
this._draco = draco;
this._attributeSemantic = attributeSemantic;
this._accessorId = accessorId;
Expand Down Expand Up @@ -265,6 +276,7 @@ async function loadFromDraco(vertexBufferLoader) {
try {
const dracoLoader = resourceCache.getDracoLoader({
gltf: vertexBufferLoader._gltf,
primitive: vertexBufferLoader._primitive,
draco: vertexBufferLoader._draco,
gltfResource: vertexBufferLoader._gltfResource,
baseResource: vertexBufferLoader._baseResource,
Expand Down Expand Up @@ -467,6 +479,7 @@ GltfVertexBufferLoader.prototype.unload = function () {
this._typedArray = undefined;
this._buffer = undefined;
this._gltf = undefined;
this._primitive = undefined;
};

export default GltfVertexBufferLoader;
Loading

0 comments on commit 99e3a9e

Please sign in to comment.