Skip to content

Improve zoom clarity by removing default blur and rendering preview at device-pixel resolution#8

Merged
iamtatenda merged 24 commits into
mainfrom
copilot/fix-zoom-quality-issue
Apr 10, 2026
Merged

Improve zoom clarity by removing default blur and rendering preview at device-pixel resolution#8
iamtatenda merged 24 commits into
mainfrom
copilot/fix-zoom-quality-issue

Conversation

Copilot AI commented Apr 8, 2026

Copy link
Copy Markdown
  • Review new actionable review-thread comment and identify required update
  • Run baseline validation for current branch state
  • Ensure canvas metrics sync runs before first render to prevent 1x1 fallback flash
  • Re-run targeted validation
  • Run final parallel validation
  • Reply to the new PR comment with commit hash and UI screenshot

Copilot AI and others added 5 commits April 8, 2026 12:03
Agent-Logs-Url: https://github.com/iamtatenda/Focra/sessions/61d6d2f5-380b-4d3a-ba37-e7598c439ba7

Co-authored-by: iamtatenda <120183671+iamtatenda@users.noreply.github.com>
Agent-Logs-Url: https://github.com/iamtatenda/Focra/sessions/61d6d2f5-380b-4d3a-ba37-e7598c439ba7

Co-authored-by: iamtatenda <120183671+iamtatenda@users.noreply.github.com>
Agent-Logs-Url: https://github.com/iamtatenda/Focra/sessions/61d6d2f5-380b-4d3a-ba37-e7598c439ba7

Co-authored-by: iamtatenda <120183671+iamtatenda@users.noreply.github.com>
Agent-Logs-Url: https://github.com/iamtatenda/Focra/sessions/61d6d2f5-380b-4d3a-ba37-e7598c439ba7

Co-authored-by: iamtatenda <120183671+iamtatenda@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR aims to improve perceived zoom/preview clarity in the editor and during zoom interactions by changing motion blur defaults, increasing preview rendering resolution, tuning blur application, and increasing desktop source thumbnail resolution.

Changes:

  • Default motionBlur to false for auto-generated and manually-added zoom keyframes.
  • Render the editor preview canvas at CSS size × devicePixelRatio and enable high-quality image smoothing.
  • Increase desktop source thumbnail capture size for clearer source selection previews.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
src/renderer/components/editor/ZoomEditor.tsx Changes newly added zoom keyframes to default motionBlur: false.
src/renderer/components/editor/VideoPreview.tsx Resizes canvas backing buffer to match displayed size × DPR; adjusts blur calculation and removes fixed canvas dimensions.
src/main/recorder.ts Increases desktopCapturer thumbnail size and updates auto-zoom defaults to motionBlur: false.

Comment on lines +78 to +85
const rect = canvas.getBoundingClientRect()
const dpr = window.devicePixelRatio
const targetWidth = Math.max(FALLBACK_CANVAS_DIMENSION, Math.round(rect.width * dpr))
const targetHeight = Math.max(FALLBACK_CANVAS_DIMENSION, Math.round(rect.height * dpr))
if (canvas.width !== targetWidth || canvas.height !== targetHeight) {
canvas.width = targetWidth
canvas.height = targetHeight
}

Copilot AI Apr 8, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The canvas is resized using getBoundingClientRect() inside renderFrame(), which will run every animation frame while playing. Reading layout each frame can force synchronous reflow and hurt playback performance; consider moving the resize logic to a ResizeObserver/window resize handler (or caching rect + only re-measuring when needed) and keep renderFrame() focused on drawing.

Copilot uses AI. Check for mistakes.
Comment on lines 87 to 88
const W = canvas.width
const H = canvas.height

Copilot AI Apr 8, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After switching the backing buffer to CSS × devicePixelRatio, all drawing now happens in device pixels (W/H), but sizes like fontSize, strokeWidth, shadowBlur, and the hard-coded padding = 40 remain in the old coordinate system. On HiDPI displays this will make text/lines/padding appear smaller by ~1/dpr. Consider scaling the context (e.g., ctx.setTransform(dpr, 0, 0, dpr, 0, 0)) and then drawing in CSS pixels, or multiply all size constants by dpr to keep the preview visually consistent.

Suggested change
const W = canvas.width
const H = canvas.height
const W = Math.max(FALLBACK_CANVAS_DIMENSION, rect.width)
const H = Math.max(FALLBACK_CANVAS_DIMENSION, rect.height)
ctx.setTransform(dpr, 0, 0, dpr, 0, 0)

