fix(mcp): frame_shot aims the live camera it places (#86) - #91
Merged
Conversation
HaD0Yun
force-pushed
the
fix/issue-86
branch
from
September 3, 2026 13:43
effbc19 to
fca63d8
Compare
frame_shot sent position and lens only, so a connected editor kept whatever orientation the last human gesture left behind. Only `front` still framed the subject — the other four views orbited the lens away and dropped the subject out of frame, behind the camera plane from profile round to back, while describe_shot kept reporting "the subject fills 98% of frame height" because the shot vocabulary is derived from distance and lens on the assumption that the lens points at the framing pivot. set_camera now carries an optional lookAtX/lookAtY/lookAtZ triple (additive to live protocol v1; an editor that ignores it degrades to the old position-only behaviour), frame_shot always sends the framing pivot the vocabulary is measured against, and the editor writes the resulting yaw/pitch into look.current as well as the camera — that ref is the orientation of record, so commitManualCameraFraming now measures the framing it just applied instead of the one the last gesture left. The regression is asserted twice: verify-live-editor-model gates in CI without pixels (the shot camera's forward vector must point at the pivot within 2deg after a profile frame_shot), and verify-live-capture renders all five views and requires visible pixels with the subject in front of the lens.
Keeps the before/after 5-view visibility measurements next to the fix: the regression suites assert the invariant, this records what a real editor on hardware GL actually rendered before and after, so the numbers can be re-derived without re-running the browser suites.
…remount (#86) A cast model whose FBX is still downloading suspends the R3F scene graph: the shot camera is unmounted, and when the mesh lands React remounts every sibling rig on a fresh camera object. Two things went wrong across that gap. The live set_camera handler wrote to shotCamRef.current, which was null, so the position and aim were dropped; and ShotRig's preset effect ran again on mount and seeded the medium preset over whatever was there. On a slow runner (CI, cold cache) frame_shot therefore landed in the gap and the editor came back on the default camera, 57deg off the subject it had just been told to frame. The preset is now applied once per (preset, nonce) using a stamp held in App, and the shot camera's last position is kept in an App-level ref that the live handler writes even when the camera is unmounted; a remount restores that position and look.current instead of re-seeding. verify-live-editor-model holds the x-bot FBX at the network layer, frames the shot while the canvas is suspended, releases it and only then reads the camera, so the runner's timing is the test's timing.
HaD0Yun
force-pushed
the
fix/issue-86
branch
from
September 3, 2026 14:25
fca63d8 to
5e01f52
Compare
|
Read through this — nice work, and thanks for picking it up. You actually took it further than my patch: exposing |
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.
Closes #86.
frame_shotsent position and lens only; a connected editor kept whatever orientation the last gesture left, so every view exceptfrontlost the subject whiledescribe_shotstill reported "98% of frame height".Fix
set_cameragains an optional, additivelookAtX/lookAtY/lookAtZtriple (complete or absent; documented inmcp/LIVE-PROTOCOL.md, no version bump).frame_shotalways sends the framing pivot the shot vocabulary is measured against (FRAMING_PIVOT_Ynamed). Theset_cameraMCP tool accepts optionallook_at_x/y/zand forwards them only when all three are given.set_camerahandler: when aimed, writes yaw/pitch intolook.current(the orientation of record) and the camera rotation, socommitManualCameraFramingmeasures the framing it just applied.Tests (RED on main, GREEN here)
mcp/verify-live.mjs:frame_shotmust forward a finitelookAt*equal to the subject pivot (node tier, gatesnpm test).mcp/verify-live-editor-model.mjs(CI mcp-live gating): afterframe_shot profile, the shot camera forward vector points at the pivot within 2 deg (was 86.4 deg off).mcp/verify-live-capture.mjs: all five views reportvisiblePixelCount > 0andbehindCameraPlane === false(before: 0 px on four views, behind the plane on three).cd mcp && npm run verify: 420 framing combinations still pass.Thanks to @popixoxipop-collab for the precise report and reference patch.