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

Conversation

umpox
Copy link
Contributor

@umpox umpox commented Feb 14, 2025

Description

closes https://linear.app/sourcegraph/issue/CODY-4995/image-appears-blurred-on-large-screen-compared-to-text
closes https://linear.app/sourcegraph/issue/CODY-4924/auto-edit-improve-image-resolution

Screenshots (Low DPI Display)

This is without the progressive enhancement, i.e. using the default pixel ratio of 1.95

Before
image

After
image

Screenshots (High DPI Display)

This is without the progressive enhancement, i.e. using the default pixel ratio of 1.95

Before
image

After
image

Test plan

  • Tested on a low DPI display with the default pixel ratio of 1.95
  • Tested on a low DPI display with the actual pixel ratio
  • Tested on a high DPI display with the default pixel ratio of 1.95
  • Tested on a high DPI display with the actual pixel ratio

@umpox umpox force-pushed the tr/img-pixel-ratio branch from f62b1a2 to 247d708 Compare February 14, 2025 15:35
@umpox umpox force-pushed the tr/img-pixel-ratio branch from 686408c to dd2afc1 Compare February 14, 2025 17:17
@umpox umpox marked this pull request as ready for review February 17, 2025 08:46
Copy link
Contributor

@hitesh-1997 hitesh-1997 left a comment

Choose a reason for hiding this comment

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

Nice, Added minor comments.

Copy link
Contributor

@hitesh-1997 hitesh-1997 left a comment

Choose a reason for hiding this comment

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

LGTM !!

@@ -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

vscode/webviews/utils/useDevicePixelRatio.ts Show resolved Hide resolved
@umpox umpox merged commit d21d2b7 into main Feb 17, 2025
20 of 21 checks passed
@umpox umpox deleted the tr/img-pixel-ratio branch February 17, 2025 12:40
abeatrix pushed a commit that referenced this pull request Feb 17, 2025
…creens (#7112)

### Description

closes
https://linear.app/sourcegraph/issue/CODY-4995/image-appears-blurred-on-large-screen-compared-to-text
closes
https://linear.app/sourcegraph/issue/CODY-4924/auto-edit-improve-image-resolution

### Screenshots (Low DPI Display)

**This is without the progressive enhancement, i.e. using the default
pixel ratio of 1.95**

**Before**
<img width="662" alt="image"
src="https://github.com/user-attachments/assets/97f11290-c41d-478f-bc8c-a95c179edc5b"
/>

**After**
<img width="663" alt="image"
src="https://github.com/user-attachments/assets/072e9822-7aaa-41fb-bd5d-65c506599d8b"
/>


### Screenshots (High DPI Display)

**This is without the progressive enhancement, i.e. using the default
pixel ratio of 1.95**

**Before**
<img width="646" alt="image"
src="https://github.com/user-attachments/assets/d960e8d7-5ea6-4402-a748-fab9e21869c3"
/>

**After**
<img width="647" alt="image"
src="https://github.com/user-attachments/assets/d75cb094-5314-478e-a3b9-c6897c43b569"
/>


## Test plan

- Tested on a low DPI display with the default pixel ratio of 1.95
- Tested on a low DPI display with the actual pixel ratio
- Tested on a high DPI display with the default pixel ratio of 1.95
- Tested on a high DPI display with the actual pixel ratio



 <br> Backport d21d2b7 from #7100

Co-authored-by: Tom Ross <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants