Lazarus Release Candidate 1 of 1.2

[Martin_fr]

@taazz: did you try the +1 pixel? (for x and y). Did it work?

[taazz]

@taazz: did you try the +1 pixel? (for x and y). Did it work?

I have tried it on 1.2rc2 only with varying margins +1 has the same results as +2,3 or 4 as you can see in the attachments. I haven't had the time to rebuild lazarus it self but I'm guessing that it will have the same results there too.

As you can see I applied the margins on both axis (x and y) the changed procedure is below complete, as far as I can see it might not be required or even wanted to patch the y axis it does have an effect on the rectangle drawn while the mouse hovers over the menu and it might not be centered if you apply it, as the +4 screen shot shows.

Code: [Select]

function GetVistaBarMenuMetrics(const AMenuItem: TMenuItem; DC: HDC): TVistaBarMenuMetrics;
var
  Theme: HTHEME;
  TextRect: TRect;
  W: WideString;
  AFont, OldFont: HFONT;
begin
  Theme := TWin32ThemeServices(ThemeServices).Theme[teMenu];
  GetThemeMargins(Theme, 0, MENU_BARITEM, 0, TMT_CONTENTMARGINS, nil, Result.ItemMargins);

  if AMenuItem.Default then
    AFont := GetMenuItemFont([cfBold])
  else
    AFont := GetMenuItemFont([]);

  OldFont := SelectObject(DC, AFont);

  W := UTF8ToUTF16(AMenuItem.Caption);
  GetThemeTextExtent(Theme, DC, MENU_BARITEM, 0, PWideChar(W), Length(W),
    DT_SINGLELINE or DT_LEFT or DT_EXPANDTABS, nil, TextRect);
  Result.TextSize.cx := TextRect.Right - TextRect.Left + 1;     // Patched line
  Result.TextSize.cy := TextRect.Bottom - TextRect.Top + 1;  // Patched line
  if OldFont <> 0 then
    DeleteObject(SelectObject(DC, OldFont));
end;             


[engkin]

@Taazz, can you post a screenshot of the component bar with the themes disabled. There are two more small mistakes that I saw but did not mention.

[taazz]

@Taazz, can you post a screenshot of the component bar with the themes disabled.

Here you go.

edit : took a new screen shot with a smaller windows to show the borders as well just in case they are needed.

[Martin_fr]

I think, ths is more complicated.

From the docs at msdn, I do think we need +1

Background:
- Microsoft rect = 11,11,  20,20 
  top left INSIDE the rect at 11,11
  bottom right INSIDE the rect at 20,20
  height = 10 (20 - 11 + 1)
- LCL rect  = 11,11,  20,20 
  top left INSIDE the rect at 11,11
  bottom right OUTSIDE the rect at 20,20
  height = 9 (20 - 11)

In LCL the coordinates are not the pixel but the border between 2 pixels.

---------------------------
However, if we adjust the width, then we have to look for other places where we get a rect form the OS.

Code: [Select]

procedure DrawVistaMenuBar(const AMenuItem: TMenuItem; const AHDC: HDC; const ARect: TRect; const ASelected, ANoAccel: Boolean; const ItemAction, ItemState: UINT);

around line 602 / 625

Code: [Select]

  TextRect := ARect;
  inc(TextRect.Left, Metrics.ItemMargins.cxLeftWidth);
  dec(TextRect.Right, Metrics.ItemMargins.cxRightWidth);
  inc(TextRect.Top, Metrics.ItemMargins.cyTopHeight);
  dec(TextRect.Bottom, Metrics.ItemMargins.cyBottomHeight);

.....

  // draw text
  TextRect.Top := (TextRect.Top + TextRect.Bottom - Metrics.TextSize.cy) div 2;
  TextRect.Bottom := TextRect.Top + Metrics.TextSize.cy;

2nd part is to take the margin off again.

If that is applied to the  rect from above (assuming margin=0 fer simpler calc), assuming the old (wrong) height of 9, we get (Y)
 ( 11 + 20 - 9 ) div 2

"bottom - to_low_heigh" (20-9) will always be equal to "top" therefore this is "2*top div 2"

If you increase the height, then it is above top. So that is why text looks not centred.

TextRect.Bottom := TextRect.Top + Metrics.TextSize.cy;
Does undo the error in the height, so with fixed height it does do much. Because it again assumes LCL rect
--------------
I compared the current menu with notepad. It is exactly the same. (except the cut off)

[taazz]

well the first calculation is to convert a rect to width/height which is end-start+1 and the second is to convert a width/height to a start end so yeah it is required when then start coordinate is include in the rect to be drawn. There is no margin included in here for this conversions at least.

[Martin_fr]

I committed a fix for the menu caption in 44034


It looks the same issue repeats itself for other stuff (e.g. the shortcut text in the menu).

Each of them must be checked individually, and checked for the same kind of double fault, before they can be fixed one by one.

Please report them on the bug tracker. So they want get lost. (I will not currently do them myself, got a full todo list already)

[Martin_fr]

I am not sure the fix is right though, may have to find another.

I noted that it spaces the entire menu out. (compared to notepad)

[Martin_fr]

Ok, I overlooked a bit on msdn, and misinterpreted some other statements, after reading again.

The rect are same as LCL. So no +1 needed.
I reverted that

------
However, I found this (but don't yet have an explanation)

Code: [Select]

function VistaBarMenuItemSize(AMenuItem: TMenuItem; ADC: HDC): TSize;
....
  // item margins. Seems windows adds that margins itself to our return values

However the margin for y, top/bottom on my PC is 3 each.

The rect received by

Code: [Select]

procedure DrawVistaMenuBar(const AMenuItem: TMenuItem; const AHDC: HDC; const ARect: TRect; const ASelected, ANoAccel: Boolean; const ItemAction, ItemState: UINT);

is only 4 pixel higher than the height that was returned.

This is fixed by

Code: [Select]

  TextRect := ARect;
  inc(TextRect.Left, Metrics.ItemMargins.cxLeftWidth);
  dec(TextRect.Right, Metrics.ItemMargins.cxRightWidth);
  inc(TextRect.Top, Metrics.ItemMargins.cyTopHeight);
  dec(TextRect.Bottom, Metrics.ItemMargins.cyBottomHeight);
....
  TextRect.Top := (TextRect.Top + TextRect.Bottom - Metrics.TextSize.cy) div 2;
  TextRect.Bottom := TextRect.Top + Metrics.TextSize.cy;

e.g
if we measure a heigh of 15
we get a rect top = 28 bottom = 47  (that is a height of 19)
but margin (top/bottom) are 3 each

Now subtracting the margin, we are 2 pixel lower than needed.

Y is ismple set to be centered with the correct height. So y is then fine.
But X is not corrected. But it PROBABLY suffers the same issue.


So that means whatever windows did add, it was not the same margin that we use.

But I do not know what it does add, so no idea how to get the right value.

[engkin]

...
So that means whatever windows did add, it was not the same margin that we use.

But I do not know what it does add, so no idea how to get the right value.

1-Windows gets basic width from VistaBarMenuItemSize.

Code: [Select]

  BasicWidth := VistaBarMenuItemSize.cx;

2-When an item needs to be painted Windows passes ARect that includes VistaBarMenuItemSize.cx plus the unknown value.

Code: [Select]

  UnknowValue := ARect.Right - ARect.Left - BasicWidth;

I assume half of this value is needed to correct the menu problem.

[Martin_fr]

That assumption only works, if padding is equal on both sides.
Window explicitly can specify padding for each of the 4 sides:

So we are looking more for something like

Code: [Select]

UnknowValue := (ARect.Right - PaddRight) - (ARect.Left + PadLeft) - BasicWidth;

which is negative in our case, and added back. But only, if half of it is not bigger than the padding on that side was to begin with.
And only if we assume the error is equal on both sides.

Though yes, if we can not find a definition, then we have to correct it somehow like this.

[Martin_fr]

Update: I just debugged the left right margins (my previous observations where the top/bottom only/ followed by an if then maybe scenario).

The left/right margins are correct (on my PC.). 
So there must be something else still to it, that we do not know.

I noticed on your patch "+4" the menubar itself did not get any higher. So the 4 extra Pixel where not allocated. If windows squeezes the item in like that, then the margins will indeed be to big for y.
But in X direction no squeezing is needed.
MAYBE that is why?

----------------------------

I wonder what the actual values are, if you debug them on your system.

Does

Code: [Select]

  GetThemeTextExtent(Theme, DC, MENU_BARITEM, 0, PWideChar(W), Length(W),
    DT_SINGLELINE or DT_LEFT or DT_EXPANDTABS, nil, TextRect);
in "function GetVistaBarMenuMetrics" actually return the correct width? Compared against pixels counted by hand?


But before I sent you off to do anything:
I don't know how much work I will spent on this. My work area is SynEdit and the debugger. Occasionally I pick up something else, and see if I can find something. But I may drop it as well....


Also what are you DPI settings?

True-type? Does window do sub-pixels? Maybe the measurement is rounded down in your case.

I also noted, your window theme uses thinner borders, and smaller fonts than mine.

------------------
You did a compare of the lazarus menu against some other (with the vertical compare lines.

How does it compare against notepad (including widow decorations, and all)?

[taazz]

I think that we should make sure what the differences are visually before going in for a debug session. As it is now I have no idea what I'm looking for and why. My test have reveled so far a small placement difference vertically in the menu's text notepad paints it one pixel lower than lazarus. That might be the difference of rounding as you have said your self or simple put the difference between GDI and GDI+ and the coordinate changes (ee integer instead of single etc). Also I do not see any difference in the actual menu height too so I need to go on themes again for more screen shots because I remember a clear difference on the box around the menu on the +4 patch.

I wonder what the actual values are, if you debug them on your system.

Does

Code: [Select]

  GetThemeTextExtent(Theme, DC, MENU_BARITEM, 0, PWideChar(W), Length(W),
    DT_SINGLELINE or DT_LEFT or DT_EXPANDTABS, nil, TextRect);
in "function GetVistaBarMenuMetrics" actually return the correct width? Compared against pixels counted by hand?


But before I sent you off to do anything:
I don't know how much work I will spent on this. My work area is SynEdit and the debugger. Occasionally I pick up something else, and see if I can find something. But I may drop it as well....


Also what are you DPI settings?

it is reported as 96 DPI by windows. No HD it is an old acer laptop around the era of vista

True-type?

We are talking about windows 7 here I haven't changed the default fonts or size not as far as I remember, so yes true type and all the default sizes as well.

Does window do sub-pixels? Maybe the measurement is rounded down in your case.

How can I test for subpixel? I'm assuming here but I would say that all program that use GDI+ instead of GDI do subpixel.

I also noted, your window theme uses thinner borders, and smaller fonts than mine.

ehm I don't remeber where I based my current settings I think it was on a free theme named "Dark Skies By Tracy Hymas" I'll revert back to windows 7 default in my next screenshot session just in case.


Any way I'll spend more time as it becomes free so the more info I have gathered before hand the better. I do not expect you to do any patching your self I'll be content if I manage to create a good bug report, probably for the next version after 1.2.


thank you for your time spend so far.

[engkin]

That assumption only works, if padding is equal on both sides.
Window explicitly can specify padding for each of the 4 sides:

So we are looking more for something like

Code: [Select]

UnknownValue := (ARect.Right - PaddRight) - (ARect.Left + PadLeft) - BasicWidth;


I agree, and with the correct values for PadLeft and PadRight:
(ARect.Right - PadRight) - (ARect.Left + PadLeft) - BasicWidth should yield 0. Without their individual values, we have their sum:
UnknownValue := PadLeft + PadRight;

Counting pixels on the menu bar of Notepad on my system (XP), I see:
  PadLeft := 7;
  PadRight := 6;
and also
  PadUp := 3;
  PadDown := 2;
 
I don't know if this is true in general, but:
  PadLeft := PadRight + 1;
  PadUp := PadDown + 1;

Update: I just debugged the left right margins (my previous observations where the top/bottom only/ followed by an if then maybe scenario).

The left/right margins are correct (on my PC.). 
So there must be something else still to it, that we do not know.

Taazz saw the problem on his Windows 7 32 bit when he enabled the themes.
Henppy has Windows 7 64 bit with themes enabled but did not see this problem.
Are you using 32 or 64 bit OS?

I noticed on your patch "+4" the menubar itself did not get any higher. So the 4 extra Pixel where not allocated. If windows squeezes the item in like that, then the margins will indeed be to big for y.
But in X direction no squeezing is needed.
MAYBE that is why?

I think the height of the menu bar has nothing to do with item height. Usually you can find its value using:
GetSystemMetrics(SM_CYMENUSIZE) and GetSystemMetrics(SM_CYMENU). The first one is identical to the value you can read/set using:
Display Properties
  Appearance
    Advanced Appearance
      Item: Menu,
        Size: ??

It might help to add DT_NOCLIP to TextFlags to disable clipping for drawing the text in DrawVistaMenuBar:

Code: [Select]

  TextFlags := DT_SINGLELINE or DT_EXPANDTABS or DT_NOCLIP;


[Martin_fr]

notepad paints it one pixel lower than lazarus

You picture is not themed? Why, we were looking at code for themed only until now. Un-themed is handled elsewhere.

As for themed, on my PC notepad and Lazarus are identical. At least placement wise. Notepad paints a 1pixel white border on the left, Lazarus does not.

Anyway differences in Y placement may be explicable, *IF* may observation/assumption on squeezing into a fixed height is correct. IF so, there would have to be a special rule on Y placement.

X placement appears fine though.

As it is now I have no idea what I'm looking for and why.

Does

Code: [Select]

GetThemeTextExtent(Theme, DC, MENU_BARITEM, 0, PWideChar(W), Length(W),
    DT_SINGLELINE or DT_LEFT or DT_EXPANDTABS, nil, TextRect);
return the correct Value.

E.g
- Say that returns left=0 (left is always 0) and right=20; which means a width of 20 pixels.
- Then you snapshot the menu, zoom and count the amount of visible pixels of that menu caption.

Now if you can see 20 pixels, and the 21st is missing, then painting is fine, and calculation in the code that we looked at is also fine. It means that measuring goes wrong.

Code: [Select]

How can I test for subpixel? I'm assuming here but I would say that all program that use GDI+ instead of GDI do subpixel.
Sorry no idea. Not exactly the area of my expertise.

But, if your systems says that the text "File" is 20.4 pixel width, and Lazarus measures with a method tha gets full pixel only, then what will Lazarus receive?
But again this is speculation. I do not know this part of windows in that much detail..

----------------------

It might help to add DT_NOCLIP to TextFlags to disable clipping for drawing the text in DrawVistaMenuBar:

Yes, but that is, IF the error is in the drawing part of the code.

If the error is some place else, we should fix it some place else. Only if we exhausted all roads and have not found the place, then may we thing about adding a workaround in the "wrong" place.