1

Using swing and awt i've created a grid. The user can click over any empty cell of the grid, marking it as a red cell. The grid is composed by 20 x 20 cells, 32 pixels each.

When the user clicks over a cell, i get the x and y coordinates of the mouse click and i perform the following calculation to find out which is the selected cell:

int mouseX = e.getX();
int mouseY = e.getY();

int row = mouseY / Model.NODE_SIZE;
int col = mouseX / Model.NODE_SIZE;

The problem is that row and col get inaccurate (wrong) when i click near the edges of a cell (right border or bottom border), resulting in the marking of the next cell (cell to the right or cell to the bottom) and not in the marking of the correct one that i'm clicking over.

The more i get closest to the right/bottom borders of the grid, the more the accuracy is bad.

This is how i draw the grid:

public class MainPanel extends JPanel {

// reference to the model
private Model model;

public MainPanel() {

    setPreferredSize(new Dimension(660, 660));

    // retrieve the model
    model = Controller.getController().getModel();

    // draw 
    repaint();

    // listen to mouse clicks
    addMouseListener(new MouseAdapter() {

        @Override
        public void mouseClicked(MouseEvent e) {

            int mouseX = e.getX();
            int mouseY = e.getY();

            System.out.println("mouseX: " + mouseX + " mouseY: " + mouseY);

            // get the row and column clicked
            int row = mouseY / Model.NODE_SIZE;
            int col = mouseX / Model.NODE_SIZE;

            if(row > Model.BOARD_SIZE - 1 || col > Model.BOARD_SIZE - 1) { // avoid out of bounds
                return;
            }

            System.out.println("row: " + row + " col: " + col);

            Controller.getController().getModel().setTarget(col, row);

            repaint();
        }
    });
}

/** 
 * Custom painting codes on this JPanel 
 * Called by repaint()
 */
@Override
public void paintComponent(Graphics g) { 
    super.paintComponent(g);    // fills background
    setBackground(Color.WHITE); // sets background color

    // draw the tiles
    Node[][] nodes = model.getBoard();

    for(int i = 0; i < Model.BOARD_SIZE; i++) {
        for(int j = 0; j < Model.BOARD_SIZE; j++) {
            if(nodes[i][j].getNodeType() == NodeType.OBSTACLE) {
                g.setColor(Color.BLACK);
                g.fillRect(i + Model.NODE_SIZE * i, j + Model.NODE_SIZE * j, Model.NODE_SIZE, Model.NODE_SIZE);
            }
            else if(nodes[i][j].getNodeType() == NodeType.SOURCE) {
                g.setColor(Color.GREEN);
                g.fillRect(i + Model.NODE_SIZE * i, j + Model.NODE_SIZE * j, Model.NODE_SIZE, Model.NODE_SIZE);
            }
            else if(nodes[i][j].getNodeType() == NodeType.TARGET) {
                g.setColor(Color.RED);
                g.fillRect(i + Model.NODE_SIZE * i, j + Model.NODE_SIZE * j, Model.NODE_SIZE, Model.NODE_SIZE);
            }
            else {
                g.setColor(Color.BLACK);
                g.drawRect(i + Model.NODE_SIZE * i, j + Model.NODE_SIZE * j, Model.NODE_SIZE, Model.NODE_SIZE);
            }
        }
    }
}

}

Link to the runnable:

https://www.dropbox.com/s/bqoimipp7i1f39s/GridExample.jar?dl=0

user3075478
  • 127
  • 11
  • What's `NODE_SIZE`'s value? Consider providing a [runnable example](https://stackoverflow.com/help/mcve) which demonstrates your problem. This is not a code dump, but an example of what you are doing which highlights the problem you are having. This will result in less confusion and better responses – MadProgrammer Mar 28 '16 at 07:16
  • Ok i've put a link to the runnable. NODE_SIZE is 32. – user3075478 Mar 28 '16 at 07:24
  • FYI- In a traditional MVC, the view and model don't know about each other (it can be argued that they don't know anything about the controller either) – MadProgrammer Mar 28 '16 at 07:25
  • The Controller must be accessible from the Model in some way. The user clicks a button, an event is fired and a method of the Controller is called (the Controller is a singleton). The Controller's method changes the model and updates the view. What's wrong with this? – user3075478 Mar 29 '16 at 09:47
  • Why? It couples your code. The view sends an event to (an observer) the controller, the controller notifies the model, the model notifies (an observer) the controller, which notifies the view. Neither the model or view actually need to know about the controller (or each other), all their notifications can be achieved through the use of an observer pattern. The idea is, you should be able to change the implementation of any of three layers and it'd still work. And no, the controller shouldn't be a singleton – MadProgrammer Mar 29 '16 at 10:17
  • So am i forced to use the Observer pattern? – user3075478 Mar 29 '16 at 10:20
  • 1
    As I said, it's more how a traditional MVC works, so, if you want to get to know how MVC's actually work, then yes. The less each layer knows about each other, the better, it makes it easier to change the implantation of one or more of the layers without it needing to be completely rewritten – MadProgrammer Mar 29 '16 at 10:23

