search

brackets-archive / bracketsIssues

Archive of issues in brackets.
0 stars 0 forks source link

iBorderWidth and iPaddedBorderWidth should be combined on Windows Vista and above #12578

Open core-ai-bot opened 3 years ago

core-ai-bot commented 3 years ago

Issue by fxb Friday Mar 21, 2014 at 20:45 GMT Originally opened as https://github.com/adobe/brackets/issues/7288

I found that when compiling with VS2013 (platform toolset "v120"), the NONCLIENTMETRICS structure receives a iBorderWidth value of 1 and an iPaddedBorderWidth value of 4. These should be combined to get the total padded border width of 5 and make it look good.

On the other hand, when compiling with VS2010, iBorderWidth is set to 5 and iPaddedBorderWidth is set to 0.

Experimenting with VS2013, the VS2010 behavior can be achieved when linking with "/subsystemversion:5.01" or changing the platform toolset to "v120_xp".

From msdn.microsoft.com - NONCLIENTMETRICS structure (Windows):

The iPaddedBorderWidth and iBorderWidth members are combined for both resizable and nonresizable windows in the Windows Aero desktop experience.

When calling SystemParametersInfo on Windows Vista or greater, include the iPaddedBorderWidth member of the NONCLIENTMETRICS structure:

OSVERSIONINFO info;
info.dwOSVersionInfoSize = sizeof(info);
if (::GetVersionEx(&info) && info.dwMajorVersion >= 6 && info.dwMinorVersion >= 0) {
  mNcMetrics.cbSize = sizeof (mNcMetrics);
} else {
  mNcMetrics.cbSize = sizeof (mNcMetrics) - sizeof (mNcMetrics.iPaddedBorderWidth);
}
::SystemParametersInfo(SPI_GETNONCLIENTMETRICS, 0, &mNcMetrics, 0);

Whenever iBorderWidth is used, combine it with iPaddedBorderWidth.

mNcMetrics.iBorderWidth

should be changed to

mNcMetrics.iBorderWidth + mNcMetrics.iPaddedBorderWidth
core-ai-bot commented 3 years ago

Comment by pthiess Monday Mar 24, 2014 at 18:54 GMT

@JeffryBooher

core-ai-bot commented 3 years ago

Comment by peterflynn Monday Mar 24, 2014 at 21:18 GMT

@fxb Would you be interested in putting up a pull request for this?

We know we eventually want to migrate to a newer Visual Studio version for building brackets-shell, and in the meantime this may fix some window-drawing issues for users with custom window border sizes (as in #6801)...

core-ai-bot commented 3 years ago

Comment by JeffryBooher Monday Mar 24, 2014 at 21:26 GMT

I think the gyp files will also need to be patched to generate VS2013 and VS2012 solution files. Some of the work may have been done already by Marshall (http://www.magpcss.net/cef_downloads/) but, since our gyp files aren't direct descendents of the cef test app's gyp files, it may require some massaging.

Ultimately we probably won't take a fix for supporting VS2012/2013 without a grunt script that can generate build solution workspace files so that we can test this. furthermore, we need to be able to build from the command line (i.e. no UI to convert the project files, etc..)

My suggestion would be to first put up a pull request to generate the VS2012/VS2013 workspace solutions from gyp then put up another pull request to fix the border issue

core-ai-bot commented 3 years ago

Comment by fxb Monday Mar 24, 2014 at 22:18 GMT

Yes. I was just opening the solution in VS2013, it got converted, and I changed the platform toolset. It makes sense to get proper project generation in place first. In the case that Marshall needs to change anything, I can poke him next Monday. We actually work together :-)

I noticed that there are some more cases where borders seem to be too thin. For example when the window is maximized. It might take some time to find out how exactly these borders shall be calculated. The above code only fixed one of those.

core-ai-bot commented 3 years ago

Comment by JeffryBooher Wednesday Mar 26, 2014 at 16:14 GMT

@fxb that's great. Can you post some screen shots of where the border is too thin? Technically there shouldn't be a left or right border when maximized just a title bar. I remember early on when I wrote the aero implementation I did compute border width as mNcMetrics.iBorderWidth + mNcMetrics.iPaddedBorderWidth and it was too thick compared to VS2013's look. FWIW I'm running on Windows 8 but it seemed to look the same on Windows 7. Also compare it to Office 2013.