0

I'm making a simple screensaver that shows a set of pictures without repeating them before showing them all. The thing is, the pictureBox is skipping some updates or something, or changing between two pictures really fast. Other times it stays on the same image for too long.

It works just fine in debug mode, even when I hit continue several times, except when i hit continue too fast.

The picturebox updates are tied to a timer which fires once each 2 seconds.

private void timer1_Tick(object sender, EventArgs e)
    {
        if (pictureBox1.Image != null) { pictureBox1.Image.Dispose();}
        pictureBox1.Image = (Image)siguienteImagenAMostrar.Clone(); 
        //siguienteImagenAMostrar is an Image, and it´s the one i want to show next 
        pictureBox1.Invalidate();
        pictureBox1.Update();
        if (siguienteImagenAMostrar != null) { siguienteImagenAMostrar.Dispose(); }
        siguienteImagenAMostrar = siguienteImagen(); //I try to preload the Image         
    }

Here is the siguienteImagen() code, just in case it has something to do with the Random I used:

private Image siguienteImagen()
        {
            int imagenAmostrar;
            do
            {
                imagenAmostrar = rand.Next(0, files.Length);
            } while (currentImage == imagenAmostrar && files.Length > 1);

            int original = imagenAmostrar;

            if (mostrada[imagenAmostrar]) {
                imagenAmostrar = (imagenAmostrar + 1) % files.Length;
                while (mostrada[imagenAmostrar] && original != imagenAmostrar)
                {
                    imagenAmostrar = (imagenAmostrar + 1) % files.Length;
                }
                if (imagenAmostrar == original) 
                {
                    mostrada = Enumerable.Repeat(false, files.Length).ToArray();
                }
            }
            mostrada[imagenAmostrar] = true;
            currentImage = imagenAmostrar;
            string imagenMostrarString = files[imagenAmostrar];
            return Image.FromFile(imagenMostrarString);

        }

Edit: This function seems not to be the problem as simply choosing the next image as currentImage = (currentImage + 1) % files.Length still has the issue of not showing the pictures properly.

Also mostrada is an array of bool which knows if that picture was already shown. files is an array with the path to the pictures. The same index represents the same image on both.

yaexiste2
  • 101
  • 1
  • 7
  • You get a random number. Then add one to it, but also modulus it. That seems redundant. – John Jul 08 '20 at 05:15
  • @John That bit of code is the way I found to circle through the bool array, trying to find some false value, meaning I haven't shown that image yet. – yaexiste2 Jul 08 '20 at 05:30
  • I would create a copy of the list and while it isn't empty pick&remove a random item. - Also [remommended](https://stackoverflow.com/questions/57754206/error-when-im-trying-to-delete-an-image-used-in-a-picturebox/57754802#57754802) was to dispose of the images – TaW Jul 08 '20 at 07:14
  • @TaW i hadn´t thought about doing that. It seems like a better approach – yaexiste2 Jul 09 '20 at 00:50

2 Answers2

0

I am betting it would make your code cleaner and less work for you and the execution if you avoided getting a random number that may have already been used. Using an array of Boolean to keep track of this is possibly wasting cycle times since the random number generator may produce an already selected value. I would think you would want to avoid this.

One possible solution is to create a List<int> of the indexes from the images collection. Example the image collection has five (5) images, then this list would look like….

Index    value
  0        0
  1        1
  2        2
  3        3
  4        4

We would want to “keep” a copy of this list since we will need it after we go through all the images. So let’s say the first random number from rand.Next(0, listAbove.Count); is 3. Then we would grab the value from index 3, which is also index three (3). We get the image at index three (3) and “remove” the three (3) from the list. After this the list would look like….

Index    value
  0        0
  1        1
  2        2
  3        4

Continue in this fashion until the list is empty at which point, we simply refill it.

Given this creates two List<int> variables…

List<int> fullImageListIndexes;
List<int> currentImageIndexes;
Random rand = new Random();

Now we want a method that returns a random image from this list. The bounds of the Random call will be zero (0) to the size of the list. After we have found a random index, then we “remove” it from the list. This will decrease the list size and guarantee that there will always be only ONE (1) call to the random number generator. Continuing in this fashion we simply need to check if the image list is empty, which would indicate that we have used all the images and need to “refill” the list with the original values. I hope that makes sense.

A method that returns the next random image as described above is below… First a check to see if the list is empty, and if it is, simply refill it. Get the next random index based on the size of the list, Get the value of the index to the original images, then get that image, and finally “remove” the image index we just used.

private Image GetNextRandomImage() {
  if (currentImageIndexes.Count == 0) {
    //textBox1.Text += "Refill" + Environment.NewLine;
    currentImageIndexes = new List<int>(fullImageListIndexes);
  }
  int randomIndex = rand.Next(0, currentImageIndexes.Count);
  int valueToRemove = currentImageIndexes[randomIndex];
  Image nextImage = imageList1.Images[valueToRemove];
  currentImageIndexes.Remove(valueToRemove);
  //textBox1.Text += "Tick : RI: " + randomIndex + "  SV: " + valueToRemove + Environment.NewLine;
  return nextImage;
}

The timer tick event…

private void timer1_Tick(object sender, EventArgs e) {
  pictureBox1.Image = GetNextRandomImage();
}

And finally putting it all together…. The imageList1 is a .Net image list object with some images added to its image collection…

public Form1() {
  InitializeComponent();
  fullImageListIndexes = new List<int>();
  for (int i = 0; i < imageList1.Images.Count; i++) {
    fullImageListIndexes.Add(i);
  }
  currentImageIndexes = new List<int>(fullImageListIndexes);
}

Hope this makes sense.

JohnG
  • 9,259
  • 2
  • 20
  • 29
  • First of all, thanks for such a detailed answer. I already tried it and it does select the images the same way my algorithm did, in a better way. The thing is, the problem is the pictures are still not always displaying, even if the GetNextRandomImage() is just currentImage+1 – yaexiste2 Jul 09 '20 at 01:39
  • @ yaexiste2… I have tested the posted code and it works as expected. It is difficult to understand what you mean by… _”even if the GetNextRandomImage() is just currentImage+1”_ … this does not make sense. What is currentimage + 1? – JohnG Jul 09 '20 at 06:03
  • The problem I had was with the updates of the picturebox, not the algorithm that selected them. I have already found out the problem, which I stated on an answer on this post, but it doesn't let me accept it for a few more hours. The problem was that the tick event was being called twice aparently, thus making weird things sometimes – yaexiste2 Jul 10 '20 at 00:16
0

The thing that happened was that i had timer1.Tick += new EventHandler(timer1_Tick); in the form_load() function. I'm still not sure why it worked in step by step mode

Removing the apparently extra EventHandler i had, solved my problem.

yaexiste2
  • 101
  • 1
  • 7