3
\$\begingroup\$

The code works fine, but the speed stays at a consistent eight frames per second. Today I fixed some pieces and got four more frames per second. I figured out that the typecasting (int) and Math.floor() consume around 50 fps.

Is there anything I can do to improve the performance of this code?

public void render(){
    for(int y = 0; y < height; y++){
        for(int x = 0; x < width; x++){
            /* 1. game screen
             * 2. loading screen
             * 3. login screen
             */
            screenPix = x + y * width;
            if(Core.loadingDone && Core.logedin){
                offsetX = Player.playerX;
                offsetY = Player.playerY;
                chunkX = (int) Math.floor((x + offsetX) / 512);
                chunkY = (int) Math.floor((y + offsetY) / 512);
                tileX = (int) Math.floor(((x + offsetX) - (chunkX * 512)) / 32);
                tileY = (int) Math.floor(((y + offsetY) - (chunkY * 512)) / 32);
                pixelX = (int) ((x + offsetX) - Math.floor(chunkX * 512) - Math.floor(tileX * 32));
                pixelY = (int) ((y + offsetY) - Math.floor(chunkY * 512) - Math.floor(tileY * 32));
                currTile = ChunkManager.chunks.get(chunkX).get(chunkY).tile[tileX][tileY];
                tilePix = pixelX + pixelY * 32;
                try{
                    Game.pixels[screenPix] = SpriteManager.groundSpritesGet(currTile.getGroundID()).pixels[tilePix];
                    if(currTile.moveAbility[pixelX][pixelY] == 1){
                        //render order: player object event
                        if(x >= Game.WIDTH / 2 - 16 && x <= Game.WIDTH / 2 + 16 && y >= Game.HEIGHT / 2 - 16 && y <= Game.HEIGHT / 2 + 16){//wrong
                            //TODO render player
                            //TODO render equip
                        }
                        if(SpriteManager.objectSpritesGet(currTile.getObjectID()).pixels[tilePix] != 0xffff00ff) Game.pixels[screenPix] = SpriteManager.objectSpritesGet(currTile.getObjectID()).pixels[tilePix];
                        if(SpriteManager.objectSpritesGet(currTile.getObjectID2()).pixels[tilePix] != 0xffff00ff) Game.pixels[screenPix] = SpriteManager.objectSpritesGet(currTile.getObjectID2()).pixels[tilePix];
                        if(SpriteManager.objectSpritesGet(currTile.getObjectID3()).pixels[tilePix] != 0xffff00ff) Game.pixels[screenPix] = SpriteManager.objectSpritesGet(currTile.getObjectID3()).pixels[tilePix];
                        if(Core.builderMode){
                            if(Core.builderModePathMode && SpriteManager.eventSpritesGet(currTile.getEventWalkPath()).pixels[tilePix] != 0xffff00ff) Game.pixels[screenPix] = SpriteManager.eventSpritesGet(currTile.getEventWalkPath()).pixels[tilePix];
                            if(Core.builderModeEventMode && SpriteManager.eventSpritesGet(currTile.getEventID()).pixels[tilePix] != 0xffff00ff) Game.pixels[screenPix] = SpriteManager.eventSpritesGet(currTile.getEventID()).pixels[tilePix];
                        }else{
                            if(SpriteManager.eventSpritesGet(currTile.getEventAlias()).pixels[tilePix] != 0xffff00ff) Game.pixels[screenPix] = SpriteManager.eventSpritesGet(currTile.getEventAlias()).pixels[tilePix];
                        }
                        continue;
                    }else{
                        //render order: object event player
                        if(SpriteManager.objectSpritesGet(currTile.getObjectID()).pixels[tilePix] != 0xffff00ff) Game.pixels[screenPix] = SpriteManager.objectSpritesGet(currTile.getObjectID()).pixels[tilePix];
                        if(SpriteManager.objectSpritesGet(currTile.getObjectID2()).pixels[tilePix] != 0xffff00ff) Game.pixels[screenPix] = SpriteManager.objectSpritesGet(currTile.getObjectID2()).pixels[tilePix];
                        if(SpriteManager.objectSpritesGet(currTile.getObjectID3()).pixels[tilePix] != 0xffff00ff) Game.pixels[screenPix] = SpriteManager.objectSpritesGet(currTile.getObjectID3()).pixels[tilePix];
                        if(Core.builderMode){
                            if(Core.builderModePathMode && SpriteManager.eventSpritesGet(currTile.getEventWalkPath()).pixels[tilePix] != 0xffff00ff) Game.pixels[screenPix] = SpriteManager.eventSpritesGet(currTile.getEventWalkPath()).pixels[tilePix];
                            if(Core.builderModeEventMode && SpriteManager.eventSpritesGet(currTile.getEventID()).pixels[tilePix] != 0xffff00ff) Game.pixels[screenPix] = SpriteManager.eventSpritesGet(currTile.getEventID()).pixels[tilePix];
                        }else{
                            if(SpriteManager.eventSpritesGet(currTile.getEventAlias()).pixels[tilePix] != 0xffff00ff) Game.pixels[screenPix] = SpriteManager.eventSpritesGet(currTile.getEventAlias()).pixels[tilePix];
                        }
                        if(x >= Game.WIDTH / 2 - 16 && x <= Game.WIDTH / 2 + 16 && y >= Game.HEIGHT / 2 - 16 && y <= Game.HEIGHT / 2 + 16){//wrong
                            //TODO render player
                            //TODO render equip
                        }
                        continue;
                    }
                }catch(NullPointerException e){
                    FileManager.missingChunk = chunkX+"v"+chunkY+"v?";
                }
            }else if(Core.logedin){
                if(Core.loading_screen){
                    Game.pixels[screenPix] = SpriteManager.guiSprites.get(2).pixels[screenPix];
                    continue;
                }
            }else{
                if(hover){
                    Game.pixels[screenPix] = SpriteManager.guiSprites.get(1).pixels[screenPix];
                }else{
                    Game.pixels[screenPix] = SpriteManager.guiSprites.get(0).pixels[screenPix];
                }
                if(y > 574 && y < 600){
                    if(x > 290 && x < 450 || x > 540 && x < 700){
                        Game.pixels[screenPix] = 0xffffffff;
                    }
                }
                continue;
            }
        }
    }
}

Some more context

TreeMap<Integer, Sprite> guiSprites = new TreeMap<Integer, Sprite>();

Game is the Main Class the produce a JFrame and some other stuff until it starts to call render() that actually calls the above code.

private void render() {
    bs = getBufferStrategy();
    if(bs == null){
        createBufferStrategy(2);
        return;
    }
    screen.render();
    g = bs.getDrawGraphics();
    g.setColor(Color.BLACK);
    g.fillRect(0, 0, getWidth(), getHeight());
    g.drawImage(image, 0, 0, getWidth(), getHeight(), null);
    g.dispose();
    bs.show();
}

And Core is a Class that handles information given by the server, but for this part now it's just a few booleans for later in the game.

\$\endgroup\$
  • 4
    \$\begingroup\$ The whole approach is questionable, rendering pixel by pixel. Take a look at the java2d API. It contains methods to draw standard graphics primitives (line, rectangle, text, images) quickly with pretty simple calls. \$\endgroup\$ – Durandal Sep 8 '15 at 18:15
  • \$\begingroup\$ To make life easier for reviewers, please add sufficient context to your question. The more you tell us about what your code does and what the purpose of doing that is, the easier it will be for reviewers to help you. See also this meta question. Please at least provide some context about what the game is about. \$\endgroup\$ – Simon Forsberg Sep 8 '15 at 18:44
  • 1
    \$\begingroup\$ What is Core? What is Game? What is SpriteManager.guiSprites.get(1)? \$\endgroup\$ – Simon Forsberg Sep 8 '15 at 19:03
  • 1
    \$\begingroup\$ @erinas The rendering of the game pixel-by-pixel is the problem, if I knew more about what library and classes you are using for the sprites I could tell you more. What are the imports at the top of the class? That detail would tell me a bit more what I need to know. \$\endgroup\$ – Simon Forsberg Sep 8 '15 at 19:23
  • 1
    \$\begingroup\$ There are a bunch of variables used like chunkX and offsetX - how, and where are they defined? Are they int values? \$\endgroup\$ – rolfl Sep 8 '15 at 19:51
4
\$\begingroup\$

This is only a quick review, since I don't know much in the area of rendering graphics

Repeated Work

screenPix = x + y * width;

chunkY = (int) Math.floor((y + offsetY) / 512);

tileY = (int) Math.floor(((y + offsetY) - (chunkY * 512)) / 32);

There are many lines that are inside of a loop that do not need to be. Every time you move a pixel in the x direction, the inner loop, these results do not change, so you can save them in a variable outside of the x loop. This alone will save you a bunch of repeated calculations.


Modular Code

//render order: player object event
if(x >= Game.WIDTH / 2 - 16 && x <= Game.WIDTH / 2 + 16 && y >= Game.HEIGHT / 2 - 16 && y <= Game.HEIGHT / 2 + 16){//wrong
    //TODO render player
    //TODO render equip
}
if(SpriteManager.objectSpritesGet(currTile.getObjectID()).pixels[tilePix] != 0xffff00ff) Game.pixels[screenPix] = SpriteManager.objectSpritesGet(currTile.getObjectID()).pixels[tilePix];
if(SpriteManager.objectSpritesGet(currTile.getObjectID2()).pixels[tilePix] != 0xffff00ff) Game.pixels[screenPix] = SpriteManager.objectSpritesGet(currTile.getObjectID2()).pixels[tilePix];
if(SpriteManager.objectSpritesGet(currTile.getObjectID3()).pixels[tilePix] != 0xffff00ff) Game.pixels[screenPix] = SpriteManager.objectSpritesGet(currTile.getObjectID3()).pixels[tilePix];
if(Core.builderMode){
    if(Core.builderModePathMode && SpriteManager.eventSpritesGet(currTile.getEventWalkPath()).pixels[tilePix] != 0xffff00ff) Game.pixels[screenPix] = SpriteManager.eventSpritesGet(currTile.getEventWalkPath()).pixels[tilePix];
    if(Core.builderModeEventMode && SpriteManager.eventSpritesGet(currTile.getEventID()).pixels[tilePix] != 0xffff00ff) Game.pixels[screenPix] = SpriteManager.eventSpritesGet(currTile.getEventID()).pixels[tilePix];
}else{
    if(SpriteManager.eventSpritesGet(currTile.getEventAlias()).pixels[tilePix] != 0xffff00ff) Game.pixels[screenPix] = SpriteManager.eventSpritesGet(currTile.getEventAlias()).pixels[tilePix];
}
continue;

The fact you need a comment to explain that the difference between this and later code is the order, shows that this is perfect to abstract to a method.

renderPlayer(x, y);
renderObject(tilePix, etc..);
renderEvent(Core.builderMode, tilePix, etc..);

This will make it much easier to read, and you don't have to change code in multiple places, such as if a value changes.


chunkX = (int) Math.floor((x + offsetX) / 512);

if x is an int, and offsetX is an int, we have

   int = (int) Math.floor((int + int) / int);
=> int = (int) Math.floor(int / int);
=> int = (int) Math.floor(int);

Basically, you don't need to round down anything if everything inside is going to be an integer. This also means you don't have to cast the double you got from Math.floor back to an int, saving even more time.

chunkX = (x + offsetX) / 512;

512, 32, 2, 16

So many pretty powers of 2, I'm guessing it is deliberate. It is a good idea, it speeds up multiplication and division because x << 3 is the same as 8x, shifting bits is the same as multiplying by a power of two, and shifting bits is really fast.

As far as I am aware, Java doesn't optimise this, so try switching division by 512 for right shifting by 9, and testing the performance. It might not make any difference because Java already did this, but it may be a micro-optimisation worth doing. But remember, the algorithm you use > the micro-optimisations you implement. Low level changes like these make code harder to read, harder to change later, and more prone to bugs, if edge cases aren't accounted for. Do them last if you need the small boost to go from 58 fps to 60


Profile the code, and with each change test it again, the only way to really improve performance is to know where the bottleneck is. Sometimes it is in the last place you expect.

