Miscellaneous UI Fixes

[William Poetra Yoga H]

OK, this is the first time I'm sending a patch as plaintext (I figured out I
have to use .txt as the extension, otherwise it's sent as a normal attachment).
I don't know if this method of sending works correctly so please try it and let
me know.

This patch fixes a number of things:
1. System menu icon (fix size and position)
2. Caption buttons (fix size and position)
3. Window caption (fix bounding rectangle)
4. Menu bar (fix size)
5. Popup menu (fix distance)

By the way, I have this kind of code in my patch (I wrote this code, the
original one doesn't look like this):
  rect.right -= (GetSystemMetrics(SM_CXSIZE) - 2) + 2;
Is it OK to write so? What I mean by the code is, the size of the button is
(GetSystemMetrics(SM_CXSIZE) - 2), and there's a 2-pixel space to the right of
the button. Or should I just write:
  rect.right -= GetSystemMetrics(SM_CXSIZE);
Which doesn't deliver the message clearly but is simpler.

I have tested the patch against theForger's tutorial (I compared the
screenshots to the actual apps running on my machine) and it doesn't generate
new errors, although some things aren't fixed yet:
1. Popup menu items are still a bit (one pixel) taller than those in Windows.
For this one I think I really need help.
2. Caption buttons' captions are still the old ones. I'll try to fix these
after this patch is committed.
3. Caption bar is still single-color. I'll fix this after this patch, because
it will intersect with this patch and possibly make an unclean patch, in the
end the two gets messy and never gets committed.

I haven't sent this to wine-patches yet. How should I do it? I mean, should I
send it all at once (the two files), or one file per message, or one file per
item (of the 5 items fixed, above)? I'm not sure (I've read the docs).

Reasons for sending the patch:
1. all at once: the patches are a group of related fixes (as said in the
website).
2. one file per message: I don't know :-P
3. one file per item: it's one fix per patch (as said in the website)

I prefer #1, but is it OK actually?
Comments, Alexandre? Guys? Gals?



		
__________________________________ 
Do you Yahoo!? 
Check out the new Yahoo! Front Page. 
www.yahoo.com 
 

Index: dlls/user/menu.c
===================================================================
RCS file: /home/wine/wine/dlls/user/menu.c,v
retrieving revision 1.7
diff -u -r1.7 menu.c
--- dlls/user/menu.c	19 Oct 2004 21:10:17 -0000	1.7
+++ dlls/user/menu.c	7 Nov 2004 04:47:47 -0000
@@ -978,7 +978,7 @@
     {
 	lpitem = &lppop->items[start];
 	orgX = maxX;
-	orgY = 2;
+	orgY = 3;
 
 	maxTab = maxTabWidth = 0;
 
@@ -1043,7 +1043,7 @@
           lprect->left, lprect->top, lprect->right, lprect->bottom);
     lppop->Width  = lprect->right - lprect->left;
     lppop->Height = 0;
-    maxY = lprect->top+1;
+    maxY = lprect->top;
     start = 0;
     helpPos = -1;
     while (start < lppop->nItems)
@@ -1077,7 +1077,7 @@
 	while (start < i) lppop->items[start++].rect.bottom = maxY;
     }
 
-    lprect->bottom = maxY;
+    lprect->bottom = maxY + 1;
     lppop->Height = lprect->bottom - lprect->top;
 
     /* Flush right all items between the MF_RIGHTJUSTIFY and */
@@ -1431,7 +1431,7 @@
     lppop = MENU_GetMenu( hMenu );
     if (lppop == NULL || lprect == NULL)
     {
-        return GetSystemMetrics(SM_CYMENU);
+        return GetSystemMetrics(SM_CYMENU) + 1;
     }
 
     if (suppress_draw)
@@ -1441,8 +1441,6 @@
 	if (lppop->Height == 0)
 		MENU_MenuBarCalcSize(hDC, lprect, lppop, hwnd);
 
-	lprect->bottom = lprect->top + lppop->Height;
-
         if (hfontOld) SelectObject( hDC, hfontOld);
 	return lppop->Height;
     }
