fix: apply orientation attribute to <extra-model> elements#5177
Merged
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
`updateModelTransforms` accepted an `orientation` parameter but never used it, so `<extra-model orientation="...">` had no effect. Parse the value with `parseExpressions`/`normalizeUnit` (supporting `deg`/`rad` units) and apply it as an Euler rotation on the model's quaternion, using the same `$roll $pitch $yaw` → `Euler(pitch, yaw, roll, 'YXZ')` convention as `applyTransform` and the documented `orientation` attribute. Also fix the multimodel example: bare numbers are treated as radians by the unit parser, and the axis ordering follows roll/pitch/yaw, so update the orientation attributes to `"0deg 0deg 30deg"` / `"0deg 0deg -30deg"` (yaw = third term) to produce the intended 30° Y-axis rotation. Add a test covering both initial application and dynamic updates of the orientation attribute.
Frank3K
force-pushed
the
fix/extra-model-orientation
branch
from
5ff1d0a to
bccc603
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
The multi-model feature introduced in #5134 is a really nice feature — this PR fixes a small gap in the initial implementation.
<extra-model orientation="...">had no effect. TheupdateModelTransformsmethod inModelScene.tsaccepted anorientationparameter but never applied it — it was prefixed_orientation, signalling intentional non-use. Onlyoffsetandscalewere wired up, leaving models always in their default rotation regardless of the declared attribute.Root cause
updateModelTransforms(source) received the orientation string from theextra-model-changedevent but did not parse or apply it to the model's quaternion.A secondary issue: the multimodel example used bare numbers (
orientation="0 30 0").Changes
packages/model-viewer/src/three-components/ModelScene.ts_orientation→orientationand implemented parsing using the existingparseExpressions/normalizeUnitutilities — the same infrastructure used byapplyTransform.$roll $pitch $yawconvention:model.quaternion.setFromEuler(new Euler(pitch, yaw, roll, 'YXZ')), consistent with the mainorientationattribute behaviour.packages/modelviewer.dev/examples/multimodel/index.htmldegunits and correct roll/pitch/yaw ordering. A 30° yaw (Y-axis) rotation is now"0deg 0deg 30deg"(third term), matching the documented$roll $pitch $yawformat.Before / After
Before
After
Robot and Astronaut now rotate around their Y-axis as declared, facing slightly toward the scene center.
Testing
Verified visually in the multimodel example page. The robot (left,
orientation="0deg 0deg 30deg") and the astronaut (right,orientation="0deg 0deg -30deg") each rotate 30° around the Y-axis toward the center.