-
Notifications
You must be signed in to change notification settings - Fork 10.5k
fix(annotation): keep preview marks on the artifact region the user selected at any UI zoom #6476
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
linghaoSu
wants to merge
20
commits into
nexu-io:main
Choose a base branch
from
linghaoSu:fix/6361-annotation-zoom-coords
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 4 commits
Commits
Show all changes
20 commits
Select commit
Hold shift + click to select a range
1a7fb5a
fix(annotation): keep preview marks on the artifact they were drawn on
linghaoSu 677fa56
test(annotation): update Draw render-mode expectation to the anchor c…
linghaoSu 6e2ba8f
fix(annotation): read mark bounds in frame layout space, not the scal…
linghaoSu fbf371b
fix(annotation): address review — URL anchor bridge, retryable probe,…
linghaoSu 8345bab
fix(annotation): probe anchors on the active frame; make probe give-u…
linghaoSu c5765ba
fix(annotation): drop stale anchor replies; keep bridge failures unan…
linghaoSu f284004
fix(annotation): bridge large streamed HTML; supersede stale probes b…
linghaoSu cbd438d
fix(annotation): invalidate probes on document load, filter invisible…
linghaoSu b716c74
fix(annotation): pre-capture sync joins the in-flight probe chain
linghaoSu 11a04bd
fix(annotation): dragged labels drop their stale anchor; suffix joins…
linghaoSu 07ccc96
fix(annotation): freeze anchor writes through capture; suffix-aware I…
linghaoSu fe12b0e
fix(annotation): capture freeze covers the resize re-anchor and the b…
linghaoSu 6e59b71
ci: retrigger after UI P0 infra flake
linghaoSu a6260db
fix(annotation): defer the inactive cleanup while a send is in flight
linghaoSu 12b53fc
fix(annotation): queue a trailing pass when a stale probe reply is di…
linghaoSu eec0415
fix(annotation): sanitize bridge anchor replies; lean bounded probe e…
linghaoSu c953192
fix(annotation): refuse oversized forged anchor replies before iterating
linghaoSu e92affd
Merge remote-tracking branch 'upstream/main' into pr6476
linghaoSu b770034
test(daemon): close keep-alive sockets before ending the raw-range suite
linghaoSu 65521e4
Merge remote-tracking branch 'upstream/main' into pr6476
linghaoSu File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,76 @@ | ||
| import { describe, expect, test } from 'vitest'; | ||
|
|
||
| import { parseCaptureClip } from '../../src/main/runtime.js'; | ||
|
|
||
| // Issue #6361: the renderer measures the preview frame with | ||
| // getBoundingClientRect() — CSS pixels — but Electron's capturePage() clips in | ||
| // DIP page coordinates. The two spaces coincide only at zoom factor 1. Without | ||
| // the conversion a mark made inside the preview at a non-100% zoom captured a | ||
| // region shifted up/left off the artifact, so the annotation PNG and the | ||
| // structured position handed to the agent described different pixels. | ||
| // | ||
| // Measured on macOS/HiDPI at zoom 1.095 (Cmd + once), marking a 40px band: | ||
| // frame rect (CSS px) {x:468, y:148, w:692, h:666} | ||
| // returned bitmap 1384 × 1332 == 692 × 666 × 2.0 | ||
| // but devicePixelRatio 2.1909 (== 2 × 1.095) | ||
| // The bitmap being exactly 2.0× the CSS rect — not 2.1909× — is the proof that | ||
| // Electron consumed the numbers as DIP. The mark landed one band high: the red | ||
| // box painted at rows 683–765 while the marked band occupied rows 777–864. | ||
| const PREVIEW_FRAME = { x: 420, y: 96, width: 1000, height: 600 }; | ||
|
|
||
| describe('parseCaptureClip zoom conversion', () => { | ||
| test('100% zoom is identity', () => { | ||
| expect(parseCaptureClip({ clip: PREVIEW_FRAME }, 1)).toEqual(PREVIEW_FRAME); | ||
| }); | ||
|
|
||
| test('125% zoom scales origin and size together', () => { | ||
| expect(parseCaptureClip({ clip: PREVIEW_FRAME }, 1.25)).toEqual({ | ||
| x: 525, | ||
| y: 120, | ||
| width: 1250, | ||
| height: 750, | ||
| }); | ||
| }); | ||
|
|
||
| test('150% zoom', () => { | ||
| expect(parseCaptureClip({ clip: PREVIEW_FRAME }, 1.5)).toEqual({ | ||
| x: 630, | ||
| y: 144, | ||
| width: 1500, | ||
| height: 900, | ||
| }); | ||
| }); | ||
|
|
||
| test('80% zoom', () => { | ||
| expect(parseCaptureClip({ clip: PREVIEW_FRAME }, 0.8)).toEqual({ | ||
| x: 336, | ||
| y: 77, | ||
| width: 800, | ||
| height: 480, | ||
| }); | ||
| }); | ||
|
|
||
| test('the clip stays inside the frame it was measured from at every zoom', () => { | ||
| // The failure users saw was the clip drifting *out* of the preview frame. | ||
| // At any zoom the converted clip must be exactly the frame in DIP space. | ||
| for (const zoom of [0.8, 1, 1.25, 1.5, 2]) { | ||
| const clip = parseCaptureClip({ clip: PREVIEW_FRAME }, zoom)!; | ||
| expect(clip.x / zoom).toBeCloseTo(PREVIEW_FRAME.x, 0); | ||
| expect(clip.y / zoom).toBeCloseTo(PREVIEW_FRAME.y, 0); | ||
| expect(clip.width / zoom).toBeCloseTo(PREVIEW_FRAME.width, 0); | ||
| expect(clip.height / zoom).toBeCloseTo(PREVIEW_FRAME.height, 0); | ||
| } | ||
| }); | ||
|
|
||
| test('defaults to identity when the zoom factor is missing or nonsensical', () => { | ||
| expect(parseCaptureClip({ clip: PREVIEW_FRAME })).toEqual(PREVIEW_FRAME); | ||
| expect(parseCaptureClip({ clip: PREVIEW_FRAME }, 0)).toEqual(PREVIEW_FRAME); | ||
| expect(parseCaptureClip({ clip: PREVIEW_FRAME }, Number.NaN)).toEqual(PREVIEW_FRAME); | ||
| }); | ||
|
|
||
| test('invalid payloads still yield a full-page capture', () => { | ||
| expect(parseCaptureClip(null, 1.25)).toBeUndefined(); | ||
| expect(parseCaptureClip({}, 1.25)).toBeUndefined(); | ||
| expect(parseCaptureClip({ clip: { x: 1, y: 2, width: 'wide', height: 4 } }, 1.25)).toBeUndefined(); | ||
| }); | ||
| }); |
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
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.