\$\endgroup\$
  • 1
    \$\begingroup\$ As far as I am aware, Java does optimise power-of-two numbers. \$\endgroup\$ – Simon Forsberg Sep 8 '15 at 19:21
  • \$\begingroup\$ @SimonForsberg Any idea how to test it? \$\endgroup\$ – spyr03 Sep 8 '15 at 19:36
3
\$\begingroup\$

The continuous testing of Core.loadingDone && Core.logedin for each pixel is a horrible waste of time (also, it's loggedin not logedin :). These values are not going to change part-way through a render, so just divide the method in three, and have:

private void renderGame() {
    .... complicated logic for complicated screens
}

private void renderSprite(Sprite sprite) {
    System.arraycopy(sprite.pixels, 0, Game.pixels, 0, sprite.pixels.length);
}

private void renderLoading() {
    renderSprite(SpriteManager.guiSprites.get(2));
}

private void blockOut(int fromx, int tox, int fromy, int toy) {
    for (int y = fromy; y <= toy; y++) {
        for (int x = fromx; y <= tox; x++) {
            Game.pixels[x + y * width] = 0xffffffff;
        }
    }
}

private void renderSplash() {
    renderSprite(SpriteManager.guiSprites.get(hover ? 1 : 0));
    blockOut(291, 449, 575, 599);
    blockout(541, 699, 575, 599);
}

Now, with the above, your main render method becomes:

public void render(){
    if(Core.loadingDone && Core.logedin) {
        renderGame();
    } else if (Core.logedin) {
        if (Core.loading_screen) {
            renderLoading();
        }
    } else {
        renderSplash();
    }
}

That logic is now performed only once for each render, instead of once per pixel!.

About the variables that are not apparently declared in your code. Here I am talking about things like offsetX, offsetY, etc. These variables should not be declared outside the method. They should be declared where they are used. I assume that the offsetX and offsetY variables are in fact int values.... this is important because they are used in this code here:

chunkX = (int) Math.floor((x + offsetX) / 512);
chunkY = (int) Math.floor((y + offsetY) / 512);
tileX = (int) Math.floor(((x + offsetX) - (chunkX * 512)) / 32);
tileY = (int) Math.floor(((y + offsetY) - (chunkY * 512)) / 32);
pixelX = (int) ((x + offsetX) - Math.floor(chunkX * 512) - Math.floor(tileX * 32));
pixelY = (int) ((y + offsetY) - Math.floor(chunkY * 512) - Math.floor(tileY * 32));

If all those values are int values (and I suspect they are), then the use of Math.floor is completely unnecessary. The pixelX and pixelY values are always required to be positive, and this in turn implies that all the int values above will be positive. Again, this is important, because it means that Math.floor(...) and simple integer truncation will both produce the same result (which they do for all positive int values). So, taking one of the above lines:

chunkX = (int) Math.floor((x + offsetX) / 512);

That line is exactly the same as (assuming x and offsetX is always positive):

chunkX = (x + offsetX) >>> 9;

Similar logic can be applied to all the calculations, resulting in:

int ox = x + offsetX;
int oy = y + offsetY;
int chunkX = ox >>> 9;
int chunkY = oy >>> 9;
int tileX = (ox >>> 4) & 0x1f;
int tileY = (oy >>> 4) & 0x1f;
int pixelX = ox & 0x0f;
int pixelY = oy & 0x0f;

That should help a bunch with the performance.

\$\endgroup\$
  • \$\begingroup\$ I still think this is unfortunately nothing compared to the fact that the rendering is done pixel-by-pixel... \$\endgroup\$ – Simon Forsberg Sep 8 '15 at 21:23
  • 1
    \$\begingroup\$ @SimonForsberg - I would agree, but I suspect this will get in as much as a few-times better performance. It's also something the OP should know about Math.floor ;-) \$\endgroup\$ – rolfl Sep 8 '15 at 21:24
  • \$\begingroup\$ @rolfl i also thought about bitshift but problem is x and y can be negativ so how do i manage this? And about all other ideas i edited the above code how that comes near to what you guys ment. \$\endgroup\$ – erinas Sep 9 '15 at 0:27

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.