Stuff after the for loops performing before it's done

  • (3 Pages)
  • +
  • 1
  • 2
  • 3

42 Replies - 2000 Views - Last Post: 27 February 2019 - 03:03 PM Rate Topic: -----

#16 Salem_c   User is online

  • void main'ers are DOOMED
  • member icon

Reputation: 2310
  • View blog
  • Posts: 4,418
  • Joined: 30-May 10

Re: Stuff after the for loops performing before it's done

Posted 23 January 2019 - 11:17 PM

The loops are finished as expected for sure.

The OP believes that the loops finish early in the mistaken belief that the static 'setter' functions in curScreenTile actually modify any external state. They do not.
$ cat foo.cpp
#include <iostream>
struct curScreenTile {
        unsigned long rgb;
        static void setColor(int b, int g, int r, curScreenTile tile) {
            tile.rgb = ((r & 0xff) << 16) + ((g & 0xff) << 8) + (b & 0xff);
        }
};
int main ( ) {
    curScreenTile d;
    curScreenTile::setColor(1,1,1,d);
    if ( d.rgb != 0 ) {
        std::cout << "Success\n";
    }
}
$ g++ -S -O2 -Wall -Wextra -std=c++11 -c foo.cpp
foo.cpp: In static member function ‘static void curScreenTile::setColor(int, int, int, curScreenTile)’:
foo.cpp:4:65: warning: parameter ‘tile’ set but not used [-Wunused-but-set-parameter]
         static void setColor(int b, int g, int r, curScreenTile tile) {
                                                                 ^
foo.cpp: In function ‘int main()’:
foo.cpp:11:12: warning: ‘d.curScreenTile::rgb’ is used uninitialized in this function [-Wuninitialized]
     if ( d.rgb != 0 ) {
            ^



So the loop finishes doing nothing, and the OP thinks the for loop is broken.
Was This Post Helpful? 1
  • +
  • -

#17 tolazytbh   User is offline

  • D.I.C Head

Reputation: 0
  • View blog
  • Posts: 67
  • Joined: 24-December 18

Re: Stuff after the for loops performing before it's done

Posted 24 January 2019 - 05:11 AM

You were right, it's because of the static declaration...
I did that to see if it would speed up,
I'm surprised you can't allocate a color for each pixel on a screen and not have it take 20-30 seconds
Do any of you know how to speed it up, or should I just go with plan b


Are operations really that much slower than loading/reading a file?

This post has been edited by andrewsw: 24 January 2019 - 05:29 AM
Reason for edit:: removed previous quote, use the Reply button

Was This Post Helpful? 0
  • +
  • -

#18 andrewsw   User is online

  • Stealth IT
  • member icon

Reputation: 6737
  • View blog
  • Posts: 27,746
  • Joined: 12-December 12

Re: Stuff after the for loops performing before it's done

Posted 24 January 2019 - 05:30 AM

There is no need to quote the immediately preceding post in full, there is a Reply button further down the page.
Was This Post Helpful? 0
  • +
  • -

#19 tolazytbh   User is offline

  • D.I.C Head

Reputation: 0
  • View blog
  • Posts: 67
  • Joined: 24-December 18

Re: Stuff after the for loops performing before it's done

Posted 24 January 2019 - 05:30 AM

sorry, forgot
Was This Post Helpful? 0
  • +
  • -

#20 Skydiver   User is offline

  • Code herder
  • member icon

Reputation: 6767
  • View blog
  • Posts: 23,071
  • Joined: 05-May 12

Re: Stuff after the for loops performing before it's done

Posted 24 January 2019 - 07:40 AM

View Posttolazytbh, on 24 January 2019 - 07:11 AM, said:

You were right, it's because of the static declaration...

Not quite. The static declaration in combination with passing the parameter by value instead of by reference is what is causing the problem.

View Posttolazytbh, on 24 January 2019 - 07:11 AM, said:

I'm surprised you can't allocate a color for each pixel on a screen and not have it take 20-30 seconds

I suspect that a large part of the slowness is due to the pass by value. When something is passed by value, the constructor for the class is called again. Since you have over a million calls, those constructor calls add up.

Additionally, using vector::at() is many times slower than using vector::operator []. This is because the former does range checking, and the latter does not.

Anyway, the first step to fix things is to just use a normal setter, instead of your current static setter. See if it is fast enough.
Was This Post Helpful? 0
  • +
  • -

#21 tolazytbh   User is offline

  • D.I.C Head

Reputation: 0
  • View blog
  • Posts: 67
  • Joined: 24-December 18

Re: Stuff after the for loops performing before it's done

Posted 24 January 2019 - 01:58 PM

I had already removed the static, just didn't think I needed to repost the code
It takes around 20 seconds-1 minute to perform the entire thing
I'll try the operator
Was This Post Helpful? 0
  • +
  • -

#22 Skydiver   User is offline

  • Code herder
  • member icon

Reputation: 6767
  • View blog
  • Posts: 23,071
  • Joined: 05-May 12

Re: Stuff after the for loops performing before it's done

Posted 24 January 2019 - 02:16 PM

Just removing static isn't going to fix things because you are still passing by value instead acting on the current object.
Was This Post Helpful? 0
  • +
  • -

#23 tolazytbh   User is offline

  • D.I.C Head

Reputation: 0
  • View blog
  • Posts: 67
  • Joined: 24-December 18

Re: Stuff after the for loops performing before it's done

Posted 24 January 2019 - 02:21 PM

Could you elaborate more?

Also the std::cout was why it was taking so long, after I removed that it took 8 ms for the for loops
loading the entire thing on screen took 11 seconds though

        case WM_PAINT:
            {
            PAINTSTRUCT ps;
            HDC hdc = BeginPaint(hwnd, &ps);
            SetGraphicsMode(hdc, GM_ADVANCED);
            SetMapMode(hdc, MM_ANISOTROPIC);
            //note HBRUSH CreateSolidBrush(COLORREF color);
            //FillRect(hdc, &ps.rcPaint, (HBRUSH) CreateSolidBrush(createRGB(0,0,109)));
                /*RECT r;
                r.left = 100;
                r.top = 100;
                r.right = 110;
                r.bottom = 110;*/
                    mTimer.reset();
                    if(engineC.getUpdate()) {
                        std::cout << "Occurred" << std::endl;
                        //int arraySize = engineC.getSize();
                        //std::cout << "Size " + arraySize << std::endl;
                        int i = 0;
                        HBRUSH hBrush = CreateSolidBrush(RGB(0,0,240));
                        while(true) {
                            bool var2 = engineC.getIsActive(i);
                            if(!var2) {
                                break;
                            }
                            RECT r;
                            r.left = engineC.getTile(i).x;
                            r.top = engineC.getTile(i).y;
                            r.right = engineC.getTile(i).x+1;
                            r.bottom = engineC.getTile(i).y+1;
                            FillRect(hdc, &r, hBrush);

                            //FloodFill(hdc, engineC.getTile(i).x,engineC.getTile(i).y, RGB(0,0,240));
                            i++;
                        }
                        std::cout << std::to_string(i) << std::endl;
                        engineC.done();
                    }
                    double d = mTimer.elapsed();
                    std::cout << "Time took: " + std::to_string(d) << std::endl;
                EndPaint(hwnd, &ps);
            }
            return 0;


struct curScreenTile {
        short x, y;
        COLORREF rgb;
        bool activeTile = false;

        void setActive(bool toSet) {
            activeTile = toSet;
        }

        bool active() {
            return activeTile;
        }
};




Do any of you know why this takes 7 seconds (7000 ms) to load the entire screen by one pixel, any suggestions on improvements? (starting to think it's more around 3 seconds and that clock is just off)


Also this is only 2-5 ms at most
        for(int i = 0; i < 1440; i++) {
            for(int b = 0; b < 900; b++) {
                curScreen[g].x = i;
                curScreen[g].y= b;
                curScreen[g].setActive(true);
                g++;
            }
        }


This post has been edited by tolazytbh: 24 January 2019 - 04:16 PM

Was This Post Helpful? 0
  • +
  • -

#24 Skydiver   User is offline

  • Code herder
  • member icon

Reputation: 6767
  • View blog
  • Posts: 23,071
  • Joined: 05-May 12

Re: Stuff after the for loops performing before it's done

Posted 24 January 2019 - 09:42 PM

View Posttolazytbh, on 24 January 2019 - 04:21 PM, said:

Could you elaborate more?

Since in post #21 you said:

View Posttolazytbh, on 24 January 2019 - 03:58 PM, said:

I had already removed the static, just didn't think I needed to repost the code

That would mean that you only changed your code from post #1:
:

struct curScreenTile {
        RECT r;
        unsigned long rgb;
        bool activeTile = false;

        static void setColor(int b, int g, int r, curScreenTile tile) {
            tile.rgb = ((r & 0xff) << 16) + ((g & 0xff) << 8) + (b & 0xff);
        }

        static void setPos(short x, short y, short pixelSize, curScreenTile tile) {
                tile.r.left = x;
                tile.r.top = y;
                tile.r.right = y+pixelSize;
                tile.r.bottom = x+pixelSize;
        }

        static void setActive(bool toSet, curScreenTile tile) {
            tile.activeTile = toSet;
        }

        static RECT getR(curScreenTile tile) {
            return tile.r;
        }

        static unsigned long getColor(curScreenTile tile) {
            return tile.rgb;
        }

        static bool active(curScreenTile tile) {
            return tile.activeTile;
        }
};

:


to
:

struct curScreenTile {
        RECT r;
        unsigned long rgb;
        bool activeTile = false;

        void setColor(int b, int g, int r, curScreenTile tile) {
            tile.rgb = ((r & 0xff) << 16) + ((g & 0xff) << 8) + (b & 0xff);
        }

        void setPos(short x, short y, short pixelSize, curScreenTile tile) {
                tile.r.left = x;
                tile.r.top = y;
                tile.r.right = y+pixelSize;
                tile.r.bottom = x+pixelSize;
        }

        void setActive(bool toSet, curScreenTile tile) {
            tile.activeTile = toSet;
        }

        RECT getR(curScreenTile tile) {
            return tile.r;
        }

        unsigned long getColor(curScreenTile tile) {
            return tile.rgb;
        }

        bool active(curScreenTile tile) {
            return tile.activeTile;
        }
};

:



So you are still passing in curScreenTile by value. Passing by value incurrs the cost of calling the constructor and destructor for the curScreenTile each time you make any of the calls.
Was This Post Helpful? 0
  • +
  • -

#25 Skydiver   User is offline

  • Code herder
  • member icon

Reputation: 6767
  • View blog
  • Posts: 23,071
  • Joined: 05-May 12

Re: Stuff after the for loops performing before it's done

Posted 24 January 2019 - 09:56 PM

View Posttolazytbh, on 24 January 2019 - 04:21 PM, said:

Do any of you know why this takes 7 seconds (7000 ms) to load the entire screen by one pixel, any suggestions on improvements?

Please post your current code. It's very hard to tell what you are doing in your calls to various things like engineC.getIsActive(i) or engineC.getTile(i) or engineC.getUpdate() and engineC.done().
Also, that is a very long time to render in WM_PAINT regardless if it's 7 seconds or 3 seconds.

As an aside, what happens when you run the profiler? Where does it say most of the time is being spent?
Was This Post Helpful? 0
  • +
  • -

#26 tolazytbh   User is offline

  • D.I.C Head

Reputation: 0
  • View blog
  • Posts: 67
  • Joined: 24-December 18

Re: Stuff after the for loops performing before it's done

Posted 25 January 2019 - 06:34 AM

GetUpdate and .done() are just Booleans to prevent a deadlock
I posted the code above, each timers are placed before and after the loops

I did 1440x900 for the resolution
1x1 takes 7 seconds (probably more like 3-4 seconds) for the whole screen, 4x4 is small (a second to load the screen), and 16x16 is instance
(aspect ratio is 16:10)
do you have any knowledge of how to speed it up besides me removing that one Boolean to check if the pixel is active/valid? (I did the worst case scenario first)

EDIT:
16x16 - 30 milliseconds
10x10 - 70 milliseconds
4x4 - 400 milliseconds

1x1 - 7400 milliseconds

removing the getIsActive(I); does not speed things up at all/make a difference

This post has been edited by tolazytbh: 25 January 2019 - 07:29 AM

Was This Post Helpful? 0
  • +
  • -

#27 Skydiver   User is offline

  • Code herder
  • member icon

Reputation: 6767
  • View blog
  • Posts: 23,071
  • Joined: 05-May 12

Re: Stuff after the for loops performing before it's done

Posted 25 January 2019 - 08:45 AM

Can you show us the code for getTile()?
Was This Post Helpful? 0
  • +
  • -

#28 tolazytbh   User is offline

  • D.I.C Head

Reputation: 0
  • View blog
  • Posts: 67
  • Joined: 24-December 18

Re: Stuff after the for loops performing before it's done

Posted 25 January 2019 - 10:36 AM

curScreenTile Engine::getTile(int position) {
    return curScreen[position];
}


Just grabs that position in the array...
I had put the WM_Paint on it's own thread and it doesn't speed things up at all (the drawing on it's own thread)

Kind of sucks because i don't want to get into shit like direct2d or opengl and crap because of how much crap is in their libraries that'd be unused...

Are you aware of anything with win32 that'd speed things up with drawing?
Was This Post Helpful? 0
  • +
  • -

#29 Skydiver   User is offline

  • Code herder
  • member icon

Reputation: 6767
  • View blog
  • Posts: 23,071
  • Joined: 05-May 12

Re: Stuff after the for loops performing before it's done

Posted 25 January 2019 - 03:14 PM

View Posttolazytbh, on 25 January 2019 - 12:36 PM, said:

Just grabs that position in the array...

Unfortunately it doesn't. It also creates a brand new copy curScreenTile because you are returning a new object instead or returning a reference to the existing copy in the array.

So in the span of these 4 lines:
r.left = engineC.getTile(i).x;
r.top = engineC.getTile(i).y;
r.right = engineC.getTile(i).x+1;
r.bottom = engineC.getTile(i).y+1;


You managed to create and destroy 4 copies of the same curScreenTile.

Also why are you using FillRect() to just draw a single pixel? Why not simply use SetPixel()?
Was This Post Helpful? 0
  • +
  • -

#30 tolazytbh   User is offline

  • D.I.C Head

Reputation: 0
  • View blog
  • Posts: 67
  • Joined: 24-December 18

Re: Stuff after the for loops performing before it's done

Posted 25 January 2019 - 04:34 PM

I think it's some kind of defense with the computer/computers, if you do a line large enough it's instant...

I don't believe the return array[] is a problem, rather not gut programming entirely for a few milliseconds...

This post has been edited by tolazytbh: 25 January 2019 - 04:36 PM

Was This Post Helpful? 0
  • +
  • -

  • (3 Pages)
  • +
  • 1
  • 2
  • 3