Java: Speed up my code

Question

I have been coding an application which renders tiles and GUI and all that. I seem to have come across a problem where my paintComponent seems to hog too much CPU and can no longer run much higher than 10 FPS on my small computer. I was wondering if there was any more efficient way of running this code or threading it or anything to enhance the calculation speed. Here is my code:

import java.awt.Color;
import java.awt.Font;
import java.awt.Graphics;
import javax.swing.JPanel;

@SuppressWarnings("serial")
public class M1 extends JPanel implements Runnable {
public static double zoom = 1.25;
public static double charZoom = 1;
public static boolean breaking = false;

public void run() {

}

public void paintComponent(Graphics g) {
    super.paintComponent(g);
    if(zoom <= 0.03) zoom = 1.25;

    for(int cy = 0; cy < 3; cy++) {
        for(int cx = 0; cx < 3; cx++) {
            for(int y = 0; y < 16; y++) {
                for(int x = 0; x < 16; x++) {
                    g.drawImage(Tiles.tileImages.get(C0.chunk[x][y][cx][cy]), 
                            (int)((C0.cX[cx][cy] * C0.chunkWidth) * zoom) + ((int)(32 * zoom) * x) + 
                                ((M0.gameFrame.getWidth() / 2)) - (int)(PEntity.x.getValue() * zoom),
                            (int)((C0.cY[cx][cy] * C0.chunkHeight) * zoom) + ((int)(32 * zoom) * y) + 
                                ((M0.gameFrame.getHeight() / 2)) - (int)(PEntity.y.getValue() * zoom) + (int)(24.25 * zoom),// <-- 24.25 used to correctly position  charatcter
                            (int)(32 * zoom), (int)(32 * zoom), this);
                    if(C0.chunk[x][y][cx][cy].equals("a05")) {
                        g.drawImage(Tiles.treetop, 
                                (int)((C0.cX[cx][cy] * C0.chunkWidth) * zoom) + ((int)(32 * zoom) * x) + 
                                    ((M0.gameFrame.getWidth() / 2)) - (int)(PEntity.x.getValue() * zoom),
                                (int)((C0.cY[cx][cy] * C0.chunkHeight) * zoom) + ((int)(32 * zoom) * y) + 
                                    ((M0.gameFrame.getHeight() / 2)) - (int)(PEntity.y.getValue() * zoom) + (int)(24.25 * zoom)
                                    - (int) (32 * zoom),// <-- 24.25 used to correctly position  charatcter
                                (int)(32 * zoom), (int)(32 * zoom), this);
                    }
                }
            }
        }
    }

    if(breaking) {
        g.drawImage(M3.currentBreak, (int)((C0.cX[M3.cx][M3.cy] * C0.chunkWidth) * zoom) + ((int)(32 * zoom) * M3.x) + 
                ((M0.gameFrame.getWidth() / 2)) - (int)(PEntity.x.getValue() * zoom),
                (int)((C0.cY[M3.cx][M3.cy] * C0.chunkHeight) * zoom) + ((int)(32 * zoom) * M3.y) + 
                ((M0.gameFrame.getHeight() / 2)) - (int)(PEntity.y.getValue() * zoom) + (int)(24.25 * zoom),
                (int)(32 * zoom), (int)(32 * zoom), this);
    }

    M3.placeX = (48 * zoom);
    M3.placeY = (48 * zoom);

    if(M0.HUDenabled) {
        g.drawImage(PEntity.currentChar, 
                (M0.gameFrame.getWidth() / 2) - (int)((16 * charZoom) * zoom), 
                (M0.gameFrame.getHeight() / 2) - (int)((32 * charZoom) * zoom),
                (int)((32 * charZoom) * zoom), (int)((64 * charZoom) * zoom), this);

        g.setColor(Color.BLACK);
        g.setFont(new Font("Dialog", 1, 12));
        g.drawString("Terrem" + " By Tyler D :)", 5, 15);
        g.drawString("X: " + PEntity.x.getValue(), 5, 28);
        g.drawString("Y: " + PEntity.y.getValue(), 5, 41);
        g.drawString("ChunkX: " + C0.currentChunkX.getValue(), 5, 54);
        g.drawString("ChunkY: " + C0.currentChunkY.getValue(), 5, 67);
        g.drawString("BlockX: " + C0.currentBlockX.getValue(), 5, 80);
        g.drawString("BlockY: " + C0.currentBlockY.getValue(), 5, 93);
        g.drawString("Zoom: " + zoom, 5, 106);
        g.drawString(M4.tileArea[0][0] + "_" + M4.tileArea[1][0] + "_" + M4.tileArea[2][0], 5, 126);
        g.drawString(M4.tileArea[0][1] + "_" + M4.tileArea[1][1] + "_" + M4.tileArea[2][1], 5, 139);
        g.drawString(M4.tileArea[0][2] + "_" + M4.tileArea[1][2] + "_" + M4.tileArea[2][2], 5, 152);
        g.drawString("FPS: " + (int) FPS.currentFPS, 5, 172);

        //GUI
        g.drawImage(M0.GUIbar, (M0.gameFrame.getWidth() - (624)) / 2, (M0.gameFrame.getHeight() - 80), 624, 40, this);
        for(int i = 0; i < 9; i++) {
            g.drawImage(Item.Item_Img.get(PEntity.PInv[i]), ((M0.gameFrame.getWidth() - (624)) / 2) + 6 + (36 * i), 
                    (M0.gameFrame.getHeight() - 74), 28, 28, this);
            if(Item.Item_Img.get(PEntity.PInv[i]) != null) {
                g.drawString("" + (PEntity.stk[i] + 1), ((M0.gameFrame.getWidth() - (624)) / 2) + 6 + (36 * i), 
                        (M0.gameFrame.getHeight() - 47));
            }
        }
    }

    repaint();
    FPS.calculateFrameRate();
}

public M1() {
    M0.gameFrame.setVisible(true);
    Clock.Start();
    setBackground(new Color(242, 220, 121));
    System.out.println("M1 loaded...");
}
}