Copilot uses AI. Check for mistakes.
Comment thread src/main/recorder.ts
Comment on lines 29 to 33
export async function getDesktopSources() {
const sources = await desktopCapturer.getSources({
types: ['window', 'screen'],
thumbnailSize: { width: 320, height: 180 }
thumbnailSize: { width: 960, height: 540 }
})

Copilot AI Apr 8, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bumping thumbnailSize from 320x180 to 960x540 will significantly increase CPU/memory usage and the IPC payload size (you convert every thumbnail to a base64 data URL). If this is only for a small on-screen preview, consider a smaller size, using JPEG (nativeImage.toJPEG(quality)) to reduce transfer size, or lazily requesting higher-res thumbnails only for the selected source.

Copilot uses AI. Check for mistakes.
@iamtatenda

Copy link
Copy Markdown
Collaborator

@copilot apply changes based on the comments in this thread

Copilot AI and others added 2 commits April 8, 2026 12:29

Copilot AI commented Apr 8, 2026

Copy link
Copy Markdown
Author

@copilot apply changes based on the comments in this thread

Implemented the requested review-thread updates in ab7e440 (plus e9715b7): canvas sizing now uses cached metrics via ResizeObserver/resize sync (no per-frame layout reads), drawing stays in CSS-space with DPR transform for HiDPI consistency, and source thumbnails were reduced to 640x360 with constants to balance quality/perf. Screenshot: UI preview after fixes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.

Comment on lines +274 to +283
let resizeObserver: ResizeObserver | null = null
if (canvas && typeof ResizeObserver !== 'undefined') {
resizeObserver = new ResizeObserver(() => {
syncCanvasMetrics()
renderSingleFrame()
})
resizeObserver.observe(canvas)
}
window.addEventListener('resize', syncCanvasMetrics)

Copilot AI Apr 9, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When ResizeObserver is unavailable, the window resize handler only calls syncCanvasMetrics(). If the canvas backing size changes, setting canvas.width/height clears the canvas, but no redraw is triggered, so the preview can go blank until the next play/seek/pause event. Consider updating the resize handler to also call renderSingleFrame() (or wrap both in a single handler) so paused previews stay visible after window resizes in environments without ResizeObserver (e.g. some test environments).

Copilot uses AI. Check for mistakes.
@iamtatenda

Copy link
Copy Markdown
Collaborator

@copilot apply changes based on the comments in this thread

Copilot AI and others added 2 commits April 9, 2026 15:57
Agent-Logs-Url: https://github.com/iamtatenda/Focra/sessions/604aeea3-4afd-4095-a491-cb196f0c617a

Co-authored-by: iamtatenda <120183671+iamtatenda@users.noreply.github.com>
Agent-Logs-Url: https://github.com/iamtatenda/Focra/sessions/604aeea3-4afd-4095-a491-cb196f0c617a

Co-authored-by: iamtatenda <120183671+iamtatenda@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.

Comment on lines +246 to +252
const rect = currentCanvas.getBoundingClientRect()
const cssWidth = Math.max(FALLBACK_CANVAS_DIMENSION, rect.width)
const cssHeight = Math.max(FALLBACK_CANVAS_DIMENSION, rect.height)
const devicePixelRatio = window.devicePixelRatio
const pixelWidth = Math.max(FALLBACK_CANVAS_DIMENSION, Math.ceil(rect.width * devicePixelRatio))
const pixelHeight = Math.max(FALLBACK_CANVAS_DIMENSION, Math.ceil(rect.height * devicePixelRatio))
const previousMetrics = canvasMetricsRef.current

Copilot AI Apr 10, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

syncCanvasMetrics stores cssWidth/cssHeight from getBoundingClientRect() (often fractional) but derives pixelWidth/pixelHeight via Math.ceil(css * devicePixelRatio). This can make scaleX/scaleY = canvas.width / W slightly different from devicePixelRatio (e.g. 667/333.33), which reintroduces subtle resampling/blur—the opposite of the goal here.

Consider deriving the CSS-space metrics from the backing size (e.g. set cssWidth = pixelWidth / devicePixelRatio and cssHeight = pixelHeight / devicePixelRatio, or otherwise ensuring scaleX/scaleY equals the intended DPR) so 1 CSS px maps to an integer number of backing pixels whenever possible.

Copilot uses AI. Check for mistakes.
@iamtatenda

Copy link
Copy Markdown
Collaborator

@copilot apply changes based on the comments in this thread

Copilot AI and others added 2 commits April 10, 2026 07:24

Copilot AI commented Apr 10, 2026

Copy link
Copy Markdown
Author

@copilot apply changes based on the comments in this thread

Applied in 31d6757 (initial alignment in 10921a6): canvas metric sync now applies fallback in CSS space, computes backing pixels from that (ceil(css * DPR)), and stores CSS metrics as pixel / DPR so transform scaling stays DPR-aligned without reintroducing subtle blur. Screenshot: UI preview

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.

Comment on lines 236 to +240
}, [project, currentTime, videoRef])

useEffect(() => {
const video = videoRef.current
const canvas = canvasRef.current

Copilot AI Apr 10, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

useEffect depends on renderFrame, but renderFrame is recreated whenever currentTime (and/or project) changes. That will repeatedly tear down and re-register the ResizeObserver and window.resize listener during playback/seek updates, which can be a noticeable perf hit and can miss resize events between cleanups. Consider making the resize/listener effect independent of currentTime by keeping the latest renderFrame in a ref (or splitting into separate effects), so observers/listeners are registered once per mounted canvas/video element.

Copilot uses AI. Check for mistakes.
Comment on lines +248 to +254
const baseCssWidth = Math.max(FALLBACK_CANVAS_DIMENSION, rect.width)
const baseCssHeight = Math.max(FALLBACK_CANVAS_DIMENSION, rect.height)
const pixelWidth = Math.ceil(baseCssWidth * devicePixelRatio)
const pixelHeight = Math.ceil(baseCssHeight * devicePixelRatio)
const cssWidth = pixelWidth / devicePixelRatio
const cssHeight = pixelHeight / devicePixelRatio
const previousMetrics = canvasMetricsRef.current

Copilot AI Apr 10, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cssWidth/cssHeight are derived from ceil(rect.* * dpr) / dpr, so W/H used for drawing can differ slightly from the actual on-screen getBoundingClientRect() size due to rounding. This can introduce subtle coordinate mismatches (e.g., annotation placement uses rect.width/height but rendering uses W/H). Consider storing the unrounded rect.width/height as the CSS-space metrics (and only rounding the backing canvas.width/height), or explicitly syncing the canvas CSS size to the rounded cssWidth/cssHeight so input and rendering coordinate spaces stay consistent.

Copilot uses AI. Check for mistakes.
@iamtatenda

Copy link
Copy Markdown
Collaborator

@copilot apply changes based on the comments in this thread

Copilot AI commented Apr 10, 2026

Copy link
Copy Markdown
Author

@copilot apply changes based on the comments in this thread

Applied in f2a66fa: resize/listener setup is now stable via a render-function ref (so observer/window handlers are no longer re-registered on playback time updates), and canvas CSS metrics now keep the unrounded displayed size while only backing pixels are rounded. Screenshot: UI preview

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

src/renderer/components/editor/VideoPreview.tsx:309

  • This comment says the effect re-renders "when currentTime changes", but the effect dependency list is now only [videoRef] so it won't re-run on currentTime updates. Either update the comment to reflect the new behavior (re-render is driven by the separate [renderFrame] effect) or re-introduce the dependency if the intent is still to render on currentTime changes.
    window.addEventListener('resize', handleCanvasResize)

    // Render one frame immediately for initial state / when currentTime changes
    renderSingleFrame()

Comment on lines +239 to +244
useEffect(() => {
renderFrameRef.current = renderFrame
if (animFrameRef.current === 0) {
renderFrameRef.current()
}
}, [renderFrame])

Copilot AI Apr 10, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The effect that syncs renderFrameRef calls renderFrameRef.current() immediately when animFrameRef.current === 0. On initial mount this runs before syncCanvasMetrics() (declared in the next effect), so canvasMetricsRef is still the 1×1 fallback. That leads to negative availW/availH (due to the fixed padding) and an incorrect first render/frame flash. Consider ensuring canvas metrics are initialized before the first render (e.g., move syncCanvasMetrics outside the effect so it can be called here, or gate the initial render until metrics are > padding*2, or convert the metrics sync to a useLayoutEffect that runs before triggering the first draw).

Copilot uses AI. Check for mistakes.
@iamtatenda

Copy link
Copy Markdown
Collaborator

@copilot apply changes based on the comments in this thread

Copilot AI commented Apr 10, 2026

Copy link
Copy Markdown
Author

@copilot apply changes based on the comments in this thread

Applied in 0f12fae: canvas metric initialization now runs in a layout effect, so syncCanvasMetrics() is executed before the first draw and avoids the initial 1×1 fallback-frame flash. Screenshot: UI preview

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 4 changed files in this pull request and generated no new comments.

@iamtatenda
iamtatenda marked this pull request as ready for review April 10, 2026 10:22
@iamtatenda
iamtatenda merged commit a3809c4 into main Apr 10, 2026
4 checks passed
iamtatenda added a commit that referenced this pull request Apr 15, 2026
Improve zoom clarity by removing default blur and rendering preview at device-pixel resolution
iamtatenda added a commit that referenced this pull request Apr 16, 2026
Improve zoom clarity by removing default blur and rendering preview at device-pixel resolution
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants