Static control overlapping siblings when Alt key is pressed

[kkurkiewicz]

In an attempt to adjust the color of a simple graphics static box intended to display a filled rectangle under a small cluster of controls, I replaced the style SS_WHITERECT that I defined initially with SS_OWNERDRAW and implemented the required WM_DRAWITEM handler, but there's something really wrong, and I just can't tell what it is.

The program in question is a modeless dialog box that displays one edit control, three regular buttons, and the filled rectangle I already mentioned. When run, the program appears to work okay but, as soon as I press Alt, the rectangle is brought to the top of the z-order, which completely hides its siblings. Interestingly, even when they're hidden, the three (standard) buttons are still clickable, and it is possible to get them back by pressing Tab or simply clicking them. However, the edit control is not fully redrawn until the window is first minimized and then restored, after which the program seems to work just perfectly (that is, after the window is restored, pressing Alt again has no effect). But that's not all.

First of all, the bug manifests itself only when I start the program by running the executable file manually—when I run it in Visual Studio (either normally or with debugging) or via Command Prompt, nothing ever happens. Using IDA, I found out that every time I press the key, the DRAWITEMSTRUCT parameter of WM_DRAWITEM is "05 00 00 00 E9 03 00 00 28 01 00 00 01 00 00 00 00 00 00 00 F6 06 0C 00 54 02 01 C0 00 00 00 00 00 00 00 00 86 01 00 00 55 00 00 00 00 00 00 00", while initially it is "05 00 00 00 E9 03 00 00 00 00 00 00 01 00 00 00 00 01 00 00 F6 06 0C 00 53 0F 01 26 00 00 00 00 00 00 00 00 86 01 00 00 55 00 00 00 00 00 00 00". That is, when the message is caught, the itemID and itemState fields are 128h and 0 (respectively), while initially the values are 0 and 100h. It doesn't make any sense to me, however, because I don't have a menu, and the value itemState=0 isn't even listed in WinUser.h. The only noteworthy thing is that the initial value itemState=100h means that the control is to be drawn without the keyboard accelerator cues, which brings us to accelerators.

The problem can be 'fixed' by going to Control Panel > Ease of Access Center > Make the keyboard easier to use and ticking the checkbox Underline keyboard shortcuts and access keys. I don't know if "access key" is different from "accelerator", but I think that this may also be related as I have three virtual-key characters associated with the buttons, e.g., "N", 1003, VIRTKEY, where 1003 is the identifier of the button New. Is it correct to define them like this? Or maybe, is it possible to define a real accelerator without using the ampersand character and still have the first letter underlined?

Also, the program exampl03\menudemo\menudemo.asm, which seems to be the only example program that makes use of owner-drawn controls in masm\examples, ends its WM_DRAWITEM handler with mov dis.itemState, ODS_DEFAULT\mov dis.hdc, 0. Is it necessary?

There are three somewhat related questions on Stack Overflow but, unfortunately, none comes with a satisfying answer. I'm listing them below. Could you please help me?

________________________________________________________________________________

1. Ownerdraw controls overlaps when main window is minimized
2. Strange behaviour when customdrawing a button
3. How to filter Alt key event for WM_DRAWITEM message

[NoCforMe]

I just took a cursory look at your code. Nothing pertaining to your problem jumps out at me yet. However, some suggestions:

• Please learn to use INVOKE instead of push-push-call: it makes your code a hell of a lot easier to read:
  
  Code Select Expand
      INVOKE    CreateDialogParamA, HINST, OFFSET DIALOGNAME, 0, OFFSET DLGPROC, 0
; versus
    PUSH  0
    PUSH  OFFSET DLGPROC
    PUSH  0
    PUSH  OFFSET DIALOGNAME
    PUSH  HINST
    CALL  CreateDialogParamA@20
(notice that you can lose the A@20 business since you're using a prototype)
  
• Don't use the old-school SEGMENTs; that's so 16-bit DOS. Just use the new simplified ones: .data, .data? and .code.
• Lose the message loop; you don't need it here, as the dialog manager establishes its own loop. Dunno if this is causing any problems, but it definitely doesn't help.
• Please don't use those horrible EBP+xxx constructs for proc parms. Define the procedure properly and you can use named variables (what a concept!). It'll make your code less error-prone and easier to maintain.
  
  Code Select Expand
  DLGPROC PROC hWin:HWIN, uMsg:DWORD, wParam:DWORD, lParam:DWORD
; instead of
;   [EBP+14H]: LPARAM -- Additional message-specific information
;   [EBP+10H]: WPARAM -- Additional message-specific information
;   [EBP+0CH]: MES -- The message
;   [EBP+08H]: HWND -- A handle to the dialog box


Sorry if I seem like a scold, but the easier you make your code for us to read, the more likely someone here can help you.

I'll look at your specific problem shortly. I should say you deserve credit for clearly documenting your problem here.

[HSE]

Static don't work like container.
You can try to replace Static with GroupBox.

[fearless]

I believe its to do with the tab index order, or the order in which the controls are created via the dialog manager that is causing the static to appear on top when the alt key is pressed. I recall vaguely something I read somewhere about the Z order not being what its supposed to be or something. Anyhow long story short is if you re-arrange the order the controls are listed in the rc file to:

Code Select Expand

DIAL1 DIALOG 0, 0, 390, 290
FONT 8, "MS Sans Serif"
STYLE WS_CAPTION | WS_SYSMENU | WS_MINIMIZEBOX | WS_VISIBLE
{


    CONTROL "", 1002, "edit",
            ES_CENTER | ES_READONLY | WS_BORDER | WS_CHILD | WS_TABSTOP | WS_VISIBLE,
                    75, 151, 240, 12

    CONTROL "&New", 1003, "button",
            BS_CENTER | BS_PUSHBUTTON | WS_CHILD | WS_TABSTOP | WS_VISIBLE,
                    125, 171, 40, 12

    CONTROL "&Copy", 1004, "button",
            BS_CENTER | BS_PUSHBUTTON | WS_CHILD | WS_TABSTOP | WS_VISIBLE,
                    175, 171, 40, 12

    CONTROL "E&xit", 1005, "button",
            BS_CENTER | BS_PUSHBUTTON | WS_CHILD | WS_TABSTOP | WS_VISIBLE,
                    225, 171, 40, 12

    CONTROL "", 1001, "static",
            SS_OWNERDRAW | WS_CHILD | WS_CLIPSIBLINGS | WS_VISIBLE,
                    65, 141, 260, 52

}

I added the & to the text in the controls, so that the accelerator is underlined when the ALT key is pressed.

Doesn't seem to be an issue with declaring the accelerator the same id as the button. For my own use and as a standard I like to keep the defines for suchlike unique, but that requires additional checking in the WM_COMMAND for them as well. Maybe some edge case issues could crop up, but cant think of anything at the moment.

Also prob no harm adding in WS_EX_TRANSPARENT to the static so that mouse clicks are ignored. But I tested and it seems to work without it, so no worries either way. Add the define to your rc:

Code Select Expand

// Extended Window Styles
#define WS_EX_TRANSPARENT 0x00002000L
and then include the extended style like so:

Code Select Expand

    CONTROL "", 1001, "static",
            SS_OWNERDRAW | WS_CHILD | WS_CLIPSIBLINGS | WS_VISIBLE,
                    65, 141, 260, 52,  WS_EX_TRANSPARENT


[NoCforMe]

Static don't work like container.

I'm not so sure of that. I've successfully used static controls as containers, but there are issues to be dealt with.

I'm wondering if the OP needs to position the window on the Z-axis so things layer as he wants them to (using SetWindowPos()).

[Greenhorn]

Looks like your program is some kind of KeyGen.
I hope this is not for illegal purposes.

Code Select Expand

; Ensure that the device context identified by HDC is in the default state and return
    MOV  PDIS.ITEMSTATE, 20h  ; ODS_DEFAULT    ; not necessary or even maybe wrong, don't touch it !
    MOV  PDIS.HDC, 0                           ; This is wrong ! (See explanation below)

"Ensure that the device context identified by HDC is in the default state" means that you must not leave any GDI object that you selected into the HDC in it.
So, always save the current object returned by SelectObject and after use select it back into the HDC.

EDIT:
Just saw that you use a copy of the DRAWITEMSTRUCT. This si not necessary.
You can use [ESI].DRAWITEMSTRUCT.HDC, etc. ...

[kkurkiewicz]

Greenhorn—the MOV PDIS.ITEMSTATE, ODS_DEFAULT\\MOV PDIS.HDC, 0 fragment is copied from exampl03\menudemo\menudemo.asm.

And addressing all of you, thank you for all the suggestions. I may not be able to try them out until Wednesday, but I'll definitely get back to you as soon as possible.

[kkurkiewicz]

fearless—when declaring the accelerators, I reused the IDs of the buttons precisely because I wanted to avoid those additional checks, but otherwise you are right. Just reordering the controls so that the static one comes last fixes the problem completely.

My only follow-up question would be if it's possible for the Alt key to show and hide the accelerator cues alternately, just as in Visual Studio or Word 2016.

[fearless]

Not sure, but probably have to look at something to do with the following:

• WM_UPDATEUISTATE
• WM_CHANGEUISTATE
• Untangling the confusingly-named WM_UPDATEUISTATE and WM_CHANGEUISTATE messages
• Who sends the initial WM_UPDATEUISTATE message?

[kkurkiewicz]

Just saw that you use a copy of the DRAWITEMSTRUCT. This si not necessary.
You can use [ESI].DRAWITEMSTRUCT.HDC, etc. ...

In masm\examples\exampl03\lcd\lcd.asm, there is the following function definition. Can I also use assume? Is there any real difference?

Code Select Expand

GetPositionFromString proc uses esi edi  lpString:DWORD,lpPosition:DWORD
; Function to convert TMSF string to CDPOSITION structure.
mov edi, lpPosition
assume edi:ptr CDPOSITION
mov esi, lpString
invoke GetNextNumberFromTime
mov [edi].Track, al
invoke GetNextNumberFromTime
mov [edi].Minute, al
invoke GetNextNumberFromTime
mov [edi].Second, al
invoke GetNextNumberFromTime
mov [edi].Frame, al
ret
GetPositionFromString endp


[NoCforMe]

Those ASSUMEs are completely superfluous. Get rid of them.

As long as esi & edi are pointing towards the appropriate places, that's all that matters.

In other words, the caller of GetPositionFromString() can be assumed (by us humans, not the assembler) to have passed the correct addresses in those two parameters, lpString and lpPosition. The subroutine simply obeys orders based on those addresses.

In other other words, if those parameters were somehow passed incorrectly, no ASSUME is going to fix that.
Ack. I'm wrong here. Forgot you were dealing with structure members. Nevermind ...

So all this just to avoid typing

Code Select Expand

[EDI].STRUCTNAME.member
Me, I just copy and paste. That way it's obvious what's being dealt with, instead of masking it behind a macro or EQU. But that's just me ...

[jj2007]

Can I also use assume?

You can, certainly. But there are more elegant ways to do that.

Code Select Expand

    cdp equ [edi.CDPOSITION]
    mov edi, lpPosition
    mov cdp.Track, al
    mov cdp.Minute, al

[NoCforMe]

Or how 'bout not being lazy and typing

Code Select Expand

[edi].CDPOSITION.Track
Is that really so hard? Are we that spoiled?
Besides, to me it's much more obvious when revisiting the code a year later and trying to figure out what the hell you were trying to do ...

[kkurkiewicz]

How would you rewrite PUSH  OFFSET PDIS.RCITEM?

[NoCforMe]

So PDIS is a global** structure, correct? And you want to push the address of the RCITEM member, yes?

I wouldn't rewrite it. Why would you want to? What's wrong with it as it is?

** We can tell it's a global because of OFFSET. If it were a LOCAL we'd have to use ADDR instead.