Search Unity

Question Slowdown in IEnumerator loop when using Input

Discussion in 'Scripting' started by RASKALOF, Apr 18, 2023.

  1. RASKALOF

    RASKALOF

    Joined:
    Jul 9, 2012
    Posts:
    56
    Hello, im trying to implement sand behaviour.
    I have image 800x600
    i have array of custom Pixel struct 800x600 (aka coordinates)(struct contains type and color fields)
    I have 2 types of Pixels: CLEAR (color = transparent), SAND (color = yellow).
    In infinite loop in IEnumerator i go thru each x and y of array and finding out what this pixel (array[x,y]) is (CLEAR or SAND) and what to do next. if array[x,y].type==CLEAR - do nothing, if array[x,y].type==SAND - move this sand doown or downleft or downright and set previous position to clear. Applying color changes in image for previous and current position. triviial.
    And now the problem:
    I adding sand by mouse hold (in Update function) and runtime slowdowns while im holding mouse (the bigger the brush (2х2, 4x4 coordinates) the slower runtime is). Sand is adding and falling down, but spread works in weird way, like i adding sand faster than it spreads.for example if i hold mouse in one coordinates i can get sand clif (not good), but after mouse button is released in 1-2 seconds duration i can see awesome 60 fps sand spreading like it must work. If im not holding mouse and bursting - the faster i bursting the slower fps becomes. I think the prooblem is in IEnumerator, but have no clue how to do it in the better way. Or maybe i just do something stupid.

    hope for help. how to remove slowdown while adding sand.

    Peaces of code:
    this IEnumerator called in void Start()
    Code (CSharp):
    1. IEnumerator UpdateLoop() {
    2.         Pixel cur_pix = PCLEAR;
    3.         while(true) {
    4.             for(var y = height - 1; y > 0; --y) { // for each coordinate in array
    5.                 for(int x = 0; x < width; ++x) {
    6.                     cur_pix = GetP(x, y); // Getting pixel to get it type
    7.                     switch(cur_pix.id) {
    8.                         case ID_TYPE.SAND: { // if this coordinate in array is sand
    9.                             UpdateSand(x, y); // spread this peace in sand way
    10.                             break;
    11.                         }
    12.                         case ID_TYPE.NONE:
    13.                         default: {
    14.                             break;
    15.                         }
    16.                     }
    17.                 }
    18.             }
    19.             yield return null;
    20.         }
    21.     }
    22.  
    UpdateSand is
    Code (CSharp):
    1. void UpdateSand(int x, int y) {
    2.         x = clampx
    3.         y = xlampy
    4.         if(candown ) {
    5.             SetPixel(x, y, PCLEAR);
    6.             SetPixel(x, y - 1, PSAND);
    7.         }else if(candownleft) {
    8.             SetPixel(x, y, PCLEAR);
    9.             SetPixel(x-1, y - 1, PSAND);
    10.         }
    11.         else if(candownright ) {
    12.             SetPixel(x, y, PCLEAR);
    13.             SetPixel(x+1, y - 1, PSAND);
    14.         }
    15.     }
    In Update()
    Code (CSharp):
    1. if(Input.GetMouseButton(0)) SetPixel((int)Input.mousePosition.x, (int)Input.mousePosition.y, PSAND);
    this is brush 1x1, another is the same but with additional SetPixel calls with another coordinates
     
  2. DouglasPotesta

    DouglasPotesta

    Joined:
    Nov 6, 2014
    Posts:
    109
    It’s difficult to know what the issue is.
    Is there more code that you can show?
     
    angrypenguin likes this.
  3. angrypenguin

    angrypenguin

    Joined:
    Dec 29, 2011
    Posts:
    15,620
    It's unlikely to be the issue, but if you've got an Update method anyway, why perform this bit of code in an enumerator (presumably a coroutine?) instead of just putting it in the Update method? If you're trying to debug / optimise something, start with it in as straightforward a state as it can be.

    Modifying textures via SetPixel(...) is "slow", though I wouldn't expect it to have the level of impact you're implying here based only on the code we're being shown.

    In this case, for optimal performance I'd probably try doing it with a RenderTexture (or two?) and a shader, rather than on the CPU at all.

    Edit: Is there any other code in your Update method?

    Edit 2: And just how "slow" are we talking about, here? You mention 60fps once things stabilise, but what's the "slow" speed it goes down to?
     
  4. angrypenguin

    angrypenguin

    Joined:
    Dec 29, 2011
    Posts:
    15,620
    Oh, I see... what's in your SetPixel(...) method?

    I've just noticed that you're not calling it on a texture object, so that's your own SetPixel method and, suspiciously, I don't see any calls to Texture2D.Apply() in any of the visible code. Are you, by any chance, applying the texture update once for every single pixel you're modifying?
     
  5. RASKALOF

    RASKALOF

    Joined:
    Jul 9, 2012
    Posts:
    56
    Oh, i found out, thanks for idea, i just move WORLD.sprite.texture.Apply(); to end of all loops instead of updating texture in each SetPixel. Thanks!

    Here is all code with fix:
    Code (CSharp):
    1. using System.Collections;
    2. using UnityEngine;
    3. using UnityEngine.UI;
    4.  
    5. public enum ID_TYPE : byte { NONE, SAND, WATER, FIRE, STATIC }
    6.  
    7. public struct Pixel {
    8.     public ID_TYPE id;
    9.     public Color   color;
    10.  
    11.     public Pixel(ID_TYPE _id, Color _color) {
    12.         id = _id;
    13.         color = _color;
    14.     }
    15. }
    16.  
    17. public class LevelManager : MonoBehaviour {
    18.     Pixel PCLEAR = new Pixel(ID_TYPE.NONE, Color.clear);
    19.     Pixel PSAND =  new Pixel(ID_TYPE.SAND, Color.yellow);
    20.     public Image WORLD = null;
    21.     bool end_game = false;
    22.     const int height = 600;
    23.     const int width =  800;
    24.     const int gravity = 1;
    25.     Pixel[,] worldp = new Pixel[width, height];
    26.  
    27.     private void Awake() {
    28.         Screen.SetResolution(width, height, FullScreenMode.ExclusiveFullScreen, 60);
    29.         ClearCanvas();
    30.     }
    31.     private void Start() {
    32.         StartCoroutine(UpdateLoop());   // infi loop
    33.     }
    34.  
    35.     private void FixedUpdate() {
    36.         if(Input.GetMouseButton(0)) SetPixel((int)Input.mousePosition.x, (int)Input.mousePosition.y, PSAND);
    37.         if(Input.GetMouseButton(0)) SetPixel((int)Input.mousePosition.x + 1, (int)Input.mousePosition.y - 1, PSAND);
    38.         if(Input.GetMouseButton(0)) SetPixel((int)Input.mousePosition.x - 1, (int)Input.mousePosition.y - 1, PSAND);
    39.         if(Input.GetMouseButton(0)) SetPixel((int)Input.mousePosition.x + 1, (int)Input.mousePosition.y + 1, PSAND);
    40.         if(Input.GetMouseButton(0)) SetPixel((int)Input.mousePosition.x - 1, (int)Input.mousePosition.y + 1, PSAND);
    41.         if(Input.GetMouseButton(0)) SetPixel((int)Input.mousePosition.x + 1, (int)Input.mousePosition.y, PSAND);
    42.         if(Input.GetMouseButton(0)) SetPixel((int)Input.mousePosition.x - 1, (int)Input.mousePosition.y, PSAND);
    43.     }
    44.  
    45.     IEnumerator UpdateLoop() {
    46.         Pixel cur_pix = PCLEAR;
    47.         while(true) {
    48.             for(var y = height - 1; y > 0; --y) {
    49.                 for(int x = 0; x < width; ++x) {
    50.                     cur_pix = GetP(x, y);
    51.                     switch(cur_pix.id) {
    52.                         case ID_TYPE.SAND: {
    53.                             UpdateSand(x, y);
    54.                             break;
    55.                         }
    56.                         case ID_TYPE.NONE:
    57.                         default: {
    58.                             break;
    59.                         }
    60.                     }
    61.                 }
    62.             }
    63.             WORLD.sprite.texture.Apply();
    64.             yield return null;
    65.         }
    66.     }
    67.  
    68.     void UpdateSand(int x, int y) {
    69.         x = Mathf.Clamp(x, 1, width-1);
    70.         y = Mathf.Clamp(y, 1, height);
    71.         if(worldp[x, y - 1].id == ID_TYPE.NONE ) {
    72.             SetPixel(x, y, PCLEAR);
    73.             SetPixel(x, y - 1, PSAND);
    74.         }else if(worldp[x - 1, y - 1].id == ID_TYPE.NONE ) {
    75.             SetPixel(x, y, PCLEAR);
    76.             SetPixel(x-1, y - 1, PSAND);
    77.         }
    78.         else if(worldp[x + 1, y - 1].id == ID_TYPE.NONE ) {
    79.             SetPixel(x, y, PCLEAR);
    80.             SetPixel(x+1, y - 1, PSAND);
    81.         }
    82.     }
    83.  
    84.     Pixel GetP(int x, int y) {
    85.         return worldp[x, y];
    86.     }
    87.  
    88.     void SetPixel(int x, int y, Pixel type) {
    89.         worldp[x, y] = type;
    90.         WORLD.sprite.texture.SetPixel(x, y, worldp[x, y].color);
    91.      
    92.     }
    93.  
    94.     void ClearCanvas() {
    95.         for(var x = 0; x < width; ++x) {
    96.             for(var y = 0; y < height; ++y) {
    97.                 worldp[x, y] = new Pixel();
    98.                 WORLD.sprite.texture.SetPixel(x, y, worldp[x, y].color);
    99.             }
    100.         }
    101.         WORLD.sprite.texture.Apply();
    102.     }
    103.  
    104.     private void OnApplicationQuit() {
    105.         ClearCanvas();
    106.     }
    107. }
    108.  
    109.  
     
    Last edited: Apr 18, 2023
  6. angrypenguin

    angrypenguin

    Joined:
    Dec 29, 2011
    Posts:
    15,620
    I use for loops in Update all the time. The issue isn't the "for", it's the "while (true)".

    In a coroutine, wrapping a "while (true)" around a "yield return null;" is basically a hacky way to replicate what Update() does, i.e. calling the code once per frame. You wouldn't need the "while (true)" part at all if you just put the rest in Update, which is already called once per frame.

    Edit: Why'd you delete your other post?
     
  7. RASKALOF

    RASKALOF

    Joined:
    Jul 9, 2012
    Posts:
    56
    i added fixed version, now will try to move all things into fixedupdated
     
  8. angrypenguin

    angrypenguin

    Joined:
    Dec 29, 2011
    Posts:
    15,620
    Yes, do all of the work on a texture, and then only send it to the GPU when you're finished.

    I'm not sure if you need your own SetPixel(...) method at all? I can't see what else you might have been doing in there since you deleted that post, though, or look for anything else which might be eating cycles. Removing method calls is only going to noticably impact performance if there's lots of them, though.

    With that in mind, if you want it faster then I'd look at a shader-based approach. Where your CPU has to iterate over pixels one-by-one, your GPU iterates over dozens simultaneously, and I believe that they're well suited to the problem you described.
     
  9. RASKALOF

    RASKALOF

    Joined:
    Jul 9, 2012
    Posts:
    56
    i need SetPixel to set pixel type and image pixel color, cos color will be a random (array) depends on Pixel type, so i cant just depends on color of pixel to define its type.
    EDIT: and tthanks gameloop works in FixedUpdate, have no clue why i thougth it will freeze =) Big thanks to you for all!
     
  10. angrypenguin

    angrypenguin

    Joined:
    Dec 29, 2011
    Posts:
    15,620
    FixedUpdate is for physics simulation, for Rigidbody components and collisions. It's decoupled from the rendering and input loop by default, which can make other things behave weirdly. You just want the normal Update method, and should probably get familiar with this.

    You're welcome! :)