Also I can tell it is the large loop which kills off about 200 FPS at this point because I commented that part off and my FPS shot up to about 250.

How fast does it run when you remove all of that string building? — RussS, Mar 13 '12 at 21:59
Well if anyone is willing to help me then they will help me. — MrDrProfessorTyler, Mar 13 '12 at 22:00
Why not pre-draw as much stuff as possible, especially any background images, onto a BufferedImage and then show the BufferedImage in your `paintComponent(...)` method via a `Graphics#drawImage(...)` method call, and then only drawing images that change in the rest of the method. — Hovercraft Full Of Eels, Mar 13 '12 at 22:04
O wait RussS I see what you mean, I got an extra 50 FPS by not trying to access the HashMap but how would I access which tile to draw otherwise? A bunch of "if" statements? — MrDrProfessorTyler, Mar 13 '12 at 22:05
Hovercraft Full Of Eels: Like load all the tiles in a buffered image and only draw the buffered image? — MrDrProfessorTyler, Mar 13 '12 at 22:05
@MrDr: Yes. And for god's sake, ***never*** call `repaint()` from within a `paint()` or `paintComponentMethod()`! Use a Swing Timer or a game loop that uses a background thread (which is what Swing Timer does) as long as all Swing calls are done on the EDT. — Hovercraft Full Of Eels, Mar 13 '12 at 22:07
Can you explain your tiles a bit. As I understand it, for each frame you draw 2304 tiles, and possibly twice as many if they are all "a05". How many layers are your drawing and how large are the tiles? Also, most of your calculations stay the same within the loop and could be moved out of it. — Roger Lindsjö, Mar 13 '12 at 22:21
Hovercraft Full Of Eels: I tried drawing it all to a buffered image whenever it updated but it destroyed my FPS. Taking the repaint out of the equation did a little bit. Also cutting the HashMap out and just using 'if' statements gave me a bit more as well — MrDrProfessorTyler, Mar 13 '12 at 22:27
Roger: The tiles are originally 16 * 16 in file but are read in when I load them as 32 * 32 and I have different zoom levels so I multiply the size of the blocks and their positions by the zoom level to acheive proper positioning and size — MrDrProfessorTyler, Mar 13 '12 at 22:28
@MrDr: no you don't "draw it all whenever it updated", but rather if possible, you draw it once and then re-use the same drawing if this is feasible with your program logic. — Hovercraft Full Of Eels, Mar 13 '12 at 22:34
Ok ok sorry I worded that wrong. I meant that I updated it whenever it need to be updated — MrDrProfessorTyler, Mar 13 '12 at 22:36

