Skip to content

Commit

Permalink
gltf: Extension handling cleanup
Browse files Browse the repository at this point in the history
  • Loading branch information
Ib Green committed Sep 26, 2019
1 parent 1a58738 commit 90e8a0d
Show file tree
Hide file tree
Showing 11 changed files with 114 additions and 103 deletions.
4 changes: 2 additions & 2 deletions dev-docs/RFCs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,12 @@ Writeups of directions in major areas of interest
| RFC | Author | Status | Description |
| -------------------------------------------------------------------- | -------- | --------- | --------------------------------------------- |
| [**Streaming JSON Loader**](v2.0/json-loader-rfc.md) | @ibgreen | **Draft** | Binary, streaming, worker-enabled JSON loader |
| [**Loader Options**](v2.0/loader-options-rfc.md) | @ibgreen | **Draft** | Nested options scheme for loaders |
| [**Loader Options**](v2.0/loader-options-rfc.md) | @ibgreen | **Draft** | Nested options scheme for loaders |
| [**Fetch Options**](v2.0/fetch-option-rfc.md) | @ibgreen | **Draft** | Options for `fetch` |
| [**JSON**](v2.0/json-support-rfc.md) | @ibgreen | **Draft** | Core support for JSON formats |
| [**Loader Auto-Detection**](v2.0/loader-auto-detection-rfc.md) | @ibgreen | **Draft** | Improved support for loader auto detection |
| [**Loader Auto-Registration**](v2.0/loader-auto-registration-rfc.md) | @ibgreen | **Draft** | Loader auto registration at import |
| [**Loader Lookup By Name**](v2.0/loader-lookup-by-name-rfc.md) | @ibgreen | **Draft** | Loader lookup among pre-registered loaders |
| [**Loader Lookup By Name**](v2.0/loader-lookup-by-name-rfc.md) | @ibgreen | **Draft** | Loader lookup among pre-registered loaders |

## v1.0 RFCs

Expand Down
2 changes: 0 additions & 2 deletions dev-docs/RFCs/v2.0/fetch-option-rfc.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,12 @@ However, problems remain:
- Some loaders (e.g. gltf) load additional files, currently they provide ad-hoc solutions for the "recursive" fetch operations.
- (Update: This will no longer apply in v2) Some loaders (mainly image loaders) only support `load`. They do not support separate parsing of data, but instead load and parse in a single operation.


### Future considerations:

- File system support (separate RFC) - Some loaders can generate virtual file systems (zip files, a list of dropped files in the browser, a dropbox loader) where files can be loaded with local names from a non-URL source. Overridable `fetch` in `load` and `parse` could be extended to support this.

## Proposals


## Proposal 1a: Allow fetch to be completely overridden

```js
Expand Down
1 change: 1 addition & 0 deletions dev-docs/RFCs/v2.0/loader-auto-detection-rfc.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ Successful auto detection of loaders depends on multiple contextual pieces of in
When autoselecting a loader, the `parse` (and sometimes `load`) functions should be supplied with contextual information.

Depending on how `parse` is called, contextual information may be available:

- The standard `fetch` `Response` object allows `url` and `headers` (in which MIME type might be present) to be queried.
- The loaders.gl `fetchFile` function returns `fetch` `Response` objects for `File` and `Blob` objects, providing a uniform interface to such classes.

Expand Down
6 changes: 2 additions & 4 deletions dev-docs/RFCs/v2.0/loader-auto-registration-rfc.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
This RFC proposes that importing loaders.gl loader modules autoregisters their loaders, removing the need to also import and call `registerLoaders`.

## Review Notes
- Sep 2019 - the `CompositeLoader` system is somewhat reducing the need to pre-register loaders.

- Sep 2019 - the `CompositeLoader` system is somewhat reducing the need to pre-register loaders.

## Motivation

Expand Down Expand Up @@ -64,12 +64,11 @@ import '@loaders.gl/draco'; // Simply importing it makes this loader available
new ScenegraphLayer({scenegraph: DRACO_COMPRESSED_GLTF_URL});
```


## Open Issues

While loader "pre-registration" would be trivial to implement, there are some concerns:

- Tree-shaking: A loader module often exports multiple loaders. By auto registering all of them, we defeat tree-shaking (mitigated by the fact that we are already publishing loaders a-la-carte).
- Tree-shaking: A loader module often exports multiple loaders. By auto registering all of them, we defeat tree-shaking (mitigated by the fact that we are already publishing loaders a-la-carte).
- Which loader(s) to register: The default main-thread loader, the streaming loader, or the worker thread loader? All of them? Create separate entry-points for each of them?

```js
Expand All @@ -83,4 +82,3 @@ This would require solving non-trivial problems in ocular-dev-tools (which would
- Increased dependency: Currently simple loader modules can be written without importing any loaders.gl helper libraries (e.g. `@loaders.gl/loader-utils`). If the loader modules have to import `registerLoaders` that changes. This is a design simplicity/elegance in loaders.gl that matters to some people, that would be lost for this convenience.
- Manual registration code? - To avoid having to import `@loaders.gl/loader-utils` in each loader module, we could just ask each loader to push their loaders to a global array. But even then, the global scope must be determined, if not by helper functions in loaders.gl, then by some code that needs to be copied into each loader.
- Conceptually, global mechanisms should normally be minimized.

5 changes: 5 additions & 0 deletions modules/gltf/docs/api-reference/gltf-extensions.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@ loaders.gl aims to provide support for glTF extensions that can be handled compl

Note that many glTF extensions affect aspects that are firmly outside of the scope of loaders.gl (e.g. rendering), and no attempt is made to process those extensions in loaders.gl.

| Extension | Description |
| -------------------------------------------------------------------------------------------------------------------------------- | ----------- |
| [KHR_draco_mesh_compression](https://github.com/KhronosGroup/glTF/tree/master/extensions/2.0/Khronos/KHR_draco_mesh_compression) | |
| [KHR_lights_punctual](https://github.com/KhronosGroup/glTF/tree/master/extensions/2.0/Khronos/KHR_lights_punctual) | |

## Official Extensions

### KHR_draco_mesh_compression
Expand Down
57 changes: 28 additions & 29 deletions modules/gltf/src/lib/extensions/KHR_draco_mesh_compression.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,45 +5,40 @@
import GLTFScenegraph from '../gltf-scenegraph';
import {KHR_DRACO_MESH_COMPRESSION} from '../gltf-constants';
import {getGLTFAccessors, getGLTFAccessor} from '../gltf-utils/gltf-attribute-utils';
export default class KHR_draco_mesh_compression {
static get name() {
return KHR_DRACO_MESH_COMPRESSION;
}

// Note: We have a "soft dependency" on Draco to avoid bundling it when not needed
static async decode(gltfData, options, context) {
if (!options.gltf.decompress) {
return;
}
// Note: We have a "soft dependency" on Draco to avoid bundling it when not needed
export async function decode(gltfData, options, context) {
if (!options.gltf.decompressMeshes) {
return;
}

const scenegraph = new GLTFScenegraph(gltfData);
const promises = [];
for (const primitive of meshPrimitiveIterator(scenegraph)) {
if (scenegraph.getObjectExtension(primitive, KHR_DRACO_MESH_COMPRESSION)) {
promises.push(decompressPrimitive(primitive, scenegraph, options, context));
}
const scenegraph = new GLTFScenegraph(gltfData);
const promises = [];
for (const primitive of meshPrimitiveIterator(scenegraph)) {
if (scenegraph.getObjectExtension(primitive, KHR_DRACO_MESH_COMPRESSION)) {
promises.push(decompressPrimitive(primitive, scenegraph, options, context));
}
}

// Decompress meshes in parallel
await Promise.all(promises);
// Decompress meshes in parallel
await Promise.all(promises);

// We have now decompressed all primitives, so remove the top-level extensions
scenegraph.removeExtension(KHR_DRACO_MESH_COMPRESSION);
}
// We have now decompressed all primitives, so remove the top-level extensions
scenegraph.removeExtension(KHR_DRACO_MESH_COMPRESSION);
}

static encode(gltfData, options = {}) {
const scenegraph = new GLTFScenegraph(gltfData);
export function encode(gltfData, options = {}) {
const scenegraph = new GLTFScenegraph(gltfData);

for (const mesh of scenegraph.json.meshes || []) {
// eslint-disable-next-line camelcase
compressMesh(mesh, options);
// NOTE: Only add the extension if something was actually compressed
scenegraph.addRequiredExtension(KHR_DRACO_MESH_COMPRESSION);
}
for (const mesh of scenegraph.json.meshes || []) {
// eslint-disable-next-line camelcase
compressMesh(mesh, options);
// NOTE: Only add the extension if something was actually compressed
scenegraph.addRequiredExtension(KHR_DRACO_MESH_COMPRESSION);
}
}

// PRIVATE
// DECODE

// Unpacks one mesh primitive and removes the extension from the primitive
// DracoDecoder needs to be imported and registered by app
Expand Down Expand Up @@ -73,6 +68,8 @@ async function decompressPrimitive(primitive, scenegraph, options, context) {
checkPrimitive(primitive);
}

// ENCODE

// eslint-disable-next-line max-len
// Only TRIANGLES: 0x0004 and TRIANGLE_STRIP: 0x0005 are supported
function compressMesh(attributes, indices, mode = 4, options, context) {
Expand Down Expand Up @@ -112,6 +109,8 @@ function compressMesh(attributes, indices, mode = 4, options, context) {
return glTFMesh;
}

// UTILS

function checkPrimitive(primitive) {
if (!primitive.attributes && Object.keys(primitive.attributes).length > 0) {
throw new Error('Empty glTF primitive detected: Draco decompression failure?');
Expand Down
79 changes: 37 additions & 42 deletions modules/gltf/src/lib/extensions/KHR_lights_punctual.js
Original file line number Diff line number Diff line change
@@ -1,56 +1,51 @@
// GLTF EXTENSION: KHR_lights_punctual
// https://github.com/KhronosGroup/glTF/tree/master/extensions/2.0/Khronos/KHR_lights_punctual

import GLTFScenegraph from '../gltf-scenegraph';
import {KHR_LIGHTS_PUNCTUAL} from '../gltf-constants';
import assert from '../utils/assert';

// GLTF EXTENSION: KHR_lights_punctual
// https://github.com/KhronosGroup/glTF/tree/master/extensions/2.0/Khronos/KHR_lights_punctual
// eslint-disable-next-line camelcase
export default class KHR_lights_punctual {
static get name() {
return KHR_LIGHTS_PUNCTUAL;
}

static decode(gltfData, options) {
const gltfScenegraph = new GLTFScenegraph(gltfData);
export function decode(gltfData, options) {
const gltfScenegraph = new GLTFScenegraph(gltfData);
const {json} = gltfScenegraph;

// Move the light array out of the extension and remove the extension
const extension = gltfScenegraph.getExtension(KHR_LIGHTS_PUNCTUAL);
if (extension) {
gltfScenegraph.json.lights = extension.lights;
gltfScenegraph.removeExtension(KHR_LIGHTS_PUNCTUAL);
}
// Move the light array out of the extension and remove the extension
const extension = gltfScenegraph.getExtension(KHR_LIGHTS_PUNCTUAL);
if (extension) {
gltfScenegraph.json.lights = extension.lights;
gltfScenegraph.removeExtension(KHR_LIGHTS_PUNCTUAL);
}

// Any nodes that have the extension, add lights field pointing to light object
// and remove the extension
for (const node of gltfScenegraph.nodes || []) {
const nodeExtension = node.extensions && node.extensions.KHR_lights_punctual;
if (nodeExtension) {
node.light = gltfScenegraph._get('lights', nodeExtension.light);
delete node.extensions.KHR_lights_punctual;
}
// Any nodes that have the extension, add lights field pointing to light object
// and remove the extension
for (const node of json.nodes || []) {
const nodeExtension = gltfScenegraph.getObjectExtension(node, KHR_LIGHTS_PUNCTUAL);
if (nodeExtension) {
node.light = nodeExtension.light;
}
gltfScenegraph.removeObjectExtension(node, KHR_LIGHTS_PUNCTUAL);
}
}

// Move the light ar ray out of the extension and remove the extension
static encode(gltfData, options) {
const gltfScenegraph = new GLTFScenegraph(gltfData);
const {json} = gltfScenegraph;
// Move the light ar ray out of the extension and remove the extension
export function encode(gltfData, options) {
const gltfScenegraph = new GLTFScenegraph(gltfData);
const {json} = gltfScenegraph;

if (json.lights) {
const extension = gltfScenegraph.addExtensions(KHR_LIGHTS_PUNCTUAL);
assert(!extension.lights);
extension.lights = json.lights;
delete json.lights;
}
if (json.lights) {
const extension = gltfScenegraph.addExtensions(KHR_LIGHTS_PUNCTUAL);
assert(!extension.lights);
extension.lights = json.lights;
delete json.lights;
}

// Any nodes that have lights field pointing to light object
// add the extension
if (gltfScenegraph.json.lights) {
for (const light of gltfScenegraph.json.lights) {
const node = light.node;
gltfScenegraph.addObjectExtension(node, KHR_LIGHTS_PUNCTUAL, light);
}
delete gltfScenegraph.json.lights;
// Any nodes that have lights field pointing to light object
// add the extension
if (gltfScenegraph.json.lights) {
for (const light of gltfScenegraph.json.lights) {
const node = light.node;
gltfScenegraph.addObjectExtension(node, KHR_LIGHTS_PUNCTUAL, light);
}
delete gltfScenegraph.json.lights;
}
}
17 changes: 9 additions & 8 deletions modules/gltf/src/lib/extensions/gltf-extensions.js
Original file line number Diff line number Diff line change
@@ -1,22 +1,23 @@
/* eslint-disable camelcase */
import KHR_draco_mesh_compression from './KHR_draco_mesh_compression';
import KHR_lights_punctual from './KHR_lights_punctual';
import * as KHR_draco_mesh_compression from './KHR_draco_mesh_compression';
import * as KHR_lights_punctual from './KHR_lights_punctual';
// import UBER_POINT_CLOUD_COMPRESSION from './KHR_draco_mesh_compression';

export const EXTENSIONS = {
KHR_draco_mesh_compression,
KHR_lights_punctual
};

export async function decodeExtensions(gltf, options, context) {
export async function decodeExtensions(gltf, options = {}, context) {
options.gltf = options.gltf || {};
for (const extensionName in EXTENSIONS) {
const disableExtension = extensionName in options && !options[extensionName];
if (!disableExtension) {
const excludes = options.gltf.excludeExtensions || {};
const exclude = extensionName in excludes && !excludes[extensionName];
if (!exclude) {
const extension = EXTENSIONS[extensionName];
// Note: We decode extensions sequentially, this might not be necessary
// Currently we only have glTF, but when we add Basis we may revisit
// Note: We decode async extensions sequentially, this might not be necessary
// Currently we only have Draco, but when we add Basis we may revisit
await extension.decode(gltf, options, context);
// TODO - warn if extension cannot be decoded synchronously?
}
}
}
1 change: 1 addition & 0 deletions modules/gltf/src/lib/gltf-constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
export const KHR_DRACO_MESH_COMPRESSION = 'KHR_draco_mesh_compression';
export const UBER_POINT_CLOUD_EXTENSION = 'UBER_draco_point_cloud_compression';
export const KHR_LIGHTS_PUNCTUAL = 'KHR_lights_punctual';
export const KHR_MATERIALS_UNLIT = 'KHR_materials_unlit';

const COMPONENTS = {
SCALAR: 1,
Expand Down
15 changes: 15 additions & 0 deletions modules/gltf/src/lib/gltf-scenegraph.js
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,21 @@ export default class GLTFScenegraph {
return this;
}

addObjectExtension(object, extensionName, data) {
assert(data);
object.extensions = object.extensions || {};
// TODO - clobber or merge?
object.extensions[extensionName] = data;
this.registerUsedExtension(extensionName);
return this;
}

removeObjectExtension(object, extensionName) {
const extensions = object.extensions || {};
delete extensions[extensionName];
return this;
}

// Add to standard GLTF top level extension object, mark as used
addExtension(extensionName, data) {
assert(data);
Expand Down
30 changes: 14 additions & 16 deletions modules/gltf/test/lib/extensions/KHR_lights_punctual.spec.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,8 @@
/* eslint-disable max-len */
// import test from 'tape-promise/tape';
/* eslint-disable camelcase */
import test from 'tape-promise/tape';

// TBA
import {decodeExtensions} from '@loaders.gl/gltf/lib/extensions/gltf-extensions.js';

/* eslint-disable max-len, camelcase */
/*
import test from 'tape-promise/tape';
const TEST_CASES = [
{
name: 'KHR_lights_punctual',
Expand Down Expand Up @@ -39,23 +36,24 @@ const TEST_CASES = [
nodes: [
{
extensions: {},
id: 'node-0',
children: [],
light: {
color: [1.0, 1.0, 1.0],
type: 'directional'
}
light: 0
}
],
lights: [
{
color: [1.0, 1.0, 1.0],
type: 'directional'
}
]
}
}
];

test('gltf#KHR_lights_punctuals', t => {
test('gltf#KHR_lights_punctuals', async t => {
for (const testCase of TEST_CASES) {
const json = getResolvedJson(testCase.input);
t.deepEqual(json, testCase.output, testCase.name);
await decodeExtensions(testCase.input);
// Modifies input
t.deepEqual(testCase.input.json, testCase.output, testCase.name);
}
t.end();
});
*/

0 comments on commit 90e8a0d

Please sign in to comment.