2 Answers2

4

Your paint code is wrong, basically...

g.drawRect(i + Model.NODE_SIZE * i, j + Model.NODE_SIZE * j, Model.NODE_SIZE, Model.NODE_SIZE);

is increasing the size of each cell by a factor of the row/cell been drawn, for example...

Bad Screen

The green line is calculated by using g.drawRect(0, 0, BOARD_SIZE * NODE_SIZE, BOARD_SIZE * NODE_SIZE);, which should be the visible area of your grid, but you're drawing beyond it.

Instead, if I use...

import java.awt.Color;
import java.awt.Dimension;
import java.awt.EventQueue;
import java.awt.Graphics;
import java.awt.event.MouseAdapter;
import java.awt.event.MouseEvent;
import javax.swing.JFrame;
import javax.swing.JPanel;
import javax.swing.UIManager;
import javax.swing.UnsupportedLookAndFeelException;

public class Test {

    public static void main(String[] args) {
        new Test();
    }

    public Test() {
        EventQueue.invokeLater(new Runnable() {
            @Override
            public void run() {
                try {
                    UIManager.setLookAndFeel(UIManager.getSystemLookAndFeelClassName());
                } catch (ClassNotFoundException | InstantiationException | IllegalAccessException | UnsupportedLookAndFeelException ex) {
                    ex.printStackTrace();
                }

                JFrame frame = new JFrame("Testing");
                frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
                frame.add(new MainPanel());
                frame.pack();
                frame.setLocationRelativeTo(null);
                frame.setVisible(true);
            }
        });
    }

    public static class MainPanel extends JPanel {

        public static final int NODE_SIZE = 32;
        public static final int BOARD_SIZE = 8;

        private int row, col = -1;

        public MainPanel() {

            // listen to mouse clicks
            addMouseListener(new MouseAdapter() {

                @Override
                public void mouseClicked(MouseEvent e) {

                    int mouseX = e.getX();
                    int mouseY = e.getY();

                    System.out.println("mouseX: " + mouseX + " mouseY: " + mouseY);

                    // get the row and column clicked
                    int row = mouseY / NODE_SIZE;
                    int col = mouseX / NODE_SIZE;

                    if (row > BOARD_SIZE - 1 || col > BOARD_SIZE - 1) { // avoid out of bounds
                        return;
                    }

                    System.out.println("row: " + row + " col: " + col);

                    repaint();
                }
            });

            addMouseMotionListener(new MouseAdapter() {
                @Override
                public void mouseMoved(MouseEvent e) {
                    int mouseX = e.getX();
                    int mouseY = e.getY();
                    row = mouseY / NODE_SIZE;
                    col = mouseX / NODE_SIZE;
                    repaint();
                }
            });
        }

        @Override
        public Dimension getPreferredSize() {
            return new Dimension(BOARD_SIZE * NODE_SIZE, BOARD_SIZE * NODE_SIZE);
        }

        /**
         * Custom painting codes on this JPanel Called by repaint()
         */
        @Override
        public void paintComponent(Graphics g) {
            super.paintComponent(g);    // fills background
            setBackground(Color.WHITE); // sets background color

            g.setColor(Color.GREEN);

            for (int i = 0; i < BOARD_SIZE; i++) {
                for (int j = 0; j < BOARD_SIZE; j++) {
                    if (row == j && col == i) {
                        g.setColor(Color.RED);
                        g.fillRect(NODE_SIZE * i, NODE_SIZE * j, NODE_SIZE, NODE_SIZE);
                    }
                    g.setColor(Color.BLACK);
                    g.drawRect(NODE_SIZE * i, NODE_SIZE * j, NODE_SIZE, NODE_SIZE);
                }
            }
        }
    }
}

It becomes decidedly more accurate..

Grid

Another idea might be to use Rectangle to define each cell of the grid, that way you could just use Rectangle#contains to test if the mouse is within any given cell bounds...as an idea

MadProgrammer
  • 343,457
  • 22
  • 230
  • 366
3

Seeing this code:

g.fillRect(i + Model.NODE_SIZE * i, j + Model.NODE_SIZE * j, Model.NODE_SIZE, Model.NODE_SIZE);

In combination with this code:

int row = mouseY / Model.NODE_SIZE;
int col = mouseX / Model.NODE_SIZE;

is suspicious, your drawing your rectangles on screen with a size of i + Model.NODE_SIZE * i - so the size of rectangles on screen are offset by 1 for every preceding rectangle. This offset would compound making your underlying "detection" of the rectangle you selected slowly get worse the further bottom right on the screen you click.

I suspect if you change your code to read

g.fillRect(Model.NODE_SIZE * i, Model.NODE_SIZE * j, Model.NODE_SIZE, Model.NODE_SIZE);

It will work as intended.

ug_
  • 11,267
  • 2
  • 35
  • 52