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.

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 Draco decoding for vertex colors that are normalized UNSIGNED_BYTE or UNSIGNED_SHORT #12417

Merged
merged 3 commits into from
Jan 13, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
- Fixed error when resetting `Cesium3DTileset.modelMatrix` to its initial value. [#12409](https://github.com/CesiumGS/cesium/pull/12409)
- Fixed the parameter types of the `ClippingPolygon.equals` function, and fixed cases where parameters to `equals` functions had erroneously not been marked as 'optional'. [#12394](https://github.com/CesiumGS/cesium/pull/12394)
- 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`. [#12417](https://github.com/CesiumGS/cesium/pull/12417)

### 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
Loading