From daabc4d5624c6a49823a5eba586f4d80f05fbb11 Mon Sep 17 00:00:00 2001 From: Danny McGee Date: Tue, 26 Mar 2024 14:07:09 -0400 Subject: [PATCH] Fix rotation inspector (#104) This PR updates the Transform > Rotation section of the Entity Inspector to display Euler angles in degrees instead of the vector component of the input quaternion. Additionally, this updates the `studio/ui/inspector` module to add a special-case function for updating the selected entity's rotation from the inspector in a way that's consistent with the behavior of the rotation gizmo. ### Known Issues The values displayed for the angles in the inspector are not always intuitive, especially while actively manipulating them. To some extent this is just the nature of Euler angles (e.g., Unreal Engine exhibits similar issues), but I suspect there's room for improvement of our implementation (e.g., Unreal's rotation display is typically _more_ sane than ours). Regardless, fixing this requires a firmer grasp of the math than I possess, so a fix for this issue is not currently planned. --- .../src/lib/forms/scalar-field.element.ts | 160 +++++++++++++++++- .../lib/inspector/quat-inspector.element.scss | 31 ++++ .../lib/inspector/quat-inspector.element.ts | 126 ++++++++++++++ .../transform-inspector.element.scss | 4 + .../inspector/transform-inspector.element.ts | 55 +++--- studio/src/ui/inspector.rs | 81 ++++++++- 6 files changed, 415 insertions(+), 42 deletions(-) create mode 100644 studio-web/src/lib/inspector/quat-inspector.element.scss create mode 100644 studio-web/src/lib/inspector/quat-inspector.element.ts diff --git a/studio-ui/src/lib/forms/scalar-field.element.ts b/studio-ui/src/lib/forms/scalar-field.element.ts index 06a592d..8a9e881 100644 --- a/studio-ui/src/lib/forms/scalar-field.element.ts +++ b/studio-ui/src/lib/forms/scalar-field.element.ts @@ -40,7 +40,76 @@ export class ScalarFieldElement extends FormFieldBaseElement { min = -Infinity; @property({ type: Number }) - sensitivity = 0.01; + step = 0.01; + + @property({ type: Number }) + microStep = 0.001; + + /** + * A format string, supporting similar syntax to Rust's numeric + * {@linkcode https://doc.rust-lang.org/std/fmt/ std::fmt} specifiers: + * + * ``` + * [minIntegerDigits]['.' minFractionalDigits ['-' maxFractionalDigits]][' ' unit] + * ``` + * where: + * + * - `minIntegerDigits` (default: `1`) : + * + * The minimum number of integer digits to use. A value with a smaller + * number of integer digits than this number will be left-padded with zeros + * (to the specified length) when formatted. Possible values are from `1` + * to `21`. + * + * - `minFractioinalDigits` (default: `1`) : + * + * The minimum number of fraction digits to use. Possible values are from + * `0` to `20`. + * + * - `maxFractionalDigits` (default: `3`) : + * + * The maximum number of fraction digits to use. Possible values are from + * `0` to `20`. If not specified, will default to the larger of + * `minFractionalDigits` and `3`. + * + * - `unit` (optional) : + * + * Arbitrary text to display after the formatted value, representing the + * unit of measurement. + * + * @example + * ```html + * + * + * + * + * + * ``` + */ + @property() + format = "1.1-3"; + + /** + * Specify which DOM event ("input" or "change") should trigger this + * component to emit a "value-change" event. + * + * - "input" events fire every time the user types into the text field + * - "change" events fire only when the updated value is "committed" (by + * pressing "Enter" or moving focus out of the text field) + * + * @default "input" + */ + @property() + emitOn: "change" | "input" = "input"; @property({ type: Number, @@ -63,11 +132,54 @@ export class ScalarFieldElement extends FormFieldBaseElement { minimumFractionDigits: 1, maximumFractionDigits: 3, }); + #fmtUnitSuffix = ""; + + protected override willUpdate(changes: PropertyValues): void { + if (changes.has("format")) { + // Optional fractional digits: https://regexr.com/7ttu5 + const FORMAT_PATTERN_OPT_FRAC = /^([0-9]+)(?:\.([0-9]+)(?:-([0-9]+))?)?( .+)?$/; + // Optional integral digits: https://regexr.com/7ttu8 + const FORMAT_PATTERN_OPT_INT = /^([0-9]+)?(?:\.([0-9]+)(?:-([0-9]+))?)( .+)?$/; + + const matches = this.format.match(FORMAT_PATTERN_OPT_FRAC) + ?? this.format.match(FORMAT_PATTERN_OPT_INT) + // Note: If any _individual_ subexpression is null, we pass that + // intention through to the formatter. If the _entire format-string_ + // is omitted (i.e. the property is not set), then we fall back to + // "1.1-3" as a sensible default. + ?? ["", "1", "1", "3"]; + const [ minInt, minFrac, maxFrac, unitSuffix] = matches.slice(1); + + this.#fmtDecimal = new Intl.NumberFormat("en-US", { + minimumIntegerDigits: minInt != null ? parseInt(minInt) : undefined, + minimumFractionDigits: minFrac != null ? parseInt(minFrac) : undefined, + maximumFractionDigits: maxFrac != null ? parseInt(maxFrac) : undefined, + }); + this.#fmtUnitSuffix = unitSuffix ?? ""; + } + + super.willUpdate(changes); + } protected override update(changes: PropertyValues): void { if (changes.has("_editing")) this.classList.toggle("editing", this._editing); + if (changes.has("value")) { + this.ariaValueText = `${this.#fmtDecimal.format(this.value)}${this.#fmtUnitSuffix}`; + + if (this._editing && this.emitOn === "change") { + // If we receive a new value from outside (i.e. not due to the + // user's own input), replace the input's content. + // + // Note: This is a bit of a hack to ensure a decent UX when tabbing + // between fields in the `` -- please don't + // change this without validating that use case still functions in + // a nice way. + this.#activateDirectInput(); + } + } + super.update(changes); } @@ -98,7 +210,9 @@ export class ScalarFieldElement extends FormFieldBaseElement { this.addEventListener("pointermove", event => { totalMovement += event.movementX; - this.#emitValue(this.value + event.movementX * this.sensitivity); + + const step = event.ctrlKey ? this.microStep : this.step; + this.#emitValue(this.value + event.movementX * step); }, { signal: pointerMoveAbortSignal, }); @@ -131,19 +245,39 @@ export class ScalarFieldElement extends FormFieldBaseElement { const input = this.#inputRef.value; if (!input) return; - input.textContent = this.#fmtDecimal.format(this.value); + // Arbitrary HTML elements with `contenteditable` don't actually emit + // a "change" event, so we'll emit one manually. + input.dispatchEvent(new Event("change", { + bubbles: true, + cancelable: true, + composed: false, + })); } - onInputChange(event: InputEvent): void { + onInputChange(event: InputEvent | Event): void { + if (event.type === this.emitOn) + this.processTextInput(event); + } + + processTextInput(event: InputEvent | Event): void { try { const input = this.#inputRef.value!; const textContent = input.textContent?.trim() ?? ""; - if (event.data && !VALID_INPUT_PATTERN.test(event.data)) { - input.textContent = this.value.toString(10); + if ( + event instanceof InputEvent + && event.data + && !VALID_INPUT_PATTERN.test(event.data) + ) { + input.textContent = this.#fmtDecimal.format(this.value); } else { const value = parseFloat(textContent); - this.#emitValue(value); + + if (!Number.isNaN(value) && Number.isFinite(value)) { + this.#emitValue(value); + } else { + input.textContent = this.#fmtDecimal.format(this.value); + } } } catch {} } @@ -160,6 +294,14 @@ export class ScalarFieldElement extends FormFieldBaseElement { throw new Error("Expected input to be available in editing mode"); input.addEventListener("blur", () => { + // Arbitrary HTML elements with `contenteditable` don't actually emit + // a "change" event, so we'll emit one manually. + input.dispatchEvent(new Event("change", { + bubbles: true, + cancelable: true, + composed: false, + })); + this._editing = false; this.tabIndex = 0; }, { @@ -167,6 +309,7 @@ export class ScalarFieldElement extends FormFieldBaseElement { }); input.focus(); + // Note: Unit suffix intentionally omitted when text field is editable input.textContent = this.#fmtDecimal.format(this.value); const range = document.createRange(); @@ -191,7 +334,7 @@ export class ScalarFieldElement extends FormFieldBaseElement { protected override render = () => html` ${!this._editing ? html` - ${this.#fmtDecimal.format(this.value)} + ${this.#fmtDecimal.format(this.value)}${this.#fmtUnitSuffix} ` : html` { if (event.key === "Enter") this.onInputEnter(event); diff --git a/studio-web/src/lib/inspector/quat-inspector.element.scss b/studio-web/src/lib/inspector/quat-inspector.element.scss new file mode 100644 index 0000000..0d440a2 --- /dev/null +++ b/studio-web/src/lib/inspector/quat-inspector.element.scss @@ -0,0 +1,31 @@ +@use "sass:math"; + +:host { + display: flex; + gap: 0.25rem; +} + +.input { + padding: 0.5rem; + width: math.div(1, 3) * 100%; + border-left-width: 4px; + + &:nth-of-type(1) { + border-left-color: rgb(220, 53, 69); + &:focus { + border-left-color: rgb(255, 48, 68); + } + } + &:nth-of-type(2) { + border-left-color: rgb(25, 135, 84); + &:focus { + border-left-color: rgb(0, 187, 100); + } + } + &:nth-of-type(3) { + border-left-color: rgb(13, 110, 253); + &:focus { + border-left-color: rgb(47, 131, 255); + } + } +} diff --git a/studio-web/src/lib/inspector/quat-inspector.element.ts b/studio-web/src/lib/inspector/quat-inspector.element.ts new file mode 100644 index 0000000..9d52614 --- /dev/null +++ b/studio-web/src/lib/inspector/quat-inspector.element.ts @@ -0,0 +1,126 @@ +import * as studio from "@storyteller/studio"; +import { RotationAxis } from "@storyteller/studio"; +import { match } from "@storyteller/utility"; +import { LitElement, PropertyValues, TemplateResult, html, unsafeCSS } from "lit"; +import { customElement, property, state } from "lit/decorators.js"; + +import { type Quat, type Vec3 } from "./inspector.types"; + +import styles from "./quat-inspector.element.scss?inline"; + +@customElement("sts-quat-inspector") +export class QuatInspectorElement extends LitElement { + static override styles = unsafeCSS(styles); + + @property({ attribute: false }) + value: Quat = { w: 1, x: 0, y: 0, z: 0 }; + + @state() _eulerX = 0; + @state() _eulerY = 0; + @state() _eulerZ = 0; + + protected override willUpdate(changes: PropertyValues): void { + if (changes.has("value")) { + const { x, y, z } = quatToEuler(this.value); + this._eulerX = rad2deg(x); + this._eulerY = rad2deg(y); + this._eulerZ = rad2deg(z); + } + + super.willUpdate(changes); + } + + #emitUpdate(axis: RotationAxis) { + return ({ detail: newAngle }: CustomEvent) => { + const prevAngle = match (axis, { + [RotationAxis.X]: () => this._eulerX, + [RotationAxis.Y]: () => this._eulerY, + [RotationAxis.Z]: () => this._eulerZ, + }); + + const deltaTheta = deg2rad(newAngle - prevAngle); + + if (Number.isFinite(deltaTheta) && !Number.isNaN(deltaTheta)) + studio.rotateLocalTransform(axis, deltaTheta); + } + } + + protected override render = (): TemplateResult => html` + + + + `; +} + +// Reference: https://gamemath.com/book/orient.html#quaternion_to_euler_angles +// TODO: The fields here "work" on a basic level, but I'm not convinced they're +// actually displaying intelligible values. For example, if you tilt the +// X-axis by any amount and rotate the Y-axis, you'll typically see all +// three values spinning wildly. I hate Euler angles. +function quatToEuler({ w, x, y, z }: Quat): Vec3 { + let sinPitch = -2 * (y*z - w*x); + // Clamp in the range of [-1,1] to avoid NaNs + sinPitch = Math.min(Math.max(sinPitch, -1), 1); + + // Check for gimbal lock + if (nearlyEq(sinPitch, 1, 1e-3)) { + const yaw = Math.atan2(-x*z + w*y, 0.5 - y*y - z*z); + const pitch = Math.PI * 0.5 * sinPitch; + const roll = 0; + + return { + x: pitch, + y: yaw, + z: roll, + } + } + + const yaw = Math.atan2(x*z + w*y, 0.5 - x*x - y*y); + const pitch = Math.asin(sinPitch); + const roll = Math.atan2(x*y + w*z, 0.5 - x*x - z*z); + + return { + x: pitch, + y: yaw, + z: roll, + } +} + +function nearlyEq(lhs: number, rhs: number, tolerance = Number.EPSILON): boolean { + return Math.abs(lhs - rhs) < tolerance; +} + +function rad2deg(rad: number): number { + return rad * (180 / Math.PI); +} + +function deg2rad(deg: number): number { + return (deg * Math.PI) / 180; +} diff --git a/studio-web/src/lib/inspector/transform-inspector.element.scss b/studio-web/src/lib/inspector/transform-inspector.element.scss index 2ccd8a2..451e37c 100644 --- a/studio-web/src/lib/inspector/transform-inspector.element.scss +++ b/studio-web/src/lib/inspector/transform-inspector.element.scss @@ -61,3 +61,7 @@ h4 { } } } + +sts-quat-inspector { + width: 100%; +} diff --git a/studio-web/src/lib/inspector/transform-inspector.element.ts b/studio-web/src/lib/inspector/transform-inspector.element.ts index bb1e204..aaecb83 100644 --- a/studio-web/src/lib/inspector/transform-inspector.element.ts +++ b/studio-web/src/lib/inspector/transform-inspector.element.ts @@ -6,6 +6,7 @@ import { v4 as uuid } from "uuid"; import { type Quat, type Vec3 } from "./inspector.types"; import "@storyteller/studio-ui/forms/scalar-field"; +import "./quat-inspector.element"; import styles from "./transform-inspector.element.scss?inline"; @@ -26,16 +27,14 @@ export class TransformInspectorElement extends LitElement { #rotationLabelId = uuid(); #scaleLabelId = uuid(); - updateField(reflectPath: `${"translation"|"rotation"|"scale"}.${"x"|"y"|"z"}`) { - return ({ detail }: CustomEvent) => { - const value = parseFloat(detail); - if (Number.isFinite(value) && !Number.isNaN(value)) { - studio.updateComponentField({ - component: "bevy_transform::components::transform::Transform", - reflectPath, - value, - }); - } + updateField(reflectPath: `${"translation"|"scale"}.${"x"|"y"|"z"}` | "rotation") { + return ({ detail: value }: CustomEvent) => { + console.log("value:", value); + studio.updateComponentField({ + component: "bevy_transform::components::transform::Transform", + reflectPath, + value, + }); } } @@ -52,18 +51,24 @@ export class TransformInspectorElement extends LitElement { @@ -76,26 +81,10 @@ export class TransformInspectorElement extends LitElement { Rotation - - - - - +
diff --git a/studio/src/ui/inspector.rs b/studio/src/ui/inspector.rs index 45481ae..5d6f3df 100644 --- a/studio/src/ui/inspector.rs +++ b/studio/src/ui/inspector.rs @@ -50,8 +50,10 @@ impl Plugin for InspectorPlugin { app.add_systems( Update, - update_field_by_ui - .map(bevy::utils::error) + ( + update_field_by_ui.map(bevy::utils::error), + update_local_rotation_by_ui, + ) .in_set(InspectorSystem::WriteChanges), ); @@ -400,8 +402,18 @@ fn update_field_by_ui(world: &mut World) -> Result<(), String> { *val = msg_value; Ok(()) } else if let Some(val) = reflect_field.downcast_mut::() { - let msg_value = serde_json::from_value(msg.value.clone()).map_err(fmt_err)?; - *val = msg_value; + #[derive(Deserialize)] + struct JsonQuat { + w: f32, + x: f32, + y: f32, + z: f32, + } + + let JsonQuat { w, x, y, z } = + serde_json::from_value(msg.value.clone()).map_err(fmt_err)?; + + *val = Quat::from_xyzw(x, y, z, w); Ok(()) } else if let Some(val) = reflect_field.downcast_mut::() { let Rgba { @@ -759,3 +771,64 @@ pub fn update_component_field(msg: JsValue) -> Result<(), JsValue> { CHANGE_FROM_UI.write_message(msg); Ok(()) } + +/// Handle transform rotation updates from the UI as a special case. +/// +/// It's impossible to reliably compute a new quaternion for a desired Euler- +/// axes or axis-angle rotation from the frontend, because the calculation is +/// dependent on the parent's orientation. To deal with this, we provide a +/// separate `rotate_local_transform` method for this operation and handle it +/// with a separate Bevy system. +static ROTATE_LOCAL_XFORM: WasmMessageBuffer<(RotationAxis, f32)> = WasmMessageBuffer::new(); + +#[wasm_bindgen] +pub enum RotationAxis { + X, + Y, + Z, +} + +#[wasm_bindgen(js_name = rotateLocalTransform)] +pub fn rotate_local_transform(axis: RotationAxis, theta: f32) { + ROTATE_LOCAL_XFORM.write_message((axis, theta)); +} + +fn update_local_rotation_by_ui( + r_selection: Option>, + q_parents: Query<&Parent>, + q_global_xforms: Query<&GlobalTransform>, + mut q_xforms: Query<&mut Transform>, +) { + let Some((axis, theta)) = ROTATE_LOCAL_XFORM.take_message() else { + return; + }; + + let Some(selection) = r_selection.map(|selection| **selection) else { + return; + }; + + let Ok(target_global_xform) = q_global_xforms.get(selection).map(|xform| xform.affine()) else { + return; + }; + + let parent_global_xform = q_parents + .get(selection) + .and_then(|parent| q_global_xforms.get(**parent)) + .unwrap_or(&GlobalTransform::IDENTITY) + .affine(); + + let axis_ws = match axis { + RotationAxis::X => target_global_xform.matrix3.x_axis, + RotationAxis::Y => target_global_xform.matrix3.y_axis, + RotationAxis::Z => target_global_xform.matrix3.z_axis, + }; + + let axis_parent_space = parent_global_xform + .inverse() + .transform_vector3a(axis_ws) + .normalize(); + + if let Ok(mut xform) = q_xforms.get_mut(selection) { + xform.rotate_axis(axis_parent_space.into(), theta); + } +}