Graphics.FillRectangle - Parameter is invalid

Question

I want to move player on the panel by using arrows. My panel has a Paint event:

private void panel1_Paint(object sender, PaintEventArgs e)
{
    var gameManager = new GameManager(this, e.Graphics);
}

GameManager.cs

public GameManager(Form1 gameForm, Graphics graphic)
{
    this.gameForm = gameForm;
    this.graphic = graphic;

    player = new Player(100, 100, graphic);
    gameForm.KeyUp += MovePlayer; // Event to handle KeyUpevent
}

private void MovePlayer(object sender, System.Windows.Forms.KeyEventArgs e)
{
     int x = 0, y = 0;

     switch (e.KeyCode)
     {
         case System.Windows.Forms.Keys.Down:
              y = 10;
              player.Move(x, y);
              break;
         }
     }
 }

Player.cs

private SolidBrush brush = new SolidBrush(Color.Black);

        public Player(int x, int y, Graphics graphic)
        {
            location = new Point(x, y);
            this.graphic = graphic;

            graphic.FillRectangle(brush, location.X, location.Y, 10, 10);
        }

        public void Move(int x, int y)
        {
            location.X += x;
            location.Y += y;

            graphic.FillRectangle(brush, location.X, location.Y, 10, 10); // Here I get an error
        }

The problem: Player is generating correctly on the Panel when the player object is called (on the constructor). But when I click down arrow key I get the following simply message:

System.ArgumentException: Parameter is invalid

But all the parameters seems to be valid, the SolidBrush object and the coordinates and dimensions. What is wrong?

Try to recreate/dispose `brush` in `Move` method, instead of use single one. — tchelidze, Mar 30 '18 at 06:32
I tried like that: `using(var brush = new SolidBrush(Color.Black)) { graphic.FillRectangle(brush, location.X, location.Y, 10, 10); }` but it didn't help — XardasLord, Mar 30 '18 at 06:37
okey, then see in debugger what are the values of `location.X` and `location.Y` — tchelidze, Mar 30 '18 at 06:38
`Location.X = 100`, `Location.Y = 110`. Seems to be as it should be — XardasLord, Mar 30 '18 at 06:41
When I call the method `Move(0,0)`, `Move(10,0)`, `Move(20,0)...` in the constructor one by one it works fine. The problem is then I try to Move player by using key arrows from a different class. — XardasLord, Mar 30 '18 at 06:44
Is a new `GameManager` meant to be created on every single call to Paint (e.g. minimize/maximize/resize window/other event that causes the window to repaint)? — ProgrammingLlama, Mar 30 '18 at 06:48
@john Yeah, I did it just for simplicity sake. But in this example it is not causing a problem :/ — XardasLord, Mar 30 '18 at 07:08
You need to trigger the Paint when needed, ie when a key is pressed by Invalidate'ing the panel. The Graphics object will get __invalid after__ the Paint event. you can pass it around but must not cache i! So here it will get invalid while waiting for a key press! — TaW, Mar 30 '18 at 07:14
Also: You need to decide what will drive the game: Either keypresses or a Timer. If nothing need to be done between the keypresses this will do but if stuff is going on a timer should be use. In the tick do stuff, test for keys and finally invalidate the field panel. in its paint event draw everything according to the new data. — TaW, Mar 30 '18 at 07:32
@TaW Yeah I will have to use a timer to drive the game anyway. But how should I test for keys input each timer tick? Like this - `if (ModifierKeys.HasFlag(Keys.Down))` etc for other keys? Because I understand that I cannot use event for handling keypress events while using a timer. — XardasLord, Mar 30 '18 at 07:46
No, this method only works for few keys. There are several ways. One is to include PresentationCore.dll and use Keyboard.IsKeyDown(Key.LeftCtrl) etc. Another is caching the key: Several ways: Set KeyPreview=true and cache the key like this: Keys lastKey; private void Form1_KeyDown(object sender, KeyEventArgs e) { lastKey = e.KeyCode; }. And then you could try use [ProcessCmdKey](https://stackoverflow.com/questions/10468200/key-events-processcmdkey) or override void OnKeyDown. — TaW, Mar 30 '18 at 08:43

score 0 · Accepted Answer · answered Mar 30 '18 at 07:08

0

According to (MSDN) you should not cache Graphics object. Instead create and dispose it as needed.

Simplest modification for you example:

public Player(int x, int y, Form1 form)
{
    _form = form;
    location = new Point(x, y);
    graphic = form.CreateGraphics();
    graphic.FillRectangle(brush, location.X, location.Y, 10, 10);
    graphic.Dispose();
}

 public void Move(int x, int y)
 {
    location.X += x;
    location.Y += y;

    graphic = _form.CreateGraphics();
    graphic.FillRectangle(brush, location.X, location.Y, 10, 10); 
    graphic.Dispose();
 }

Also you probably should not recreate your GameManager on every repaint but maybe it is just for simplicity sake.

answered Mar 30 '18 at 07:08

Roman

259
1
3
8

While well meant you really shouldn't do a form.CreateGraphics. This is bad advice, unless the graphics is meant to be replaced immediately.. - In fact he tries to use the e.Graphics object froma Paint event. But caching sneaks in; most of the other stuff you wrote is correct, though.. – TaW Mar 30 '18 at 07:17
I meant it just to illustrate the point, the whole original example seems done just to show what is failing. – Roman Mar 30 '18 at 07:22
Well, replacing one (classic) error by another one is not really helping matters, imo. – TaW Mar 30 '18 at 07:26

Graphics.FillRectangle - Parameter is invalid

1 Answers1