gotchajavaMinor
Unexpected Low FPS while drawing images
Viewed 0 times
whilelowdrawingunexpectedfpsimages
Problem
I am in the process of making a game, and during this endeavour, I have come across problems maintaining a good frames per second while drawing my sprites. When I draw my background image, my frames drop from ~65 to ~30. My background image is simply a green tile 2000x2000 wide, constructed from 900 50*50 tiles.
What can I do to increase my frames per second?
My Main class
```
import java.awt.*;
import java.awt.event.*;
import java.awt.geom.AffineTransform;
import java.awt.image.BufferedImage;
import java.io.File;
import java.io.IOException;
import java.util.*;
import javax.imageio.ImageIO;
import javax.swing.*;
import javax.swing.Timer;
public class Main extends JPanel implements ActionListener, KeyListener{
static int WIDTH;
static int HEIGHT;
static int SCREENWIDTH;
static int SCREENHEIGHT;
static int GRAVITY = 2;
BufferedImage background;
Tower tower;
Entity debug = new Entity(0, 0, 0, 30, 30, 300);
ArrayList entities = new ArrayList();
Random random = new Random();
public Main(){
addKeyListener(this);
setFocusable(true);
setFocusTraversalKeysEnabled(false);
requestFocus();
Thread thread = new Thread(new Runnable() {
@Override
public void run() {
gameLoop();
}
});
thread.start();
tower = new Tower(WIDTH, HEIGHT, 10, 10, 7);
background = generateBackground();
entities.add(debug);
}
public void paintComponent(Graphics g){
super.paintComponent(g);
Graphics2D g2d = (Graphics2D) g;
AffineTransform plain = g2d.getTransform();
float xscale = (float)SCREENWIDTH/WIDTH;
float yscale = (float)SCREENHEIGHT/HEIGHT;
g2d.scale(xscale, yscale);
g2d.rotate(Math.toRadians(45), WIDTH/2, HEIGHT/2);
AffineTransform normal = g2d.getTransform();
g2d.drawImage(background, 0, 0, this);
for (int e=0;e= tower.x-tow
What can I do to increase my frames per second?
My Main class
```
import java.awt.*;
import java.awt.event.*;
import java.awt.geom.AffineTransform;
import java.awt.image.BufferedImage;
import java.io.File;
import java.io.IOException;
import java.util.*;
import javax.imageio.ImageIO;
import javax.swing.*;
import javax.swing.Timer;
public class Main extends JPanel implements ActionListener, KeyListener{
static int WIDTH;
static int HEIGHT;
static int SCREENWIDTH;
static int SCREENHEIGHT;
static int GRAVITY = 2;
BufferedImage background;
Tower tower;
Entity debug = new Entity(0, 0, 0, 30, 30, 300);
ArrayList entities = new ArrayList();
Random random = new Random();
public Main(){
addKeyListener(this);
setFocusable(true);
setFocusTraversalKeysEnabled(false);
requestFocus();
Thread thread = new Thread(new Runnable() {
@Override
public void run() {
gameLoop();
}
});
thread.start();
tower = new Tower(WIDTH, HEIGHT, 10, 10, 7);
background = generateBackground();
entities.add(debug);
}
public void paintComponent(Graphics g){
super.paintComponent(g);
Graphics2D g2d = (Graphics2D) g;
AffineTransform plain = g2d.getTransform();
float xscale = (float)SCREENWIDTH/WIDTH;
float yscale = (float)SCREENHEIGHT/HEIGHT;
g2d.scale(xscale, yscale);
g2d.rotate(Math.toRadians(45), WIDTH/2, HEIGHT/2);
AffineTransform normal = g2d.getTransform();
g2d.drawImage(background, 0, 0, this);
for (int e=0;e= tower.x-tow
Solution
Firstly, I recommend you format your code. In Eclipse, the shortcut is CTRL + SHIFT + F, in IntelliJ, it is CTRL + ALT + L. This is because of the following:
-
A lot of your functions are defined like
We usually add a space to make it
-
You don't have any blank lines between functions. Usually we have one blank line to improve the readability.
-
Other similar formatting issues.
These names seem to suggest that they are constants, so let's make them constants:
However, there's no real reason why we need to separate out the ideas of width and height; why not just use the
Don't declare fields with default visibility. Declare them as private:
Also, it's better to "code to the interface", meaning that rather than using
That is ... quite the constructor call. If you have six arguments to your constructor, one of the following is probably true:
-
-
-
You should use the Builder design pattern. Note that this is least likely to be the correct solution. The Builder design pattern would make it so that you could construct the
In this case, the design pattern basically gives you a way to pass "named arguments" to the constructor.
I actually recommend you take this as a parameter in the constructor (this is known as Dependency Injection). In doing so, should you want to debug by setting the Random to a seed, you could just call
With Java 8, this code could be reduced down to:
Good job in overriding
Don't return an
Also, the function doesn't actually find the smallest value; you actually look for the closest to 0. A better name would be
Furthermore, this entire function ca
-
A lot of your functions are defined like
public void myFunction(){We usually add a space to make it
public void myFunction() {-
You don't have any blank lines between functions. Usually we have one blank line to improve the readability.
-
Other similar formatting issues.
static int WIDTH;
static int HEIGHT;
static int SCREENWIDTH;
static int SCREENHEIGHT;
static int GRAVITY = 2;These names seem to suggest that they are constants, so let's make them constants:
public static final int WIDTH = 2000;
public static final int HEIGHT = 2000;
public static final int SCREEN_WIDTH = Toolkit.getDefaultToolkit().getScreenSize().width;
public static final int SCREEN_HEIGHT = Toolkit.getDefaultToolkit().getScreenSize().height;
public static final int GRAVITY = 2;However, there's no real reason why we need to separate out the ideas of width and height; why not just use the
Dimension class directly? Something like this:public static final Dimension BACKGROUND_SIZE = new Dimension(2000, 2000); // instead of WIDTH and HEIGHT
public static final Dimension SCREEN_DIMENSION = Toolkit.getDefaultToolkit().getScreenSize();
public static final int GRAVITY = 2;BufferedImage background;
Tower tower;
Entity debug = new Entity(0, 0, 0, 30, 30, 300);
ArrayList entities = new ArrayList();
Random random = new Random();Don't declare fields with default visibility. Declare them as private:
private BufferedImage background;Also, it's better to "code to the interface", meaning that rather than using
ArrayList, use List; most of the time, the rest of the code stays the same, but in the future, if you decide a different List, such as LinkedList, then all you have to do is change new ArrayList() to new LinkedList():private List entities = new ArrayList<>(); // Note the diamond operator, Java 7+. Use it; avoid repeating yourself.... = new Entity(0, 0, 0, 30, 30, 300);That is ... quite the constructor call. If you have six arguments to your constructor, one of the following is probably true:
-
Entity is doing too many things. The Single Responsibility Principle states that each class should ideally do one thing, and only one thing. Perhaps Entity should be split into multiple classes.-
Entity should be composed of multiple classes. For example, making more classes to describe Entity's data might result in a constructor call similar to:new Entity(new Location(0, 0), 0, new SubEntity(30, 30, 300))-
You should use the Builder design pattern. Note that this is least likely to be the correct solution. The Builder design pattern would make it so that you could construct the
Entity along the lines of:Entity.builder()
.setLocation(new Location(0, 0))
.setHeight(0)
.setSubEntity(new SubEntity(30, 30, 300)) // SubEntity is not an appropriate name; name it according to the data it holds
.create();In this case, the design pattern basically gives you a way to pass "named arguments" to the constructor.
Random random = new Random();I actually recommend you take this as a parameter in the constructor (this is known as Dependency Injection). In doing so, should you want to debug by setting the Random to a seed, you could just call
new Main(new Random(0)) instead of having to modify this file (your main function may not always be in this file, and it is cleaner anyway). Furthermore, should you want to switch Random implementations away from the default, say to a Mersenne Twister, you'd simply pass in a new MersenneTwister() and the implementation of Main wouldn't have to change.Thread thread = new Thread(new Runnable() {
@Override
public void run() {
gameLoop();
}
});
thread.start();With Java 8, this code could be reduced down to:
Thread thread = new Thread(this::gameLoop);
thread.start();public void paintComponent(Graphics g){Good job in overriding
paintComponent instead of paint!public int[] findSmallest(int[] arr){
int smallest = arr[0];
int index = 0;
for (int i=0; i<arr.length;i++){
if (arr[i]<0){
arr[i]*=-1;
}
if (arr[i]<smallest){
smallest=arr[i];
index = i;
}
}
int[] returnArr = {smallest, index};
return returnArr;
}Don't return an
int[] for this. Instead, just return the index (so rename the function to indexOfSmallest). If you want the index, just use it to get the value.Also, the function doesn't actually find the smallest value; you actually look for the closest to 0. A better name would be
indexOfSmallestAbs. Also, you never access state in this function, so make it static to signal the intent.Furthermore, this entire function ca
Code Snippets
public void myFunction(){public void myFunction() {static int WIDTH;
static int HEIGHT;
static int SCREENWIDTH;
static int SCREENHEIGHT;
static int GRAVITY = 2;public static final int WIDTH = 2000;
public static final int HEIGHT = 2000;
public static final int SCREEN_WIDTH = Toolkit.getDefaultToolkit().getScreenSize().width;
public static final int SCREEN_HEIGHT = Toolkit.getDefaultToolkit().getScreenSize().height;
public static final int GRAVITY = 2;public static final Dimension BACKGROUND_SIZE = new Dimension(2000, 2000); // instead of WIDTH and HEIGHT
public static final Dimension SCREEN_DIMENSION = Toolkit.getDefaultToolkit().getScreenSize();
public static final int GRAVITY = 2;Context
StackExchange Code Review Q#133238, answer score: 6
Revisions (0)
No revisions yet.