We help IT Professionals succeed at work.

Simple multi-threading synchronization question

thresher_shark
on
272 Views
Last Modified: 2013-11-20
Hi,

I am working on my first multi-threaded program. It generates a star field on the screen which is continually moving around; tough to explain.

Anyway, I thought that I would create a separate thread for the drawing of the stars. The main program thread would perform all of the calculations for where the stars are moving to.

So, when drawing, it leaves garbage behind. I know I am doing it right because I have done many programs like this (i.e. erase prev star, draw new star, calculate new pos, loop), and the problem hasn't started occurring until I tried multi-threading.

What I think is happening is the stars coordinates are being updated at the same time they're being drawn, which is kind of what I want (i.e. I want the program to go faster :-)).

I think I can solve the problem using something like CSemaphore, CMutex, or CCriticalLock and the CSingleLock classes, but I don't understand how to use them.

Here is a large part of the source code:

void CSpaceSaverDlg::OnTimer(UINT nIDEvent)
{ int i;
  int j = 0;

  dat->s = stars;
  dat->num = m_Number;
  dat->wnd = this;

  AfxBeginThread (ThreadProc, dat); // Passing global variable?  That's life.
  // Sleep (50); // Fixes some
  // Sleep (100); // Fixes all the way.

  if (rand () % 1000 == 0)
  { m_Direction = rand () % 8; }

  for (i = 0; i < m_Number; i++)
  { CSingleLock sLock (&stars[i].m, FALSE);
    if (sLock.Lock () == 0)
    { MessageBox ("Error");
      break;
    }

    stars[i].px = stars[i].x;
    stars[i].py = stars[i].y;

    switch (m_Direction)
    { case (0): stars[i].x += stars[i].z / m_Speed;
                break;

      case (1): stars[i].x += stars[i].z / m_Speed;
                stars[i].y += stars[i].z / m_Speed;
                break;

      case (2): stars[i].y += stars[i].z / m_Speed;
                break;

      case (3): stars[i].x -= stars[i].z / m_Speed;
                stars[i].y += stars[i].z / m_Speed;
                break;

      case (4): stars[i].x -= stars[i].z / m_Speed;
                break;

      case (5): stars[i].x -= stars[i].z / m_Speed;
                stars[i].y -= stars[i].z / m_Speed;
                break;

      case (6): stars[i].y -= stars[i].z / m_Speed;
                break;

      case (7): stars[i].y -= stars[i].z / m_Speed;
                stars[i].x += stars[i].z / m_Speed;
                break;
    }

    if (stars[i].x > m_MaxX)
    { stars[i].x -= m_MaxX; }
    else if (stars[i].x < 0.0)
    { stars[i].x += m_MaxX; }

    if (stars[i].y > m_MaxY)
    { stars[i].y -= m_MaxY; }
    else if (stars[i].y < 0.0)
    { stars[i].y += m_MaxY; }
    sLock.Unlock ();
  }

  CDialog::OnTimer(nIDEvent);
}

UINT ThreadProc (LPVOID pParam)
{ Data* dat = (Data *) pParam;

  if (dat == NULL)
  { return -1; }   // illegal parameter

  CDC *dc = dat->wnd->GetDC ();
  CBrush blankbrush (RGB (0, 0, 0));
  CBrush brush;
  int i;

  int m_Number = dat->num;
  Star *stars = dat->s;

  for (i = 0; i < m_Number; i++)
  { CSingleLock sLock (&stars[i].m, FALSE);
    if (sLock.Lock () == 0)
    { dat->wnd->MessageBox ("Error");
      break;
    }

    brush.CreateSolidBrush (stars[i].color);

    dc->SelectObject (blankbrush);
    dc->Ellipse ((int) stars[i].px - 2, (int) stars[i].py - 2, (int) stars[i].px + 2, (int) stars[i].py + 2);
    dc->SelectStockObject (NULL_BRUSH);
    //dc->SetPixel ((int) stars[i].px, (int) stars[i].py, RGB (0, 0, 0));

    dc->SelectObject (brush);
    dc->Ellipse ((int) stars[i].x - 2, (int) stars[i].y - 2, (int) stars[i].x + 2, (int) stars[i].y + 2);
    dc->SelectStockObject (NULL_BRUSH);
    //dc->SetPixel ((int) stars[i].x, (int) stars[i].y, stars[i].color);

    brush.DeleteObject ();
    sLock.Unlock ();
  }

  blankbrush.DeleteObject ();

  return 0;    // thread completed successfully
}

