CGDipSnapShot - GDI replacement for PixelGetColor

Post a reply


In an effort to prevent automatic submissions, we require that you complete the following challenge.
Smilies
:D :) ;) :( :o :shock: :? 8-) :lol: :x :P :oops: :cry: :evil: :twisted: :roll: :!: :?: :idea: :| :mrgreen: :geek: :ugeek: :arrow: :angel: :clap: :crazy: :eh: :lolno: :problem: :shh: :shifty: :sick: :silent: :think: :thumbup: :thumbdown: :salute: :wave: :wtf: :yawn: :facepalm: :bravo: :dance: :beard: :morebeard: :xmas: :HeHe: :trollface: :cookie: :rainbow: :monkeysee: :monkeysay: :happybday: :headwall: :offtopic: :superhappy: :terms: :beer:
View more smilies

BBCode is ON
[img] is OFF
[flash] is OFF
[url] is ON
Smilies are ON

Topic review
   

Expand view Topic review: CGDipSnapShot - GDI replacement for PixelGetColor

Re: CGDipSnapShot - GDI replacement for PixelGetColor

by +MeleaB+ » 23 Nov 2017, 02:35

I think there are still memory leakage issues caused from Snapshot.

When I tried using code such as that shown below, it appears that Snapshot was causing a conflict as I was having to create a new token each time I wanted to create and save a bitmap. So, I couldn't use the code as shown, and instead had to use pToken := gdip_startup() and Gdip_Shutdown(pToken) EACH TIME I wanted to perform the task. That wasn't a long-term solution of course and so I proceeded to replace all of my routines that used SnapShot with those from the GDI+ library and it now works as required. I'm learning as I go with AHK so there's plenty I still don't fully grasp. If I'm missing something then I'd be grateful if you could explain how else I could have resolved the memory leak.

Code: Select all

pToken := gdip_startup()
.
.
file=%a_scriptdir%\PMData\thumbnail%thumbnail_tableno%.png
thumbnail_id := tableid[thumbnail_tableno]
pBitmap := Gdip_BitmapFromHWND(thumbnail_id)
Gdip_SaveBitmapToFile(pBitmap,file)
Gdip_DisposeImage(pBitmap)
.
.
Gdip_Shutdown(pToken)

Re: CGDipSnapShot - GDI replacement for PixelGetColor

by +MeleaB+ » 18 Nov 2017, 23:24

Hi again, evilC

Is it possible for SnapShot to work with an image file, rather than just from the screen?

For example, if one had previously saved an image with SnapShot and wished to re-examine it, or- in my case- with an image captured from a window that is hidden.

Thanks.

Re: CGDipSnapShot - GDI replacement for PixelGetColor

by +MeleaB+ » 05 Nov 2017, 06:36

Hi.

Just reporting back that since you made the alterations, everything seems to be working perfectly! Thanks very much for quickly fixing it.

Re: CGDipSnapShot - GDI replacement for PixelGetColor

by +MeleaB+ » 01 Nov 2017, 12:25

No, there aren't really any fixed regions, but it's good to know that I don't have to create a new snap if I do happen to have a fixed region that I need to examine at some future point.

Thanks very much for making the change to the code. I'll try it out and report back.

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

I can only be vague with the issue where the snap was returning a zero value each time. It was from over a year ago, and I just recall that the first pixelsnap read returned a zero result each time, and that the next read produced the actually result, so I had to add a "dummy peek" to get around the problem. I don't remember the precise conditions though. I just wondered if it was similar to the problem I mentioned a few posts back: "Also, during the checkseatstatus function, I had to give the two uses of the CGDipSnapShot different names (originally they both used the variable "snap", now they have been changed to "snap1" and "snap2") even though they are used one after the other, because originally the second use was returning a result of zero each time the red colour component was read..." I'll let you know if I experience it again.

Re: CGDipSnapShot - GDI replacement for PixelGetColor

by evilC » 31 Oct 2017, 07:22

Are there a fixed number of different regions that you want to examine using AveCol?

