Avoid requiring document as a function parameter unnecessarily (#787)

* [core] Add Document.fromGraph(graph) static accessor. * [functions] Avoid requiring document as a function parameter unnecessarily. Removes document from the following method signatures: - listTextureChannels(texture) - getTextureChannelMask(texture) - listTextureInfo(texture) - listTextureSlots(texture)
donmccurdy · Jan 19, 2023 · def1733 · def1733
1 parent 14f8907
commit def1733
Show file tree

Hide file tree

Showing 12 changed files with 66 additions and 35 deletions.
diff --git a/packages/cli/src/transforms/ktxfix.ts b/packages/cli/src/transforms/ktxfix.ts
@@ -21,7 +21,7 @@ export function ktxfix(): Transform {
 
 			const ktx = read(image);
 			const dfd = ktx.dataFormatDescriptor[0];
-			const slots = listTextureSlots(doc, texture);
+			const slots = listTextureSlots(texture);
 
 			// Don't make changes if we have no information.
 			if (slots.length === 0) continue;

diff --git a/packages/cli/src/transforms/toktx.ts b/packages/cli/src/transforms/toktx.ts
@@ -133,8 +133,8 @@ export const toktx = function (options: ETC1SOptions | UASTCOptions): Transform
 		const numTextures = textures.length;
 		const promises = textures.map((texture, textureIndex) =>
 			limit(async () => {
-				const slots = listTextureSlots(doc, texture);
-				const channels = getTextureChannelMask(doc, texture);
+				const slots = listTextureSlots(texture);
+				const channels = getTextureChannelMask(texture);
 				const textureLabel =
 					texture.getURI() ||
 					texture.getName() ||

diff --git a/packages/core/src/document.ts b/packages/core/src/document.ts
@@ -80,14 +80,34 @@ export class Document {
 	private _root: Root = new Root(this._graph);
 	private _logger: ILogger = Logger.DEFAULT_INSTANCE;
 
+	/**
+	 * Enables lookup of a Document from its Graph. For internal use, only.
+	 * @internal
+	 * @experimental
+	 */
+	private static _GRAPH_DOCUMENTS = new WeakMap<Graph<Property>, Document>();
+
+	/**
+	 * Returns the Document associated with a given Graph, if any.
+	 * @hidden
+	 * @experimental
+	 */
+	public static fromGraph(graph: Graph<Property>): Document | null {
+		return Document._GRAPH_DOCUMENTS.get(graph) || null;
+	}
+
+	/** Creates a new Document, representing an empty glTF asset. */
+	public constructor() {
+		Document._GRAPH_DOCUMENTS.set(this._graph, this);
+	}
+
 	/** Returns the glTF {@link Root} property. */
 	public getRoot(): Root {
 		return this._root;
 	}
 
 	/**
 	 * Returns the {@link Graph} representing connectivity of resources within this document.
-	 *
 	 * @hidden
 	 */
 	public getGraph(): Graph<Property> {

diff --git a/packages/core/src/properties/property.ts b/packages/core/src/properties/property.ts
@@ -75,6 +75,15 @@ export abstract class Property<T extends IProperty = IProperty> extends GraphNod
 	 */
 	protected abstract init(): void;
 
+	/**
+	 * Returns the Graph associated with this Property. For internal use.
+	 * @hidden
+	 * @experimental
+	 */
+	public getGraph(): Graph<Property> {
+		return this.graph;
+	}
+
 	/**
 	 * Returns default attributes for the property. Empty lists and maps should be initialized
 	 * to empty arrays and objects. Always invoke `super.getDefaults()` and extend the result.

diff --git a/packages/functions/src/list-texture-channels.ts b/packages/functions/src/list-texture-channels.ts
@@ -1,4 +1,4 @@
-import type { Document, Texture } from '@gltf-transform/core';
+import { Document, Texture } from '@gltf-transform/core';
 import { Material, TextureChannel, PropertyType } from '@gltf-transform/core';
 
 /**
@@ -10,13 +10,13 @@ import { Material, TextureChannel, PropertyType } from '@gltf-transform/core';
  * Example:
  *
  * ```js
- * const channels = listTextureChannels(document, texture);
+ * const channels = listTextureChannels(texture);
  * if (channels.includes(TextureChannel.R)) {
  *   console.log('texture red channel used');
  * }
  */
-export function listTextureChannels(document: Document, texture: Texture): TextureChannel[] {
-	const mask = getTextureChannelMask(document, texture);
+export function listTextureChannels(texture: Texture): TextureChannel[] {
+	const mask = getTextureChannelMask(texture);
 	const channels = [];
 	if (mask & TextureChannel.R) channels.push(TextureChannel.R);
 	if (mask & TextureChannel.G) channels.push(TextureChannel.G);
@@ -34,13 +34,14 @@ export function listTextureChannels(document: Document, texture: Texture): Textu
  * Example:
  *
  * ```js
- * const mask = getTextureChannelMask(document, texture);
+ * const mask = getTextureChannelMask(texture);
  * if (mask & TextureChannel.R) {
  *   console.log('texture red channel used');
  * }
  * ```
  */
-export function getTextureChannelMask(document: Document, texture: Texture): number {
+export function getTextureChannelMask(texture: Texture): number {
+	const document = Document.fromGraph(texture.getGraph())!;
 	let mask = 0x0000;
 	for (const edge of document.getGraph().listParentEdges(texture)) {
 		const parent = edge.getParent();

diff --git a/packages/functions/src/list-texture-info.ts b/packages/functions/src/list-texture-info.ts
@@ -1,4 +1,4 @@
-import { Document, Texture, TextureInfo } from '@gltf-transform/core';
+import { Texture, TextureInfo } from '@gltf-transform/core';
 
 /**
  * Lists all {@link TextureInfo} definitions associated with a given {@link Texture}.
@@ -7,15 +7,15 @@ import { Document, Texture, TextureInfo } from '@gltf-transform/core';
  *
  * ```js
  * // Find TextureInfo instances associated with the texture.
- * const results = listTextureInfo(document, texture);
+ * const results = listTextureInfo(texture);
  *
  * // Find which UV sets (TEXCOORD_0, TEXCOORD_1, ...) are required.
  * const texCoords = results.map((info) => info.getTexCoord());
  * // → [0, 0, 1]
  * ```
  */
-export function listTextureInfo(document: Document, texture: Texture): TextureInfo[] {
-	const graph = document.getGraph();
+export function listTextureInfo(texture: Texture): TextureInfo[] {
+	const graph = texture.getGraph();
 	const results: TextureInfo[] = [];
 
 	for (const textureEdge of graph.listParentEdges(texture)) {

diff --git a/packages/functions/src/list-texture-slots.ts b/packages/functions/src/list-texture-slots.ts
@@ -1,18 +1,19 @@
-import type { Document, Texture } from '@gltf-transform/core';
+import { Document, Texture } from '@gltf-transform/core';
 
 /**
  * Returns names of all texture slots using the given texture.
  *
  * Example:
  *
  * ```js
- * const slots = listTextureSlots(document, texture);
+ * const slots = listTextureSlots(texture);
  * // → ['occlusionTexture', 'metallicRoughnesTexture']
  * ```
  */
-export function listTextureSlots(doc: Document, texture: Texture): string[] {
-	const root = doc.getRoot();
-	const slots = doc
+export function listTextureSlots(texture: Texture): string[] {
+	const document = Document.fromGraph(texture.getGraph())!;
+	const root = document.getRoot();
+	const slots = texture
 		.getGraph()
 		.listParentEdges(texture)
 		.filter((edge) => edge.getParent() !== root)

diff --git a/packages/functions/src/texture-compress.ts b/packages/functions/src/texture-compress.ts
@@ -8,7 +8,7 @@ import { TextureResizeFilter } from './texture-resize';
 
 const NAME = 'textureCompress';
 
-type Format = typeof FORMATS[number];
+type Format = (typeof FORMATS)[number];
 const FORMATS = ['jpeg', 'png', 'webp'] as const;
 const SUPPORTED_MIME_TYPES = ['image/jpeg', 'image/png', 'image/webp'];
 
@@ -92,8 +92,8 @@ export const textureCompress = function (_options: TextureCompressOptions): Tran
 
 		await Promise.all(
 			textures.map(async (texture, textureIndex) => {
-				const slots = listTextureSlots(document, texture);
-				const channels = getTextureChannelMask(document, texture);
+				const slots = listTextureSlots(texture);
+				const channels = getTextureChannelMask(texture);
 				const textureLabel =
 					texture.getURI() ||
 					texture.getName() ||

diff --git a/packages/functions/src/texture-resize.ts b/packages/functions/src/texture-resize.ts
@@ -65,7 +65,7 @@ export function textureResize(_options: TextureResizeOptions = TEXTURE_RESIZE_DE
 				continue;
 			}
 
-			const slots = listTextureSlots(doc, texture);
+			const slots = listTextureSlots(texture);
 			if (options.slots && !slots.some((slot) => options.slots?.test(slot))) {
 				logger.debug(`${NAME}: Skipping, [${slots.join(', ')}] excluded by "slots" parameter.`);
 				continue;

diff --git a/packages/functions/test/list-texture-channels.test.ts b/packages/functions/test/list-texture-channels.test.ts
@@ -19,14 +19,14 @@ test('@gltf-transform/functions::listTextureChannels', (t) => {
 		.setBaseColorTexture(textureA)
 		.setExtension('KHR_materials_sheen', sheen);
 
-	t.deepEquals(listTextureChannels(document, textureA), [R, G, B, A], 'baseColorTexture RGBA');
-	t.deepEquals(listTextureChannels(document, textureB), [A], 'sheenColorTexture A');
+	t.deepEquals(listTextureChannels(textureA), [R, G, B, A], 'baseColorTexture RGBA');
+	t.deepEquals(listTextureChannels(textureB), [A], 'sheenColorTexture A');
 
 	material.setAlphaMode('OPAQUE');
-	t.deepEquals(listTextureChannels(document, textureA), [R, G, B], 'baseColorTexture RGB');
+	t.deepEquals(listTextureChannels(textureA), [R, G, B], 'baseColorTexture RGB');
 
 	sheen.setSheenColorTexture(textureB);
-	t.deepEquals(listTextureChannels(document, textureB), [R, G, B, A], 'sheenColorTexture RGBA');
+	t.deepEquals(listTextureChannels(textureB), [R, G, B, A], 'sheenColorTexture RGBA');
 	t.end();
 });
 
@@ -42,13 +42,13 @@ test('@gltf-transform/functions::getTextureChannelMask', (t) => {
 		.setBaseColorTexture(textureA)
 		.setExtension('KHR_materials_sheen', sheen);
 
-	t.equals(getTextureChannelMask(document, textureA), R | G | B | A, 'baseColorTexture RGBA');
-	t.equals(getTextureChannelMask(document, textureB), A, 'sheenColorTexture A');
+	t.equals(getTextureChannelMask(textureA), R | G | B | A, 'baseColorTexture RGBA');
+	t.equals(getTextureChannelMask(textureB), A, 'sheenColorTexture A');
 
 	material.setAlphaMode('OPAQUE');
-	t.equals(getTextureChannelMask(document, textureA), R | G | B, 'baseColorTexture RGB');
+	t.equals(getTextureChannelMask(textureA), R | G | B, 'baseColorTexture RGB');
 
 	sheen.setSheenColorTexture(textureB);
-	t.equals(getTextureChannelMask(document, textureB), R | G | B | A, 'sheenColorTexture RGBA');
+	t.equals(getTextureChannelMask(textureB), R | G | B | A, 'sheenColorTexture RGBA');
 	t.end();
 });
diff --git a/packages/functions/test/list-texture-info.test.ts b/packages/functions/test/list-texture-info.test.ts
@@ -17,14 +17,14 @@ test('@gltf-transform/functions::listTextureInfo', (t) => {
 	document.createMaterial().setOcclusionTexture(textureB).setMetallicRoughnessTexture(textureB);
 
 	t.deepEquals(
-		listTextureInfo(document, textureA)
+		listTextureInfo(textureA)
 			.map((info) => info.getName())
 			.sort(),
 		['baseColorTextureInfo', 'sheenRoughnessTextureInfo'],
 		'texture A'
 	);
 	t.deepEquals(
-		listTextureInfo(document, textureB)
+		listTextureInfo(textureB)
 			.map((info) => info.getName())
 			.sort(),
 		['metallicRoughnessTextureInfo', 'occlusionTextureInfo'],

diff --git a/packages/functions/test/list-texture-slots.test.ts b/packages/functions/test/list-texture-slots.test.ts
@@ -12,7 +12,7 @@ test('@gltf-transform/functions::listTextureSlots', (t) => {
 	const sheenExtension = document.createExtension(KHRMaterialsSheen);
 	const sheen = sheenExtension.createSheen().setSheenColorTexture(textureB);
 	document.createMaterial().setBaseColorTexture(textureA).setExtension('KHR_materials_sheen', sheen);
-	t.deepEquals(listTextureSlots(document, textureA), ['baseColorTexture'], 'baseColorTexture');
-	t.deepEquals(listTextureSlots(document, textureB), ['sheenColorTexture'], 'sheenColorTexture');
+	t.deepEquals(listTextureSlots(textureA), ['baseColorTexture'], 'baseColorTexture');
+	t.deepEquals(listTextureSlots(textureB), ['sheenColorTexture'], 'sheenColorTexture');
 	t.end();
 });