Skip to content
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

fix(auto-edit): Fix low resolution images on low DPI screens #7100

Merged
merged 8 commits into from
Feb 17, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions vscode/src/autoedits/renderer/decorators/default-decorator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -373,8 +373,7 @@ export class DefaultDecorator implements AutoEditsDecorator {
): void {
// Blockify the added lines so they are suitable to be rendered together as a VS Code decoration
const blockifiedAddedLines = blockify(this.editor.document, addedLinesInfo)

const { dark, light } = generateSuggestionAsImage({
const { dark, light, pixelRatio } = generateSuggestionAsImage({
decorations: blockifiedAddedLines,
lang: this.editor.document.languageId,
})
Expand All @@ -390,8 +389,8 @@ export class DefaultDecorator implements AutoEditsDecorator {
position: 'absolute',
// Make sure the decoration is rendered on top of other decorations
'z-index': '9999',
// Scale to decoration to the correct size (upscaled to boost resolution)
scale: '0.5',
// Scale the decoration to the correct size (upscaled to boost resolution)
scale: String(1 / pixelRatio),
'transform-origin': '0px 0px',
height: 'auto',
// The decoration will be entirely taken up by the image.
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
13 changes: 8 additions & 5 deletions vscode/src/autoedits/renderer/image-gen/canvas/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import type {
SyntaxHighlightedTextDecorationRange,
} from '../../decorators/default-decorator'
import type { SYNTAX_HIGHLIGHT_MODE } from '../highlight'
import { type RenderConfig, type UserProvidedRenderConfig, getRenderConfig } from './render-config'
import type { RenderConfig } from './render-config'

type CanvasKitType = Awaited<ReturnType<typeof CanvasKitInit>>
type RenderContext = {
Expand Down Expand Up @@ -138,7 +138,7 @@ function drawDiffColors(
export function drawDecorationsToCanvas(
decorations: AddedLinesDecorationInfo[],
mode: SYNTAX_HIGHLIGHT_MODE,
userConfig: UserProvidedRenderConfig
config: RenderConfig
): EmulatedCanvas2D {
if (!canvasKit || !fontCache) {
// TODO: Log these errors, useful to see if we run into issues where we're not correctly
Expand All @@ -150,7 +150,6 @@ export function drawDecorationsToCanvas(
CanvasKit: canvasKit,
font: fontCache,
}
const config = getRenderConfig(userConfig)

// In order for us to draw to the canvas, we must first determine the correct
// dimensions for the canvas. We can do this with a temporary Canvas that uses the same font
Expand All @@ -173,11 +172,15 @@ export function drawDecorationsToCanvas(
const canvasWidth = Math.min(requiredWidth + config.padding.x, config.maxWidth)
const canvasHeight = tempYPos + config.padding.y

// Round to the nearest pixel, using sub-pixels will cause CanvasKit to crash
Copy link
Member

Choose a reason for hiding this comment

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

Interesting, is it expected? 😃

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope, didn't find many issues online about it either but it's hard to search because CanvasKit is just a small part of Skia which is a much bigger project (used for rendering Chrome/Android UIs etc).

Thankfully the image tests work really well here and help catch any regressions

const height = Math.round(canvasHeight * config.pixelRatio)
const width = Math.round(canvasWidth * config.pixelRatio)

// Now we create the actual canvas, ensuring we scale it accordingly to improve the output resolution.
const { canvas, ctx } = createCanvas(
{
height: canvasHeight * config.pixelRatio,
width: canvasWidth * config.pixelRatio,
height,
width,
fontSize: config.fontSize,
// We upscale the canvas to improve resolution, this will be brought back to the intended size
// using the `scale` CSS property when the decoration is rendered.
Expand Down
65 changes: 60 additions & 5 deletions vscode/src/autoedits/renderer/image-gen/canvas/render-config.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import { isMacOS } from '@sourcegraph/cody-shared'
import * as vscode from 'vscode'
import { localStorage } from '../../../../services/LocalStorageProvider'

/**
* This is the ratio that VS Code uses to automatically determine the line height based on the font size.
Expand All @@ -12,10 +14,60 @@ const GOLDEN_LINE_HEIGHT_RATIO = isMacOS() ? 1.5 : 1.35
*/
const DEFAULT_FONT_SIZE = isMacOS() ? 12 : 14

export function getLineHeight(fontSize: number): number {
/**
* Use a default pixel ratio that works for both high and low DPI screens.
* Note: A pixel ratio is 2 is preferred for high DPI screens, however this
* causes significant blurriness when the image is downscaled on low DPI screens.
*
* This value is significantly preferrable to '2' for low DPI screens. I am unsure
* exactly why this is the case. It possibly could be an issue with how VS Code handles image scaling.
* You can see the diference in this PR: https://github.com/sourcegraph/cody/pull/7100
*
* Changes to this value should be manually tested, this is not covered in any CI tests.
* It can be difficult to manually simulate a low-DPI resolution especially on MacOS, unless you have a physical monitor to hand.
* One way to test this:
* 1. Install an Ubuntu VM via UTM, ensure retina mode is disabled in UTM display settings.
* Docs: https://docs.getutm.app/guides/ubuntu/.
* 2. Install VS Code and Cody on the VM.
* 3. Test image decorations, you can confirm the pixel ratio by inspecting `window.devicePixelRatio` in VS Code DevTools.
*/
const DEFAULT_PIXEL_RATIO = 1.95

function getUserLineHeight(fontSize: number): number {
return Math.round(GOLDEN_LINE_HEIGHT_RATIO * fontSize)
}

function getUserFontSize(): number | undefined {
// Extract the font size from VS Code user settings.
// Note: VS Code warns but technically supports string-based font sizes, e.g. "14".
// TODO: Support this for other editors. We should respect the font size in editors like JetBrains.
const userFontSize = Number(vscode.workspace.getConfiguration('editor').get('fontSize'))

if (Number.isNaN(userFontSize) || userFontSize <= 0) {
// We cannot use this font size, we will use a platform specific default
return
}

return userFontSize
}

/**
* In order to generate the most optimal image, we need to know the pixel ratio of the device.
* We cannot get this through Node, we need to interface with the Webview.
* This implementation is a form of progressive enhancement, where we use a suitable default that
* works for both high and low DPI screens. We then use the pixel ratio available from the Webview
* if it becomes available.
*/
function getUserPixelRatio(): number | undefined {
const devicePixelRatio = localStorage.getDevicePixelRatio()
if (!devicePixelRatio) {
// No pixel ratio available. User has not opened a Webview yet.
return
}

return Math.max(devicePixelRatio, 1)
}

/**
* Options to render the auto-edit suggestion to the canvas.
* This should be configurable by the user and/or the client where suitable.
Expand All @@ -38,17 +90,20 @@ export interface RenderConfig {
export interface UserProvidedRenderConfig {
fontSize?: number
lineHeight?: number
pixelRatio?: number
}

export function getRenderConfig(userProvidedConfig: UserProvidedRenderConfig): RenderConfig {
const fontSize = userProvidedConfig.fontSize || DEFAULT_FONT_SIZE
const lineHeight = userProvidedConfig.lineHeight || getLineHeight(fontSize)
export function getRenderConfig(userProvidedConfig?: UserProvidedRenderConfig): RenderConfig {
const pixelRatio = userProvidedConfig?.pixelRatio || getUserPixelRatio() || DEFAULT_PIXEL_RATIO
const fontSize = userProvidedConfig?.fontSize || getUserFontSize() || DEFAULT_FONT_SIZE
const lineHeight = userProvidedConfig?.lineHeight || getUserLineHeight(fontSize)

return {
fontSize,
lineHeight,
padding: { x: 6, y: 2 },
maxWidth: 1200,
pixelRatio: 2,
pixelRatio,
diffHighlightColor: 'rgba(35, 134, 54, 0.2)',
}
}
2 changes: 2 additions & 0 deletions vscode/src/autoedits/renderer/image-gen/index.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { toMatchImageSnapshot } from 'jest-image-snapshot'
import { describe, expect, it } from 'vitest'
import { generateSuggestionAsImage, initImageSuggestionService } from '.'
import { mockLocalStorage } from '../../../services/LocalStorageProvider'
import type {
AddedLinesDecorationInfo,
DiffedTextDecorationRange,
Expand Down Expand Up @@ -45,6 +46,7 @@ async function generateImageForTest(
decorations: AddedLinesDecorationInfo[],
lang: string
): Promise<{ darkBuffer: Buffer; lightBuffer: Buffer }> {
mockLocalStorage()
await initImageSuggestionService()

const { light, dark } = generateSuggestionAsImage({
Expand Down
39 changes: 17 additions & 22 deletions vscode/src/autoedits/renderer/image-gen/index.ts
Original file line number Diff line number Diff line change
@@ -1,27 +1,12 @@
import * as vscode from 'vscode'
import type { AddedLinesDecorationInfo } from '../decorators/default-decorator'
import { drawDecorationsToCanvas, initCanvas } from './canvas'
import type { UserProvidedRenderConfig } from './canvas/render-config'
import { type UserProvidedRenderConfig, getRenderConfig } from './canvas/render-config'
import { initSyntaxHighlighter, syntaxHighlightDecorations } from './highlight'

export async function initImageSuggestionService() {
return Promise.all([initSyntaxHighlighter(), initCanvas()])
}

function getFontSizeFromUserSettings(): number | undefined {
// Extract the font size from VS Code user settings.
// Note: VS Code warns but technically supports string-based font sizes, e.g. "14".
// TODO: Support this for other editors. We should respect the font size in editors like JetBrains.
const userFontSize = Number(vscode.workspace.getConfiguration('editor').get('fontSize'))

if (Number.isNaN(userFontSize) || userFontSize <= 0) {
// We cannot use this font size, we will use a platform specific default
return
}

return userFontSize
}

interface SuggestionOptions {
decorations: AddedLinesDecorationInfo[]
lang: string
Expand All @@ -32,18 +17,28 @@ interface SuggestionOptions {
config?: UserProvidedRenderConfig
}

export function generateSuggestionAsImage(options: SuggestionOptions): { light: string; dark: string } {
const { decorations, lang } = options
const renderConfig: UserProvidedRenderConfig = options.config || {
// The image should be rendered using the same font size as the existing text in the editor.
fontSize: getFontSizeFromUserSettings(),
}
interface GeneratedSuggestion {
/* Base64 encoded image suitable for rendering in dark editor themes */
dark: string
/* Base64 encoded image suitable for rendering in light editor themes */
light: string
/**
* The pixel ratio used to generate the image. Should be used to scale the image appropriately.
* Has a minimum value of 1.
*/
pixelRatio: number
}

export function generateSuggestionAsImage(options: SuggestionOptions): GeneratedSuggestion {
const { decorations, lang, config } = options
const renderConfig = getRenderConfig(config)

const darkDecorations = syntaxHighlightDecorations(decorations, lang, 'dark')
const lightDecorations = syntaxHighlightDecorations(decorations, lang, 'light')

return {
dark: drawDecorationsToCanvas(darkDecorations, 'dark', renderConfig).toDataURL('image/png'),
light: drawDecorationsToCanvas(lightDecorations, 'light', renderConfig).toDataURL('image/png'),
pixelRatio: renderConfig.pixelRatio,
}
}
4 changes: 4 additions & 0 deletions vscode/src/chat/chat-view/ChatController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -515,6 +515,10 @@ export class ChatController implements vscode.Disposable, vscode.WebviewViewProv
logger(message.filterLabel, message.message)
break
}
case 'devicePixelRatio': {
localStorage.setDevicePixelRatio(message.devicePixelRatio)
break
}
}
}

Expand Down
1 change: 1 addition & 0 deletions vscode/src/chat/protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ export type WebviewMessage =
selectedFilters: NLSSearchDynamicFilter[]
}
| { command: 'action/confirmation'; id: string; response: boolean }
| { command: 'devicePixelRatio'; devicePixelRatio: number }

export interface SmartApplyResult {
taskId: FixupTaskID
Expand Down
9 changes: 9 additions & 0 deletions vscode/src/services/LocalStorageProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ class LocalStorage implements LocalStorageForModelPreferences {
private readonly MODEL_PREFERENCES_KEY = 'cody-model-preferences'
private readonly CODY_CHAT_MEMORY = 'cody-chat-memory'
private readonly AUTO_EDITS_ONBOARDING_NOTIFICATION_COUNT = 'cody-auto-edit-notification-info'
private readonly DEVICE_PIXEL_RATIO = 'device-pixel-ratio'

public readonly keys = {
deepCodyLastUsedDate: 'DEEP_CODY_LAST_USED_DATE',
Expand Down Expand Up @@ -352,6 +353,14 @@ class LocalStorage implements LocalStorageForModelPreferences {
await this.set(this.keys.deepCodyLastUsedDate, lastUsed)
}

public getDevicePixelRatio(): number | null {
return this.get<number>(this.DEVICE_PIXEL_RATIO)
}

public async setDevicePixelRatio(ratio: number): Promise<void> {
await this.set(this.DEVICE_PIXEL_RATIO, ratio)
}

public get<T>(key: string): T | null {
return this.storage.get(key, null)
}
Expand Down
5 changes: 5 additions & 0 deletions vscode/webviews/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import { updateDisplayPathEnvInfoForWebview } from './utils/displayPathEnvInfo'
import { TelemetryRecorderContext, createWebviewTelemetryRecorder } from './utils/telemetry'
import { ClientConfigProvider } from './utils/useClientConfig'
import { type Config, ConfigProvider } from './utils/useConfig'
import { useDevicePixelRatioNotifier } from './utils/useDevicePixelRatio'
import { LinkOpenerProvider } from './utils/useLinkOpener'

export const App: React.FunctionComponent<{ vscodeAPI: VSCodeWrapper }> = ({ vscodeAPI }) => {
Expand Down Expand Up @@ -168,6 +169,10 @@ export const App: React.FunctionComponent<{ vscodeAPI: VSCodeWrapper }> = ({ vsc
}
}, [config, webviewTelemetryService])

// Notify the extension host of the device pixel ratio
// Currently used for image generation in auto-edit.
useDevicePixelRatioNotifier()

const wrappers = useMemo<Wrapper[]>(
() => getAppWrappers({ vscodeAPI, telemetryRecorder, config, clientConfig }),
[vscodeAPI, telemetryRecorder, config, clientConfig]
Expand Down
21 changes: 21 additions & 0 deletions vscode/webviews/utils/useDevicePixelRatio.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { useEffect } from 'react'
import { getVSCodeAPI } from './VSCodeApi'

export function useDevicePixelRatioNotifier(): void {
useEffect(() => {
const updatePixelRatio = () => {
getVSCodeAPI().postMessage({
command: 'devicePixelRatio',
devicePixelRatio: window.devicePixelRatio,
})
}

updatePixelRatio()

// Listen for changes in pixel ratio (e.g., zoom in/out)
const mediaQuery = window.matchMedia(`(resolution: ${window.devicePixelRatio}dppx)`)
mediaQuery.addEventListener('change', updatePixelRatio)

return () => mediaQuery.removeEventListener('change', updatePixelRatio)
}, [])
}
Loading