large images are not scaled to stay inside diff panel

[shiftkey]

Description

With the new image diff improvements in #2383 there's a scenario where a large image is drawn outside the diff panel. We shouldn't do this.

Version

GitHub Desktop version: current master e.g. 82b1d23
OS version: Windows 10

Steps to Reproduce

1. Clone https://github.com/shiftkey/image-examples
2. Switch to History tab
3. Select this commit shiftkey/image-examples@10c1cf4
4. Ensure you have 2-up selected (but the others show this problem too)

Expected behavior:

• Image does not display outside diff view.
• Can toggle between different modes.

Actual behavior:

• Image drawn over top menu.
• Unable to change diff mode.

Reproduces how often: 100%

[joshaber]

😱

I think we should fix this for 1.0.

[joshaber]

Huh, interestingly I can't reproduce this on Mac

[joshaber]

Hrm, I can't reproduce this in my Windows 8 VM either. Gonna try getting a Windows 10 VM running.

[joshaber]

I'm having VMWare troubles with Windows 10, so I'm gonna unassign myself until I'm able to reproduce it.

[iAmWillShepherd]

I'm unable to reproduce this issue. Are you still having it?

[tierninho]

Was just looking at this too, can't reproduce either. Tested in Windows 10.

[joshaber]

Gonna 👢 this until we can reproduce it.

[shiftkey]

Still reproducible on 0.8.1-beta2 on a Windows 10 VM:

👍 to defer until someone else can reproduce this.

[PragmaticEd]

I uploaded git project here, that reproduces the problem. @tierninho please open the issue again, till fixed.

[niik]

@PragmaticEd thank you so much for taking the time to create a reproduction repository.

Unfortunately I wasn't able to reproduce this using the repo provided on macOS, haven't tested it on Windows 10 yet.

@PragmaticEd Are you using multiple monitors or different DPI scaling settings by any chance?

[tierninho]

@PragmaticEd thanks for checking in and adding your project.

As this issue is still open, we will be tracking it here. Your #3075 issue is noted.

[PragmaticEd]

@tierninho Yes, that might be it. I have 4k screen, set to 4k resolution and 200% scaling. Just tested. By setting the scaling to 100%, this problem goes away. So the problem is with scaling. I tried disabling the scaling for the app, like so: http://take.ms/jbXkW That, usually, fixes the problem for most other apps, but not this time..

[niik]

Yes, that might be it. I have 4k screen, set to 4k resolution and 200% scaling. Just tested. By setting the scaling to 100%, this problem goes away.

Excellent. I think I've got a good sense of what's going on then. The image diff uses a ResizeObserver which returns native pixels and then applies them as logical pixels. This has been a problem for us in the past. This should be a fairly straightforward fix of using offsetWidth and offsetHeight instead like we do in list.tsx. See #1200 and #1201 (but note that getClientBoundingRect isn't the answer due to zooming, see 03a86e5)

[j-f1]

I have a retina screen and I’ve encountered this with (tall?) images.

[shiftkey]

#5215 was another report that seems related to this issue

I added an image to my repository, it's size was 2600x3100 pixels. I previewed it before commiting changes. The 2-up view were selected. The image hided some of the navbar's components, like a layer whichs z-index is incorrect.

[ChiriVulpes]

I use 125% zoom and I have this issue very frequently in an image-heavy repository. It can cover the branch/fetch buttons in the changes view, and always covers the diff view options in the history view. I have to resize down to 100% to work with this repository.

[fdmarcin]

Hi! This affects me too on Windows 10, GH Desktop 1.4.3.
The image dimensions are 1532 x 794, and my screen resolution is 1920 x 1080 with 150% scaling.
The menu bar in the screenshot is part of the image in the diff btw.

[ghost]

I am still able to reproduce this in 1.5.0

[shiftkey]

I've been able to reproduce this on Windows 10 and I think it's some combination of scaling and screen real-estate.

This is the commit I'm working with (thanks @AndyThePie for the setup): 12beesinatrenchcoat/pizzaRPG-repo@0233f1c

At 100% scaling on a 1440x900px display, the image diff is scaled down to fit into the space:

At 200% scaling on a 2560x1440px display, the image is no longer scaled down:

Probably requires some refreshing of what the image diff controls are rendering and whether they care about the available space...