[GenAI] [DRAFT] Add a offset slider to modify the placement of the video#1959
[GenAI] [DRAFT] Add a offset slider to modify the placement of the video#1959boweiliu wants to merge 6 commits into
Conversation
Co-authored-by: Sculptor <sculptor@imbue.com>
Co-authored-by: Sculptor <sculptor@imbue.com>
Co-authored-by: Sculptor <sculptor@imbue.com>
Co-authored-by: Sculptor <sculptor@imbue.com>
Co-authored-by: Sculptor <sculptor@imbue.com>
| build: | ||
| context: ./apps/media-server | ||
| dockerfile: Dockerfile.standalone | ||
| image: cap-media-server:local |
There was a problem hiding this comment.
Replacing the media-server build block with cap-media-server:local makes docker compose up require a local image that this PR does not build or document. A developer using the existing compose workflow can now fail with an image-not-found error instead of Compose building ./apps/media-server/Dockerfile.standalone.
Prompt To Fix With AI
This is a comment left during a code review.
Path: docker-compose.yml
Line: 44
Comment:
**Local Media Image Is Required**
Replacing the `media-server` build block with `cap-media-server:local` makes `docker compose up` require a local image that this PR does not build or document. A developer using the existing compose workflow can now fail with an image-not-found error instead of Compose building `./apps/media-server/Dockerfile.standalone`.
How can I resolve this? If you propose a fix, please make it concise.| cap-web: | ||
| container_name: cap-web | ||
| image: ghcr.io/capsoftware/cap-web:latest | ||
| image: cap-web:local |
There was a problem hiding this comment.
Changing the default cap-web image from the registry tag to cap-web:local makes a normal docker compose up fail unless the user first runs the separate Depot build. The compose file used to pull a public image, so this should live in an override file or keep the registry image as the default path.
Prompt To Fix With AI
This is a comment left during a code review.
Path: docker-compose.yml
Line: 6
Comment:
**Default Web Image Is Local**
Changing the default `cap-web` image from the registry tag to `cap-web:local` makes a normal `docker compose up` fail unless the user first runs the separate Depot build. The compose file used to pull a public image, so this should live in an override file or keep the registry image as the default path.
How can I resolve this? If you propose a fix, please make it concise.| y: project.background.positionOffsetY ?? 0, | ||
| }} | ||
| onChange={(v) => | ||
| batch(() => { | ||
| setProject("background", "positionOffsetX", v.x); | ||
| setProject("background", "positionOffsetY", v.y); | ||
| }) | ||
| } | ||
| /> |
There was a problem hiding this comment.
This new control calls setProject on every raw slider change, but it does not use the editor slider wrapper that pauses project history during a drag. Dragging the offset control can record many tiny intermediate states, so undo only reverts the last tick instead of the whole drag.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/routes/editor/ConfigSidebar.tsx
Line: 2031-2039
Comment:
**Slider Drag Floods History**
This new control calls `setProject` on every raw slider change, but it does not use the editor slider wrapper that pauses project history during a drag. Dragging the offset control can record many tiny intermediate states, so undo only reverts the last tick instead of the whole drag.
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| let room = output_size - target_size; | ||
| let shift_x = (project.background.position_offset_x / 100.0) * room.x; | ||
| let shift_y = (project.background.position_offset_y / 100.0) * room.y; | ||
| let raw_offset = centered_offset + XY::new(shift_x, shift_y); | ||
| let max_x = room.x.max(0.0); | ||
| let max_y = room.y.max(0.0); | ||
| let offset = XY::new( | ||
| raw_offset.x.clamp(0.0, max_x), | ||
| raw_offset.y.clamp(0.0, max_y), | ||
| ); |
There was a problem hiding this comment.
Minor robustness: room can go slightly negative (float rounding / padding clamp). Right now shift_* uses the raw room.* but the clamp uses max_*, which can invert behavior in those edge cases.
| let room = output_size - target_size; | |
| let shift_x = (project.background.position_offset_x / 100.0) * room.x; | |
| let shift_y = (project.background.position_offset_y / 100.0) * room.y; | |
| let raw_offset = centered_offset + XY::new(shift_x, shift_y); | |
| let max_x = room.x.max(0.0); | |
| let max_y = room.y.max(0.0); | |
| let offset = XY::new( | |
| raw_offset.x.clamp(0.0, max_x), | |
| raw_offset.y.clamp(0.0, max_y), | |
| ); | |
| let room_x = (output_size.x - target_size.x).max(0.0); | |
| let room_y = (output_size.y - target_size.y).max(0.0); | |
| let shift_x = (project.background.position_offset_x / 100.0) * room_x; | |
| let shift_y = (project.background.position_offset_y / 100.0) * room_y; | |
| let raw_offset = centered_offset + XY::new(shift_x, shift_y); | |
| let offset = XY::new( | |
| raw_offset.x.clamp(0.0, room_x), | |
| raw_offset.y.clamp(0.0, room_y), | |
| ); |
| let crop = ProjectUniforms::get_crop(options, project); | ||
| let output_size = ProjectUniforms::get_output_size(options, project, resolution_base); | ||
| let padding_offset = ProjectUniforms::display_offset(options, project, resolution_base); | ||
|
|
||
| let output_size = XY::new(output_size.0, output_size.1).map(|v| v as f64); | ||
| let display_size = ProjectUniforms::display_size(options, project, resolution_base); |
There was a problem hiding this comment.
Nit: this now computes the same layout twice. Since display_layout already exists, you can grab both offset + size in one call (also applies in Coord<FrameSpace>::to_zoomed_frame_space).
| let crop = ProjectUniforms::get_crop(options, project); | |
| let output_size = ProjectUniforms::get_output_size(options, project, resolution_base); | |
| let padding_offset = ProjectUniforms::display_offset(options, project, resolution_base); | |
| let output_size = XY::new(output_size.0, output_size.1).map(|v| v as f64); | |
| let display_size = ProjectUniforms::display_size(options, project, resolution_base); | |
| let crop = ProjectUniforms::get_crop(options, project); | |
| let (padding_offset, display_size) = | |
| ProjectUniforms::display_layout(options, project, resolution_base); |
Sorry, probably not satisfying the contributing guidelines but I thought I'd put this out there for folks to make use of if they wish. (I'm running this fork locally).
Enables slides to position the recorded screen recording video inside the exported video frame. I use this to move my landscape screen recording upwards when recording for a vertical video format.
Greptile Summary
This PR adds a background video position offset control for desktop exports. The main changes are:
positionOffsetXandpositionOffsetYbackground config fields.Confidence Score: 4/5
The Docker Compose workflow can fail unless local images are built ahead of time.
cap-webnow requires a separate Depot build before compose can start it.media-serverno longer builds from its Dockerfile and has no documented local-image build step.docker-compose.yml, apps/desktop/src/routes/editor/ConfigSidebar.tsx
Important Files Changed
Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "depot" | Re-trigger Greptile
Context used: