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

Update UI library used for Web Viewer #1618

Merged

Conversation

kwokcb
Copy link
Contributor

@kwokcb kwokcb commented Dec 19, 2023

Change

  • Switch to lil-gui which fixes a number of dat-gui issues including all input which did not show up anymore (regresssion).
  • Same GUI lib as used for ThreeJS examples for consistency.

Examples

  • lil-gui is mostly a 1:1 replacement. The clearing of UI on material change or initialization (to handle threaded load), is simplified to finding the appropriate DOM element.
  • performance "appears" to be a a bit better, possibly due to delayed UI building / display after material loading.

image

image

Same GUI lib as used for ThreeJS examples.
@@ -9,7 +9,7 @@ import { RGBELoader } from 'three/examples/jsm/loaders/RGBELoader.js';

import { prepareEnvTexture, getLightRotation, findLights, registerLights, getUniformValues } from './helper.js'
import { Group } from 'three';
import { GUI } from 'dat.gui';
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the library which is an add-on to Three.js but can directly be pulled in.

@@ -445,13 +445,10 @@ export class Editor
//
clearFolders()
{
Array.from(document.getElementsByClassName('folder')).forEach(
Array.from(document.getElementsByClassName('lil-gui')).forEach(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The DOM element to clear is different for lil-gui than dat.gui.

Copy link
Member

@jstone-lucasfilm jstone-lucasfilm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a great improvement to me, thanks @kwokcb!

@jstone-lucasfilm jstone-lucasfilm merged commit cbff01a into AcademySoftwareFoundation:main Dec 19, 2023
30 checks passed
@kwokcb kwokcb deleted the webview_gui_library branch December 20, 2023 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants