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

Window layout style bug on open #146683

Closed
laurelkeys opened this issue Apr 4, 2022 · 11 comments · Fixed by #158025
Closed

Window layout style bug on open #146683

laurelkeys opened this issue Apr 4, 2022 · 11 comments · Fixed by #158025
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug electron Issues and items related to Electron electron-17-update Issues related to electron 17 update insiders-released Patch has been released in VS Code Insiders upstream Issue identified as 'upstream' component related (exists outside of VS Code) upstream-issue-linked This is an upstream issue that has been reported upstream verified Verification succeeded windows VS Code on Windows issues
Milestone

Comments

@laurelkeys
Copy link

Does this issue occur when all extensions are disabled?: Yes

  • VS Code Version: 1.66.0 (x64)
  • OS Version: 10.0.19044 Build 19044 (Window 10)

Steps to Reproduce:

  1. Launch VS Code
  2. Open File > New Window (or use the shortcut Ctrl+Shift+N)
  3. For the first miliseconds after opening a new instance of VS Code, a "Window 7"-like window frame layout appears, before the usual VS Code look shows up (see the attached GIF).

The GIF below first shows the expected behavior, opening version 1.65.0, followed by the current behavior of version 1.66.0.

inbetween

The two executables were downloaded from: https://update.code.visualstudio.com/1.65.0/win32-x64-archive/stable and https://update.code.visualstudio.com/1.66.0/win32-x64-archive/stable.

@sbatten sbatten assigned deepak1556 and unassigned sbatten Apr 4, 2022
@deepak1556 deepak1556 added bug Issue identified by VS Code Team member as probable bug upstream Issue identified as 'upstream' component related (exists outside of VS Code) electron Issues and items related to Electron upstream-issue-linked This is an upstream issue that has been reported upstream electron-17-update Issues related to electron 17 update windows VS Code on Windows issues labels Apr 4, 2022
@deepak1556
Copy link
Collaborator

Refs electron/electron#30760

@rokimiftah
Copy link

that's the reason I went back to 1.65, frames are so annoying.

@laurelkeys
Copy link
Author

Hi @deepak1556, just a small update: I've downloaded the latest VS Code Insiders .zip and the problem now only appears when launching VS Code for the first time, but no longer when opening new windows from a running instance of it.

@laurelkeys
Copy link
Author

This issue persists in the new latest version release, 1.67.0.

@laurelkeys
Copy link
Author

The issue persists in today's release, 1.68.0.

@eddiezato
Copy link

Since the related bug in the Electron repo has a 19-x-y label, and today's VSCode uses 17.4.7, I believe we will be experiencing this bug for months. 😔

@rzhao271
Copy link
Contributor

rzhao271 commented Aug 11, 2022

Current status of investigation

The upstream Electron issue is at electron/electron#30760.
The issue was introduced in Electron v14.0.0-beta.5 and the commit that introduced the issue is electron/electron@172ac25.
The PR electron/electron#35189 fixes the issue where frameless resizable windows show Windows 7 frames during startup, but there's a larger issue that is still unresolved even with that PR. I also searched through the thousands of Chromium commits associated with that Electron commit to see whether they had anything to do with the issue, but I didn't find anything relevant even after reverting several suspicious commits.

Minimal repro

const { app, BrowserWindow } = require('electron')
function createWindow() {
  new BrowserWindow({
    width: 800,
    height: 600,
    frame: false
  })
}
app.whenReady().then(() => {
  createWindow()
})
app.on('window-all-closed', function () {
  app.quit()
})

Running <path-to-electron-exe> <path-to-js-file-above> with Electron v14.0.0-beta.5 or newer on Windows 10 reproduces the issue.

Going through the Chromium code with the PR fix for frameless resizable windows

The PR electron/electron#35189 fixes the issue for frameless resizable windows by calling set_can_resize() at an earlier stage of program initialization. Assume the window is frameless and resizable. Then, the program sets can_resize to true earlier in widget_delegate.cc, and the following occurs:

  1. widget_hwnd_utils.cc:68 is not run, meaning WS_THICKFRAME is not removed from the style set in Line 62. This style parameter is passed in to a CreateWindowEx call at window_impl.cc:213.
  2. hwnd_message_handler.cc:1029 is not run. Instead, the if block above is run, which adds a WS_THICKFRAME style to the style passed into the SetWindowLong call.
  3. Somehow, because of these WS_THICKFRAME styles being passed in, after the window is shown, we get a single WM_DWMNCRENDERINGCHANGED message, and no WM_NCPAINT messages after. NC in this case refers to non-client area, which is the frame. DWM stands for desktop windows manager.

After adding a long setTimeout of 30 seconds to the createWindow call above, I was able to use Spy++ to log messages to that window and see the following:

<000157> 0000000000170B9C R WM_WINDOWPOSCHANGING
<000158> 0000000000170B9C S WM_GETICON nType:ICON_BIG
<000159> 0000000000170B9C R WM_GETICON hicon:00000000
<000160> 0000000000170B9C P WM_DWMNCRENDERINGCHANGED fRenderingEnabled:True
<000161> 0000000000170B9C S WM_GETICON nType:ICON_SMALL2
<000162> 0000000000170B9C R WM_GETICON hicon:00000000
<000163> 0000000000170B9C P message:0x0400 [User-defined:WM_USER+0] wParam:00000003 lParam:00000000
<000164> 0000000000170B9C S WM_GETICON nType:ICON_SMALL
<000165> 0000000000170B9C R WM_GETICON hicon:00000000
<000166> 0000000000170B9C P message:0xC112 [Registered:"TaskbarButtonCreated"] wParam:00000000 lParam:00000000
<000167> 0000000000170B9C S WM_GETMINMAXINFO lpmmi:0000002354FFE8F0
<000168> 0000000000170B9C R WM_GETMINMAXINFO lpmmi:0000002354FFE8F0
<000169> 0000000000170B9C P WM_PAINT hdc:00000000

I have also attached full logs to the end of this comment.

The PR doesn't fix the case of frameless non-resizable windows

For frameless windows that cannot resize, even if we call set_can_resize() earlier, because the program would pass in false, for the CreateWindowEx and SetWindowLong calls above, the WS_THICKFRAME style would not be in the styles sent to the calls. As a result, what I see in Spy++ is WM_DWMNCRENDERINGCHANGED being toggled three times, and several WM_WINDOWPOSCHANGING calls that result in WM_NCPAINT messages being emitted. Using Spy++, I confirmed that the WM_WINDOWPOSCHANGING calls have the flag SWP_FRAMECHANGED set.

<000174> 0000000000130AAA P WM_DWMNCRENDERINGCHANGED fRenderingEnabled:True
<000175> 0000000000130AAA S WM_GETICON nType:ICON_SMALL2
<000176> 0000000000130AAA R WM_GETICON hicon:00000000
<000177> 0000000000130AAA P WM_DWMNCRENDERINGCHANGED fRenderingEnabled:False
<000178> 0000000000130AAA S WM_WINDOWPOSCHANGING lpwp:000000E709BFE280
<000179> 0000000000130AAA R WM_WINDOWPOSCHANGING
<000180> 0000000000130AAA S WM_NCCALCSIZE fCalcValidRects:True lpncsp:000000E709BFE250
<000181> 0000000000130AAA R WM_NCCALCSIZE fuValidRect:0000 lpncsp:000000E709BFE250
<000182> 0000000000130AAA S WM_NCPAINT hrgn:00000001
<000183> 0000000000130AAA R WM_NCPAINT
<000184> 0000000000130AAA S WM_ERASEBKGND hdc:5B01182B
<000185> 0000000000130AAA R WM_ERASEBKGND fErased:True
<000186> 0000000000130AAA S WM_WINDOWPOSCHANGED lpwp:000000E709BFE280
<000187> 0000000000130AAA S WM_WINDOWPOSCHANGING lpwp:000000E709BFDB20
<000188> 0000000000130AAA R WM_WINDOWPOSCHANGING
<000189> 0000000000130AAA S WM_NCCALCSIZE fCalcValidRects:True lpncsp:000000E709BFDAF0
<000190> 0000000000130AAA R WM_NCCALCSIZE fuValidRect:0000 lpncsp:000000E709BFDAF0
<000191> 0000000000130AAA S WM_NCPAINT hrgn:00000001
<000192> 0000000000130AAA R WM_NCPAINT
<000193> 0000000000130AAA S WM_ERASEBKGND hdc:FFFFFFFF87011810
<000194> 0000000000130AAA R WM_ERASEBKGND fErased:True
<000195> 0000000000130AAA S WM_WINDOWPOSCHANGED lpwp:000000E709BFDB20
<000196> 0000000000130AAA R WM_WINDOWPOSCHANGED
<000197> 0000000000130AAA R WM_WINDOWPOSCHANGED
<000198> 0000000000130AAA S WM_GETICON nType:ICON_SMALL
<000199> 0000000000130AAA R WM_GETICON hicon:00000000
<000200> 0000000000130AAA P message:0x0400 [User-defined:WM_USER+0] wParam:00000003 lParam:00000000
<000201> 0000000000130AAA P WM_DWMNCRENDERINGCHANGED fRenderingEnabled:True
<000202> 0000000000130AAA S WM_WINDOWPOSCHANGING lpwp:000000E709BFE280
<000203> 0000000000130AAA R WM_WINDOWPOSCHANGING
<000204> 0000000000130AAA S WM_NCCALCSIZE fCalcValidRects:True lpncsp:000000E709BFE250
<000205> 0000000000130AAA R WM_NCCALCSIZE fuValidRect:0000 lpncsp:000000E709BFE250
<000206> 0000000000130AAA S WM_NCPAINT hrgn:00000001
<000207> 0000000000130AAA R WM_NCPAINT
<000208> 0000000000130AAA S WM_ERASEBKGND hdc:010118D7
<000209> 0000000000130AAA R WM_ERASEBKGND fErased:True
<000210> 0000000000130AAA S WM_WINDOWPOSCHANGED lpwp:000000E709BFE280
<000211> 0000000000130AAA S WM_WINDOWPOSCHANGING lpwp:000000E709BFDB20
<000212> 0000000000130AAA R WM_WINDOWPOSCHANGING
<000213> 0000000000130AAA S WM_NCCALCSIZE fCalcValidRects:True lpncsp:000000E709BFDAF0
<000214> 0000000000130AAA R WM_NCCALCSIZE fuValidRect:0000 lpncsp:000000E709BFDAF0
<000215> 0000000000130AAA S WM_NCPAINT hrgn:00000001
<000216> 0000000000130AAA R WM_NCPAINT
<000217> 0000000000130AAA S WM_ERASEBKGND hdc:5B01182B
<000218> 0000000000130AAA R WM_ERASEBKGND fErased:True
<000219> 0000000000130AAA S WM_WINDOWPOSCHANGED lpwp:000000E709BFDB20
<000220> 0000000000130AAA R WM_WINDOWPOSCHANGED
<000221> 0000000000130AAA R WM_WINDOWPOSCHANGED

More information

I have searched the Chromium and Electron codebases for WS_THICKFRAME and SWP_FRAMECHANGED (as well as SWP_DRAWFRAME) to see why the frame might be rendering more often when WS_THICKFRAME is not passed in. However, I haven't found anything within the codebase itself.

Searching Windows documentation, I found the following:

Also, Electron calls SetWindowLong at native_window_views.cc:350. I noticed that if I don't call SetWindowLong there, the Windows 7 frame doesn't show up, but then the Electron window doesn't render with the proper style (see electron/electron#32981). I also noticed that when the Windows 7 frame shows up, it has the style of whatever was passed into that specific SetWindowLong call. For example, if I add WS_SYSMENU to the call, then for a second I see a Windows 7 frame with WS_SYSMENU applied.

I also tried creating a minimal project in VS to reproduce the issue but wasn't successful. The frame that rendered seemed to have a Windows 10 style rather than a Windows 7 one.

I also tried registering a listener in hwnd_message_handler.cc for the WM_DWMNCRENDERINGCHANGED messages, and it did seem to get hit, though I couldn't do anything with it. There's also the mysterious ScopedRedrawLock, but I wasn't able to apply it to resolve the issue.

Attached log files

A text file showing the good case where WM_DWMNCRENDERINGCHANGED is only called once. The window is frameless and resizable.
A text file showing the bad case where WM_DWMNCRENDERINGCHANGED is called three times. The window is frameless and not resizable.

@rzhao271
Copy link
Contributor

rzhao271 commented Aug 12, 2022

The PR that @deepak1556 linked above (partially based on electron/electron#35189) should resolve the issue for VS Code Insiders and hence close this issue, but there's still more I'd like to investigate, especially for frameless non-resizable windows.

More logging

I added some lines to log the hwnd and style passed into each relevant CreateWindowEx and SetWindowLong call. One example of the styles that show up:

[22824:0812/111914.541:ERROR:window_impl.cc(217)] window_impl.cc CreateWindowEx hwnd 0000000000060218 style 06cc0000
WS_CLIPCHILDREN | WS_CLIPSIBLINGS | WS_CAPTION | WS_THICKFRAME | WS_SYSMENU
[22824:0812/111914.541:ERROR:window_impl.cc(218)] window_impl.cc CreateWindowEx hwnd 0000000000060218 ex_style 0
[22824:0812/111914.548:ERROR:hwnd_message_handler.cc(1043)] hwnd_message_handler SetWindowLong hwnd 0000000000060218 style 060e0000
WS_CLIPCHILDREN | WS_CLIPSIBLINGS | WS_THICKFRAME | WS_SYSMENU | WS_MINIMIZEBOX
[22824:0812/111914.549:ERROR:hwnd_message_handler.cc(1043)] hwnd_message_handler SetWindowLong hwnd 0000000000060218 style 060a0000
WS_CLIPCHILDREN | WS_CLIPSIBLINGS | WS_SYSMENU | WS_MINIMIZEBOX
[22824:0812/111914.549:ERROR:native_window_views.cc(360)] native_window_views.cc SetWindowLong hwnd 0000000000060218 style 00c30000
WS_CAPTION | WS_MINIMIZEBOX | WS_MAXIMIZEBOX
[22824:0812/111914.601:ERROR:hwnd_message_handler.cc(1043)] hwnd_message_handler SetWindowLong hwnd 0000000000060218 style 04c20000
WS_CLIPSIBLINGS | WS_CAPTION | WS_MINIMIZEBOX

There's also another window being created really early, but I don't think it influences the issue.

[24748:0812/115319.325:ERROR:window_impl.cc(217)] window_impl.cc CreateWindowEx hwnd 000000000029053C style 02cf0000
[24748:0812/115319.325:ERROR:window_impl.cc(218)] window_impl.cc CreateWindowEx hwnd 000000000029053C ex_style 0
WS_CLIPCHILDREN | WS_CAPTION | WS_THICKFRAME | WS_SYSMENU | WS_MINIMIZEBOX | WS_MAXIMIZEBOX

Good scenario (meaning no frame showing) with can_resize set to true and staying true

[4124:0812/124902.900:ERROR:window_impl.cc(217)] window_impl.cc CreateWindowEx hwnd 00000000000C0BC8 style 2cf0000
[4124:0812/124902.900:ERROR:window_impl.cc(218)] window_impl.cc CreateWindowEx hwnd 00000000000C0BC8 ex_style 0
[4124:0812/124902.996:ERROR:window_impl.cc(217)] window_impl.cc CreateWindowEx hwnd 0000000000060D16 style 6cc0000
[4124:0812/124902.996:ERROR:window_impl.cc(218)] window_impl.cc CreateWindowEx hwnd 0000000000060D16 ex_style 0
[4124:0812/124903.007:ERROR:hwnd_message_handler.cc(1043)] hwnd_message_handler SetWindowLong hwnd 0000000000060D16 style 60f0000
[4124:0812/124903.009:ERROR:native_window_views.cc(360)] native_window_views.cc SetWindowLong hwnd 0000000000060D16 style c70000
[4124:0812/124903.065:ERROR:hwnd_message_handler.cc(1043)] hwnd_message_handler SetWindowLong hwnd 0000000000060D16 style 4c70000

Bad scenario with can_resize set to true then false

[7740:0812/124938.950:ERROR:window_impl.cc(217)] window_impl.cc CreateWindowEx hwnd 00000000003A0240 style 2cf0000
[7740:0812/124938.950:ERROR:window_impl.cc(218)] window_impl.cc CreateWindowEx hwnd 00000000003A0240 ex_style 0
[7740:0812/124939.033:ERROR:window_impl.cc(217)] window_impl.cc CreateWindowEx hwnd 0000000000740736 style 6cc0000
[7740:0812/124939.033:ERROR:window_impl.cc(218)] window_impl.cc CreateWindowEx hwnd 0000000000740736 ex_style 0
[7740:0812/124939.041:ERROR:hwnd_message_handler.cc(1043)] hwnd_message_handler SetWindowLong hwnd 0000000000740736 style 60e0000
[7740:0812/124939.043:ERROR:hwnd_message_handler.cc(1043)] hwnd_message_handler SetWindowLong hwnd 0000000000740736 style 60a0000
[7740:0812/124939.044:ERROR:native_window_views.cc(360)] native_window_views.cc SetWindowLong hwnd 0000000000740736 style c30000
[7740:0812/124939.099:ERROR:hwnd_message_handler.cc(1043)] hwnd_message_handler SetWindowLong hwnd 0000000000740736 style 4c20000

Bad scenario with can_resize set to false and staying false

[6636:0812/125425.559:ERROR:window_impl.cc(217)] window_impl.cc CreateWindowEx hwnd 0000000000230986 style 2cf0000
[6636:0812/125425.559:ERROR:window_impl.cc(218)] window_impl.cc CreateWindowEx hwnd 0000000000230986 ex_style 0
[6636:0812/125425.662:ERROR:window_impl.cc(217)] window_impl.cc CreateWindowEx hwnd 00000000001E0BBC style 6c80000
[6636:0812/125425.662:ERROR:window_impl.cc(218)] window_impl.cc CreateWindowEx hwnd 00000000001E0BBC ex_style 0
[6636:0812/125425.668:ERROR:hwnd_message_handler.cc(1043)] hwnd_message_handler SetWindowLong hwnd 00000000001E0BBC style 60a0000
[6636:0812/125425.671:ERROR:native_window_views.cc(360)] native_window_views.cc SetWindowLong hwnd 00000000001E0BBC style c30000
[6636:0812/125425.730:ERROR:hwnd_message_handler.cc(1043)] hwnd_message_handler SetWindowLong hwnd 00000000001E0BBC style 4c20000

The SetWindowLong call with style 60a0000 causes the Windows 7 frame to render, and its style corresponds to WS_CLIPCHILDREN | WS_CLIPSIBLINGS | WS_SYSMENU | WS_MINIMIZEBOX
60e0000 is actually the same style just with WS_THICKFRAME, once again showing how mysterious the WS_THICKFRAME constant is.

Delaying the SetCanResize call

While working on electron/electron#35189, I also noticed that changing the order of the calls seemed to fix the issue for frameless non-resizable windows. In particular, on the Electron side, in native_window_views.cc, if I first called set_can_resize(true) to pretend that the window is resizable, and then called SetCanResize(resizable_) (where resizable_ = false) after the SetWindowLong call around line 365, then the issue is resolved even for frameless non-resizable windows. However, this workaround delays the SetCanResize call to hundreds of lines later, and I wouldn't be surprised if it would cause a regression elsewhere.

Another log file for the scenario above.

Just like in the log file of the good case of the previous comment, WM_DWMNCRENDERINGCHANGED only shows up once, rather than three times.

For completeness, here are the logs with the styles for this scenario, excluding the CreateWindowEx logs:

[11236:0812/145829.596:ERROR:hwnd_message_handler.cc(1043)] hwnd_message_handler SetWindowLong hwnd 0000000000830BCA style 60e0000
[11236:0812/145829.601:ERROR:native_window_views.cc(367)] native_window_views.cc SetWindowLong hwnd 0000000000830BCA style c30000
[11236:0812/145829.601:ERROR:hwnd_message_handler.cc(1043)] hwnd_message_handler SetWindowLong hwnd 0000000000830BCA style 4c20000
[11236:0812/145829.666:ERROR:hwnd_message_handler.cc(1043)] hwnd_message_handler SetWindowLong hwnd 0000000000830BCA style 4c20000

In this case, the style 60a0000 never comes up!

I also tried delaying the SetCanResize(resizable_) call and calling set_can_resize(resizable_) instead of set_can_resize(true), but that doesn't work because it fails to pass WS_THICKFRAME to the CreateWindowEx call as well as the first SetWindowLong call, both of which need WS_THICKFRAME for the frame not to render:

[26344:0812/150627.310:ERROR:hwnd_message_handler.cc(1043)] hwnd_message_handler SetWindowLong hwnd 0000000000170B7C style 60a0000
[26344:0812/150627.312:ERROR:native_window_views.cc(367)] native_window_views.cc SetWindowLong hwnd 0000000000170B7C style c30000
[26344:0812/150627.359:ERROR:hwnd_message_handler.cc(1043)] hwnd_message_handler SetWindowLong hwnd 0000000000170B7C style 4c20000

A note on DwmSetAttribute

DwmSetAttribute can be used to change the non-client (NC) rendering policy and hence result in a WM_DWMNCRENDERINGCHANGED call. However, I did not find any references to DwmSetAttribute within the Electron and Chromium codebases that were actually called when running my minimal repro.

@deepak1556
Copy link
Collaborator

@rzhao271 lets continue the investigation in #158065

@VSCodeTriageBot VSCodeTriageBot added insiders-released Patch has been released in VS Code Insiders and removed unreleased Patch has not yet been released in VS Code Insiders labels Aug 15, 2022
@roblourens roblourens added the author-verification-requested Issues potentially verifiable by issue author label Aug 25, 2022
@VSCodeTriageBot
Copy link
Collaborator

This bug has been fixed in the latest release of VS Code Insiders!

@laurelkeys, you can help us out by commenting /verified if things are now working as expected.

If things still don't seem right, please ensure you're on version 9529e11 of Insiders (today's or later - you can use Help: About in the command palette to check), and leave a comment letting us know what isn't working as expected.

Happy Coding!

@laurelkeys
Copy link
Author

/verified

@VSCodeTriageBot VSCodeTriageBot added verified Verification succeeded and removed author-verification-requested Issues potentially verifiable by issue author labels Aug 25, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Sep 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug electron Issues and items related to Electron electron-17-update Issues related to electron 17 update insiders-released Patch has been released in VS Code Insiders upstream Issue identified as 'upstream' component related (exists outside of VS Code) upstream-issue-linked This is an upstream issue that has been reported upstream verified Verification succeeded windows VS Code on Windows issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants