Skip to content

Commit de463ca

Browse files
committed
fix: remove double-rounding in viewport offset calculation
Remove Math.round from injectWebviewOverlay to preserve float precision. In getMobileViewPortPosition, derive dimensions from edges and cap to screen bounds to guarantee consistent sums. Made-with: Cursor
1 parent bc8d4c4 commit de463ca

4 files changed

Lines changed: 94 additions & 14 deletions

File tree

packages/image-comparison-core/src/clientSideScripts/injectWebviewOverlay.test.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,40 @@ describe('injectWebviewOverlay', () => {
6767
})
6868
})
6969

70+
it('should preserve float precision with non-integer DPR (Android)', () => {
71+
Object.defineProperty(window, 'devicePixelRatio', {
72+
value: 2.625,
73+
configurable: true,
74+
})
75+
Object.defineProperty(window, 'innerWidth', {
76+
value: 412,
77+
configurable: true,
78+
})
79+
Object.defineProperty(document.documentElement, 'clientHeight', {
80+
value: 363,
81+
configurable: true,
82+
})
83+
84+
injectWebviewOverlay(true)
85+
86+
const overlay = document.querySelector('[data-test="ics-overlay"]') as HTMLDivElement
87+
const event = new window.MouseEvent('click', {
88+
clientX: 206,
89+
clientY: 181,
90+
bubbles: true,
91+
})
92+
overlay.dispatchEvent(event)
93+
94+
const parsedData = JSON.parse(overlay.dataset.icsWebviewData!)
95+
96+
expect(parsedData).toEqual({
97+
x: 206 * 2.625,
98+
y: 181 * 2.625,
99+
width: 412 * 2.625,
100+
height: 363 * 2.625,
101+
})
102+
})
103+
70104
it('should use DPR = 1 for iOS (isAndroid = false)', () => {
71105
injectWebviewOverlay(false)
72106

packages/image-comparison-core/src/clientSideScripts/injectWebviewOverlay.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,10 @@ export function injectWebviewOverlay(isAndroid: boolean): void {
2121
overlay.onclick = (event) => {
2222
const { clientX: x, clientY: y } = event
2323
const data = {
24-
x: Math.round(x * dpr),
25-
y: Math.round(y * dpr),
26-
width: Math.round(window.innerWidth * dpr),
27-
height: Math.round(document.documentElement.clientHeight * dpr),
24+
x: x * dpr,
25+
y: y * dpr,
26+
width: window.innerWidth * dpr,
27+
height: document.documentElement.clientHeight * dpr,
2828
}
2929

3030
overlay.dataset.icsWebviewData = JSON.stringify(data)

packages/image-comparison-core/src/helpers/utils.test.ts

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -691,6 +691,49 @@ describe('utils', () => {
691691
expect(result).toMatchSnapshot()
692692
})
693693

694+
it('should handle float overlay values from non-integer DPR and ensure consistent dimensions', async () => {
695+
const dpr = 2.625
696+
const screenW = 1080
697+
const screenH = 2424
698+
699+
const cssClickX = 206
700+
const cssClickY = 385
701+
const cssWidth = 412
702+
const cssHeight = 363
703+
704+
vi.mocked(mockBrowserInstance.execute)
705+
.mockResolvedValueOnce(undefined) // loadBase64Html
706+
.mockResolvedValueOnce(undefined) // checkMetaTag (iOS)
707+
.mockResolvedValueOnce(undefined) // injectWebviewOverlay
708+
.mockResolvedValueOnce(undefined) // executeNativeClick
709+
.mockResolvedValueOnce({
710+
x: cssClickX * dpr,
711+
y: cssClickY * dpr,
712+
width: cssWidth * dpr,
713+
height: cssHeight * dpr,
714+
}) // getMobileWebviewClickAndDimensions (floats, not rounded)
715+
716+
const result = await getMobileViewPortPosition({
717+
browserInstance: mockBrowserInstance,
718+
...baseOptions,
719+
screenHeight: screenH,
720+
screenWidth: screenW,
721+
})
722+
723+
const viewportTop = Math.max(0, Math.round(screenH / 2 - cssClickY * dpr))
724+
const viewportLeft = Math.max(0, Math.round(screenW / 2 - cssClickX * dpr))
725+
const viewportWidth = Math.min(Math.round(cssWidth * dpr), screenW - viewportLeft)
726+
const viewportHeight = Math.min(Math.round(cssHeight * dpr), screenH - viewportTop)
727+
728+
expect(result.viewport.y).toBe(viewportTop)
729+
expect(result.viewport.x).toBe(viewportLeft)
730+
expect(result.viewport.width).toBe(viewportWidth)
731+
expect(result.viewport.height).toBe(viewportHeight)
732+
expect(result.statusBarAndAddressBar.height).toBe(viewportTop)
733+
expect(result.viewport.y + result.viewport.height + result.bottomBar.height).toBe(screenH)
734+
expect(result.viewport.x + result.viewport.width + result.rightSidePadding.width).toBe(screenW)
735+
})
736+
694737
it('should return initialDeviceRectangles if not WebView (native context)', async () => {
695738
const result = await getMobileViewPortPosition({
696739
browserInstance: mockBrowserInstance,

packages/image-comparison-core/src/helpers/utils.ts

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -526,21 +526,24 @@ export async function getMobileViewPortPosition({
526526
const { y, x, width, height } = await browserInstance.execute(getMobileWebviewClickAndDimensions, '[data-test="ics-overlay"]')
527527
// 4.b reset the url
528528
await browserInstance.url(currentUrl)
529-
// 5. Calculate the position of the viewport based on the click position of the native click vs the overlay
529+
// 5. Calculate the position of the viewport based on the click position of the native click vs the overlay.
530+
// The overlay values may be floats (CSS pixels * DPR for Android, CSS pixels for iOS).
531+
// We round edges once and derive dimensions from edges to guarantee
532+
// viewportTop + viewportHeight + bottomBarHeight === screenHeight (no gaps).
530533
const viewportTop = Math.max(0, Math.round(nativeClickY - y))
531534
const viewportLeft = Math.max(0, Math.round(nativeClickX - x))
532-
const statusBarAndAddressBarHeight = Math.max(0, Math.round(viewportTop))
533-
const bottomBarHeight = Math.max(0, Math.round(screenHeight - (viewportTop + height)))
534-
const leftSidePaddingWidth = Math.max(0, Math.round(viewportLeft))
535-
const rightSidePaddingWidth = Math.max(0, Math.round(screenWidth - (viewportLeft + width)))
535+
const viewportWidth = Math.min(Math.round(width), screenWidth - viewportLeft)
536+
const viewportHeight = Math.min(Math.round(height), screenHeight - viewportTop)
537+
const bottomBarHeight = Math.max(0, screenHeight - viewportTop - viewportHeight)
538+
const rightSidePaddingWidth = Math.max(0, screenWidth - viewportLeft - viewportWidth)
536539
const deviceRectangles = {
537540
...initialDeviceRectangles,
538-
bottomBar: { y: viewportTop + height, x: 0, width: screenWidth, height: bottomBarHeight },
539-
leftSidePadding: { y: viewportTop, x: 0, width: leftSidePaddingWidth, height: height },
540-
rightSidePadding: { y: viewportTop, x: viewportLeft + width, width: rightSidePaddingWidth, height: height },
541+
bottomBar: { y: viewportTop + viewportHeight, x: 0, width: screenWidth, height: bottomBarHeight },
542+
leftSidePadding: { y: viewportTop, x: 0, width: viewportLeft, height: viewportHeight },
543+
rightSidePadding: { y: viewportTop, x: viewportLeft + viewportWidth, width: rightSidePaddingWidth, height: viewportHeight },
541544
screenSize: { height: screenHeight, width: screenWidth },
542-
statusBarAndAddressBar: { y: 0, x: 0, width: screenWidth, height: statusBarAndAddressBarHeight },
543-
viewport: { y: viewportTop, x: viewportLeft, width: width, height: height },
545+
statusBarAndAddressBar: { y: 0, x: 0, width: screenWidth, height: viewportTop },
546+
viewport: { y: viewportTop, x: viewportLeft, width: viewportWidth, height: viewportHeight },
544547
}
545548

546549
return deviceRectangles

0 commit comments

Comments
 (0)