What I meant is, that if you want to examine x5 y5 w100 h100 multiple times, then it is wasteful creating a new instance each time. Every time you want to take a snapshot of the same region, you should use the same instance and re-snapshot.

I had a quick look at the code re: the bloating issue, and all my classes seem to free their memory.
However, I did find some issue where GDI+ did not like me creating a new token each time you newed up a CGDipSnapshot class.

So I altered the code so that all CGDipSnapshot classes share the same GDI+ token.
This should also increase performance.

Regarding the "snap not updating" issue, do you mean if you re-snapshot or something? Could you maybe clarify a little?

Re: CGDipSnapShot - GDI replacement for PixelGetColor

by +MeleaB+ » 30 Oct 2017, 21:03

OK, so I've spent a little time trying to understand everything better but I'm still having a problem: Forgive me if I've missed something, but I don't understand how I can re-use the same snapshot object. Each time the AveCol function is called, different co-ordinates and dimensions of the area to snapshot are used, so wouldn't I need to create a new snapshot each time?

As I was unable to successfully make the change you mentioned, I reverted to the code as it stood, other than now using the correct syntax of snap.TakeSnapShot(), and even added in a further check to verify coordinates are valid, but I'm still seeing a steady increase in memory usage until the program slows to such a pace that it needs to be restarted.

Help! :)

EDIT: I was looking through the other AHK program I wrote last summer, which also included CGdipSnapShot, as I recalled an issue I had back then. I found this code and comment:

Code: Select all

;Take a peek else next snap won't update
DummyPeek := snap.PixelSnap[5, 5].r
IIRC, often the initial attempt to read a colour component resulted in a value of zero (similar to the comment I made in my last post) and so I initiated a "dummy" read to get around this. I don't know if this is in any way related to my current issue, but I thought I'd mention it in case it may be.

Re: CGDipSnapShot - GDI replacement for PixelGetColor

by +MeleaB+ » 30 Oct 2017, 18:29

Thanks very much for the prompt response! I'll make the change you suggested and see how it goes.

(I hadn't noticed the missing "()" as it still worked despite that. However I now just tested using an infinite loop and undefined, and then off-screen, variabless, and whereas it continued to work fine with the correct syntax, with the parenthesis omitted it caused the program to crash.)

Re: CGDipSnapShot - GDI replacement for PixelGetColor

by evilC » 30 Oct 2017, 15:47

Hmm, it is entirely possible that I messed up and when the snap variable goes out of scope, the memory is not freed up. (Probably due to a self-reference).

In order to work around this, you could do try to re-use the same snapshot object, just re-take the snapshot
At the moment, you create a new snapshot each time the function is called.
You could maybe do something like

Code: Select all

snap := new CGdipSnapshot(x,y,w,h)
AveCol(x,y,w,h, snap)
{
	blue := 0
	green := 0
	red := 0
	snap.TakeSnapShot() 
This would also make your code quicker, as you do not need to new up the snapshot each time.

Btw, your sample code uses snap.TakeSnapShot when it should use snap.TakeSnapShot()

I will try and get around to looking into whether snapshot objects get properly garbage collected, thanks for the heads up.

Re: CGDipSnapShot - GDI replacement for PixelGetColor

by +MeleaB+ » 30 Oct 2017, 11:44

It appears as if CGDipSnapShot is gradually increasing the amount of memory it uses, the longer the program is running. The main loop of my program calls the following functions. It's memory usage increases from an initial ~7MB until it eventually lags so badly that it has to be restarted.

The program [initially] runs so much quicker than with the original pixelgetcolor code, but unfortunately I won't be able to use it if I can't get this issue fixed. I would be very grateful if you could offer any advice.

(Also, during the checkseatstatus function, I had to give the two uses of the CGDipSnapShot different names (originally they both used the variable "snap", now they have been changed to "snap1" and "snap2") even though they are used one after the other, because originally the second use was returning a result of zero each time the red colour component was read. NB: Parts of the code have been revised since I made those name changes- in case that may be relevant.)

Thank you!

Code: Select all

;------------------------------------------------------------------------
;------------------------------------------------------------------------
;------------------------CGdipSnapShot Functions-------------------------
;------------------------------------------------------------------------
;------------------------------------------------------------------------

;Average Colour Function
;Checks 100* pixels within the rectangle to find the average colour
;(*as long as dimensions are greater than 20, else it's one check per pixel per dimension <= 20)
AveCol(x,y,w,h)
{
	blue := 0
	green := 0
	red := 0
	snap := new CGdipSnapshot(x,y,w,h)
	snap.TakeSnapShot
	
	if (w > 20) then
	{
		loopw := 10
	} else {
		loopw := w
	}
	
	if (h > 20) then
	{
		looph := 10
	} else {
		looph := h
	}
	
	stepw := floor(w/10)
	steph := floor(h/10)
	
	count1 := 0
	Loop %loopw%
	{
		count2 := 0
		Loop %looph%
		{
			blue := blue + snap.Pixelsnap[count1,count2].b
			green := green + snap.Pixelsnap[count1,count2].g
			red := red + snap.Pixelsnap[count1,count2].r
			count2 := count2 + steph
		}
		count1 := count1 + stepw
	}
	Blue := (blue/(loopw*looph))
	Green := (green/(loopw*looph))
	Red := (red/(loopw*looph))
	;return Blue + Green + Red
	return [Blue,Green,Red]
}




CheckBattery(Tablex,Tabley,Tablewidth,Tableheight)
{
	Global head
	
	x := Tablex + (0.919 * TableWidth)
	y := Tabley + Head + (0.015 * (TableHeight-Head))		
	w := ((0.948 - 0.919) * TableWidth)
	h := ((0.019 - 0.015) * (TableHeight-Head))
	
	x1 := Tablex + (0.467 * TableWidth)
	y1 := Tabley + Head + (0.947 * (TableHeight-Head))
	
	colourcheck := AveCol(x,y,w,h)
	blue := colourcheck[1]
	green := colourcheck[2]
	red := colourcheck[3]
	if ((abs(blue-45)<25) and (abs(green-70)<25) and (abs(red-25)<15)) then
	{
		BatteryShown := 1
		GreenShown := 1
	} else {
		BatteryShown := 0
		GreenShown := 0
	}

	colourcheck := AveCol(x1,y1,1,1)
	blue := colourcheck[1]
	green := colourcheck[2]
	red := colourcheck[3]	
	if ((abs(Blue-158)<20) and (abs(Green-223)<20) and (abs(Red-254)<20)) or ((abs(Blue-52)<10) and (abs(Green-39)<10) and (abs(Red-31)<10)) then
	{
		;Logo (or results page) showing, so Battery not showing
	} else {
		BatteryShown := 1
	}
	
	return [greenshown,batteryshown]
}



FeltColourCheck(x,y,w,h)
{
	colourcheck := avecol(x,y,w,h)
	blue := colourcheck[1]
	green := colourcheck[2]
	red := colourcheck[3]
	tabcol := 0
	if ((abs(blue-80)<40) and (abs(green-45)<40) and (abs(red-0)<30)) then
	{
		TabCol := 1
	}
	if ((abs(blue-40)<20) and (abs(green-40)<20) and (abs(red-40)<20) and (abs(blue-red)<5)) then
	{
		TabCol := 2
	}
	if ((abs(blue-130)<40) and (abs(green-70)<25) and (abs(red-50)<20)) then
	{
		TabCol := 3
	}
	if ((abs(blue-40)<20) and (abs(green-85)<25) and (abs(red-17)<30)) then
	{
		TabCol := 4
	}
	;msgbox %blue%, %green%, %red%
	return tabcol
}



CheckSeatStatus(x,y,w,h,x1,y1,t)
{
	Global head
	fx := x + ((x1 - 0.033) * w)
	fy := y + head + ((y1 - 0.013) * (h - head))
	fw := (0.033 + 0.033) * w
	fh := (0.013 + 0.013) * (h - head)
	
	colourcheck := AveCol(fx,fy,fw,fh)
	blue := colourcheck[1]
	green := colourcheck[2]
	red := colourcheck[3]
	
	filled := 1
	open := 0
	balance := 0
	
	if ((abs(blue-120)<45) and (abs(green-75)<25) and (abs(red-0)<22) and (t = 1)) then
	{
		filled := 0
	}
	if ((abs(blue-35)<20) and (abs(green-35)<20) and (abs(red-35)<20) and (abs(blue-red)<10) and (t = 2)) then
	{
		filled := 0
	}
	if ((abs(blue-115)<25) and (abs(green-70)<25) and (abs(red-50)<20) and (t = 3)) then
	{
		filled := 0
	}
	if ((abs(blue-40)<15) and (abs(green-75)<20) and (abs(red- 20)<10) and (t = 4)) then           
	{
		filled := 0
	}
	
	if (filled = 0) then
	{
		;perfrom one more filled check, on the balance/writing under avatar, in case avatar colour is similar to background
		qx := x + ((x1 - 0.025) * w)
		qy := y + head + ((y1 + 0.055) * (h - head))
		qw := (0.025 + 0.025) * w
		qh := (0.060 - 0.055) * (h - head)

		snap1 := new CGdipSnapshot(qx,qy,qw,qh)
		snap1.TakeSnapShot
		count1 := 0
		Loop %qw%
		{
			count2 := 0
			Loop %qh%
			{
				red := snap1.Pixelsnap[count1,count2].r
				if (red > 100) then 
				balance := balance + 1
				count2 := count2 + 1
			}
			count1 := count1 + 1
		}
		if (balance > 1) then
		{
			filled := 1
		}
	}
	
	if (filled < 2) then
	{
		;check for Open seat
		ox := x + ((x1 - 0.033) * w)
		oy := y + head + ((y1 - 0.003) * (h - head))
		ow := (0.033 + 0.033) * w
		oh := (0.003 + 0.003) * (h - head)

		snap2 := new CGdipSnapshot(ox,oy,ow,oh)
		snap2.TakeSnapShot
		count1 := 0
		Loop %ow%
		{
			count2 := 0
			Loop %oh%
			{
				red := snap2.Pixelsnap[count1,count2].r
				if ((t = 1) and (red > 20)) then 
				{
					open := open + 1
				}
				if ((t = 2) and (red > 48)) then
				{
					open := open + 1
				}
				if ((t = 3) and (red > 65)) then
				{
					open := open + 1
				}
				if ((t = 4) and (red > 37)) then
				{
					open := open + 1
				}
				count2 := count2 + 1
			}
			count1 := count1 + 1
		}
	}
	return [filled,open]
}

Re: CGDipSnapShot - GDI replacement for PixelGetColor

by +MeleaB+ » 26 Oct 2017, 19:00

EDIT: I just switched my default version of AHK from Unicode 64-bit to Unicode 32-bit. It looks like this may have stopped the crashing.

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

Hello evilC

I taught myself AHK a year ago, and used your CGDipSnapShot library to dramatically speed up the program I wrote. However, I ran into a problem which I am again experiencing now, as I attempt to include it in another program I am writing. The issue seems to be that CGDipSnapShot is causing AHK to crash whenever a GUI is closed. If I run, say, your 'example.ahk" then the program works as it should until I attempt to close it. Then I receive the following message:

"AutoHotKey Unicode 64-bit has stopped working. A problem caused the program to stop working correctly. Windows will close the program and notify you if a solution is available."

If I remove the GUI lines from your example.ahk, then the program runs as it should. if I use it within my own program with a GUI, then it produces the error if I try and close the GUI (and also leaves the PC with the mouse "stuttering" and only moving very slowly, as it there's a process stealing all the focus.)

CGDipSnapShot is amazing, but unfortunately I'm prevented from using it with my GUI (which is imperative!) Any help would be greatly appreciated!

Cheers!

Re: CGDipSnapShot - GDI replacement for PixelGetColor

by evilC » 26 Jan 2017, 15:06

No no no, it *IS* possible at the moment.

col2 := new CGDipSnapShot._CColor(0xff0000)

Re: CGDipSnapShot - GDI replacement for PixelGetColor

by Entropy42 » 26 Jan 2017, 14:33

Ah, ok, so it is not possible currently. I will just do a compare manually. Thanks!

Re: CGDipSnapShot - GDI replacement for PixelGetColor

by evilC » 26 Jan 2017, 12:01

Code: [Select all]GeSHi © Codebox Plus

col2 := new CGDipSnapShot._CColor(0xff0000)


Maybe I should expose it as a method, eg CGDipSnapShot.CreateColor(0xff0000)

Re: CGDipSnapShot - GDI replacement for PixelGetColor

by Entropy42 » 26 Jan 2017, 11:22

I was trying to use Compare function of this library, but I'm not sure how to make the color object to compare to.
I have a pixel color that I get from
col1 := snap.PixelSnap[x,y].rgb
From the documentation, it sounds like this is going to be a color object, so to compare it I need to make another Color object, but don't know how. My colors are in RGB format, like [115,24,41], so I tried defining it by:
col2 := { r:115, g:24, b:41 }
And then comparing using:
match := col1.Compare(col2, 10)

But that didn't work, presumably because I made the color object wrong.

Re: CGDipSnapShot - GDI replacement for PixelGetColor

by evilC » 24 Jan 2017, 14:21

Changes pushed to the GitHub repo,

Re: CGDipSnapShot - GDI replacement for PixelGetColor

by Entropy42 » 24 Jan 2017, 12:34

As far as I can tell, this fixed the issue. Thanks!

Re: CGDipSnapShot - GDI replacement for PixelGetColor

by evilC » 24 Jan 2017, 11:32

The only things that seem to use the parent property are:

Code: [Select all]GeSHi © Codebox Plus

		; Returns the Difference between two colors
Diff(c2){
return this._parent.Diff(this, c2)
}

The parent class does not have a Diff() method, so this code seems pointless.

Plus a bunch of checks like this:

Code: [Select all]GeSHi © Codebox Plus

if (!this._parent._SnapshotTaken){
return -1
}

But I do not see the point in this code. If the class exists, it will have been passed an RGB value, so it can return something.

So I removed this offending code, please try out this version of the library and let me know if it fixes your issues.

Code: [Select all] [Expand]GeSHi © Codebox Plus

Re: CGDipSnapShot - GDI replacement for PixelGetColor

by evilC » 24 Jan 2017, 11:23

OK, it seems that the problem is caused by passing a reference to the CGDipSnapShot class into the _CColor class

ie the problem line is:

Code: [Select all]GeSHi © Codebox Plus

	Class _CColor {
__New(RGB, parent){
this._RGB := RGB
this._parent := parent ; <--- This line causes the problem
}


It is entirely possible that the system can be re-engineered to automatically fire the destructor, or maybe I could add a method to delete.

Re: CGDipSnapShot - GDI replacement for PixelGetColor

by just me » 24 Jan 2017, 10:14

This one doesn't call the destructor here:

Code: [Select all] [Expand]GeSHi © Codebox Plus

I added a MsgBox to__Delete().

Re: CGDipSnapShot - GDI replacement for PixelGetColor

by Entropy42 » 24 Jan 2017, 10:12

I didn't mean to say that the script above will just eat up 4 MB per loop iteration. As you can see, in that function, the gemColorsRGB[x, y, 1] := snap.PixelSnap[px,py].r line gets called 64 times, but it only goes up by 4 MB the first time it encounters that line after the snap is taken. The scanGridInnerLoop() function gets called inside another loop, and the memory usage is going up every time around THAT loop. I cut it way down and verified that this version eats up 4 MB every time I press F10. I was just calling this with my SciTE4AHK window in focus to test it.

Code: [Select all] [Expand]GeSHi © Codebox Plus


Top