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 babel-plugin-inline-webgl-constants #1859

Merged
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
11 changes: 5 additions & 6 deletions babel.config.cjs
Original file line number Diff line number Diff line change
@@ -1,23 +1,22 @@
const {getBabelConfig} = require('ocular-dev-tools/configuration');

/**
* NOTE: To debug our babel plugins, reference the local modules using
* `./dev-modules/<plugin>/index.js`, instead of the npm package names.
*/
module.exports = getBabelConfig({
overrides: {
plugins: [
// inject __VERSION__ from package.json
// 'version-inline',
// NOTE: To debug our babel plugins, just reference the local modules
// './dev-modules/babel-plugin-inline-gl-constants',
// 'babel-plugin-inline-webgl-constants',
// TODO - Restore. Some import issue....
// TODO(v9): Publish `babel-plugin-inline-webgl-constants` and re-enable.
// 'babel-plugin-inline-webgl-constants',
// [
// 'babel-plugin-remove-glsl-comments',
// {
// patterns: ['**/shadertools/src/modules/**/*.js']
// }
// ]
// NOTE: To debug our babel plugins, just reference the local modules
// './dev-modules/babel-plugin-inline-gl-constants',
// ['./dev-modules/babel-plugin-remove-glsl-comments', {
// patterns: ['**/shadertools/src/modules/**/*.js']
// }
Expand Down
17 changes: 14 additions & 3 deletions dev-modules/babel-plugin-inline-webgl-constants/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,12 @@ export default function _(opts) {
ImportDeclaration(path, state) {
// specifiers: [ ImportSpecifier | ImportDefaultSpecifier | ImportNamespaceSpecifier ];
// source: Literal;
const specifiers = path.get('specifiers');

let modified = false;
let specifiers = path.get('specifiers');

specifiers.forEach((specifier) => {
if (specifier.type === 'ImportDefaultSpecifier') {
if (specifier.type === 'ImportDefaultSpecifier' || specifier.type === 'ImportSpecifier') {
const local = specifier.node.local;
if (local.type === 'Identifier' && local.name === 'GL') {
if (state.opts.debug) {
Expand All @@ -23,10 +26,18 @@ export default function _(opts) {
`${COLOR_YELLOW}${filename}:${line} Dropping GL import${COLOR_RESET}`
);
}
path.remove();
specifier.remove();
modified = true;
}
}
});

specifiers = path.get('specifiers');

// If the last imported specifier was removed, remove the entire import.
if (modified && specifiers.length === 0) {
path.remove();
}
},

MemberExpression(path, state) {
Expand Down
6 changes: 5 additions & 1 deletion modules/constants/src/constants-enum.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@
* Standard WebGL, WebGL2 and extension constants (OpenGL constants)
* @note (Most) of these constants are also defined on the WebGLRenderingContext interface.
* @see https://developer.mozilla.org/en-US/docs/Web/API/WebGL_API/Constants
* @privateRemarks Locally called `GLEnum` instead of `GL`, because `babel-plugin-inline-webl-constants`
* both depends on and processes this module, but shouldn't replace these declarations.
*/
export enum GL {
enum GLEnum {
// Clearing buffers
// Constants passed to clear() to clear buffer masks.

Expand Down Expand Up @@ -1052,3 +1054,5 @@ export enum GL {
/** A Boolean indicating whether or not the GPU performed any disjoint operation. */
GPU_DISJOINT_EXT = 0x8fbb // A Boolean indicating whether or not the GPU performed any disjoint operation.
}

export {GLEnum as GL};
21 changes: 12 additions & 9 deletions modules/gltf/src/gltf/gltf-instantiator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,10 @@ export class GLTFInstantiator {
}
}

enum GL {
donmccurdy marked this conversation as resolved.
Show resolved Hide resolved
// NOTE: Modules other than `@luma.gl/webgl` should not import `GL` from
// `@luma.gl/constants`. Locally we use `GLEnum` instead of `GL` to avoid
// conflicts with the `babel-plugin-inline-webgl-constants` plugin.
enum GLEnum {
POINTS = 0x0,
LINES = 0x1,
LINE_LOOP = 0x2,
Expand All @@ -210,16 +213,16 @@ enum GL {
}

export function convertGLDrawModeToTopology(
drawMode: GL.POINTS | GL.LINES | GL.LINE_STRIP | GL.LINE_LOOP | GL.TRIANGLES | GL.TRIANGLE_STRIP | GL.TRIANGLE_FAN,
drawMode: GLEnum.POINTS | GLEnum.LINES | GLEnum.LINE_STRIP | GLEnum.LINE_LOOP | GLEnum.TRIANGLES | GLEnum.TRIANGLE_STRIP | GLEnum.TRIANGLE_FAN,
): PrimitiveTopology {
switch (drawMode) {
case GL.POINTS: return 'point-list';
case GL.LINES: return 'line-list';
case GL.LINE_STRIP: return 'line-strip';
case GL.LINE_LOOP: return 'line-loop-webgl';
case GL.TRIANGLES: return 'triangle-list';
case GL.TRIANGLE_STRIP: return 'triangle-strip';
case GL.TRIANGLE_FAN: return 'triangle-fan-webgl';
case GLEnum.POINTS: return 'point-list';
case GLEnum.LINES: return 'line-list';
case GLEnum.LINE_STRIP: return 'line-strip';
case GLEnum.LINE_LOOP: return 'line-loop-webgl';
case GLEnum.TRIANGLES: return 'triangle-list';
case GLEnum.TRIANGLE_STRIP: return 'triangle-strip';
case GLEnum.TRIANGLE_FAN: return 'triangle-fan-webgl';
default: throw new Error(drawMode);
}
}
23 changes: 18 additions & 5 deletions modules/gltf/src/pbr/parse-pbr-material.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import type {Device, Texture, Binding, Parameters} from '@luma.gl/core';
import {log} from '@luma.gl/core';
import {GL} from '@luma.gl/constants';
import {PBREnvironment} from './pbr-environment';

/* eslint-disable camelcase */
Expand All @@ -26,6 +25,20 @@ export type ParsedPBRMaterial = {
readonly generatedTextures: Texture[];
};

// NOTE: Modules other than `@luma.gl/webgl` should not import `GL` from
// `@luma.gl/constants`. Locally we use `GLEnum` instead of `GL` to avoid
// conflicts with the `babel-plugin-inline-webgl-constants` plugin.
enum GLEnum {
FUNC_ADD = 0x8006,
ONE = 1,
SRC_ALPHA = 0x0302,
ONE_MINUS_SRC_ALPHA = 0x0303,
TEXTURE_MIN_FILTER = 0x2801,
LINEAR = 0x2601,
LINEAR_MIPMAP_NEAREST = 0x2701,
UNPACK_FLIP_Y_WEBGL = 0x9240,
}

/**
* Parses a GLTF material definition into uniforms and parameters for the PBR shader module
*/
Expand Down Expand Up @@ -144,8 +157,8 @@ function parseMaterial(device: Device, material, parsedMaterial: ParsedPBRMateri

// GL parameters
parsedMaterial.glParameters.blend = true;
parsedMaterial.glParameters.blendEquation = GL.FUNC_ADD;
parsedMaterial.glParameters.blendFunc = [GL.SRC_ALPHA, GL.ONE_MINUS_SRC_ALPHA, GL.ONE, GL.ONE_MINUS_SRC_ALPHA];
parsedMaterial.glParameters.blendEquation = GLEnum.FUNC_ADD;
parsedMaterial.glParameters.blendFunc = [GLEnum.SRC_ALPHA, GLEnum.ONE_MINUS_SRC_ALPHA, GLEnum.ONE, GLEnum.ONE_MINUS_SRC_ALPHA];

break;
}
Expand Down Expand Up @@ -197,7 +210,7 @@ function addTexture(
if (image.compressed) {
textureOptions = image;
specialTextureParameters = {
[GL.TEXTURE_MIN_FILTER]: image.data.length > 1 ? GL.LINEAR_MIPMAP_NEAREST : GL.LINEAR
[GLEnum.TEXTURE_MIN_FILTER]: image.data.length > 1 ? GLEnum.LINEAR_MIPMAP_NEAREST : GLEnum.LINEAR
};
} else {
// Texture2D accepts a promise that returns an image as data (Async Textures)
Expand All @@ -211,7 +224,7 @@ function addTexture(
...specialTextureParameters
},
pixelStore: {
[GL.UNPACK_FLIP_Y_WEBGL]: false
[GLEnum.UNPACK_FLIP_Y_WEBGL]: false
},
...textureOptions
});
Expand Down
1 change: 0 additions & 1 deletion modules/webgpu/src/adapter/helpers/accessor-to-format.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
/*
import {assert} from '@luma.gl/core';
import {GL} from '@luma.gl/constants';

type Accessor = Record<string, any>;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,17 @@ const TEST_CASES = [
const x = GL.FLOAT;
`,
output: 'const x = 5126;'
},
{
title: 'remove import specifier',
donmccurdy marked this conversation as resolved.
Show resolved Hide resolved
input: `
import {GL, UnknownItem} from '@luma.gl/constants';
const x = GL.FLOAT;
`,
output: `
import { UnknownItem } from '@luma.gl/constants';
const x = 5126;
`
}
];

Expand All @@ -51,9 +62,7 @@ function clean(code) {
return code.replace('"use strict";', '').replace(/\n\s+/g, '\n').trim();
}

// TODO - restore
/* eslint-disable */
test.skip('InlineGLSLConstants Babel Plugin', (t) => {
test('InlineGLSLConstants Babel Plugin', (t) => {
TEST_CASES.forEach((testCase) => {
const transform = () =>
babel.transform(testCase.input, {
Expand Down
2 changes: 1 addition & 1 deletion test/dev-modules/index.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
import './babel-plugin-inline-gl-constants.spec';
import './babel-plugin-inline-webgl-constants.spec';
import './babel-plugin-remove-glsl-comments.spec';
import './eslint-plugin-luma-gl-custom-rules.spec';
Loading