Need Help Fixing GDI Leaks

Thread view:

Enable EMail Alerts

Start New Thread

Thread rating:

Phillip N Rounds - 24 Feb 2005 23:11 GMT

I have an application which is heavily graphics intensive, all the graphics
being custom.

Scattered throughout by app, I have

MyView->OnDraw( this->GetDC() );

Apparently, each call to this->GetDC() creates a GDI object and, 16,000 or
so calls to OnDraw() results in the Application hanging because it can no
longer create any new GDI objects ( I know, 16,384 )

I tried
CDC* pTheDC = this->GetDC();
this->OnDraw( pTheDC );
pTheDC->DeleteDC();

This took care of the GDI leak, but causes problems elsewhere, specfically
in the CView::OnPaint() which calls CView::OnDraw( locally created DC )
which destroys the locally created DC somewhere.

I tried to declare a global CDC* pTheDC in the MyView, instantiate it in
OnInitialUpdate(), and where ever I need to call OnDraw(), implement

if ( pTheDC == NULL ) pTheDC = this->GetDC();
this->OnDraw( pTheDC );

And dispose of pTheDC somewhere appropriate. This fails somewhere down the
line. My OnDraw( ) function does some checking, and based on some factors
either does or does not call
ClearDrawingPalette( CDC* pDC);
DrawTitle( CDC* pDC);
DrawLegend( CDC* pDC);
DrawFootNote( CDC* pDC);
then calls
DrawMyStuff( CDC* pDC );
The above call to OnDraw( pTheDC) fails in one of the subsequent graphics
calls, whereas it does not in the original OnDraw( this->GetDC() );

How do I do this correctly?

TIA

Phil

Reply to this Message

AliR - 24 Feb 2005 23:19 GMT

> I have an application which is heavily graphics intensive, all the graphics
> being custom.
[quoted text clipped - 11 lines]
> this->OnDraw( pTheDC );
> pTheDC->DeleteDC();

Ouch! Try ReleaseDC instead of DeleteDC. Read the docs for GetDC a bit
more carefully.

CDC* pTheDC = GetDC();
OnDraw( pTheDC );
ReleaseDC(pTheDC);

May I suggest posting a WM_PAINT message to your view, which in turn calls
the views OnDraw.
you can do it like this:

MyView->Invalidate();
MyView->UpdateWindow();

AliR.

Reply to this Message

Scott McPhillips [MVP] - 25 Feb 2005 00:07 GMT

> I have an application which is heavily graphics intensive, all the graphics
> being custom.
>
> Scattered throughout by app, I have
>
> MyView->OnDraw( this->GetDC() );

You are breaking rule 1 of Windows painting. Don't do that. Do

MyView->Invalidate();
... and optionally, if you really want it painted immediately,
MyView->UpdateWindow();

The OnPaint/OnDraw functions use BeginPaint/EndPaint APIs, which are
only valid when processing a Windows-generated WM_PAINT message.

Signature

Scott McPhillips [VC++ MVP]

Reply to this Message

Phillip N Rounds - 25 Feb 2005 23:11 GMT

The reason OnDraw() gets called throughout this app is that the app
controls an external piece of lab equipment ( actually 3 ), and collects &
presents data from one of the machines. Each of the machines, and of course
the user, can instigate a need to redraw one of the views, with one machine
instigating a need to redraw the view every second. The OnDraw() function
switches on some data in the associated Doc to determine if the entire view
needs to be redrawn, or simply has to add the next line segment(s) in the
graph. What I was trying to do in all of this was to minimize the amount of
work involved in each update to the view.

So, the real reason I am responding except to say thanks is that it is
entirely possible that I have taken the wrong approach to this and I wanted
to understand your response better. I really can't just repaint the entire
for each new data point, that would be too graphics intensive. With this
further input, do you still think I have taken the wrong approach?

Any further input would be appreciated.

Phil

>> I have an application which is heavily graphics intensive, all the
>> graphics being custom.
[quoted text clipped - 11 lines]
> The OnPaint/OnDraw functions use BeginPaint/EndPaint APIs, which are only
> valid when processing a Windows-generated WM_PAINT message.

Reply to this Message

Larry Brasfield - 26 Feb 2005 00:30 GMT

[Top-posting undone for clarity.]

>>> I have an application which is heavily graphics intensive, all the graphics being custom.
>>>
[quoted text clipped - 3 lines]
>>
>> You are breaking rule 1 of Windows painting. Don't do that.

[responding to Scott]
I would not go quite that far. For most applications, that is
a reasonable guideline, but not all and I believe Mr. Rounds
has one that does not quite fit the norm. More below.

>> Do
>> MyView->Invalidate();
[quoted text clipped - 3 lines]
>> The OnPaint/OnDraw functions use BeginPaint/EndPaint APIs, which are only valid when processing a Windows-generated WM_PAINT
>> message.

It would be more correct to say that the caller
of OnDraw() utilizes the {Begin,End}Paint APIs.
I believe the contract for OnDraw() is that it will
draw given a display context, as Mr. Rounds has
supposed.

> The reason OnDraw() gets called throughout this app is that the app controls an external piece of lab equipment ( actually 3 ),
> and collects & presents data from one of the machines. Each of the machines, and of course the user, can instigate a need to
> redraw one of the views, with one machine instigating a need to redraw the view every second. The OnDraw() function switches on
> some data in the associated Doc to determine if the entire view needs to be redrawn, or simply has to add the next line segment(s)
> in the graph. What I was trying to do in all of this was to minimize the amount of work involved in each update to the view.

