Preventing memory and resource leaks with GDI Objects ???
in [MFC]
|
Prev: Posting 60 messages per second to CView locks up UI (animation continues...)
Next: Turning on ClearType for TextOut() ???
From: Goran on 14 Mar 2010 13:05 On Mar 12, 4:07 pm, "Peter Olcott" <NoS...(a)OCR4Screen.com> wrote: > 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.
From: Goran on 14 Mar 2010 13:12 On Mar 14, 5:46 am, "Peter Olcott" <NoS...(a)OCR4Screen.com> wrote: > 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.
From: Peter Olcott on 14 Mar 2010 13:22 The point is that the function listed below can be called 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.pusic(a)gmail.com> wrote in message news:bc8e5ce9-773d-42df-b509-097b70e021d1(a)33g2000yqj.googlegroups.com... On Mar 12, 4:07 pm, "Peter Olcott" <NoS...(a)OCR4Screen.com> wrote: > 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.
From: Peter Olcott on 14 Mar 2010 13:29 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. 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.pusic(a)gmail.com> wrote in message news:06b6b099-4b40-4809-868b-982febcb6066(a)t23g2000yqt.googlegroups.com... On Mar 14, 5:46 am, "Peter Olcott" <NoS...(a)OCR4Screen.com> wrote: > 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.
From: Goran on 14 Mar 2010 14:44
On Mar 14, 6:29 pm, "Peter Olcott" <NoS...(a)OCR4Screen.com> wrote: > 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. |