score 2 · Accepted Answer · answered Mar 13 '12 at 22:09

Seems like you could multi-thread some of your outer for-loops at the top of the method, if the Graphics2D instance you are using is thread-safe. Might be worthwhile to keep a ThreadPoolExecutor around for this and then break up your outer for-loops into instances of Runnable. This totally depends on whether or not the order of the draw matters to you - it's hard to tell just from the code you've posted.

One other thing that jumps out at me is how you're accessing your 4-D image array. Recall that multi-dimensional Java arrays are actually arrays of references to other arrays. You'd probably be better off getting a reference to a specific sub-array at the top of each loop, and accessing the sub-array reference that you've saved, rather than indexing the original array directly. This will save you a lot of unnecessary memory fetches.

How could I thread the for-loops and still test everything inside of them? I have never heard of being able to do that before... — MrDrProfessorTyler, Mar 13 '12 at 22:33
You'd have to implement Runnable. In your custom Runnable implementation, you'd have to maintain state that told the Runnable what iteration of the for loop it represented. Pass your instances of Runnable either to new Threads or to existing ones managed by a ThreadPoolExecutor, your choice - you just have to make sure you wait for those threads to finish each Runnable you submitted so that you guarantee the draw actually finished by the time your paint method returned. — CodeBlind, Mar 13 '12 at 22:39

DNA · Answer 2 · 2012-03-13T22:13:44.950

1

As a general rule, try to precalculate as much as possible - e.g. the Strings that you are constructing.

Don't create a new Font on every frame, create it once and reuse it.

As the comments suggest, predraw tiles onto a reusable BufferedImage if that makes sense for your application.

You have quite a few common subexpressions, M0.gameFrame.getHeight() and (32 * zoom), although the HotSpot compiler may well sort these out if you do repeated runs (a single frame test is no good). It will clarify the code if you factor these out, so is a good thing anyway.

Beyond that, you need to do some profiling to see which parts are taking the most time...

edited Mar 13 '12 at 22:13

answered Mar 13 '12 at 22:08

DNA

42,007
12
107
146

Good ideas and I have tried most of them but the BufferedImage for predrawing it all destroys my FPS more than the looping and everything else. – MrDrProfessorTyler Mar 13 '12 at 22:31
It might help a bit I'll try it. Thanks for the help – MrDrProfessorTyler Mar 13 '12 at 22:45

score 1 · Answer 3 · answered Mar 13 '12 at 22:08

Instead of building new strings (string concatenation) on every repaint, you could cache the strings and only rebuild them if data has changed.

Another idea: draw the static part of each string separately from the dynamic part of the string. You could use FontMetrics to determine the width of certain strings, to help you arrange the dynamic part next to the static part.

Also caching in general is good performance strategy which can be applied in many situations. See e.g. Bufferedimage.

How could I cache the strings instead of rebuilding them? – MrDrProfessorTyler Mar 13 '12 at 22:34 — MrDrProfessorTyler, Mar 13 '12 at 22:34

score 1 · Answer 4 · edited Mar 13 '12 at 22:20

It seems you redraw the entire screen from scratch each time paintComponent is called.

What would be better is to draw the initial blank image to an offscreen buffer. Each time a tile is updated mark it as dirty. When paintComponent is drawn only redraw the parts of the offscreen buffer which are out of date. That should save you a lot of effort. Then just draw the entire buffer to the screen in one go. Drawing single large images is generally much quicker than drawing lots of small images.

eg.

public void paintComponent(Graphics g) {

    updateOffscreenBuffer(); 
    // ^-- contains all the nested for loops, but does minimal work

    g.drawImage(getOffscreenBuffer());
    // ^-- draw the entire buffer all in one go to the screen

    drawHUD(g);

}

Java: Speed up my code

4 Answers4

Linked