Hi,

The problem is that in the WM_COMMAND message, instead of wNotifyCode = 0, we get random values.

After some debugging I found out that MENUINFO.WID is a 32 bit integer, but it contains a 16 bit application defined value. It seems the application is responsible for providing an increasing set of numbers in this identifier. Currently (see win32object.inc:2610) it is assigned a pointer which is a 32 bit value and thus too wide. It could be that the upper 16 bits are used for the WM_COMMAND.wNotifyCode but I am not sure.

This is just a suggestion, because sometimes the menu's simply disappear if you change the wid to something else.

HTH,

Micha.

Okay, the patch is ready and attached. Half of the first item of the TODO can be stripped now (implement menu item ids). It worked with a simple menu structure, but we need to test it with lots of menu items to see if the TBits are functioning well. We can also try checking whether disabling/enabling checking/unchecking works as expected, ie. not call the handler when disabled.

It assumes the FCommand field of TMenuItem has been moved to the public section. (See my previous mail). Maybe there is a cleaner solution for this.

HTH,

Micha.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: bin00029.bin
Type: application/octet-stream
Size: 4757 bytes
Desc: "menupool.patch"
Url : http://localhost/pipermail/lazarus/attachments/20030624/293223f5/bin00029.bin

Thanks. Applied.

I would like to hear your comments in the patch (probably not correct coding style), it's a bit ugly (using pointers for the item ids), but the current code is just wrong (with respect to the menu item ids).

I can't test it, so I can't say much. Can you create a short list which example and/or component does/does not work and what needs to be done? Just start it, it does not need to be complete. Maybe this will help other win32 devels.

Mattias

As for the above patch: there should be the possibility to store the command ID in the menu item records, and the application should keep track of these commands. In Delphi, it is done using the TBits class:

var
  CommandPool: TBits;
function UniqueCommand: Word;
begin
  Result := CommandPool.OpenBit;
  CommandPool[Result] := True;
end;

and in TMenuItem.Create:

...

  FCommand := UniqueCommand;

..

When the menu item is destroyed, the bit is freed again:

  if FCommand  0 then
    CommandPool[FCommand] := False;

Just clipping the lower 16 bits may work most of the time, but does not guarantee a unique ID.

Michael.

Hi,

Michael Van Canneyt wrote

Just clipping the lower 16 bits may work most of the time, but does not guarantee a unique ID.

My concern exactly. Does the fcl have a compatible TBits class? (In terms of functionality)? That would solve it easily, no?

Regards,

Micha.

Hi,

I can't test it, so I can't say much. Can you create a short list which example and/or component does/does not work and what needs to be done? Just start it, it does not need to be complete. Maybe this will help other win32 devels.

The major things which I noticed: (not complete)

1) the menus (of course), testing and proper implementation of menu item ids.
2) form sizes are not honoured, they are often way too big.
3) some examples use TPromptDialog (but problem could be general), in these dialogs there is no text. In particular, TBitBtn's don't have a caption, but they do have an image.
4) TTimers crash the application internally...application hangs.

HTH,

Micha.

Ah, I see. I will add the Command functions.

The alternative is to create a separate mapping menu-item->command, and look up the the menu item from the command, but this requires an extra list handling, in addition to the TBits array.

Mattias

Hi.

I have confirmed my above statements. The attached (very ugly) patch shows how to make the menus work. It basically cuts off the high part of the pointer, forcing it to be a 16 bit value as windows expects. Ideally, one would keep a counter of some sort to keep track of used menu item ids (also known as commands).

The patch attached proves that using 16 bit integers, the menus will work. Apply it in the lazarus/lcl/interfaces/win32 directory.

I would like to hear your comments in the patch (probably not correct coding style), it's a bit ugly (using pointers for the item ids), but the current code is just wrong (with respect to the menu item ids).

HTH,

Micha.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: bin00028.bin
Type: application/octet-stream
Size: 18142 bytes
Desc: "win32object.patch"
Url : http://localhost/pipermail/lazarus/attachments/20030624/84b54e52/bin00028.bin

Can I get write access to TMenuItem.FCommand from Win32Object? I also would prefer a name like MenuItemID, but that's up to you, because that is the term most used in Win32API docs, command is only used in BY_COMMAND flags. We might want to keep it to be alike delphi. It seems that FCommand is not used anywhere else, is it?

I think the UniqueCommand code needs to be stripped from lcl/menus.pp. It is only confusing there IMHO.

Regards,

Micha.

Hi,

Michael Van Canneyt wrote

> My concern exactly. Does the fcl have a compatible TBits class? (In > terms of functionality)? That would solve it easily, no? TBits is implemented, so you can try it.

Hmm, it seems it uses 4 bytes per bit entry. A little overkill if you ask me :). They surely could have simpy used 1 byte? Anyway, I will try it. Shall I create the bit array in win32int.pp?

Regards,

Micha.

Win32 has a handle per menu item, but the *command* associated with the menu can be shared between menu items, so this needs to be stored with the menu item. The alternative is to create a separate mapping menu-item->command, and look up the the menu item from the command, but this requires an extra list handling, in addition to the TBits array.

Michael.

TBits is implemented, so you can try it.

Michael.

Eh, I don't know why FCommand is still in the LCL. Probably as we started, we didn't know, if it is needed for Delphi compatibility. But this is not the case. The FCommand is a win32 specific thing to identify handles. So, it should not be stored in the LCL, but in the win32 interface. A Handle must be enough to identify any interface component.
Maybe the win32 api identifies it otherwise. If this is the case, then the win32 interface should map the handles.

I will have a look ...

Mattias

Wouldn't limit that forms to 256 controls?

After a second look, I see it doesn't use 4 bytes per bit, only 1 bit per bit, but they do allocate in 4 byte blocks, probably for efficient integer operations. They first div by 32 * 4 and then multiply by 4 and add some.

Anyway, the implementation doesn't matter much, as long as I can find a way to dynamically increase the size and find out how to determine if all bits are set. The return value of OpenBit still confuses me a bit :). Sometimes it sets Result to FSize * 32, but sometimes it does not.

Regards,

Micha.

Heh. No. Please take a and you will understand what I mean. TBits is a long array of 'bits'. Undeterminate length in delphi, in fcl it seems you need to know in advance :(. Each bit takes a certain amount of space. The smallest is 1/8th of a byte (as delphi uses), but the fcl uses 4 bytes.

Regards,

Micha.

Right.
Although the ID should be stored somewhere in the win32 interface, since they are win32 specific.

Mattias

And another patch to fix the check/enable issue. Like the other patch: apply in lcl/interfaces/win32.

Regards,

Micha.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: bin00030.bin
Type: application/octet-stream
Size: 1017 bytes
Desc: "menucheckenable.patch"
Url : http://localhost/pipermail/lazarus/attachments/20030624/8e7bef3e/bin00030.bin

Please do so.

I have added your todos to docs/TODO.

Mattias

The author of Synapse is working on a TCP/IP suite for FPC and uses cross-compiling. After some makefile woes, he has everything compiling without problems, it seems.

Michael.

The MenuItemIdPool is unneeded, since the LCL does it like the VCL in menus.pp. Every TMenuItem creates its own unique Command.

Mattias

Applied.

I have added the public property Command: integer; to TMenuItem. The FCommand ist set just like Delphi on creation and destruction. I replaced the FCommand in the win32 intf by Command. Please check if this is correct. (I have to find out, how this cross compiling works ...)

Mattias

+ From: Micha Nelissen
+ Sent: dinsdag 24 juni 2003 18:03
+
+ Michael Van Canneyt wrote:
+ > On Tue, 24 Jun 2003, Mattias Gaertner wrote:
+ >
+ >>
+ >> Eh, I don't know why FCommand is still in the LCL. Probably as we
+ >> started, we didn't know, if it is needed for Delphi compatibility.
+ >> But this is not the case. The FCommand is a win32 specific thing to
+ >> identify handles. So, it should not be stored in the LCL, but in the
+ >> win32 interface. A Handle must be enough to identify any interface
+ >> component.
+ >> Maybe the win32 api identifies it otherwise. If this is the case,
+ >> then the win32 interface should map the handles.
+ >>
+ >> I will have a look ...
+ >
+ > Win32 has a handle per menu item, but the *command* associated with
+ > the menu can be shared between menu items, so this needs to be stored
+ > with the
+
+ I don't know how useful it would be to have two menu items firing the same
+ event... :).

Hmm.. a bit digging in my memory, from the time I was programming win 3.11 API...
I think the M$ idea behind it was a bit the same as Delphi does with actions.

You could associate a commandID to buttons, toolbuttons, menuitems and popup menus, and implement the command at one place. Specially for popmenus and shared MDI menus you must be able to assign the same command. (Popmenus were hidden menuitems)

I think nowadays you don't need it.

Marc

Hehe, try my adapted progressbar example, better to be called menu example. (Not to be committed to CVS :) ). It creates lots of menus.

Regards,

Micha.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: bin00031.bin
Type: application/octet-stream
Size: 9803 bytes
Desc: "progressbar.pp"
Url : http://localhost/pipermail/lazarus/attachments/20030625/9f1f4528/bin00031.bin

I don't know how useful it would be to have two menu items firing the same event... :).

Regards,

Micha.

So basically, in it's current state, the win32 port should now have working menus ? This I should test tomorrow :-)

Michael.

Right you are.

Mattias

Ah ok. But then still, in win32object.inc you need something like:

@@ -2614,6 +2610,8 @@
     fMask:=Mask;
     fType:=Style;
     {fState:=MFS_ENABLED;} {not needed}
+    wID:=(Sender as TMenuItem).Command; {value may only be 16 bit wide!}
     hSubmenu:=MenuHandle;
     {hbmpChecked:=0;
     hbmpUnchecked:=0;} {not needed}
---end patch---

If you don't assign the ID, then windows doesn't know about it and it was all for nothing :) !

Regards,

Micha.

I think you have deleted some by accident; you still need to apply:

@@ -537,6 +534,7 @@
           Begin
             DeleteMenu((Sender as TMenuItem).Parent.Handle, Handle,
MF_BYCOMMAND);
             { release menu item id }
+            MenuItemIdPool.Bits[(Sender as TMenuItem).Command] := False;
             DrawMenuBar(((Sender as TMenuItem).Owner as
TWinControl).Handle);
           End
           Else
@@ -2614,6 +2610,8 @@
     fMask:=Mask;
     fType:=Style;
     {fState:=MFS_ENABLED;} {not needed}
+    wID:=UniqueMenuItemId; {value may only be 16 bit wide!}
+    (Sender as TMenuItem).Command := wID;
     hSubmenu:=MenuHandle;
     {hbmpChecked:=0;
     hbmpUnchecked:=0;} {not needed}
---end patch---

Regards,

Micha.