NOTE: 2nd page branches into an optimization discussion and bug fixes.
http://wolfshade.home.ro/project1.asm
(http://img246.imageshack.us/img246/34/erroryo1.th.jpg) (http://img246.imageshack.us/my.php?image=erroryo1.jpg)
or
http://wolfshade.home.ro/error.JPG
loop uses the value of ecx for counting the number of loops - of course, you know this because you're setting it for 4 loops :wink
When you call another function (MoveWindow, in this case), you should assume it will mess up the value of ecx (esi, edi, ebx, and ebp are generally safe, assume all others messed up; esp is a special case, but it's considered safe) - meaning every time you call MoveWindow, the value for ecx is made 'random' and it's no longer your counter.
Simple solution - push the value before the call, pop it afterwards.
xor ecx, ecx
mov ecx, 4
push esi
xor esi, esi
label2::
mov i, esi
.
.
push ecx ; <****************
invoke MoveWindow, [ChildIDsArray+esi], temp1, 0, temp2, temp3, TRUE
pop ecx ; <****************
inc esi
loop label2
pop esi
P.S. Post long code 'snippets' as zipped attachments :wink
If you would've assembled it you would've got a "cannot read memory location err" as the title suggests.
I didn't care for the MoveWindow part because the trouble starts since the CreateWindow loop at array indexing part i think where i try to move the returns of CreateWindow(child handles) into an array. The error is quite serious me thinks.
This part contains the problem i can't solve:
mov ecx, 13; i = 12
push esi
xor esi, esi
label1::
mov edx, 13000
add edx, ecx
mov temp, edx
push ecx
push esi
invoke CreateWindowEx, NULL, ADDR ChildClassName, NULL,\
WS_CHILDWINDOW + WS_VISIBLE,\
CW_USEDEFAULT, CW_USEDEFAULT, CW_USEDEFAULT, CW_USEDEFAULT,\
hwnd, [temp], hInstance, NULL
pop esi
mov [ChildIDsArray+esi], eax
add esi, 4
pop ecx
loop label1
pop esi
I did assemble and run the code, and got the error.
I fixed it and then showed you how to fix it.
Try again before you start whining ::)
Thank you, this did the trick, my problem is i don't know how to debug, i tought the problem was still in the first loop:
Now i got another problem :'( :boohoo: i made this program in c aswell an now they both don't do what i intended them to do:
make each child window appear as a rectangle and when it gets a mouse click draw an "X" on it, it uses a flag made out of window extra bytes; the code is made after i learned the "checker3" program of Charles Petzold's Programming Windows.
The rectangles are supposed to appear at the top of the client area at x == 0/4, 1/4, 2/4, 3/4 times cxClient(client area pixels), y == 0 and have the width 1/4 and height 1/7 times cyClient.
Can you point me to whats going wrong please? Again when i made the C replica, my poor debugging skills pointed me to the array in wich the child handles sit(it was not getting filled for some reason).
Can you give me a quick explanation of the OllyDebugger for example, commands(except step into, step over, go to address?)
I can't jump to a specific address w/o having to use "go to address" functionality, and i have to use that a lot to look at the values the registers hold, also i don't understand what that assemble after an double click on code does.
mov temp2, 4
fld1 ; 1
fld temp2 ; 4, 1
fdivp ST(1), ST(0) ; 1/4
fld cxClient ; cxClient, 1/4
fmulp ST(1), ST(0) ; cxClient*1/4
fstp temp2 ; temp2 = ST(0), FPU stack balanced
mov temp3, 7
fld1 ; 1
fld temp3 ; 7, 1
fdivp ST(1), ST(0) ; 1/7
fld cxClient ; cxClient, 1/7
fmulp ST(1), ST(0) ; cxClient*1/7
fstp temp3 ; temp3 = ST(0), FPU stack balanced
mov ecx, 4
push esi
xor esi, esi
push edi
xor edi, edi
label2::
mov i, edi
mov temp1, 3
fld i ; i
fld temp1 ; 4, i
fdivp ST(1), ST(0) ; i/4
fld cxClient ; cxClient, i/4
fmulp ST(1), ST(0) ; cxClient*(i/4)
fstp temp1 ; temp1 = ST(0), FPU stack balanced
push ecx
push esi
push edi
invoke MoveWindow, [ChildIDsArray+esi], temp1, 0, temp2, temp3, TRUE
pop edi
inc edi
pop esi
add esi, 4
pop ecx
loop label2
pop edi
pop esi
this doesn't work like this:
case WM_SIZE :
cxClient = LOWORD(lParam) ;
cyClient = HIWORD(lParam) ;
for (i = 0 ; i < 4 ; i++)
MoveWindow (hwndChild[i],
cxClient * i/4, 0,
cxClient * 1/4, cyClient *1/7, TRUE) ;
1. Use "finit" before you start using the fpu (only needs doing once in your program)
2. "cxClient*(1/7)" is the same as "cxClient/7"
3. "fld" and "fst" move floating point numbers from and to memory. You're using integers, so you want "fild" and "fist" (with or without 'p')
4. "fdivp ST(1), ST(0)" -- if st(0)=1 and st(1)=4, then this is 4/1 (not 1/4) which results in 4 (not 0.25)
5. there are mul and div instructions for integers anyway, so you don't actually need to use the fpu
6. MoveWindow changes the position of the window -- which is why neither program does what you want
for drawing rectangles you want Rectangle (inside a WM_PAINT message)
7. functions mess up the values of ecx and edx, but not esi, edi, or ebx, so there's no need to keep push-popping these ;)
Now, here's your fpu stuff fixed. (Note this won't magically make rectangles appear, because you're using MoveWindow, not Rectangle)
Look at it carefully, there are a lot of small changes :wink
mov eax,lParam
mov edx,eax
and eax,0ffffh ;width
shr edx,16 ;height
mov cxClient,eax
mov cyClient,edx
mov temp2, 4
fld1 ; 1
;st(0) = 1.0
fild temp2 ; 4, 1
;st(0) = 4.0, st(1) = 1.0
fdivp ST(1), ST(0) ; 1/4
;st(0) = 0.25
fild cxClient ; cxClient, 1/4
;st(0) = cxClient, st(1) = 0.25
fmulp ST(1), ST(0) ; cxClient*(1/4)
;st(0) = (cxClient*0.25)
fistp temp2 ; temp2 = ST(0), FPU stack balanced
mov temp3, 7
fld1 ; 1
fild temp3 ; 7, 1
fdivp ST(1), ST(0) ; 1/7
fild cxClient ; cxClient, 1/7
fmulp ST(1), ST(0) ; cxClient*(1/7)
fistp temp3 ; temp3 = ST(0), FPU stack balanced
xor ecx, ecx
mov ecx, 4
push esi
xor esi, esi
label2::
mov i, esi
mov temp1, 3
fild i ; i
fild temp1 ; 3, i
fdivp ST(1), ST(0) ; i/3
fild cxClient ; cxClient, i/3
fmulp ST(1), ST(0) ; cxClient*(i/3)
fistp temp1 ; temp1 = ST(0), FPU stack balanced
You don't need to learn to debug, you need to learn how your program works. Debugging isn't much use unless you understand your program first.
Thank you, i will look carefully, i was about to start debugging(without much success perhaps).
Judging from your last post and status on forum i'm sure it will be fixed after i implement it all:).
finit initializes FPU, meaning it lets the assembler know when to initialize it right?
P.S. The C code was working by getclientrect() passing to rectangle() and drawing rectangle as big as the child windows from the childwndproc() in wm_paint; sry i only included the code needed to compare to the asm snippet i didn't think you would go that deep :)
Here is the whole C code if it matters, its the replica of this asm code hopefully it will help some1 else http://wolfshade.home.ro/project.txt *
*for some reason seems there is a block from this site i presume preventing me to link a cpp source?(link hit goes:403 Forbidden, you must copy/paste the link)
finit initialises the fpu - it's an actual instruction for the fpu to clean its registers and go to 'default settings'
You won't need GetClientRect, as you already got the width and height in WM_SIZE, from which you calculated cxClient and cyClient, so you can go straight to drawing the rectangles in WM_PAINT :U
Quote from: w0lfshad3 on August 22, 2006, 01:21:49 AMmy problem is i don't know how to debug...
Please consider this challenge as being the most important skill you could learn.
Regards, P1 :8)
In OLLYDBG i found that i get lasterr: ERROR_INVALID_WINDOW_HANDLE (00000578) on MoveWindow;
That comes from the initialization of the ChildIDsArray i mentioned in the first post of this thread.
Debugging further it seems that [temp] inserts the right value as an argument to the formal parameter hMenu.
However after i pass with F7 after that parameter it disappears? what does that mean?
When exiting CreateWindowEx EAX is 0 ??? It was supposed to contain a handle.This seems to be the problem but can't fix.
http://wolfshade.home.ro/Project.Asm (keep refreshing it s a crappy ftp)
Very good - you found the problem :wink
When CreateWindow returns zero, that means it failed -- creation of the child window failed.
Now you have to figure out why :bdg
Hint: When each child window is created, its ChildWndProc gets called, any messages you don't handle are passed onto DefWindowProc (this is correct). BUT what is the result of DefWindowProc? And what is the result of ChildWndProc? -- Are these the same? Should they be?
BIG HINT: ret
I had ret after DefWindowProc in ChildWndProc, still nothing :'( :boohoo: In my code i had it in the whole time.
updated: http://wolfshade.home.ro/Project.Asm
.else
invoke DefWindowProc, hwnd, uMsg, wParam, lParam
;THIS looks like a nice spot
.endif
xor eax, eax
ret
ChildWndProc endp
end start
Are you sure?
yes, updated link: http://wolfshade.home.ro/Project.Asm
same CreateWindowEx, same return
:dance: Hurray, it just stroke me, i'm using WNDCLASSEX but only RegisterClass, now some child windows started appearing after i registered them right. Thanks for the help. :U
Well you must have changed that some time in between posting and me fixing it, because in the version I fixed there was already RegisterClassEx - and so, simply adding the ret made it work.
(Even the earlier version: http://wolfshade.home.ro/project1.asm has RegisterClassEx)
Anyway, it WORKS :cheekygreen:
Note: there's no need to go crazy with the ret for every message; most can fall through to return zero at the bottom, but DefWindowProc should return its own value, so you can't let it fall through.
Note: there's no need to go crazy with the ret for every message;Charles Petzold, Programming Windows(read last line):
Quote
Get In and Out Fast
Windows 98 and Windows NT are preemptive multitasking environments. This means that as one program is doing a lengthy job, Windows can allow the user to switch control to another program. This is a good thing, and it is one advantage of the current versions of Windows over the older 16-bit versions.
However, because of the way that Windows is structured, this preemptive multitasking does not always work the way you might like. For example, suppose your program spends a minute or two processing a particular message. Yes, the user can switch to another program. But the user cannot do anything with your program. The user cannot move your program's window, resize it, minimize it, close it, nothing. That's because your window procedure is busy doing a lengthy job. Oh, it may not seem like the window procedure performs its own moving and sizing operations, but it does. That's part of the job of DefWindowProc, which must be considered as part of your window procedure.
If your program needs to perform lengthy jobs while processing particular messages, there are ways to do so politely that I'll describe in Chapter 20. Even with preemptive multitasking, it's not a good idea to leave your window sitting inert on the screen. It annoys users. It annoys users just as much as bugs, nonstandard behavior, and incomplete help files. Give the user a break, and return quickly from all messages.
And your point is...?
I didn't say don't return, I meant it's unnecessary to place "xor eax,eax; ret" directly after the processing of every message. The reason is simply because, if you look at the structure of your program, then you will see that when each message has been processed it will drop out of the .IF statements and be caught by the "xor eax,eax; ret" at the bottom. The extra few clock-cycles it takes to jump will not slow down the user interface.
WndProc PROC hwnd:HWND, uMsg:UINT, wParam:WPARAM, lParam:LPARAM
.if uMsg==WM_CREATE
;blah
.elseif uMsg==WM_SIZE
;blah
.elseif uMsg==WM_DESTROY
;blah
.else
invoke DefWindowProc, hwnd, uMsg, wParam, lParam
ret
.endif
xor eax,eax
ret
WndProc ENDP
Now, look at how each message returns after it has finished processing ("blah"). Except for DefWindowProc, each message will get to "xor eax,eax; ret" once it's done - so there's no need to keep adding that extra code for every message. The only time you need to cause an alternate exit is when the message doesn't want to return zero, this is necessary in some cases, but for most messages there's no need.
Or in fancier words... "single entry single exit" (just easier to maintain) :green :green
Cheers,
Shantanu
Oh you mean if any of the tests catch and process it will not check the others that follow thus fall directly into "xor eax, eax; ret;", you are correct.
I got cought in Charles Petzold coding style(anyway i don't knwo what the switch/case stuff looks like from C to asm);
he uses this structure:
switch(message)
{
case WM_CREATE
code;
return 0;
case WM_SIZE
code;
return 0;
case WM_DESTROY
code;
return 0;
DefWindowProc(hwnd, uMsg, wParam, lParam)
}
P.S. I'll also throw a cute optimisation trick i found recently(perhaps Tedd will have something to say on this :)): i will use local variables when possible from now on because they will exist in the cache since they will be passed on the stack at procedure call thus will speed up fetching(its called fetching right?) EDIT: oh lol hutch said that
P.S. *taking a big breath* DefWindowProc cannot be moved at the top of the control tree i guess even if i test the incoming message against all the messages i process because i would have to single out the particularities of even the messages i process because if i process a message i am also taking responsability of all the possible processing through that message that has nowhere to fall because DefWindowProc would be at top corect?
*taking another big breath*
Alternatively:
switch(uMsg) {
case WM_CREATE:
//code
break;
case WM_SIZE:
//code
break;
case WM_DESTROY:
//code
break;
default:
return DefWindowProc(hwnd, uMsg, wParam, lParam);
}
return 0;
(You missed out default in yours, so the DefWindowProc would never actually be reached.)
Perhaps I will have something to say? -- What exactly are you trying to imply, my friend? :P
Locality is generally a good thing. So yes, using local variables where appropriate is a good idea, not just for keeping things in cache, but also helping with organising your code. (Global variables that are accessed often would equally be in cache anyway, but local variables are created on the stack and you can expect the stack to already be cached.) Some people would go as far to say that global variables are evil and should never EVER be used, but this is a little ridiculous. (Like similar comments about "goto" ::))
I'm not sure I get exactly what you mean by moving DefWindowProc to the top.. It's meant as a catch-all for any messages you don't process. So, moving it to the top would cause it to be given every message, whether you intend to process them or not. Alternatively, you could check if the message is one you want, and if not pass it on to DefWindowProc. But this is what we're doing already. The only difference is that you'd be testing against your list of wanted messages twice. In the current arrangement, DefWindowProc is called only for messages you don't otherwise catch and process.
What Mr. Petzold is saying is, that if you call something from within your WndProc, no other messages can be processed until your call returns. This can cause a serious problem; say when WM_CLOSE is executed your app verifies a 2GB database then closes each entry and writes all buffers to disk. During this process, which could take several minutes, the program will appear completely unresponsive because no other WndProc messages can be handled (such as WM_SIZE or WM_MOVE, etc.)
The solution to this, is to spawn a second thread and execute your lengthy process in there. That way, the WndProc is free to continue processing messages while the second thread executes. Of course if this were in the WM_CLOSE message then you'd only want to close the app after your cleanup thread had completed.
Returning after each message is not necessary, and serves little purpose because like you saw, each message just falls through. The SWITCH/BREAK syntax is a remnant of C language and is provided only for compatibility.
QuoteIf your program needs to perform lengthy jobs while processing particular messages, there are ways to do so politely that I'll describe in Chapter 20.
That chapter talks of multithreading. For now i was talking of returns. In masm the .if .elseif .else conditional structure generates the corect code, no need to return immediately. I do not know what switch/case does but i believe Charles Petzold has a good reason to return after each message processing in a switch/case structure; i was analizing a C compiled switch/case in the c replica of this program and besides lots of spaghetti code, because of jmp wich is an unconditional jump i could not see the code skip the rest of the messages after identifying one and processing.
P.S. The only reason to return even in masm after every message process could be that the jump is very long and it takes more clocks than using the return after each meassages(but i do not know anything about that yet).
QuoteI'm not sure I get exactly what you mean by moving DefWindowProc to the top.. It's meant as a catch-all for any messages you don't process. So, moving it to the top would cause it to be given every message, whether you intend to process them or not. Alternatively, you could check if the message is one you want, and if not pass it on to DefWindowProc. But this is what we're doing already. The only difference is that you'd be testing against your list of wanted messages twice. In the current arrangement, DefWindowProc is called only for messages you don't otherwise catch and process.
With the current knowledge i gained while looking in ollydbg at my code, i can explain:
When winprok gets a message it will go through all the compares and jumps(or .if .elseif part) thus wasting time. Since i believe DefWindowProc will process more messages than i ussually do it would be a good ideea to check the message against the messages i'm processing(for example i'm only processing WM_DESTROY and WM_CREATE) thus i check against those two and if the message i'm testing isn't one of these two i call DefWindowProc, thus instead of jumps for every message test that turns out false, i process it first calling DefWinproc. This would be good in the event that DefWindowProc possibility of getting more messages in an app lifetime than the ones i'm processing. If the messages i'm processing are in greater number probability than the ones that would go to DefWindowProc then DefWindowProc would indeed be better off to be last. In this manner since WM_CREATE occurs when the app is created and only once it would be better to be amongst the last, even if at first the message would be checked against all the other messages at first, afterwards this message would not get in the way, when the test is done; since the other messages will occur more than once it will also result in an improvement in number of clocks used during the app execution.
P.S. i need a spellchecker in this forum :bg
P.S.2 jerking around the size of the window makes aproximatively 80% CPU usage on this code, however on the C replica only 30% CPU usage :dazzled:
Help, my code also produces a memory leak:
Jerk the window size around while having the task manager open and you'll notice the size of the app in memory increase.
http://wolfshade.home.ro/mem_leak.asm
hmm also temp3 doesn't hold 1/7 out of cyClient as i mean to, more like 1/4 but i guess i can debug that, the mem leak i have no ideea.
Yes, I agree with you that if you expect to be passing most of the messages on to DefWindowProc, then it would be nicer to check if you wanted the message first and if not then pass it straight on. But the problem is that you can't do that without testing each one anyway. How would know if it's a message you want without comparing it against each message you want?? But this is exactly what .IFs are doing anyway! Okay, so there is an extra jump or two, but really it's not worth worrying about.
(This is a small lie - there IS a way to do it. You can make an array representing the message values, and then put the address of the handler for each of the messages in its array element - checking for the message is simply an index into the array to see where to jump, or if it's zero then go to DefWindowProc. However, this isn't worth doing unless you have a large number of messages to handle.)
The memory leak is interesting, it seems to be coming in as a result of MoveWindow. Though not the MoveWindow function itself, but the fact that it causes the children to be resized/redrawn. Funny how minimizing the window removes the 'leak.' Which would indicate it's something to do with the repainting. I can't say much more than that without checking further.
As for your 1/7th child-height. Notice that changing the height of the parent does NOT change the children's heights, but changing the width does! Meaning temp3 is 1/7th of the WIDTH, and not the height. This mistake should be obvious if you go through your code and find where you've put cxClient instead of cyClient :bdg
Thanks for the cxClient catch.
What is not funny at all is that through jerking i noticed my win32asm app is using 80-90% CPU unlike its C replica who uses only 20-30% CPU. Can not figure why this huge difference of 3x speed would exist. So far i think i noticed that the C code doesn't use the FPU, alltough i can very well be wrong, the C compiled and disassembled code is very twisted i don't think i can follow it.
C replica for speed comparison purpose.
http://wolfshade.home.ro/project.cpp (ctrl+c, ctrl+n, ctrl+v :) somehow c and cpp code links get stuck in a server error so copy paste.
At least the C code doesn't leek :)
P.S. Hmm i'm having a deja-vous on the event that minimizing a window in relation to some memory thing, doesn't mean anything yet to me.
P.S.2 Found out that windows operating systems reclaim memory when a window is minimized. Nice trick :) personal note: i think this need to be prevented to some extent in a game app or even just a "large" app if speed is in danger :)
Hurray you done it :dance:
QuoteWhich would indicate it's something to do with the repainting.
Pointing me the right way, i went straight to the EndPaint() in wich i had passed the handle to the device context instead the handle to the window(wich caused the double trouble of speed loss and memory loss; i don't want to think about the havoc it created while working w/o any errors :lol, it wasn't releasing what it was supposed to release). It was the only thing i read warnings upon, and used in my code.
Now the speed issue is fixed, doesn't seem to be faster but at least its bug free. My asm programming mood is back on track :dance:
Well C code 23,040 bytes, assembly code 4096 bytes, that certainly stands for something(don't know why tough the C code would be so huge, i wonder if it increases this way the bigger the source is or it just creates some initial size overhead out of something).
Hmm weird last one got double posted as a quote, either because i got careless or i got 2 ips and an electrical storm outside.