Despite my comments about the OnDraw() contract,
I believe you would be better off to decompose the
drawing work into at least two parts: (1) the part
needed to refresh the display when windows are
shown or uncovered; and (2) the part that causes
recent incremental update of the displayed data.
Then, OnDraw() can call upon (or be) part 1 and
whatever knows about new updates can call upon
part 2, passing in its special knowledge of what the
minimal update might be. Part 2 may also need to
be abortable or deferable under circumstances where
updates come in too rapidly and your app does not
get the priority needed to keep up.

> So, the real reason I am responding except to say thanks is that it is entirely possible that I have taken the wrong approach to
> this and I wanted to understand your response better. I really can't just repaint the entire for each new data point, that would
> be too graphics intensive. With this further input, do you still think I have taken the wrong approach?

Your presenting problem is readily cured by simply
being more careful with DC acquisition and disposal.

There is no reason that you cannot simply get the DC,
clear some minimal background, draw what is new or
cleared, and relinquish the DC. The GDI is very good
about detecting and discarding draws that will not show
up at all and clipping draws that will partially appear.
Just don't hack up OnDraw() to optimize the distinction
between ordinary refresh and incremental update.

By the way, I have solved this same problem for
a continuously updating sonar display.

> Any further input would be appreciated.

Please refrain from top-posting. There are many
good reasons that people learn to avoid it.

> Phil

Signature

--Larry Brasfield
email: donotspam_larry_brasfield@hotmail.com
Above views may belong only to me.

Reply to this Message

Scott McPhillips [MVP] - 26 Feb 2005 00:43 GMT

> The reason OnDraw() gets called throughout this app is that the app
> controls an external piece of lab equipment ( actually 3 ), and collects &
[quoted text clipped - 11 lines]
> for each new data point, that would be too graphics intensive. With this
> further input, do you still think I have taken the wrong approach?

You are still breaking Rule 1 of Windows painting.

Have you experimented with covering and then uncovering parts of your
view with some other program's window? What happens, for example, if
you bring up the Windows clock and move that window around? According
to your description, your view would not properly repaint when uncovered.

In other words, your doc variables do not know enough to determine how
much of the view needs to be redrawn.

The solution is to call the view Invalidate() to force the whole view to
repaint, or you can call InvalidateRect() if you want to request only a
partial repaint. To reduce or avoid unnecessary repainting the OnDraw
code can call GetClipBox to determine how much of the view to redraw.
GetClipBox gives you the union of the area you invalidated and the area
that Windows may have invalidated.

Signature

Scott McPhillips [VC++ MVP]

Reply to this Message

Joseph M. Newcomer - 25 Feb 2005 01:57 GMT

Absolutely. There is no reason to call GetDC anyway. Instead, do

CClientDC dc(this);
MyView->OnDraw(&dc);

and your problem will go away. In fact, you can be almost certain that using GetDC is a
mistake in an MFC program.

I think the problem is that you are missing the point entirely. Just forget that GetDC
exists.

What is deeply suspicious in all of this is why you need "scattered throughout the app"
any drawing code at all. This is so rarely needed that it is hard to believe it is
necessary, except in very, very restricted contexts such as rubber-banding. In that case,
you would not be calling OnDraw. In fact, the whole notion that you are calling OnDraw is
deeply suspect. I would suggest rewriting the code to eliminate ALL such calls; they are
inappropriate. That's what InvalidateRect() and OnPaint are supposed to deal with.

The whole notion that OnDraw "does some checking and based on some factors, either does or
does not call" seems inappropriate. The ONLY thing that should matter is whether or not
the area being drawn has been invalidated, and in general the cost of drawing an area that
is clipped is lower than the cost of attempting to determine if it should be drawn at all.

I think you have misunderstood most of the basic principles of Windows graphics.

(a) unless rubberbanding, ALL drawing is done in the OnDraw/OnPaint handler
(b) everything is drawn, at all times, since the assumption is that any arbitrary part of
the screen has been erased and needs to be redrawn
    b.1 All conditionals apply uniformly in all contexts, wihtout exception. Things
        are drawn or not drawn based solely upon whether they are
        supposed to be displayed, not because they needed to be updated.
    b.2 Unless there is a known performance issue, there is no reason to
        check to see if something "needs to be" redrawn. If was supposed
        to be drawn, draw it.

Therefore, you should immediately remove all calls on GetDC, and all calls on OnDraw. And
all conditionals that suggest something "should" be updated; either it is present or
absent. If it is present, OnDraw must draw it. If it is absent, then, and only then, does
OnDraw NOT draw it. It draws everything that is SUPPOSED to be displayed, all the time,
without exception. If, and ONLY IF, you have a performance reason, does it make sense to
limit what is drawn. The GDI clipping will generally be much more efficient than any
testing you can do.
                joe

>I have an application which is heavily graphics intensive, all the graphics
>being custom.
[quoted text clipped - 39 lines]
>
>Phil

Joseph M. Newcomer [MVP]
email: newcomer@flounder.com
Web: http://www.flounder.com
MVP Tips: http://www.flounder.com/mvp_tips.htm

Reply to this Message

Rate this thread:

Need Help Fixing GDI Leaks

Free Magazines