Preventing memory and resource leaks with GDI Objects ???
Peter Olcott
class and want to add TextOut() capability to this class. To
do this I must convert some of my GDI local function
variables into GDI object member variables. This means that
I must be able to re-use several GDI objects, instead of
constructing them and destroying them after a single use.
What issues are raised with resource and memory leaks by
using CDC and CBitmap objects? How are these issues
mitigated?
Tom Serface
think it matters how or where. I think you get like 5000 GDI resources
before they run out and I think that number is shared among programs. Still
converting them is no worse that destroying and recreating them, but you'll
have to destroy them ultimately at some point, or, if the number is not to
arduous you could let them get recovered when the program exits (but I'm not
a big fan of that behavior).
Tom
"Peter Olcott" <NoS...@OCR4Screen.com> wrote in message
news:5oWdnfO3M6HKQArW...@giganews.com...
Peter Olcott
"Tom Serface" <t...@camaswood.com> wrote in message
news:%23B3xq%23HwK...@TK2MSFTNGP06.phx.gbl...
> You will have to manage the resources somehow and
> someplace, but I don't think it matters how or where. I
> think you get like 5000 GDI resources before they run out
> and I think that number is shared among programs. Still
> converting them is no worse that destroying and recreating
> them, but you'll have to destroy them ultimately at some
> point, or, if the number is not to arduous you could let
> them get recovered when the program exits (but I'm not a
> big fan of that behavior).
>
> Tom
The main thing that I want to do is to manually free any
memory or resources that the CBitmap object is using. I
don't know what resources it is using, and I don't know how
to free them.
Peter Olcott
"Peter Olcott" <NoS...@OCR4Screen.com> wrote in message
>
> "Tom Serface" <t...@camaswood.com> wrote in message
> news:%23B3xq%23HwK...@TK2MSFTNGP06.phx.gbl...
>> You will have to manage the resources somehow and
>> someplace, but I don't think it matters how or where. I
>> think you get like 5000 GDI resources before they run out
>> and I think that number is shared among programs. Still
>> converting them is no worse that destroying and
>> recreating them, but you'll have to destroy them
>> ultimately at some point, or, if the number is not to
>> arduous you could let them get recovered when the program
>> exits (but I'm not a big fan of that behavior).
>>
>> Tom
>
> The main thing that I want to do is to manually free any
> memory or resources that the CBitmap object is using. I
> don't know what resources it is using, and I don't know
> how to free them.
>
I think that I got it: Simply call the inherited member
DeleteObject().
Tom Serface
GDI Objects column in Task Manager so I can watch what my program is doing.
Tom
"Peter Olcott" <NoS...@OCR4Screen.com> wrote in message
news:H8mdnSY2XczicgrW...@giganews.com...
Peter Olcott
> Yeah, that's it... Lots of time people forget to call
> that. I turn on the GDI Objects column in Task Manager so
> I can watch what my program is doing.
Can I execute CBitmap::DeleteObject() while the CBitmap is
currently selected into a DC?
(This is just before I create another CBitmap to be selected
into the same DC).
Joseph M. Newcomer
call DeleteObject explivitly, or you will get an assertion failure.
CFont f;
f.CreateFontIndirect(lf);
...
f.CreateFontIndirect(&lif);
the second one will assert.
If you do
cFont f;
f.CreatFontIdrect(&lf);
f.DeleteOjbect();
f.CreateFontIndirect(&lf);
would work correctly. There's nothing more complex than that required.
Note that if you don't want the objet to persiste, you must either explitily DeleteObject,
or rely on scope descruction to delete the object, which must not be selected into a DC at
that point (DeleteObject will appear to succeed if the object is selected into a DC, but
the object won't be deleted)
joe
Joseph M. Newcomer [MVP]
email: newc...@flounder.com
Web: http://www.flounder.com
MVP Tips: http://www.flounder.com/mvp_tips.htm
Joseph M. Newcomer
selected into a DC. It won't do the deletion. It is you responsibility to make sure the
objec tis not seelected. I suggest SaveDC/RestoreDC to make sure any object selected are
released.
joe
Peter Olcott
CFont* oldFont;
CFont EmptyFont;
oldFont = MemoryDC.SelectObject(&EmptyFont);
oldFont->DeleteObject();
I want to do this before this instruction:
cfont.CreateFontIndirect(&LogFont);
cfont is an object member variable that may be re-used
over-and-over.
"Joseph M. Newcomer" <newc...@flounder.com> wrote in
message news:d9jgp55513t0kb34p...@4ax.com...
Peter Olcott
> DeleteObject on any object which is
> selected into a DC. It won't do the deletion.
Apparently this is no longer true under Windows 7. I spent
several hours re-arranging my code to meet your
specification, and ended up having to change it back. When I
tested it against your prior post:
If you have an object like CFont,
you cannot create more than one
font in it unles you call DeleteObject
explivitly, or you will get an assertion failure.
and got an assertion failure when I removed the
DeleteObject() for a CFont object that was currently
selected into a MemoryDC. This apparently shows that
DeleteObject() must work (at least for CFont) even if the
CFont is currently selected into a MemoryDC.
I had great difficulty trying to unselect the CFont and
CBitmap objects. (see my prior post) The only way that I
knew to unselect a GDI object was to select another GDI
object. Since I had no other object, I tried to select an
empty one. This did not work.
Goran
> I am have built a general purpose bitmap/image handling
> class and want to add TextOut() capability to this class. To
> do this I must convert some of my GDI local function
> variables into GDI object member variables. This means that
> I must be able to re-use several GDI objects, instead of
> constructing them and destroying them after a single use.
Well, certainly nothing stops you from doing that, e.g.
(Warning: works with head-compiler and in head-debugger, YMMV)
typedef shared_ptr<CGdiObject> SPGdiObject;
class COhSoGreat
{
typedef KeyType int;
std::map<KeyType, SPGdiObject> m_gdiObjects;
struct CTempGdiObjectSelection : noncopyable
{
CTempGdiObjectSelection(CDC& dc, const CGdiObject& o) : m_dc(dc),
m_new(o) { m_pOld = dc.SelectObject(&o); }
~CTempGdiObjectSelection() { m_dc.SelectObject(m_pOld); }
CDC& m_dc;
CGdiObject* m_pOld;
CGdiObject& m_new
}
void SoGreatDrawingCode(CDC& dc) const /*an important const*/
{
{ CTempGdiObjectSelection SelectSomething(dc,
*m_gdiObject[someObjectKey]);
// Draw, baby, draw.
{ CTempGdiObjectSelection SelectSomething(dc,
*m_gdiObject[someObjectKey]);
// Draw, baby, draw.
}
}
}
void OtherFunctionsThatMustFillYourGdiObjectContainer()
{ // e.g.
SPGdiObject P(new CFont);
if (!P->CreateXYZ(params))
AfxThrowResourceException();
m_gdiObjects[some_key] = P;
}
};
Use an approach like this and it will be rather hard to leak any
resources.
You also said:
"The main thing that I want to do is to manually free any
memory or resources that the CBitmap object is using. I
don't know what resources it is using, and I don't know how
to free them."
Just let CBitmap C++ object be destroyed. As Joe said, don't try to
get smart with leaving it selected in a DC while destroyed. That's why
I proposed CTempGdiObjectSelection up here. If you can make your gdi
object container constant while drawing (note the "const" qualifier on
"SoGreatDrawingCode"), you're practically locked out of a resource
leak (bar limitless ingenuity in shooting oneself in the foot :-) ).
You say that you tried destroying gdi object while selected and that
this works, but did you verify this with some GDI handle monitor
program? There are some on the internet. Try this first.
> What issues are raised with resource and memory leaks by
> using CDC and CBitmap objects? How are these issues
> mitigated?
Listening to Joe gets you far :-).
Goran.
Peter Olcott
it. My great difficulty is with the tedious little
syntactical details, thus Joe's abstract examples don't work
for me. I can't simply let my GDI object get destroyed
because the objects must retain their state in case
subsequent operations must be performed on them.
I open a bitmap and it may be written to, or saved or
another bitmap may be opened. If another bitmap is opened, I
must create another CBitmap object of this new size.
Currently I only have the single CBitmap object as an object
member variable.
I am guessing that I could implement your design by making
pairs of GDI object member and toggling between them upon
every re-use. For example CBitmap cbitmap[2]; and then
toggle its subscript between one and zero.
I looked at your design again and there were too many things
that I did not understand.
Here is my currently working code. It does not work if I
remove the:
this->cfont.DeleteObject();
It does work with repeated invocations, thus must be
deleting the selected CFont object.
// Displays a Font Selection DialogBox with
// default "Times New Roman", Bold, Italic, 8 Point
//
inline void ScreenCaptureType::SelectFont() {
this->cfont.DeleteObject();
int PointSize = 8;
int FontHeight = -MulDiv(PointSize,
GetDeviceCaps(MemoryDC.m_hDC, LOGPIXELSY), 72);
LOGFONT LogFont = {FontHeight, 0, 0, 0, FW_BOLD, 1, 0, 0,
ANSI_CHARSET, OUT_DEFAULT_PRECIS,
CLIP_DEFAULT_PRECIS, DEFAULT_QUALITY, DEFAULT_PITCH |
FF_DONTCARE, L"Times New Roman"};
CFontDialog dlg(&LogFont);
if (dlg.DoModal() == IDOK)
cfont.CreateFontIndirect(&LogFont);
this->MemoryDC.SelectObject(&cfont);
}
"Goran" <goran...@gmail.com> wrote in message
news:0a7d9ef0-f706-4832...@q16g2000yqq.googlegroups.com...
Joseph M. Newcomer
believe it is undefined. So it may be that the old font was not deselected.
joe
Joseph M. Newcomer
On Thu, 11 Mar 2010 10:14:12 -0600, "Peter Olcott" <NoS...@OCR4Screen.com> wrote:
>
>"Joseph M. Newcomer" <newc...@flounder.com> wrote in
>message news:d9jgp55513t0kb34p...@4ax.com...
>> As I indicated in an earlier reply, you cannot call
>> DeleteObject on any object which is
>> selected into a DC. It won't do the deletion.
>
>Apparently this is no longer true under Windows 7. I spent
>several hours re-arranging my code to meet your
>specification, and ended up having to change it back. When I
>tested it against your prior post:
>
> If you have an object like CFont,
> you cannot create more than one
> font in it unles you call DeleteObject
> explivitly, or you will get an assertion failure.
****
This is not a Windows feature, but an MFC feature, and is usually caught by an assertion
in the ocde that creates a font, code of the form
ASSERT(something == NULL)
and if there is a handle associated with the font, the 'something' (I forget the variable
name) will com up non-NULL. This has alsays been a feature of MFC and it is there to
prevent errors cause by trying to create a font in a variable that already has a font. I'm
surprised that you did not encounter problems with it if you code assumed this was
possible. This is true for all the GDI objects (fonts, palettes, bitmaps, etc.)
You did not say which version of VS you are using, but it was true though VS 2008.
PTPH, the failure ot DeleteObject to delete an object selected into a DC has always been
trhough of as a bug, and it wouldn't surprise me if they fixed it but failed to tell us.
We are never told of bug ffixes.
*****
>
>and got an assertion failure when I removed the
>DeleteObject() for a CFont object that was currently
>selected into a MemoryDC. This apparently shows that
>DeleteObject() must work (at least for CFont) even if the
>CFont is currently selected into a MemoryDC.
******
CGDIObject::DeleteObject calls the kernel ::DeleteObject, fails to test the result, and
simply sends the m_hGdiObject field to NULL. This means that CGDIObject::DeleteObject
will meet the MFC requirements but does not guarantee that the objec is *actually*
deleteed. Under versions < Win7, and possibly in Win7 (since you have no evidence that
they fixed this "bug") the object remains in the kernel, consuming GDI resources, but MFC
works under the delusional system that the object has been deleted.
*******
>
>I had great difficulty trying to unselect the CFont and
>CBitmap objects. (see my prior post) The only way that I
>knew to unselect a GDI object was to select another GDI
>object. Since I had no other object, I tried to select an
>empty one. This did not work.
****
There was a reason I mentioned CDC::RestoreDC(). this will deselect all elements back to
the last SaveDC() and you don't need to have an "empty" object to select.
****
Tom Serface
You can have different font objects if you want to have multiple fonts set
up. You are right about not deleting it when it is selected, but I don't
think you'd want that to happen. It typically make my font objects members
of the class so they are always persistent while the class object is...
Also, the assertion messages won't display for release builds which is
convenient, but if you get them in debug you would definitely want to fix
the problem.
Tom
"Peter Olcott" <NoS...@OCR4Screen.com> wrote in message
news:hI6dnaNzGM1LiwTW...@giganews.com...
Joseph M. Newcomer
Vista) a font which is selected into a DC will not be deleted if ::DeleteObject is called,
whether explicitly via CGdiObject::DeleteObject or via the implicit destructor, so if the
shared pointer reference count goe to 0, the delete method will end up calling
::DeleteObject, and if the font is selected into a DC it will not actually delete it and
there will be a genuine GDI resource leak.
I tried to explain this several times, but he keeps confusing MFC behavior with actulal
system behavior, and they are not the same!
joe
Joseph M. Newcomer
you would think that in any sane system, such as one that uses kernel object reference
counts [like Windows], that "deleting" a font that was actively selected into a DC would
reduce the reference count to 1, so that when the font was deselected, the reference count
would go to 0 and at that point the now-unused font would be freed and the SelectObject
would rreturn a NULL handle. But that would probably be sensible, so instead, the font is
left live and you continue execution with the impression that the HFONT is now referring
to a nonexistent font, so you just ovewrite the handle and thus lose any access to the
font, which continues to exist. (for HFONT, you can substitute HPEN, HBRUSH, HPALETTE,
HBITMAP, etc.).
Apparently, because I forgot to put the :: in front of ::DeleteObject, I left it ambiguous
as to what I was talking about. CGdiObject;:DeleteObject produces the illusion that it
"worked", because it allows the CFont to be reused, even though the actual font object in
the kernel has not been deleted and is still occupying GDI kernel space. My bad. The
truth is that ::DeleteObject will not delete (a) any GDI object selected into a DC (b) any
stock object. While we don't expect stock objects to be deleted, the fact that objects
can be left "dangling" always comes as a surprise, and it is not helped by the fact that
CGdiObject::DeleteObject preserves the correct MFC state so that it *appears* that it
worked succesfully (even though it didn't actually delete the object).
joe
Peter Olcott
> Note, however, that in all versions of Windows I'm
> familiar with (which is up through
> Vista) a font which is selected into a DC will not be
> deleted if ::DeleteObject is called,
> whether explicitly via CGdiObject::DeleteObject or via the
> implicit destructor, so if the
> shared pointer reference count goe to 0, the delete method
> will end up calling
> ::DeleteObject, and if the font is selected into a DC it
> will not actually delete it and
> there will be a genuine GDI resource leak.
>
> I tried to explain this several times, but he keeps
> confusing MFC behavior with actulal
> system behavior, and they are not the same!
> joe
>
According to my understanding about what you said about this
the below code could not work, and yet it does work. The
CFont object is deleted repeatedly even when selected into
the MemoryDC. It must be getting deleted because when I
comment out the DeleteObject() statement it fails with an
assertion. If I leave it in, then it does not fail with the
assertion. I don't see how this could possibly mean that the
CFont object is not getting deleted while it is selected.
It could very well be (and probably is) that my
understanding is less than complete. What am I missing here?
//
// Displays a Font Selection DialogBox with
// default "Times New Roman", Bold, Italic, 8 Point
//
inline void ScreenCaptureType::SelectFont() {
this->cfont.DeleteObject();
int PointSize = 8;
int FontHeight = -MulDiv(PointSize,
GetDeviceCaps(MemoryDC.m_hDC, LOGPIXELSY), 72);
LOGFONT LogFont = {FontHeight, 0, 0, 0, FW_BOLD, 1, 0, 0,
ANSI_CHARSET, OUT_DEFAULT_PRECIS,
CLIP_DEFAULT_PRECIS, DEFAULT_QUALITY, DEFAULT_PITCH |
FF_DONTCARE, L"Times New Roman"};
CFontDialog dlg(&LogFont);
if (dlg.DoModal() == IDOK)
this->cfont.CreateFontIndirect(&LogFont);
this->MemoryDC.SelectObject(&cfont);
Peter Olcott
So what is an example of simple clean minimal syntax for
making sure that a single set of GDI object member variables
always does get properly deleted? The best that I could
come up with is to duplicate everything such as CFont
cfont[2]; and toggle the subscript.
This seems like far too much of a kludge.
Goran
> I will study your concrete example an attempt to implement
> it. My great difficulty is with the tedious little
> syntactical details, thus Joe's abstract examples don't work
> for me. I can't simply let my GDI object get destroyed
> because the objects must retain their state in case
> subsequent operations must be performed on them.
I see in your code below that you use a memory DC. Without having any
further info, only thing I have to say is that normally a memory DC is
used to draw something, then e.g. stored in a bitmap or BitBlt-ed on
screen. From that standpoint, normal drawing we all know is to select
your GDI objects into the DC, call drawing code, then un-select them.
If that is what you are doing, then you have no correlation between
your GDI objects (e.g. bitmaps, pens, fonts, brushes) except that they
have to outlive time when they are selected into the DC for drawing.
So I honestly do not see your difficulty. Just make sure that, during
the drawing code, you do stuff by the book (hence e.g. my
CTempGdiObjectSelection), and as for lifetime of your GDI object,
well, just handle their lifetime (hence my shared_ptr-based map
there).
> I open a bitmap and it may be written to, or saved or
> another bitmap may be opened. If another bitmap is opened, I
> must create another CBitmap object of this new size.
> Currently I only have the single CBitmap object as an object
> member variable.
So make that two of them. Note also that there's nothing wrong in
creating whatever GDI object you might need on the heap.
> I am guessing that I could implement your design by making
> pairs of GDI object member and toggling between them upon
> every re-use. For example CBitmap cbitmap[2]; and then
> toggle its subscript between one and zero.
Here, either I am not very smart, either there's a lot of things you
have in your head that weren't put into writing here, because I don't
understand what you are talking about :-).
> I looked at your design again and there were too many things
> that I did not understand.
I'd say, points to take from it:
1. use something like CTempGdiObjectSelection to ensure correct GDI
object selection idiom, which is "select it/draw/un-select it".
2. use some C++ object lifetime handling technique to handle your GDI
object (font, bitmap, pen...) lifetime. Hence I proposed shared_ptr -
it gets you far. If you indeed use a std::map, attention to
map::operator[], that might not do what you think (hence my insistence
on that "const").
3. divorce lifetime of a DC and GDI objects used to draw on it (you
seem to be having trouble with this). In other words, when you select
an object into a DC, do not try to destroy it. But I don't understand
why you even started down that route.
> Here is my currently working code. It does not work if I
> remove the:
> this->cfont.DeleteObject();
> It does work with repeated invocations, thus must be
> deleting the selected CFont object.
>
> // Displays a Font Selection DialogBox with
> // default "Times New Roman", Bold, Italic, 8 Point
> //
> inline void ScreenCaptureType::SelectFont() {
> this->cfont.DeleteObject();
>
> int PointSize = 8;
> int FontHeight = -MulDiv(PointSize,
> GetDeviceCaps(MemoryDC.m_hDC, LOGPIXELSY), 72);
> LOGFONT LogFont = {FontHeight, 0, 0, 0, FW_BOLD, 1, 0, 0,
> ANSI_CHARSET, OUT_DEFAULT_PRECIS,
> CLIP_DEFAULT_PRECIS, DEFAULT_QUALITY, DEFAULT_PITCH |
> FF_DONTCARE, L"Times New Roman"};
> CFontDialog dlg(&LogFont);
> if (dlg.DoModal() == IDOK)
> cfont.CreateFontIndirect(&LogFont);
> this->MemoryDC.SelectObject(&cfont);
>
> }
Without knowing anything about the rest of your code, the purpose of
this function is to select some font into a memory DC (providing that
MemoryDC indeed is a CMemoryDC). Normally, one selects a font into a
DC to do some drawing, so I personally find it hugely surprising that
a modal dialog is displayed just before selecting it into a DC. I see
that working well in some bigger design only if said font is the only
font that will be used while drawing. But, as I said before, I don't
actually understand your difficulty at all, so I might be completely
off the mark.
By the way, please, PLEASE do something about that CreateFontIndirect
call. It stands out like a sore thumb. It's a friggin resource
allocation and that can fail (if so, it returns FALSE). Imagine that
this indeed happens. Would you want to do
MemoryDC.SelectObject(&cfont) then?
Goran.
Goran
> So what is an example of simple clean minimal syntax for
> making sure that a single set of GDI object member variables
> always does get properly deleted? The best that I could
> come up with is to duplicate everything such as CFont
> cfont[2]; and toggle the subscript.
> This seems like far too much of a kludge.
(I'll presume that you want to use multiple fonts to draw on some DC).
If so, this works:
0. create any GDI objects (e.g. fonts)
1. (optional) create your DC ( or receive it in OnPaint :-) )
2. select any font into DC, draw, un-select it
Rinse, repeat 2 (you can also "stack" what you do in 2 if you wish so)
3. (optional) destroy DC (don't if it's not yours)
4. let all go out of scope (IOW, let your enclosing class be
destroyed).
Goran.
Peter Olcott
repeatedly, and it does not work if the
this->font.DeleteObject();
is removed which as far as I can tell must mean that a GDI
object is definitely being deleted while selected in a
device context. Proof that a GDI object is being deleted
while selected into a device context contradicts Joe's
statement that it can't be done.
What I suspect is there is something that I am missing here,
and Joe is not wrong, yet the explicit contradiction still
remains unresolved.
"Goran" <goran...@gmail.com> wrote in message
news:bc8e5ce9-773d-42df...@33g2000yqj.googlegroups.com...
Peter Olcott
a GDI object is to select another GDI object. I can't select
another GDI object because I have to get rid of the first
one to make room to create the second one. If there is
another way to unselect a GDI object besides selecting
another GDI object, then this would be easy.
Ideally I only want to have a single CFont and a single
CBitmap and a single MemoryDC that I use over and over.
These must all be stored as object members, and will live as
long as the app lives.
"Goran" <goran...@gmail.com> wrote in message
news:06b6b099-4b40-4809...@t23g2000yqt.googlegroups.com...
Goran
> The point is that the function listed below can be called
> repeatedly, and it does not work if the
> this->font.DeleteObject();
> is removed
Well, that part is clear. If you call CreateXYZ on your font, it is an
error to call CreateXYZ again if you did not call DeleteObject. Look
at CreateXYZ code to see the assert, and you will find that it just
asserts on a non-null GDI handle.
> which as far as I can tell must mean that a GDI
> object is definitely being deleted while selected in a
> device context. Proof that a GDI object is being deleted
> while selected into a device context contradicts Joe's
> statement that it can't be done.
Did you try some GDI resource spy program? Try it and check that your
GDI handle count indeed stays stable if you do what you are doing.
But, that still does not mean it is an allowed thing to do. And
indeed, doc for DeleteObject function, that can be found at
http://msdn.microsoft.com/en-us/library/dd183539%28VS.85%29.aspx
clearly states this:
"Do not delete a drawing object (pen or brush) while it is still
selected into a DC." And there's no reason to do it, either, and I
can't even get why you think you want that.
Goran.
Goran
> The problem is that the only way that I know how to unselect
> a GDI object is to select another GDI object. I can't select
> another GDI object because I have to get rid of the first
> one to make room to create the second one. If there is
> another way to unselect a GDI object besides selecting
> another GDI object, then this would be easy.
OK, then you seem to have a major case of bad understanding of the
API. SelectObject returns previously selected object (of the given
type).
So you do:
CGdiObject* pOld = pDC->SeletObject(&mySuperFont);
// Draw, baby, draw...
pDC->SeletObject(pOld);
That's it!
You don't ever, ever care about that first pOld - this baby "belongs"
to a DC you are using, and is most likely one of "stock" gdi objects
that are not yours to do anything with.
> Ideally I only want to have a single CFont and a single
> CBitmap and a single MemoryDC that I use over and over.
> These must all be stored as object members, and will live as
> long as the app lives.
Well, I have to ask, why all that? It doesn't seem normal. For
example, if your purpose is to "draw" a bitmap for later use, then, if
my memory serves me correctly, you
* create a memory DC,
* call CreateCompatibleBitmap,
* call SelectObject for your font
* draw stuff on DC
* unselect the font from DC, destroy font and DC.
There's your bitmap, you can keep it.
If not, tell us what do you do with the three?
You also seem to be misguided in wanting to reuse e.g. the font. I
don't think you want it around while it's not used. If nothing else,
it's a resource strain on your system. Just create it when you're
drawing - what's wrong with that?
Goran.
Peter Olcott
off. What is it that turns quoting off?
(1) I am not deleting either a Pen or a Brush
(2) I am deleting a CFont and a CBitmap
(3) Both of these consistently return 1 indicating
successful deletion, except the first time that delete is
called, which is also before there is anything to delete.
I won't bother to explain the reasoning for my design
because that would take more time to make clear than I want
to spend right now. It boils down to the goal of minimizing
user operations, while minimizing program complexity and
maximizing performance.
"Goran" <goran...@gmail.com> wrote in message
news:7703e033-65a5-41fc...@g28g2000yqh.googlegroups.com...
Peter Olcott
(DeleteObject() consistently returns a 1 indicating
successful deletion) I won't bother to explain the reasoning
for my current design right now. There are a very good set
of reasons that seem to not be able to be otherwise
achieved.
"Goran" <goran...@gmail.com> wrote in message
news:9b1a59c6-b009-47a4...@g10g2000yqh.googlegroups.com...
Joseph M. Newcomer
****
I don't see what that can do to help. You either know what is selected, or you don't. It
isn't hard to track what is selected, and you can tell immediately if you have selected
it.
joe
****
Joseph M. Newcomer
with the font, you will revert to whatever the default font was. Not a big deal. The
other common practice is to note that if you do SelectObject() of a font, the result is a
CFont * that had been selected and you simply re-selecte this. This technique generalizes
to all 30 or so parameters of an HDC (changing a parameter returns you the prvious
setting) but this means you have to keep variables around to know what value to
re-selected, and I find RestoreDC() to be a simpler apprroach.
joe
Joseph M. Newcomer
selected into a DC. If an object is selected into a DC, you must deselect it. You should
not use the DC as a "repository" of any object; the simplest approach is to start with a
DC which has nothing active, select into it what you need to do drawing, do the drawing,
deselect all the objects (RestoreDC()), and repeat as required. It is considered poor
practicce to leave an object selected into a DC across, say, an message pump invovation,
because temporary objects exhibit odd behavior under such conditions. This is just good
programming practice.
joe
Peter Olcott
> What part of "use RestoreDC()" did you miss? If you
> simply RestoreDC() when you are done
> with the font, you will revert to whatever the default
> font was. Not a big deal. The
> other common practice is to note that if you do
> SelectObject() of a font, the result is a
> CFont * that had been selected and you simply re-selecte
> this. This technique generalizes
> to all 30 or so parameters of an HDC (changing a parameter
> returns you the prvious
> setting) but this means you have to keep variables around
> to know what value to
> re-selected, and I find RestoreDC() to be a simpler
> apprroach.
> joe
Its not just Font its also CBitmap, and there is no default
bitmap to restore. It looks like all of this may be moot
because DeleteObject() is confirming that the object is
being deleted while it is selected by its return value of 1,
so my code is already good the way that it is. I only tested
this with VS 2008, but, it works on Windows 7 and XP.
Peter Olcott
> It is very, very simple: you, and you alone, are
> responsible for knowing what objects are
> selected into a DC. If an object is selected into a DC,
> you must deselect it. You should
> not use the DC as a "repository" of any object; the
> simplest approach is to start with a
> DC which has nothing active, select into it what you need
> to do drawing, do the drawing,
> deselect all the objects (RestoreDC()), and repeat as
> required. It is considered poor
> practicce to leave an object selected into a DC across,
> say, an message pump invovation,
> because temporary objects exhibit odd behavior under such
> conditions. This is just good
> programming practice.
> joe
>
I want to minimize the complexity of the code, I want to
minimize human user actions. To minimize the complexity of
the code I have a minimum number of objects and I perform
the minimum number of operations upon these objects to
achieve the required functional result.
I have exactly one CBitmap, CFont, and MemoryDC. Once a user
has performed an operation upon these objects I want this
operation to stick until the user performs another operation
upon them. (These operations can occur in any order or
sequence) I implemented this requirement by make these GDI
objects the members of the CDialog instance.
The user may load a graphics file into the MemoryDC's
CBitmap, capture the screen to the bitmap, save whatever is
in the bitmap to a graphics file, or write to whatever is in
the bitmap using TextOut(). The user may do these operations
in any order or sequence. A default 100 * 100 bitmap is
created when ScreenCapture object that does all of these
operations is created.
The trick of always deselecting the currently selected GDI
object is too difficult to implement and not required for
the following reasons:
(1) The only way to unselect a CBitmap is to select another
CBitmap.
(2) The other CBitmap does not exist until I create it, and
to minimize program complexity there is only one of these.
(3) So (according to your requirements) I can't create a
CBitmap without deleting the original CBitmap, and I can't
delete the original CBitmap without creating a new CBitmap.
(4) Apparently I can delete the original CBitmap while it is
selected into a MemoryDC, because DeleteObject() returns a 1
indicating successful deletion.
Joseph M. Newcomer
On Tue, 16 Mar 2010 08:13:07 -0500, "Peter Olcott" <NoS...@OCR4Screen.com> wrote:
>
>"Joseph M. Newcomer" <newc...@flounder.com> wrote in
>message news:vuetp51t69g884pi2...@4ax.com...
>> It is very, very simple: you, and you alone, are
>> responsible for knowing what objects are
>> selected into a DC. If an object is selected into a DC,
>> you must deselect it. You should
>> not use the DC as a "repository" of any object; the
>> simplest approach is to start with a
>> DC which has nothing active, select into it what you need
>> to do drawing, do the drawing,
>> deselect all the objects (RestoreDC()), and repeat as
>> required. It is considered poor
>> practicce to leave an object selected into a DC across,
>> say, an message pump invovation,
>> because temporary objects exhibit odd behavior under such
>> conditions. This is just good
>> programming practice.
>> joe
>>
>
>I want to minimize the complexity of the code, I want to
>minimize human user actions. To minimize the complexity of
>the code I have a minimum number of objects and I perform
>the minimum number of operations upon these objects to
>achieve the required functional result.
***
If you leave a bitmap selected into a DC (such as a memory DC) you are implicitly
increasing the complexity of the code, because the code requires that this fact be
"remembered". Simple code would not leave a bitmap selected into a DC, but select it in
each time and make sure the DC was resstored to its pristine state upon completion, Then
there is no state to "remember". Until you know the cost of an operation, adding extra
complexity to eliminate the cost is a meaningless activity. We are erroneously taught
that the total number of operations is the peformance metric that matters, when years of
experience ultimately tell you that code simplicity and robustnesss are far more
important. If you think the operational cost of something matters, and you haven't
measured it, you have made a fundamental error in logic. Because until you measure it,
you don't know. I spent 15 years optimizing code on slow machines, and the one thing I
learned was that optimizing lines of code can buy, at best, single-digit-percentage
performance improvement, while optimizing architecture can buy orders of magnitude
performance improvement. And sometimes the lines-of-code optimization (LOCO) really is
loco, and will produce negative performance improvement, and in other cases, a fraction of
a percent in end-to-end thoughput (e..g, "I made this function twice as fast after working
on it for a week" and my measurements said "Yes, you probably did. It now takes 0.025% of
your total end-to-end time, and it probably took 0.05% before you wasted a week of your
time rewriting it" and then point out where 30% of the time is going, and a one-line
change in the architecture would eliminate that 30%. You cannot optimize without measured
performance data.
*****
>
>I have exactly one CBitmap, CFont, and MemoryDC. Once a user
>has performed an operation upon these objects I want this
>operation to stick until the user performs another operation
>upon them. (These operations can occur in any order or
>sequence) I implemented this requirement by make these GDI
>objects the members of the CDialog instance.
****
Then you have the responsibility of tracking this state, adding to the complexity of your
code.
****
>
>The user may load a graphics file into the MemoryDC's
>CBitmap, capture the screen to the bitmap, save whatever is
>in the bitmap to a graphics file, or write to whatever is in
>the bitmap using TextOut(). The user may do these operations
>in any order or sequence. A default 100 * 100 bitmap is
>created when ScreenCapture object that does all of these
>operations is created.
>
>The trick of always deselecting the currently selected GDI
>object is too difficult to implement and not required for
>the following reasons:
>(1) The only way to unselect a CBitmap is to select another
>CBitmap.
>(2) The other CBitmap does not exist until I create it, and
>to minimize program complexity there is only one of these.
****
Not true; when you selected the first CBitmap in, you were given a CBitmap* which was the
previous (1-pixel monochrome) bitmap! Read about SelectObject!
****
>(3) So (according to your requirements) I can't create a
>CBitmap without deleting the original CBitmap, and I can't
>delete the original CBitmap without creating a new CBitmap.
****
You can't delete an existing CBitmap if it is selected into a DC. So the solution would
be a class which held a CBitmap and a CDC, and kept the previous CBitmap* around as well,
and work in terms of this class to manage your birtmaps, instead of working with raw
CBimapand a CDC objects! Think C++!
*****
>(4) Apparently I can delete the original CBitmap while it is
>selected into a MemoryDC, because DeleteObject() returns a 1
>indicating successful deletion.
****
And how often do I have to say "It lies, it doesn't actually delete the bitmap" before you
believe me? This was a known bug in Win16 and it has not been changed in Win32, in order
ot maintain bug-for-bug compatibility.
joe
*****
Joseph M. Newcomer
***
You k,eeep missing the point here. It *doesn't* actually delete the object if it is
selected, and you are confusing the behavior of MFC with the behavior of the kernel, and I
keep repeeating that this doesn't work, and you keep inisisting that based on something
you once saw in an unrelated piece of code (MFC is not the Kernel), that it works. YOu
are seeing the consequence of MFC believing that the object is deleted, and the ASSERT
comes from violating MFC's image of what is going on; but MFC will maintain what it
*thinks* is a consistent image of reality, even though I keep telling you that this is
illusory.
And here I can truly say RTFM: CBDiObject::DeleteObject says, explicitly, "An application
should not call DeleteObject on a CGdiObject that is currently selected into a device
context".
There's a reason that sentence exist!
I can also say "Trust the source"; if you look at the implementation of
CGdiObject::~CGdiObject, (see afxwin1.inl) you will see that it does NOT test the return
value from the DeleteObject method. However, the API does say that it will return 0 if
the object is currently selected into a DC.
You are confusng MFC behavior with kenel behavior, and you even explicitrly say you are
relying on MFC's behavior and you "cannot see how this could possibly mean that the CFont
objec tis not getting deleted". This means you have not read the code, in which MFC
explicitly IGNORES the return type from ::DeleteObject (which is what is returned from
CGdiObject::DeleteObject, (see wingdi.cpp in the MFC sources). So it is pretty obvious
why MFC runs under a delusional system, and in spite of the fact I have tried to say this
several times, you have ignored what I have been trying to tell you.
****
>
>It could very well be (and probably is) that my
>understanding is less than complete. What am I missing here?
****
Litenening to people who are telling you that it doens't work like you want to believe.
*****
>
>//
>// Displays a Font Selection DialogBox with
>// default "Times New Roman", Bold, Italic, 8 Point
>//
>inline void ScreenCaptureType::SelectFont() {
> this->cfont.DeleteObject();
>
> int PointSize = 8;
> int FontHeight = -MulDiv(PointSize,
>GetDeviceCaps(MemoryDC.m_hDC, LOGPIXELSY), 72);
> LOGFONT LogFont = {FontHeight, 0, 0, 0, FW_BOLD, 1, 0, 0,
>ANSI_CHARSET, OUT_DEFAULT_PRECIS,
> CLIP_DEFAULT_PRECIS, DEFAULT_QUALITY, DEFAULT_PITCH |
>FF_DONTCARE, L"Times New Roman"};
> CFontDialog dlg(&LogFont);
> if (dlg.DoModal() == IDOK)
> this->cfont.CreateFontIndirect(&LogFont);
> this->MemoryDC.SelectObject(&cfont);
***
Use of 'this->" is silly. Since MemoryDC is already a class member, standard scope rules
would apply. And note that the return value is a CFont*, which is the old font, which
means you don't have to create a special font to "deselect" the font. note also that you
can STILL use SaveDC/RestoreDC, you shoul wrap the font/DC pair in a class that handle
this.
joe
****
Goran
> It looks like all of this may be moot
> because DeleteObject() is confirming that the object is
> being deleted while it is selected by its return value of 1,
> so my code is already good the way that it is. I only tested
> this with VS 2008, but, it works on Windows 7 and XP.
Man, Joe is telling you repeatedly that you are mistaken. I told you
to try some GDI resource monitor program and see whether your GDI
objects indeed do get destroyed. I now believe that you didn't try
that and that you don't actually know more about that except
DeleteObject returned true. That is NOT ENOUGH.
But fine, don't trust us. Try this: start your task manager and add
"GDI objects" column. Make a test program where you create (or get) a
DC, then create a font, select it into a DC, and then call
DeleteObject on it. Watch GDI object count for your process. Did you
see that count dropped by 1 when you called DeleteObject? No you
haven't, despite the fact that DeleteObject returned TRUE.
BTW, if you do this, you WILL see that GDI object count drops, but
only when your DC (not font, DC) is destroyed. So that might mean that
some sort of reference counting is employed for fonts, or it might
mean absolutely NOTHING, because, AFAIK, none of that is actually
documented.
Now, do the same for e.g. a pen. There, you will see that GDI object
count indeed does drop. This is in line with doc for DeleteObject who
says "Do not delete a drawing object (pen or brush) while it is still
selected into a device context.". That, IMO, hints at the following:
if you delete e.g. a pen, and then do LineTo, what pen is a DC
supposed to use to draw? There is no pen anymore! Perhaps situation is
different for a font, but wouldn't you say now that it's a mess
playing with DeleteObject like that? Just unselect it before
destruction and that's all.
So what you choose to do, is to distrust the documentation and
disregard honest advice of people you often see here, and to rely on
undocumented behavior that you observe in your particular use case. Do
you actually think that is a good way to write code? If so, then
perhaps you don't deserve to be helped.
Goran.
Tom Serface
Windows behavior. Not so. My first real .NET application sucked up GDI
resources because I was creating Image objects and not specifically
Dispose()'ing them. Turns out they don't give back handles even when
garbage collected if you don't do that manually.
The worse thing is if you have another thread and it tries to get resource
that has been garbage collected, that you haven't destroyed, it will just
throw an "out of memory" exception which causes you to go down a whole
complete red herring path while debugging.
I've found managing these sorts of resource "trickies" to be some of the
most annoying parts of coding multimedia software. I can't blame Peter for
being a little confused about it. The whole font creation and usage thing
in MFC is just too cumbersome. That is something that .NET got right. You
just use the font (properties or otherwise) and you don't have to worry
about destroying it or whether it's still available or ...
Tom
"Goran" <goran...@gmail.com> wrote in message
news:a3fc4a6b-0a96-4674...@m37g2000yqf.googlegroups.com...
Joseph M. Newcomer
On Tue, 16 Mar 2010 07:54:20 -0500, "Peter Olcott" <NoS...@OCR4Screen.com> wrote:
>
>"Joseph M. Newcomer" <newc...@flounder.com> wrote in
>message news:hoetp55bqv7f9lm31...@4ax.com...
>> What part of "use RestoreDC()" did you miss? If you
>> simply RestoreDC() when you are done
>> with the font, you will revert to whatever the default
>> font was. Not a big deal. The
>> other common practice is to note that if you do
>> SelectObject() of a font, the result is a
>> CFont * that had been selected and you simply re-selecte
>> this. This technique generalizes
>> to all 30 or so parameters of an HDC (changing a parameter
>> returns you the prvious
>> setting) but this means you have to keep variables around
>> to know what value to
>> re-selected, and I find RestoreDC() to be a simpler
>> apprroach.
>> joe
>
>Its not just Font its also CBitmap, and there is no default
>bitmap to restore.
***
Have you not been paying attention? THere is ALWAYS a bitmap to restore, and it is the
one that is returned by SelectObject of the bitmap, OR the one that is restored when you
call RestoreDC, which is a 1x1 monochrome bitmap. But you didn't have to know about the
1x1 bitmap, just read about SelectObject OR about RestoreDC, both of which state
explicitly what is going to happen! Your mistaken believe that there is no default bitmap
is the heart of the problem, and it means you have ignored everything we have been telling
you! If you ask questions in this forum and get answers, you should pay attention to them
instead of insisting that it works differently than we are telling you.
****
>It looks like all of this may be moot
>because DeleteObject() is confirming that the object is
>being deleted while it is selected by its return value of 1,
>so my code is already good the way that it is. I only tested
>this with VS 2008, but, it works on Windows 7 and XP.
***
That's a change in behavior from the documented and measured past behavor. It would be
nice if someone from Microosoft could confirm what really happens here.
joe
****
Peter Olcott
Yes I now have this point and the complete answer to this
thread has been finally addressed and proven by you. You can
skip answering any of my posts on this thread, any ones that
you have not yet answered already have their answer now. The
key missing piece was the default 1 bit monochrome bitmap. I
already know that there is a default font. Also I am now
convinced that advice provided on this forum most often
supersedes advice provided by Microsoft.
One more little nuance in case that I need it, Is there also
a default memory device context?
Peter Olcott
//
// Displays a Font Selection DialogBox with
// default "Times New Roman", Bold, Italic, 8 Point
//
inline void ScreenCaptureType::SelectFont() {
int PointSize = 8;
int FontHeight = -MulDiv(PointSize,
GetDeviceCaps(MemoryDC.m_hDC, LOGPIXELSY), 72);
LOGFONT LogFont = {FontHeight, 0, 0, 0, FW_BOLD, 1, 0, 0,
ANSI_CHARSET, OUT_DEFAULT_PRECIS,
CLIP_DEFAULT_PRECIS, DEFAULT_QUALITY, DEFAULT_PITCH |
FF_DONTCARE, L"Times New Roman"};
CFontDialog dlg(&LogFont);
CFont tempfont;
CFont *OldFont;
tempfont.CreateFontIndirect(&LogFont);
OldFont = this->MemoryDC.SelectObject(&tempfont);
OldFont->DeleteObject();
if (dlg.DoModal() == IDOK)
this->cfont.CreateFontIndirect(&LogFont);
this->MemoryDC.SelectObject(&cfont);
}
So basically the messy little five line block of code in the
middle is the cleanest way to implement my functional
requirements within the requirements of MS Windows resource
management?
A simpler way that could possibly work would be:
Get rid of the five line block and replace the last line
with these two lines:
CFont* OldFont = this->MemoryDC.SelectObject(&cfont);
OldFont->DeleteObject();
This would work if the MemoryDC maintains its own pointer
the original CFont object, and does not rely upon
this->cfont to retain it.
Joseph M. Newcomer
****
if(OldFont != NULL)
****
>OldFont->DeleteObject();
>
>This would work if the MemoryDC maintains its own pointer
>the original CFont object, and does not rely upon
>this->cfont to retain it.
****
Here's what happens, really:
THe DC maintains the HFONT to a font. Note that a DC has NO CONCEPT of a CFont object!
WHen ::SelectObject returns the HFONT, the MFC runtime looks this up in the thread's
handle map. If it finds a CFont* associated with that HFONT in the permanent handle map,
it returns the pointer to the CFont* object. If it fails to find one, it creates a
TEMPORARY CFont* object in the temporary handle map, and returns a pointer to that.
When MFC finally invokes CWinApp::OnIdle, the code there wanders down the handle map,
deleting all temporary objects. Because they are temporary, their destructors do not
delete the kernel objects associated with the object.
It is best to understand how this works.
****
Peter Olcott
CFont* OlldFont = this->MemoryDC.SelectObject(&cfont);
if (OldFont != NULL)
OldFont->DeleteObject();
So the above code will always work?
I was concerned that the
this->cfont.CreateFontIndirect(&LogFont);
may erase the only pointer to the CFont object, even the one
that MemoryDC refers to.
>>
>>This would work if the MemoryDC maintains its own pointer
>>the original CFont object, and does not rely upon
>>this->cfont to retain it.
> ****
> Here's what happens, really:
>
> THe DC maintains the HFONT to a font. Note that a DC has
> NO CONCEPT of a CFont object!
>
Since I have no idea what an HFONT is (other than a handle
to a font) or how it works, knowing this does not help.
Learning MFC programming in terms of Win32 programming only
works if you know Win32 programming.
Goran
> Since I have no idea what an HFONT is (other than a handle
> to a font) or how it works, knowing this does not help.
> Learning MFC programming in terms of Win32 programming only
> works if you know Win32 programming.
That's a silly thing to say, and thinking that way won't get you far
with MFC.
MFC really is a rather thin layer over Win32. That influences very
much how it's made and what it does. And indeed, if you look at docs
for MFC, there is often corresponding Win32 function link at the end,
and doc for it is pretty much the same then.
For example, you work with LOGFONT. That has nothing to do with MFC
and all to do with Win32. So you in fact do Win32, not MFC,
programming, when you handle it. Your trouble here with selecting
stuff into DC stems from poorly understanding both Win32 and MFC in
that area. What's worse, your "functional requirements" you throw
around are wrong because of that.
You just shouldn't be doing this. There's no abstraction that doesn't
leak (see http://en.wikipedia.org/wiki/Leaky_abstraction), and MFC is
indeed an abstraction that leaks a lot, probably more than it has to.
If you want stronger abstractions, use a better framework.
Goran.
Joseph M. Newcomer
what happens when the Win32 API meets MFC, and it is not not always obvious what happens
unless you understand both the API and MFC. And not at a superficial level.
You can't really understand MFC unless you understand the concept of handle maps and
temporary objects. Yes, I successfully worked in it for two years before I got the deep
insights, but I found that they were critical to my future success. Now I teach how to
use both STL and MFC collections at the interfaces.
joe
Joseph M. Newcomer
****
No. What this does if the user hits "Cancel" for the font dialog is delete the existing
font. You did not validate that CreatFontIndirect successfully creating a new font.
The correct code would be
if(dlg.DoModal() != IDOK)
return;
if(!cfont.CreatFontIndirect(&LogFont))
{ /* failed to create font */
DWORD err = ::GetaLastError();
...maybe report it here, inluding decoding of err using FormatMessage
... see my ErrorString function on my MVP Tips site
return;
} /* failed to create font */
CFont * OldFont = MemoryDC.SelectObject(&cfont);
if(OldFot != NULL)
OldFont->DeleteObject();
Not sure why you felt compelled to put the inline directive there (which is actually
__inline; the "inline" keyword is recognized only for backward compatibility with obsolete
code); the optimizing compiler will automatically inline it it this produces better code,
and if it wouldn't, you are producing suboptimal code.
****
>I was concerned that the
> this->cfont.CreateFontIndirect(&LogFont);
>may erase the only pointer to the CFont object, even the one
>that MemoryDC refers to.
****
There is no "pointer" to the CFont object, because the variable is
CFont cfont;
and the only time you ever have a "pointer" to it is when you specify &cfont.
So I don't understand the question.
joe
****
Peter Olcott
I will be porting my system without the need of a framework.
As long as LibPNG does not leak, I should be good to go.
Peter Olcott
OK so your code is better because it handles a possible
error condition. It may still be wrong though. It looks like
it selects the font, even there was an error in creating
this font. In the case where there is an error creating the
font, I want to keep the original font. So I am guessing
that wrapping the last three lines with an else from the if
statement would fix this.
>
> Not sure why you felt compelled to put the inline
> directive there (which is actually
> __inline; the "inline" keyword is recognized only for
> backward compatibility with obsolete
> code); the optimizing compiler will automatically inline
> it it this produces better code,
> and if it wouldn't, you are producing suboptimal code.
> ****
I keep most of my code in header files that won't compile if
I don't use the inline keyword. This is one aspect the way
that I beat MS VC++ 6.0 and Borland C++ Builder std::string
by a fifty-fold faster performance. I think there was even
one benchmark where I beat Borland by 125-fold.
>>I was concerned that the
>> this->cfont.CreateFontIndirect(&LogFont);
>>may erase the only pointer to the CFont object, even the
>>one
>>that MemoryDC refers to.
> ****
> There is no "pointer" to the CFont object, because the
> variable is
> CFont cfont;
> and the only time you ever have a "pointer" to it is when
> you specify &cfont.
There is a pointer to something otherwise DeleteObject()
would have nothing to work with. This member function is
available from the CFont object's interface.
I wanted to avoid the sort of memory leak error shown in the
code below. If you create an object using the same pointer
where another object resides, you lose the capability to
remove the original object. It looks like your corrected
code above still has this same problem, depending on how
Windows/Win32/MFC works under the covers. The CFont object
is created in the same location where the old CFont object
is later deleted.
'#include <stdio.h>
#include <malloc.h>
int main() { // Intentioanal Memory Leak
int* X;
int Z;
X = (int*)malloc(104857600);
for (int N = 0; N < (104857600 / 4); N++)
*(X + N) = N;
printf("Hit Return");
scanf("%c",&Z); // System reports 10 MB allocated
X = (int*)malloc(104857600); // memory leak
for (int N = 0; N < (104857600 / 4); N++)
*(X + N) = N;
printf("Hit Return");
scanf("%c",&Z); // System reports 20 MB allocated
free(X);
printf("Hit Return");
scanf("%c",&Z); // System still reports 10 MB
allocated
return 0; // because of memory leak