/////////////////////////////
// Here is the stars structure:

#ifndef STARH
#define STARH

#include <afxmt.h>

class Star
{ public:
  void Init (double, double);

  volatile double x, y, z;
  volatile double px, py;
  volatile COLORREF color;

  CMutex m;
};

#endif

// And the stars.cpp implementation:
#include "stdafx.h"
#include "star.h"

void Star::Init (double MaxX, double MaxY)
{ x = (double) (rand () % (int) MaxX);
  y = (double) (rand () % (int) MaxY);
  z = (double) (rand () % (int) 384 + 128.0);
  px = 0.0;
  py = 0.0;

  color = RGB ((int) z / 2, (int) z / 2, (int) z / 2);

  //color = RGB (255, 255, 255);
  return;
}


What am I doing wrong?  Why is garbage still being left behind?

Thank you for any help you can provide.  Note that if the solution is more complex than it seems, I can increase the number of points.
Comment
Watch Question

Author

Commented:
Edited text of question
Unlock this solution and get a sample of our free trial.
(No credit card required)
UNLOCK SOLUTION

Author

Commented:
Thank you for your ideas.  I hadn't thought about the name of the mutex.  How would you recommend I generate a unique name for every star?

I also see what you mean about adjusting the stars coordinates all the time, as opposed to your idea which would be better in this case.

Thanks for your help.

Author

Commented:
Shoot, forget the first paragraph of the above comment.  I didn't read your answer thoroughly :-)

Author

Commented:
Well, this is strange, after trying your suggestions, the problem still occurs.

I have named each mutex individually.  I then changed the calculating code so it uses temporary variables.  Here is the new calculating code:

void CSpaceSaverDlg::OnTimer(UINT nIDEvent)
{ int i;
  int j = 0;
  double xinc = 0.0, yinc = 0.0, ztemp = 0.0;

  dat->s = stars;
  dat->num = m_Number;
  dat->wnd = this;

  AfxBeginThread (ThreadProc, dat); // Passing global variable?

  // Sleep (50); // Fixes some
  // Sleep (100); // Fixes all the way.

  if (rand () % 1000 == 0)
  { m_Direction = rand () % 8; }

  for (i = 0; i < m_Number; i++)
  { CSingleLock sLock (stars[i].m, FALSE);
    if (sLock.Lock () == 0)
    { MessageBox ("Error");
      break;
    }

    xinc = stars[i].x;
    yinc = stars[i].y;
    ztemp = stars[i].z;

    stars[i].px = xinc;
    stars[i].py = yinc;

    switch (m_Direction)
    { case (0): xinc += ztemp / m_Speed;
                break;

      case (1): xinc += ztemp / m_Speed;
                yinc += ztemp / m_Speed;
                break;

      case (2): yinc += ztemp / m_Speed;
                break;

      case (3): xinc -= ztemp / m_Speed;
                yinc += ztemp / m_Speed;
                break;

      case (4): xinc -= ztemp / m_Speed;
                break;

      case (5): xinc -= ztemp / m_Speed;
                yinc -= ztemp / m_Speed;
                break;

      case (6): yinc -= ztemp / m_Speed;
                break;

      case (7): yinc -= ztemp / m_Speed;
                xinc += ztemp / m_Speed;
                break;
    }

    if (xinc > m_MaxX)
    { xinc -= m_MaxX; }
    else if (xinc < 0.0)
    { xinc += m_MaxX; }

    if (yinc > m_MaxY)
    { yinc -= m_MaxY; }
    else if (yinc < 0.0)
    { yinc += m_MaxY; }

    stars[i].x = xinc;
    stars[i].y = yinc;
    //stars[i].z = ztemp; // No changes to z so no need to change
    sLock.Unlock ();
  }

  CDialog::OnTimer(nIDEvent);
}

/////////////////////////////////
//
// I have not changed the drawing code at all.  Here is the new // star structure (for now):
//
/////////////////////////////////
/////// Stars.cpp:

#include "stdafx.h"
#include "star.h"

void Star::Init (double MaxX, double MaxY, int index)
{ x = (double) (rand () % (int) MaxX);
  y = (double) (rand () % (int) MaxY);
  z = (double) (rand () % (int) 384 + 128.0);
  px = 0.0;
  py = 0.0;

  color = RGB ((int) z / 2, (int) z / 2, (int) z / 2);
  //color = RGB (255, 255, 255);

  CString temp;
  temp.Format ("NameOfMutex%d", index);
  m = new CMutex (FALSE, temp);
  return;
}

Star::~Star ()
{ delete m;
}

///////////////////////////
// Stars.h

#ifndef STARH
#define STARH

#include <afxmt.h>

class Star
{ public:
  ~Star ();
  void Init (double, double, int);

  volatile double x, y, z;
  volatile double px, py;
  volatile COLORREF color;

  CMutex *m;
};

#endif


/////

But now for the really strange part.  If I un-rem the line in the Init function that sets the star's color to 255, 255, 255, it WORKS!  But otherwise it doesn't work!

It is probably something very simple, that I am just missing, but I don't know.

In any case, thank you very much for your help.

Author

Commented:
If you want, I can email you the whole project and you can see this "phenomenon" in action.

Author

Commented:
Here is some information I collected:

color = RGB (255, 255, 255); // Works
color = RGB (254, 254, 254); // Doesn't work
color = RGB (253, 253, 253); // Doesn't work
color = RGB (252, 252, 252); // Doesn't work

color = RGB (128, 128, 128); // Works
color = RGB (129, 129, 129); // Doesn't work

color = RGB (192, 192, 192); // Works

It seems things that have to do with a power of two work well.  But 255 doesn't have much to do with that except 2^8 - 1 = 255.  Anyway... I still don't understand.  Thanks for your help.
This color thing is really strange. Send me the code if you can as an attachment.
My email address is cdjindia@bestweb.net

Also lemme know what compiler and what version are you using for this project.
 

Author

Commented:
I have sent the project.  I have Visual C++ with SP3.  I hope your mail reader understands the MIME format for email.
I  have lost your email address. please send a fake mail either at
cdjindia@bestweb.net or cdjindia@hotmail.com and I can mail you the
new code.


Author

Commented:
I have sent you the message.

Author

Commented:
Have you sent the new code yet?  I am just dying to see what the problem is :-)

Yes, I sent it already on Friday. Earliest I can send it again is now on Monday because I don't have that file at home machine. I sent it to your @gte account.


On second thoughts (to keep you from dying ... :-; )

Implementation that you have for painting the stars is not actually as per microsoft API
specifications.

Author

Commented:
It might not have arrived yet because it's large, the mail servers probably have priorities and all kinds of other stuff to prevent themselves from getting too overloaded.

Author

Commented:
Hello again,

I found a small problem with my drawing code, I was not releasing the DC after I was done with it, however the garbage problem is still ocurring.  Can you please be more specific with what the problem was?  After all, anyone following this thread would probably like to know, especially if they had to pay for it :-)

Thank you so much for all of your help so far.

Author

Commented:
Adjusted points to 100
Unlock the solution to this question.
Thanks for using Experts Exchange.

Please provide your email to receive a sample view!

*This site is protected by reCAPTCHA and the Google Privacy Policy and Terms of Service apply.

OR

Please enter a first name

Please enter a last name

8+ characters (letters, numbers, and a symbol)

By clicking, you agree to the Terms of Use and Privacy Policy.