Add webcam mask shapes for picture-in-picture overlays#304
Open
justynroberts wants to merge 1 commit intosiddharthvaddem:mainfrom
Open
Add webcam mask shapes for picture-in-picture overlays#304justynroberts wants to merge 1 commit intosiddharthvaddem:mainfrom
justynroberts wants to merge 1 commit intosiddharthvaddem:mainfrom
Conversation
📝 WalkthroughWalkthroughThis pull request adds a Changes
Sequence DiagramsequenceDiagram
actor User
participant SettingsPanel
participant VideoEditor
participant EditorHistory
participant VideoPlayback
participant LayoutUtils
participant CompositeLayout
participant FrameRenderer
User->>SettingsPanel: Select webcam shape
SettingsPanel->>VideoEditor: onWebcamMaskShapeChange(shape)
VideoEditor->>EditorHistory: pushState({webcamMaskShape})
EditorHistory->>VideoEditor: Update editor state
VideoEditor->>VideoPlayback: Pass webcamMaskShape prop
VideoPlayback->>LayoutUtils: layoutVideoContentUtil({webcamMaskShape})
LayoutUtils->>CompositeLayout: computeCompositeLayout({webcamMaskShape})
CompositeLayout->>CompositeLayout: resolveWebcamMaskShape()
CompositeLayout->>CompositeLayout: Calculate shape-specific dimensions
CompositeLayout-->>LayoutUtils: WebcamCompositeLayout with shape
LayoutUtils-->>VideoPlayback: Layout result with webcamRect
VideoPlayback->>VideoPlayback: Apply CSS borderRadius based on shape
VideoPlayback-->>User: Render webcam with updated shape
VideoEditor->>FrameRenderer: Pass webcamMaskShape to exporter
FrameRenderer->>CompositeLayout: computeCompositeLayout({webcamMaskShape})
CompositeLayout-->>FrameRenderer: Layout with shape applied
FrameRenderer-->>User: Export with correct shape rendering
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/exporter/frameRenderer.ts (1)
671-701:⚠️ Potential issue | 🟠 MajorCircle mask shape not rendered in export.
The
compositeWithShadowsmethod always usesctx.roundRect()for the webcam mask (lines 676-682) without checkingwebcamRect.shape. The preview renders circles correctly using CSS (borderRadius: 50%), but the export canvas rendering ignores the shape property, preventing circle masks from being applied during video/GIF export.ctx.roundRect( webcamRect.x, webcamRect.y, webcamRect.width, webcamRect.height, webcamRect.borderRadius, );When
webcamMaskShapeis"circle", the shape should be rendered using canvas arc operations or alternative clipping path instead ofroundRect.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/exporter/frameRenderer.ts` around lines 671 - 701, The compositeWithShadows block always draws the webcam mask with ctx.roundRect, ignoring webcamRect.shape (or webcamMaskShape) so circle masks aren't exported; update the rendering to branch on the mask shape inside compositeWithShadows (or the method that contains the shown code) and when the shape is "circle" build the clipping path with arc/ellipse (center = webcamRect.x + width/2, webcamRect.y + height/2; radius = Math.min(width,height)/2) (or ctx.ellipse) instead of ctx.roundRect, apply the same shadow/fill/clip logic used for rounded-rects, and fall back to the existing ctx.roundRect for other shapes; reference webcamRect, ctx.roundRect, and getWebcamLayoutPresetDefinition to locate and implement the change.
🧹 Nitpick comments (1)
src/lib/compositeLayout.test.ts (1)
36-78: Consider adding a portrait vertical-stack test case as the primary scenario.Current assertions validate a degenerate landscape stack geometry (e.g., zero-height screen area). That’s fine as an edge case, but adding a portrait case would better protect the expected user flow.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/compositeLayout.test.ts` around lines 36 - 78, The current tests for computeCompositeLayout only cover a degenerate landscape "vertical-stack" arrangement; add a new test case that uses a portrait canvas (e.g., canvasSize {width:1080, height:1920}) and both screenSize and webcamSize present to assert the normal vertical-stack behavior (non-zero screenRect and webcamRect heights, proper x/y positions, widths matching canvas width, and expected borderRadius/shape on webcamRect). Locate the test suite in compositeLayout.test.ts and add a test (named e.g., "fills the canvas with the combined screen and webcam stack in portrait vertical-stack mode") that calls computeCompositeLayout with layoutPreset: "vertical-stack" and checks screenRect and webcamRect are non-null and have sensible portrait split dimensions and positioning.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/lib/exporter/frameRenderer.ts`:
- Around line 671-701: The compositeWithShadows block always draws the webcam
mask with ctx.roundRect, ignoring webcamRect.shape (or webcamMaskShape) so
circle masks aren't exported; update the rendering to branch on the mask shape
inside compositeWithShadows (or the method that contains the shown code) and
when the shape is "circle" build the clipping path with arc/ellipse (center =
webcamRect.x + width/2, webcamRect.y + height/2; radius =
Math.min(width,height)/2) (or ctx.ellipse) instead of ctx.roundRect, apply the
same shadow/fill/clip logic used for rounded-rects, and fall back to the
existing ctx.roundRect for other shapes; reference webcamRect, ctx.roundRect,
and getWebcamLayoutPresetDefinition to locate and implement the change.
---
Nitpick comments:
In `@src/lib/compositeLayout.test.ts`:
- Around line 36-78: The current tests for computeCompositeLayout only cover a
degenerate landscape "vertical-stack" arrangement; add a new test case that uses
a portrait canvas (e.g., canvasSize {width:1080, height:1920}) and both
screenSize and webcamSize present to assert the normal vertical-stack behavior
(non-zero screenRect and webcamRect heights, proper x/y positions, widths
matching canvas width, and expected borderRadius/shape on webcamRect). Locate
the test suite in compositeLayout.test.ts and add a test (named e.g., "fills the
canvas with the combined screen and webcam stack in portrait vertical-stack
mode") that calls computeCompositeLayout with layoutPreset: "vertical-stack" and
checks screenRect and webcamRect are non-null and have sensible portrait split
dimensions and positioning.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 47ffeca1-455e-4ec6-99fa-b89bd476b53d
📒 Files selected for processing (16)
src/components/video-editor/SettingsPanel.tsxsrc/components/video-editor/VideoEditor.tsxsrc/components/video-editor/VideoPlayback.tsxsrc/components/video-editor/projectPersistence.test.tssrc/components/video-editor/projectPersistence.tssrc/components/video-editor/types.tssrc/components/video-editor/videoPlayback/layoutUtils.tssrc/hooks/useEditorHistory.tssrc/i18n/locales/en/settings.jsonsrc/i18n/locales/es/settings.jsonsrc/i18n/locales/zh-CN/settings.jsonsrc/lib/compositeLayout.test.tssrc/lib/compositeLayout.tssrc/lib/exporter/frameRenderer.tssrc/lib/exporter/gifExporter.tssrc/lib/exporter/videoExporter.ts
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
1 participant
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.
Summary
Adds configurable webcam mask shapes for picture-in-picture overlays.
This introduces a new webcam mask shape setting with support for Rounded Rectangle and Circle.
The selected shape now stays consistent across live editor preview, video export, GIF export, and saved project state.
Vertical stack remains unchanged and continues to use the existing rectangular treatment.
Changes
Validation
Notes
Summary by CodeRabbit
Release Notes