C / C++ / MFC

18-May-01 4:41

Hi:
I'm new in C++, and visual c++ programming and I have problems in a program I'm doing. I have a class like this:

class CDatos : public CDocument
{
protected:
DECLARE_SERIAL(CDatos)
CDatos();
public:
CDatos* GetDocument();

int dia;
int mes;
int anno;
int horaActual;
CString Titulo;

int hora[24];
int temp[24];
int rocio[24];
int pres[24];
int vel[24];
char direccion[24][9];
public:

virtual void Serialize (CArchive& ar);
virtual ~CDatos();
};

After it, I do this (in a .cpp file:
CDatos datosF;

Then I read from a file and put data into member variables, something like this: fscanf(fp,"%d.%d.%d %d", &datosF.anno,&datosF.mes,&datosF.dia,&datosF.horaActual);.....etc
, and I draw a graphic with this data. If I open 2 or 3 files I don't have problems, but when I try to open more, the system memory goes down quickly and the graphics began to do strange things. I suppose the problem is than something overwrites memory.
Can anybody help me to clean memory data after reading a new file? I suppose the solution is using 'new', 'delete' or destructor class, but I don`t know how to do it.
Thank you very much.

James R. Twine18-May-01 4:50

18-May-01 4:50

> but when I try to open more, the system memory goes down quickly and
> the graphics began to do strange things.

   That sounds more like a GDI resource leak than a memory leak (to me, anyway).  The class, as you described it above, does not take up a great amount of memory, so I do not see how having a few of them floating around in memory would cause that problem.

   Overwrites are something to consider, but we would have to see a bit more of your code.

   Peace!

-=- James.

José Luis Sogorb18-May-01 5:16

18-May-01 5:16

WELL, THIS IS THE FUNCTION DRAWING GRAPHIC:

/***************************

void CMeteoGraficaView::dibuja(CDC *pDC)
{
static int horaInicio;
static char *dir[8]= {"N","NE","E","SE","S","SO","O","NO" };

int x,x1,y,y1,maxT, minT, maxP, minP, maxV, minV,escalaT, escalaP, escalaV;
CPen *m_pPincel;
m_pPincel=NULL;
CFont* m_pLetra;
m_pLetra=NULL;
CString dirViento, actual, actual1;
int i;
float hH;
short nCount;

horaInicio=datosF.hora[0]+horaPref;
for(i=0;i<24;i++)
{
if(datosF.temp[i]!=666){
maxT=datosF.temp[i];
hayDatos=1;
}
}
for(i=0;i<24;i++)
{
if(datosF.rocio[i]!=666) minT=datosF.rocio[i];
}
for(i=0;i<24;i++)
{
if(datosF.pres[i]!=666) maxP=minP=datosF.pres[i];
}
for(i=0;i<24;i++)
{
if(datosF.vel[i]!=666) maxV=minV=datosF.vel[i];
}
for (i=0;i<24;i++)
{
if((datosF.temp[i]>maxT)&&(datosF.temp[i]!=666)) maxT=datosF.temp[i];

if ((datosF.rocio[i]<mint)&&(datosf.rocio[i]!=666)) mint="datosF.rocio[i];

if((datosF.pres[i]">maxP)&&(datosF.pres[i]!=666)) maxP=datosF.pres[i];

if ((datosF.pres[i]<minp)&&(datosf.pres[i]!=666)) minp="datosF.pres[i];

if((datosF.vel[i]">maxV)&&(datosF.vel[i]!=666)) maxV=datosF.vel[i];

if ((datosF.vel[i]<minv)&&(datosf.vel[i]!=666)) minv="datosF.vel[i];
}

escalaT=((maxT-minT)/8)+1;
escalaP=((maxP-minP)/8)+1;
escalaV=((maxV-minV)/8)+1;

" if(!lstrcmp("n",datosf.direccion[23]))="" dirviento="Norte" ;
="" else="" if(!lstrcmp("nne",datosf.direccion[23]))="" if(!lstrcmp("ne",datosf.direccion[23]))="" if(!lstrcmp("calm",datosf.direccion[23]))="" hh="(float)datosF.horaActual/100;
datosF.Titulo.MakeUpper();
actual.Format("Ultimos" datos="" del="" día="" %02d="" %d="" a="" las="" %.2f="" horas:="" %9s",="" datosf.dia,datosf.mes,datosf.anno,hh,datosf.titulo);
actual1.format("temperatura:="" %dºc,="" punto="" de="" rocio:="" presión="" atmosférica:="" mb.="" viento:="" km="" h,="" con="" dirección:="" %s."="" ,datosf.temp[23],datosf.rocio[23],datosf.pres[23],datosf.vel[23],dirviento);

pdc-="">SetMapMode(MM_LOENGLISH);
m_pLetra= new CFont();

m_pLetra->CreateFont(16,0,0,0,FW_SEMIBOLD,FALSE,FALSE,FALSE,
DEFAULT_CHARSET,OUT_DEFAULT_PRECIS,CLIP_DEFAULT_PRECIS,DEFAULT_QUALITY,
18,"Times New Roman");

pDC->SelectObject(m_pLetra);

pDC->TextOut(25,-2,actual);
pDC->TextOut(25,-16,actual1);

pDC->SelectObject(m_pLetra);
delete m_pLetra;
m_pLetra=NULL;
m_pLetra= new CFont();

m_pLetra->CreateFont(15,0,0,0,FW_NORMAL,FALSE,FALSE,FALSE,
DEFAULT_CHARSET,OUT_DEFAULT_PRECIS,CLIP_DEFAULT_PRECIS,DEFAULT_QUALITY,
18,"Times New Roman");

pDC->SelectObject(m_pLetra);

//COORDENADAS TEMPERATURA
pDC->MoveTo(60,-60);
pDC->LineTo(60,-220);
pDC->MoveTo(60,-220);
pDC->LineTo(310,-220);
pDC->TextOut(55,-45,"º",1); //1 o 2??
pDC->TextOut(63,-45,"C",1); //1 o 2??
coorH(pDC,80,215,horaInicio);
coorV(pDC,escalaT,maxT,40,60,0);

//COORDENADAS PRESION
pDC->MoveTo( 405,-60);
pDC->LineTo( 405,-220);
pDC->MoveTo( 405,-220);
pDC->LineTo( 655,-220);
pDC->TextOut( 400,-45,"mb",2); // 2 o 3??
coorH( pDC,425,215,horaInicio);
coorV( pDC,escalaP,maxP,385,60,0);

//COORDENADAS VELOCIDAD
pDC->MoveTo( 60,-280);
pDC->LineTo( 60,-440);
pDC->MoveTo( 60,-440);
pDC->LineTo( 310,-440);
pDC->TextOut( 50,-262,"km/h",4); //4 o 5??
coorH( pDC,80,435,horaInicio);
coorV(pDC, escalaV,maxV,40,280,1);

//COORDENADAS DIRECCION
pDC->MoveTo( 405,-280);
pDC->LineTo( 405,-440);
pDC->MoveTo( 405,-440);
pDC->LineTo( 655,-440);
coorH( pDC,425,435,horaInicio);

for (i=0,y=280;i<8;i++)
{
pDC->MoveTo( 400,-y);
pDC->LineTo( 410,-y);
nCount=sprintf(ch,dir[i]);
pDC->TextOut( 383,-(y-6),ch,nCount);
y=y+20;
}

//COORDENADAS MEDIDAS TEMPERATURA
m_pPincel= new CPen();
m_pPincel->CreatePen(PS_SOLID,2,RGB(255,0,0));
pDC->SelectObject(m_pPincel);

medida (pDC,80,60,maxT,escalaT,datosF.temp);
pDC->SelectObject(m_pPincel);
delete m_pPincel;
m_pPincel=NULL;

//COORDENADAS MEDIDAS ROCIO
m_pPincel= new CPen();
m_pPincel->CreatePen(PS_SOLID,2,RGB(255,96,0));
pDC->SelectObject(m_pPincel);

medida (pDC,80,60,maxT,escalaT,datosF.rocio);
pDC->SelectObject(m_pPincel);
delete m_pPincel;
m_pPincel=NULL;

//COORDENADAS MEDIDAS PRESION
m_pPincel= new CPen();
m_pPincel->CreatePen(PS_SOLID,2,RGB(0,255,255));
pDC->SelectObject(m_pPincel);

medida (pDC,425,60,maxP,escalaP,datosF.pres);
pDC->SelectObject(m_pPincel);
delete m_pPincel;
m_pPincel=NULL;

//COORDENADAS MEDIDAS VELOCIDAD
m_pPincel= new CPen();
m_pPincel->CreatePen(PS_SOLID,2,RGB(255,0,255));
pDC->SelectObject(m_pPincel);
medida (pDC,80,280,maxV,escalaV,datosF.vel);
pDC->SelectObject(m_pPincel);
delete m_pPincel;
m_pPincel=NULL;

//COORDENADAS MEDIDAS DIRECCION

for(i=0;i<24;i++)
{
x=425+(10*i);
x1=425+(10*(i+1));
y=y1=280;
...

m_pPincel= new CPen();
m_pPincel->CreatePen(PS_SOLID,2,RGB(0,255,0));
pDC->SelectObject(m_pPincel);

if (i<23&&lstrcmp("No",datosF.direccion[i])&&lstrcmp("No",datosF.direccion[i+1])&&lstrcmp("Calm",datosF.direccion[i])&&lstrcmp("Calm",datosF.direccion[i+1])&&lstrcmp("Variable",datosF.direccion[i])&&lstrcmp("Variable",datosF.direccion[i+1]))
{
pDC->MoveTo(x,-y);
pDC->LineTo (x1,-y1);
}
if (i<23&&lstrcmp("No",datosF.direccion[i])&&!lstrcmp("No",datosF.direccion[i+1]))
{
pDC->MoveTo(x,-y);
pDC->LineTo(x+3,-y);
}

pDC->SelectObject(m_pLetra);
delete m_pLetra;
m_pLetra=NULL;

pDC->SelectObject(m_pPincel);
delete m_pPincel;
m_pPincel=NULL;
}
}

/*****************************/
I CALL THE FUCTION FROM ONDRAW, THIS WAY:

void CMeteoGraficaView::OnDraw(CDC* pDC)
{
CPen *m_pPincel;
m_pPincel=NULL;
CFont* m_pLetra;
m_pLetra=NULL;

CMeteoGraficaDoc* pDoc = GetDocument();
ASSERT_VALID(pDoc);

CMeteoGraficaView::dibuja(pDC);

}
/**************************************/

AND THIS IS ONE OF THE WAYS ACQUIRING DATA (WITH SERIALIZE)

void CMeteoGraficaDoc::Serialize(CArchive& ar)
{

// CDocument::Serialize(ar);
datosF.Serialize(ar);
if (ar.IsStoring())
{ ar<<datosf.dia<<datosf.mes<<datosf.anno<<datosf.horaactual<<datosf.titulo;

for(int="" i="0;" i<24;i++)
="" {
="" ar<<datosf.hora[i]<<datosf.temp[i]<<datosf.rocio[i]<<datosf.pres[i]<<datosf.vel[i];
="" ar.write(datosf.direccion[i],3);
="" }

="" }
="" else
="" ar="">>datosF.dia>>datosF.mes>>datosF.anno>>datosF.horaActual>>datosF.Titulo;

for(int i = 0; i<24;i++)
{
ar>>datosF.hora[i]>>datosF.temp[i]>>datosF.rocio[i]>>datosF.pres[i]>>datosF.vel[i];
ar.Read(datosF.direccion[i],3);
}
ficheroLeido=TRUE;
ficheroDisco=TRUE;
}

}

/*******************************/

AFTER EACH FILE OPENED I LOOSE A 5% MEMORY( 41%, 37%, 32% ...)

James R. Twine18-May-01 8:32

18-May-01 8:32

> WELL, THIS IS THE FUNCTION DRAWING GRAPHIC [...]

   Whenever you call CDC::SelectObject(...), you need to store the object that is returned from that function call, and restore it back into the GDI context.  Also, deleting a GDI object while it is still Selected into a Device Context (DC) is a bad thing.

   My suggestions would be to:
     1: Keep track of the objects returned from the call to SelectObject(...), or take a look at the SaveDC(...) and RestoreDC(...) functions.  You will need to make sure to restore previously selected objects back into the DC before you delete the CGdiObjects that are currently selected into it.
     2: Do not use dynamic memory allocation for the temporary GDI objects you are using (the CPen and CFont objects). Allocate them locally, on the stack, and just reuse them as needed.

   Peace!

-=- James.

José Luis Sogorb18-May-01 9:40

18-May-01 9:40

Well, I took the idea from a Microsoft Visual C++ example (In the offical book):

void CMyDrawView::SetPenBrush(CDC *pDC, bool bTransparent, UINT nColor)
{
ASSERT(pDC != NULL);
if(bTransparent)
{
m_pBrushOld = (CBrush*)pDC->SelectStockObject(NULL_BRUSH);
}
else
{
m_pBrushOld = (CBrush*)pDC->SelectStockObject(WHITE_BRUSH);
}
ASSERT(m_pBrushOld != NULL);

ASSERT(nColor - ID_COLOR_BLACK >= 0 &&
nColor - ID_COLOR_BLACK <=
(sizeof(arColors) / sizeof(arColors[0])));
m_pPenNew = new CPen(); //------->> here

m_pPenNew->CreatePen(PS_INSIDEFRAME, 0,
arColors[nColor - ID_COLOR_BLACK]);
m_pPenOld = (CPen*)pDC->SelectObject(m_pPenNew);
}

void CMyDrawView::ResetPenBrush(CDC *pDC)
{
ASSERT(pDC != NULL);
pDC->SelectObject(m_pBrushOld);
pDC->SelectObject(m_pPenOld);

delete m_pPenNew; //--------->>>>> here
m_pPenNew = NULL;
m_pPenOld = NULL;
m_pBrushOld = NULL;
}

Which is the solution then?

James R. Twine18-May-01 10:14

18-May-01 10:14

> Well, I took the idea from a Microsoft Visual C++ example [...]
> Which is the solution then?

The solution is to never use Microsoft source code as an example of good coding practices (when was the last time you crashed a Microsoft product?)! Smile | :)

   The example code uses dynamic memory allocation because the object is allocated in the scope of one function, and used in the scope of another.  While not the best solution, it gets around the scoping problem.

   Note, however, that the example code DOES restore the existing objects back into the DC in the ResetPenBrush(..) function.  That is what the m_pPenOld and m_pBrushOld are used for.

   The original code you posted does not suffer from a scoping problem, so you should be able to use a locally (stack) allocated object.

   Peace!

-=- James.

José Luis Sogorb18-May-01 23:22

18-May-01 23:22

I've found an interesting article about this. Take a look at: http://www.pgh.net/~newcomer/savedc.htm

James R. Twine19-May-01 2:16