68 replies to this topic

[wraithdu]

Nice update!

Edited by wraithdu, 26 June 2009 - 11:37 PM.

[Yashied]

Thanks, wraithdu.

You call _GDIPlus_Startup() and _GDIPlus_Shutdown() in a couple functions. You probably shouldn't do that and leave it up to the user to do it once in the beginning and end of script.

I'm so written it in the description of functions. But if I remove _GDIPlus_Startup() from the code, it will not work. This is not good, in my opinion. How many times do not call _GDIPlus_Startup() function, DLL is loaded only once, the user may call once _GDIPlus_Startup() at the beginning of its code. But if the user wants to display the image only once (eg when creating a GUI), then encumber its challenges _GDIPlus_Startup() and _GDIPlus_Shutdown() does not need it.

You should also return the handles of any resources that you don't dispose of in the functions, so the user can dispose of them if they need to do so (always updating a GUI with different images for example, would lead to a memory leak).

All resources (I hope I have nothing not missed) are disposed. Objects that are in control, are disposed if you delete this control or window. Or I did not understand you?

[wraithdu]

*snip*

Edited by wraithdu, 26 June 2009 - 11:37 PM.

[trancexx]

1) It's not so much a problem that you call _GDIPlus_Startup(), but that you call _GDIPlus_Shutdown(). What if a user is actually using GDI+ in their script, then calls your function, and then GDI+ gets shutdown? I think it would be better to document that GDI+ is used and it's the user's responsibility to initialize / close it.

Another option (although not one I really endorse) is to check the state of the global variable $ghGDIPDll. If it is 0, then GDI+ hasn't been started. If it is >0, then GDI+ is already started. You could use this as a flag of sorts in your functions - if it is not started already then your are free to startup / shutdown GDI+, if it IS already started, then don't startup / shutdown to preserve the user's state.

2) Suppose a user creates a pic control, then uses _SetImage() to set an image to the pic control. Now if he just wants to update the image (not totally deleting the control) then the memory for the first image resource is lost since there is no handle to free the resource. Suppose this pic control is changed many times as part of a looping GUI update...you can see the problem. Besides, I am not positive an image resource like this would be disposed of by deleting the control with GuiCtrlDelete() anyway.

You are right about the first, but not second. Return of STM_SETIMAGE is the old handle that is deleted btw.

edit:
I would go with custom GdiplusStartup/GdiplusShutdown functions.

Edited by trancexx, 26 June 2009 - 09:21 PM.

[Yashied]

@wraithdu, trancexx

Func _GDIPlus_Shutdown()
    If $ghGDIPDll = 0 Then Return SetError(-1, -1, False)

    $giGDIPRef -= 1
    If $giGDIPRef = 0 Then
        DllCall($ghGDIPDll, "none", "GdiplusShutdown", "ptr", $giGDIPToken)
        DllClose($ghGDIPDll)
        $ghGDIPDll = 0
    EndIf
    Return True
EndFunc   ;==>_GDIPlus_Shutdown

Edited by Yashied, 11 July 2009 - 08:36 AM.

[trancexx]

Excellent. It's even dealt with in _GDIPlus_Startup(). $giGDIPRef is always 0 or 1. <- it's not, but that is irrelevant
So, there is no problem there.

I think wraithdu is been smoking pot.

Edited by trancexx, 26 June 2009 - 10:15 PM.

[wraithdu]

Ack, I missed the reference counters in Startup(), Shutdown(). My bad.

And I must be smoking something, cause I didn't put the whole STM_SETIMAGE thing together. Sorry, I suck today

Edited by wraithdu, 26 June 2009 - 11:36 PM.

[wraithdu]

Ok, I'm not totally crazy. There IS a memory leak. Try this example with your posted UDF -

Plain Text        expandcollapse  popup

Func Example5()

    GUICreate('Example5', 253, 244)
    $Pic1 = GUICtrlCreatePic('', 10, 10, 193, 184)
    GUISetState()

    _SetImage($Pic1, $sPng)

    MsgBox(0, "Pause", "Ok, check AutoIt memory usage...")

    For $i = 1 To 100
        _SetImage($Pic1, $sPng)
    Next

    MsgBox(0, "Pause", "Now check it again...")

    Do
    Until GUIGetMsg() = -3

    GUIDelete()

EndFunc;==>Example5

See the leak? Now replace the _SetImage() function with this one and run it again -

AutoIt        expandcollapse  popup

Func _SetImage($hWnd, $sImage)

    $hWnd = _Icons_CheckHandle($hWnd)
    If $hWnd = 0 Then
        Return SetError(1, 0, 0)
    EndIf

    _GDIPlus_Startup()

    Local $hImage = _GDIPlus_BitmapCreateFromFile($sImage)
    Local $hFit = _Icons_FitTo($hWnd, $hImage)
    Local $hBitmap = _GDIPlus_BitmapCreateHBITMAPFromBitmap($hFit)

    _GDIPlus_ImageDispose($hFit)
    _GDIPlus_ImageDispose($hImage)
    _GDIPlus_Shutdown()

    If $hBitmap = 0 Then
        Return SetError(1, 0, 0)
    EndIf
    Local $success = _Icons_SetImage($hWnd, $hBitmap, $IMAGE_BITMAP)
    _WinAPI_DeleteObject($hBitmap)
    If $success Then
        Return 1
    Else
        Return SetError(1, 0, 0)
    EndIf
EndFunc   ;==>_SetImage

The leak is gone. You have to dispose of $hFit returned by the resizing function and the $hBitmap after setting the image to the control. This is probably the case in all your current functions, this is just the one I tested.

Edited by wraithdu, 26 June 2009 - 11:58 PM.

[wraithdu]

*snip*

Edited by wraithdu, 27 June 2009 - 05:16 PM.

[Yashied]

It works on 50%, only for the Bitmap. The Icons completely killed in controls. For the function _Icons_FitTo() memory leaks do not, so it no sense again to destroy the object. Tomorrow investigate.

[Yashied]

You're right.

Important

In version 6 of the Microsoft Win32 controls, a bitmap passed to a static control using the STM_SETIMAGE message was the same bitmap returned by a subsequent STM_SETIMAGE message. The client is responsible to delete any bitmap sent to a static control.

With Microsoft Windows XP, if the bitmap passed in the STM_SETIMAGE message contains pixels with non-zero alpha, the static control takes a copy of the bitmap. This copied bitmap is returned by the next STM_SETIMAGE message. The client code may independently track the bitmaps passed to the static control, but if it does not check and release the bitmaps returned from STM_SETIMAGE messages, the bitmaps are leaked.

[wraithdu]

Hmm, I just noticed I'm having a problem on Win7, that in example 1 if I minimize the GUI then the icons disappear. This happens with both your original code and my modified code. Does this happen on XP? It's not a problem with the images, just the icons.

[Yashied]

Ok, wraithdu.

1. I got rid of memory leaks (just two lines).

2.

Hmm, I just noticed I'm having a problem on Win7, that in example 1 if I minimize the GUI then the icons disappear. This happens with both your original code and my modified code. Does this happen on XP? It's not a problem with the images, just the icons.

It was a "mistake" in the example. However, from the description to GUICtrlCreateIcon() function should be that the default size - 32x32.

GUICtrlCreateIcon('', 0, 30, 38, 32, 32) - I just forced specify the size.

UDF is ready!

Edited by Yashied, 27 June 2009 - 09:08 AM.

[wraithdu]

That is an odd bug in the GUICtrlCreateIcon function. Maybe submit a bug report?

There is one last memory leak though. You can see it in the looping example I posted. In the _SetImage() function, there is an image object returned by _Icons_FitTo() that is not disposed because you don't save the handle, you use it directly in the _GDIPlus_BitmapCreateHBITMAPFromBitmap() function. Maybe this doesn't leak on XP? But it sure does on Win7. Here's the small change to fix it -

Local $hImage = _GDIPlus_BitmapCreateFromFile($sImage)
Local $hFit = _Icons_FitTo($hWnd, $hImage)
Local $hBitmap = _GDIPlus_BitmapCreateHBITMAPFromBitmap($hFit)

_GDIPlus_ImageDispose($hFit)
_GDIPlus_ImageDispose($hImage)
_GDIPlus_Shutdown()

[Yashied]

wraithdu, I made changes in accordance with the requirements of Microsoft. Also fixed a memory leak related to _Icons_FitTo(). I hope now everything is working OK?

Thanks.

[wraithdu]

Nice work. It looks good now. Took me a minute to figure out how you were changing it, but it makes sense. Really good UDF!

[Yashied]

Nice work. It looks good now. Took me a minute to figure out how you were changing it, but it makes sense. Really good UDF!

Thanks.

[Yashied]

Small update. The size of icons may be taken from the controls.

[KaFu]

Yashied, another excellent UDF you wrote here, exactly what I was in need of ...

[CodyBarrett]

this is awesome!!! though is it possible to remove the picture? cause i need this:

i have a Label control...
_setimage () on it works beautifully!!
but then i need the label to be able to have text written on it... Guictrlsetdata(label,'text')
i just used _setimage (label, '') to make it blank... but i dunno how to turn it back into a label control