@@ -3765,7 +3763,7 @@
     lppop = MENU_GetMenu( hMenu );
     if (lppop == NULL || lprect == NULL)
     {
-        retvalue = GetSystemMetrics(SM_CYMENU);
+        retvalue = GetSystemMetrics(SM_CYMENU) + 1;
         goto END;
     }
 
@@ -3776,17 +3774,17 @@
     if (lppop->Height == 0)
         MENU_MenuBarCalcSize(hDC, lprect, lppop, hwnd);
 
-    lprect->bottom = lprect->top + lppop->Height;
-
     FillRect(hDC, lprect, GetSysColorBrush(COLOR_MENU) );
 
+#if 0
     SelectObject( hDC, SYSCOLOR_GetPen(COLOR_3DFACE));
     MoveToEx( hDC, lprect->left, lprect->bottom, NULL );
     LineTo( hDC, lprect->right, lprect->bottom );
+#endif
 
     if (lppop->nItems == 0)
     {
-        retvalue = GetSystemMetrics(SM_CYMENU);
+        retvalue = GetSystemMetrics(SM_CYMENU) + 1;
         goto END;
     }
 

Index: windows/nonclient.c
===================================================================
RCS file: /home/wine/wine/windows/nonclient.c,v
retrieving revision 1.126
diff -u -r1.126 nonclient.c
--- windows/nonclient.c	19 Oct 2004 22:54:24 -0000	1.126
+++ windows/nonclient.c	7 Nov 2004 04:56:16 -0000
@@ -400,7 +400,7 @@
 	    winRect->top +=
 		MENU_GetMenuBarHeight( hwnd,
 				       winRect->right - winRect->left,
-				       -tmpRect.left, -tmpRect.top ) + 1;
+				       -tmpRect.left, -tmpRect.top );
 	}
 
 	SetRect(&tmpRect, 0, 0, 0, 0);
@@ -655,9 +655,9 @@
     {
         RECT rect;
         NC_GetInsideRect( hwnd, &rect );
-        DrawIconEx (hdc, rect.left + 1, rect.top + 1, hIcon,
-                    GetSystemMetrics(SM_CXSIZE) - 1,
-                    GetSystemMetrics(SM_CYSIZE) - 1, 0, 0, DI_NORMAL);
+        DrawIconEx (hdc, rect.left + 2, rect.top + 1, hIcon,
+                    GetSystemMetrics(SM_CXSMICON),
+                    GetSystemMetrics(SM_CYSMICON), 0, 0, DI_NORMAL);
     }
     return (hIcon != 0);
 }
@@ -682,21 +682,21 @@
     /* A tool window has a smaller Close button */
     if (GetWindowLongA( hwnd, GWL_EXSTYLE ) & WS_EX_TOOLWINDOW)
     {
-        INT iBmpHeight = 11; /* Windows does not use SM_CXSMSIZE and SM_CYSMSIZE   */
-        INT iBmpWidth = 11;  /* it uses 11x11 for  the close button in tool window */
-        INT iCaptionHeight = GetSystemMetrics(SM_CYSMCAPTION);
+        INT iCaptionHeight = GetSystemMetrics(SM_CYSMCAPTION) - 1;
+        INT iBmpHeight = iCaptionHeight - 4;
+        INT iBmpWidth = iCaptionHeight - 5;
 
-        rect.top = rect.top + (iCaptionHeight - 1 - iBmpHeight) / 2;
-        rect.left = rect.right - (iCaptionHeight + 1 + iBmpWidth) / 2;
+        rect.top += 2;
+        rect.right -= 2;
         rect.bottom = rect.top + iBmpHeight;
-        rect.right = rect.left + iBmpWidth;
+        rect.left = rect.right - iBmpWidth;
     }
     else
     {
-        rect.left = rect.right - GetSystemMetrics(SM_CXSIZE) - 1;
-        rect.bottom = rect.top + GetSystemMetrics(SM_CYSIZE) - 1;
         rect.top += 2;
         rect.right -= 2;
+        rect.bottom = rect.top + (GetSystemMetrics(SM_CYSIZE) - 4);
+        rect.left = rect.right - (GetSystemMetrics(SM_CXSIZE) - 2);
     }
     DrawFrameControl( hdc, &rect, DFC_CAPTION,
                       (DFCS_CAPTIONCLOSE |
@@ -722,12 +722,12 @@
     flags = IsZoomed(hwnd) ? DFCS_CAPTIONRESTORE : DFCS_CAPTIONMAX;
 
     NC_GetInsideRect( hwnd, &rect );
-    if (GetWindowLongA( hwnd, GWL_STYLE) & WS_SYSMENU)
-        rect.right -= GetSystemMetrics(SM_CXSIZE) + 1;
-    rect.left = rect.right - GetSystemMetrics(SM_CXSIZE);
-    rect.bottom = rect.top + GetSystemMetrics(SM_CYSIZE) - 1;
     rect.top += 2;
+    if (GetWindowLongA( hwnd, GWL_STYLE) & WS_SYSMENU)
+        rect.right -= (GetSystemMetrics(SM_CXSIZE) - 2) + 2;
     rect.right -= 2;
+    rect.bottom = rect.top + (GetSystemMetrics(SM_CYSIZE) - 4);
+    rect.left = rect.right - (GetSystemMetrics(SM_CXSIZE) - 2);
     if (down) flags |= DFCS_PUSHED;
     if (bGrayed) flags |= DFCS_INACTIVE;
     DrawFrameControl( hdc, &rect, DFC_CAPTION, flags );
@@ -750,14 +750,14 @@
         return;
 
     NC_GetInsideRect( hwnd, &rect );
+    rect.top += 2;
     if (style & WS_SYSMENU)
-        rect.right -= GetSystemMetrics(SM_CXSIZE) + 1;
+        rect.right -= (GetSystemMetrics(SM_CXSIZE) - 2) + 2;
+    rect.right -= 2;
     if (style & (WS_MAXIMIZEBOX|WS_MINIMIZEBOX))
         rect.right -= GetSystemMetrics(SM_CXSIZE) - 2;
-    rect.left = rect.right - GetSystemMetrics(SM_CXSIZE);
-    rect.bottom = rect.top + GetSystemMetrics(SM_CYSIZE) - 1;
-    rect.top += 2;
-    rect.right -= 2;
+    rect.left = rect.right - (GetSystemMetrics(SM_CXSIZE) - 2);
+    rect.bottom = rect.top + (GetSystemMetrics(SM_CYSIZE) - 4);
     if (down) flags |= DFCS_PUSHED;
     if (bGrayed) flags |= DFCS_INACTIVE;
     DrawFrameControl( hdc, &rect, DFC_CAPTION, flags );
@@ -871,7 +871,7 @@
 
     if ((style & WS_SYSMENU) && !(exStyle & WS_EX_TOOLWINDOW)) {
 	if (NC_DrawSysButton (hwnd, hdc, FALSE))
-	    r.left += GetSystemMetrics(SM_CYCAPTION) - 1;
+	    r.left += GetSystemMetrics(SM_CXSMICON) + 2;
     }
 
     if (style & WS_SYSMENU)
@@ -885,18 +885,22 @@
 	/* Draw a grayed close button if disabled and a normal one if SC_CLOSE is not there */
 	NC_DrawCloseButton (hwnd, hdc, FALSE,
 			    ((((state & MF_DISABLED) || (state & MF_GRAYED))) && (state != 0xFFFFFFFF)));
-	r.right -= GetSystemMetrics(SM_CYCAPTION) - 1;
+	r.right -= (GetSystemMetrics(SM_CXSIZE) - 2) + 2;
 
 	if ((style & WS_MAXIMIZEBOX) || (style & WS_MINIMIZEBOX))
 	{
+            /* There's a 2-pixel space between Maximize and Close buttons */
+
+            r.right -= 2;
+
 	    /* In win95 the two buttons are always there */
 	    /* But if the menu item is not in the menu they're disabled*/
 
 	    NC_DrawMaxButton( hwnd, hdc, FALSE, (!(style & WS_MAXIMIZEBOX)));
-	    r.right -= GetSystemMetrics(SM_CXSIZE) + 1;
+	    r.right -= GetSystemMetrics(SM_CXSIZE) - 2;
 
 	    NC_DrawMinButton( hwnd, hdc, FALSE,  (!(style & WS_MINIMIZEBOX)));
-	    r.right -= GetSystemMetrics(SM_CXSIZE) + 1;
+	    r.right -= GetSystemMetrics(SM_CXSIZE) - 2;
 	}
     }
 
@@ -915,6 +919,7 @@
 	else SetTextColor( hdc, GetSysColor( COLOR_INACTIVECAPTIONTEXT ) );
 	SetBkMode( hdc, TRANSPARENT );
 	r.left += 2;
+        r.right -= 2;
 	DrawTextW( hdc, buffer, -1, &r,
 		     DT_SINGLELINE | DT_VCENTER | DT_NOPREFIX | DT_LEFT );
 	DeleteObject (SelectObject (hdc, hOldFont));
@@ -1010,12 +1015,12 @@
     {
         RECT  r = rect;
         if (dwExStyle & WS_EX_TOOLWINDOW) {
-            r.bottom = rect.top + GetSystemMetrics(SM_CYSMCAPTION);
             rect.top += GetSystemMetrics(SM_CYSMCAPTION);
+            r.bottom = rect.top;
         }
         else {
-            r.bottom = rect.top + GetSystemMetrics(SM_CYCAPTION);
             rect.top += GetSystemMetrics(SM_CYCAPTION);
+            r.bottom = rect.top;
         }
         if( !clip || IntersectRect( &rfuzz, &r, &rectClip ) )
             NC_DrawCaption(hdc, &r, hwnd, dwStyle, dwExStyle, active);
@@ -1029,7 +1034,7 @@
 	TRACE("Calling DrawMenuBar with rect (%ld, %ld)-(%ld, %ld)\n",
               r.left, r.top, r.right, r.bottom);
 
-	rect.top += MENU_DrawMenuBar( hdc, &r, hwnd, suppress_menupaint ) + 1;
+	rect.top += MENU_DrawMenuBar( hdc, &r, hwnd, suppress_menupaint );
     }
 
     TRACE("After MenuBar, rect is (%ld, %ld)-(%ld, %ld).\n",

[Dimitrie O. Paun]

On Sat, Nov 06, 2004 at 10:14:36PM -0800, William Poetra Yoga H wrote:
> Reasons for sending the patch:
> 1. all at once: the patches are a group of related fixes (as said in the
> website).
> 2. one file per message: I don't know :-P
> 3. one file per item: it's one fix per patch (as said in the website)

3, as documented on the website, is the preferred method. Otherwise,
we wouldn't have documented it as such :)

-- 
Dimi.

[William Poetra Yoga H]

--- "Dimitrie O. Paun" <dpaun(a)rogers.com> wrote:
> On Sat, Nov 06, 2004 at 10:14:36PM -0800, William Poetra Yoga H wrote:
> > Reasons for sending the patch:
> > 1. all at once: the patches are a group of related fixes (as said in the
> > website).
> > 2. one file per message: I don't know :-P
> > 3. one file per item: it's one fix per patch (as said in the website)
>
> 3, as documented on the website, is the preferred method. Otherwise,
> we wouldn't have documented it as such :)
>
> -- 
> Dimi.

You mean 5 mails with one patch each? Or one mail with 5 diff files?



		
__________________________________ 
Do you Yahoo!? 
Check out the new Yahoo! Front Page. 
www.yahoo.com

[Dimitrie O. Paun]

On Sat, Nov 06, 2004 at 10:57:27PM -0800, William Poetra Yoga H wrote:
> You mean 5 mails with one patch each? Or one mail with 5 diff files?

One patch per mail, always. So 5 mails with one patch each, yes.

-- 
Dimi.

[Duane Clark]

William Poetra Yoga H wrote:
> --- "Dimitrie O. Paun" <dpaun(a)rogers.com> wrote:
> > On Sat, Nov 06, 2004 at 10:14:36PM -0800, William Poetra Yoga H wrote:
> > > Reasons for sending the patch:
> > > 1. all at once: the patches are a group of related fixes (as said in the
> > > website).
> > > 2. one file per message: I don't know :-P
> > > 3. one file per item: it's one fix per patch (as said in the website)
> >
> > 3, as documented on the website, is the preferred method. Otherwise,
> > we wouldn't have documented it as such :)
> >
> > -- 
> > Dimi.
>
> You mean 5 mails with one patch each? Or one mail with 5 diff files?

Just to be clarify a bit more, a "patch" fixes one thing, or several 
closely related things. A patch can contain changes to multiple files, 
when multiple files need to be changed to fix something. After applying 
a single patch, Wine should be able to be recompiled and run, presumably 
with whatever was being fixed now working.

In the case of your original email, where there is one attachment for 
menu size and one for nonclient size, they probably should be sent in 
separate patches/emails.

[William Poetra Yoga H]

--- Duane Clark <dclark(a)akamail.com> wrote:
> > You mean 5 mails with one patch each? Or one mail with 5 diff files?
>
> Just to be clarify a bit more, a "patch" fixes one thing, or several 
> closely related things. A patch can contain changes to multiple files, 
> when multiple files need to be changed to fix something. After applying 
> a single patch, Wine should be able to be recompiled and run, presumably 
> with whatever was being fixed now working.
>
> In the case of your original email, where there is one attachment for 
> menu size and one for nonclient size, they probably should be sent in 
> separate patches/emails.

OK, but the size patch in menu.c and nonclient.c are actually a fix for a
thing: the size of the menubar. So I think it should be sent in one mail,
right?



		
__________________________________ 
Do you Yahoo!? 
Check out the new Yahoo! Front Page. 
www.yahoo.com

[Tony Lambregts]

William Poetra Yoga H wrote:
> --- Duane Clark <dclark(a)akamail.com> wrote:
> > > You mean 5 mails with one patch each? Or one mail with 5 diff files?
> >
> > Just to be clarify a bit more, a "patch" fixes one thing, or several 
> > closely related things. A patch can contain changes to multiple files, 
> > when multiple files need to be changed to fix something. After applying 
> > a single patch, Wine should be able to be recompiled and run, presumably 
> > with whatever was being fixed now working.
> >
> > In the case of your original email, where there is one attachment for 
> > menu size and one for nonclient size, they probably should be sent in 
> > separate patches/emails.
>
> OK, but the size patch in menu.c and nonclient.c are actually a fix for a
> thing: the size of the menubar. So I think it should be sent in one mail,
> right?

If the resulting patch is small yes. If it is larger the typicaly you should
send several smaller patches with the same subject line + some indication
that they are patch 1 of N.

--

Tony Lambregts

[James Hawkins]

On Tue, 09 Nov 2004 09:36:17 -0700, Tony Lambregts
<tony_lambregts(a)telusplanet.net> wrote:
> William Poetra Yoga H wrote:
> > --- Duane Clark <dclark(a)akamail.com> wrote:
> > > > You mean 5 mails with one patch each? Or one mail with 5 diff files?
> > >
> > > Just to be clarify a bit more, a "patch" fixes one thing, or several
> > > closely related things. A patch can contain changes to multiple files,
> > > when multiple files need to be changed to fix something. After applying
> > > a single patch, Wine should be able to be recompiled and run, presumably
> > > with whatever was being fixed now working.
> > >
> > > In the case of your original email, where there is one attachment for
> > > menu size and one for nonclient size, they probably should be sent in
> > > separate patches/emails.
> >
> > OK, but the size patch in menu.c and nonclient.c are actually a fix for a
> > thing: the size of the menubar. So I think it should be sent in one mail,
> > right?
>
> If the resulting patch is small yes. If it is larger the typicaly you should
> send several smaller patches with the same subject line + some indication
> that they are patch 1 of N.
>
> --
>
> Tony Lambregts

> If the resulting patch is small yes. If it is larger the typicaly you should
> send several smaller patches with the same subject line + some indication
> that they are patch 1 of N.

If these 1 to N patches rely on each other to be committed to compile
correctly, then this is not what we want.  You need to send one fix
per email, one patch per fix, and the subject and changelog should
specifically state what you are changing in each fix.

-- 
James Hawkins

[Duane Clark]

William Poetra Yoga H wrote:
> OK, but the size patch in menu.c and nonclient.c are actually a fix for a
> thing: the size of the menubar. So I think it should be sent in one mail,
> right?

Then yes, they would be one email. While it is not real important, I 
think it is a little clearer in that case to combine the patches into a 
single attachment. You can do "cvs diff dlls/user/menu.c 
windows/nonclient.c > menu.diff" and they will be combined for you.