Click here to Skip to main content
14,668,141 members
Rate this:
Please Sign up or sign in to vote.
So i'm trying to create a digit clock using only WinAPI with C++ (see the screenshot)[^]
Function drawDash() is called 7 times to draw one digit. i use Timer which redraws the digits every 100 milliseconds.
Everything looks fine, but for some reason function CreatePolygonRgn() starts to fail after certain amount of calls. Here is the code where everything happens:

void drawDash(HWND hwnd, const RECT& rect, UINT color) 
						 // this RECT is {left, top, width, height}!!!			
{
	// define some variables...
	HDC hdc = GetDC(hwnd);
	int trLen = min(rect.right, rect.bottom) *0.5;  // ◣ length of this triangle (there will be 4 of them)
	bool bHorizontal = rect.right > rect.bottom; // should we rotate the thing?
	
	// make clipping rgn that will make rgn look like a dash in digit clock
	POINT pointsVert[] = { trLen, 0,   trLen *2, trLen,    trLen *2, rect.bottom -trLen,
			trLen, rect.bottom,    0, rect.bottom -trLen,    0, trLen};
	POINT pointsHorz[] = { trLen, 0,   rect.right -trLen, 0,    rect.right, trLen,
			rect.right -trLen, rect.bottom,    trLen, rect.bottom,    0, trLen };
	
	POINT *ptArray = bHorizontal ? pointsHorz : pointsVert;

	// draw colored rgn!
	HRGN hrgnClip = CreatePolygonRgn(&ptArray[0], sizeof(pointsHorz) / sizeof(pointsHorz[0]), ALTERNATE);
	
	static int timesCalled = 0; // fails exactly at 9989-th time
	timesCalled++;

	OffsetRgn(hrgnClip, rect.left, rect.top);
	FillRgn(hdc, hrgnClip, CreateSolidBrush(color));

	DeleteObject(SelectObject(hdc, GetStockObject(WHITE_BRUSH)));
	ReleaseDC(hwnd, hdc);
	DeleteObject(hrgnClip);
}


What I have tried:

So CreatePolygonRgn() starts to fail exactly at 9989-th call. It returns 0, thus nothing is drawn afterwards. i tried GetLastError(), but it returns 0 (msdn documentation CreatePolygonRgn() doesn't say that i can use GetLastError() to receive the error code).
i had two same points in the arrays - documentation says "Each vertex can be specified only once" so i removed them, but it didn't change anything.
Posted
Updated 19-Sep-20 15:41pm
v4
Comments
Gerry Schmitz 19-Sep-20 12:06pm
   
You point ptArray at different arrays at different times; yet CreatePolygonRgn is hard-coded to one.
Avtem 19-Sep-20 12:09pm
   
But it changes the arrays, you can see that in the screenshot

Rate this:
Please Sign up or sign in to vote.

Solution 1

The only thing I can see that may be an issue is the following:
FillRgn(hdc, hrgnClip, CreateSolidBrush(color));

DeleteObject(SelectObject(hdc, GetStockObject(WHITE_BRUSH)));

Are you certain that DeleteObject is actually deleting the solid brush?
   
Comments
Avtem 19-Sep-20 12:17pm
   
Oh, that's it! i know that i have to release and delete everything with GDI, but i missed up those lines!
So, i fixed it like this:
"FillRgn(hdc, hrgnClip, hGlobalBrush);"
and of course the call "DeleteObject()" is no longer needed, but i do remember to delete "hGlobalBrush" at WM_DESTROY.

Thank you so much!
Richard MacCutchan 19-Sep-20 12:19pm
   
Happy to help. It's a pity that GDI (and GDI+) are so poor at telling you what is wrong when it fails.
Avtem 19-Sep-20 12:32pm
   
And i've just read that amount of GDI objects is limited, so it seems like my program was allowed to create only 9989 brushes =)
By the way, what it was deleting all the time? If it was deleting stock objects, then it should recreate them somehow...
Richard MacCutchan 19-Sep-20 13:21pm
   
You do not actually need to delete StockObjects, but it does no harm if you do.
Rate this:
Please Sign up or sign in to vote.

Solution 4

your code is very simple minded, and it is bad.
Quote:
i use Timer which redraws the digits every 100 milliseconds.

A simple test can divide by 10 the workload of redraw:
if (LastTime != NewTime) {
    redrawClock();
    LastTime = NewTime;
}

Rather than rebuilding the 2 same polygons every times
int trLen = min(rect.right, rect.bottom) *0.5;  // ◣ length of this triangle (there will be 4 of them)
bool bHorizontal = rect.right > rect.bottom; // should we rotate the thing?

// make clipping rgn that will make rgn look like a dash in digit clock
POINT pointsVert[] = { trLen, 0,   trLen *2, trLen,    trLen *2, rect.bottom -trLen,
        trLen, rect.bottom,    0, rect.bottom -trLen,    0, trLen};
POINT pointsHorz[] = { trLen, 0,   rect.right -trLen, 0,    rect.right, trLen,
        rect.right -trLen, rect.bottom,    trLen, rect.bottom,    0, trLen };

POINT *ptArray = bHorizontal ? pointsHorz : pointsVert;

wgeb you know that only 1 will be useful
int trLen = min(rect.right, rect.bottom) *0.5;  // ◣ length of this triangle (there will be 4 of them)
// make clipping rgn that will make rgn look like a dash in digit clock
if (rect.right > rect.bottom) { // should we rotate the thing?
    POINT pointsHorz[] = { trLen, 0,   rect.right -trLen, 0,    rect.right, trLen,  rect.right -trLen, rect.bottom,    trLen, rect.bottom,    0, trLen };
    POINT *ptArray = pointsHorz;
}
else {
    POINT pointsVert[] = { trLen, 0,   trLen *2, trLen,    trLen *2, rect.bottom -trLen, trLen, rect.bottom,    0, rect.bottom -trLen,    0, trLen};
    POINT *ptArray = pointsVert;
}

This rebuild only the useful polygon.
You can also the 2 polygons once for all and keep then in persistent variables between calls.
   
v2
Comments
Avtem 20-Sep-20 0:21am
   
Thank you for your answer! That's a great improvement to the code! i really like it.
Rate this:
Please Sign up or sign in to vote.

Solution 2

Your problem is here:
FillRgn(hdc, hrgnClip, CreateSolidBrush(color));
in creating a brush and NOT deleting it.

Asign it to a var use it and than delete it later, or if the color is constant you need to create it only once.

Your error case is typical for not release GUI objects correctly ;-)
   
Comments
Richard MacCutchan 19-Sep-20 13:21pm
   
Did you see my answer, and OP's comments?
Avtem 19-Sep-20 13:48pm
   
Yes, it's a bit strange to see such answer when it was all understood and fixed.
Rate this:
Please Sign up or sign in to vote.

Solution 3

Since you are using C++, I recommend writing a class to handle this for you. Here's one possibility :
template< typename T >
class handleobj
{
public:
   handleobj( T h )
   {
       m_handle = h;
   }

   virtual ~handleobj()
   {
       if( m_prevObj )  // restore previous object, for fonts and such
       {
          ::SelectObject( m_hDC, m_prevObj );
       }
       if( m_handle )
       {
          ::DeleteObject( m_handle );
          m_handle = nullptr;
       }
   }

   operator T()    { return m_handle; }

public:
    HDC m_hDC     = { nullptr };
    T   m_handle  = { nullptr };
    T   m_prevObj = { nullptr };   // to support selection
};

// usage example :

   // the brush object will delete itself automatically
   handleobj< HBRUSH > solidBrush( CreateSolidBrush( color ) );
   FillRgn(hdc, hrgnClip, solidBrush );
I have not tried this in operation but it will compile. If you want, you can add methods to the class that will select a GDI object and save the DC and previous object. The destructor of the class is ready to deal with that. It's also ready if you never do that. Another option is to make derived classes for things like fonts that create them and select them into a DC. There are a lot of possibilities.

You might think that's a lot of work to destroy a brush handle BUT once you have this and use it you will never have to remember to destroy a brush again because this class will do it for you automatically. It fact, it can be used with any GDI object that uses DeleteObject so deleting GDI objects will be automatic.
   
Comments
Avtem 20-Sep-20 0:13am
   
Thank you for the answer! i will consider this solution for the future.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)




CodeProject, 503-250 Ferrand Drive Toronto Ontario, M3C 3G8 Canada +1 416-849-8900 x 100