2

I'm new at Qt and I'm trying to design a simple application that draw lines for now. I managed to draw lines using QImage and the MouseEvent (the line starts at the mouse click and ends at the mouse release).

Now I would like to create a "ghost" line that appear during the mouseMoveEvent only. I would like to do that using a Stack (which will allow me later to manage the undo-redo) of QImage. However, I can't manage to even construct the stack, the program crashes without any explanation

Here is my declaration in my header class

private:
    QImage image;
    QStack <QImage> *history

Here is my constructor

Painty::Painty() : image(1920,1080, QImage::Format_ARGB32)
{
    image.fill(Qt::white);
    history = new QStack <QImage>;
}

Here are my functions :

void Painty::mousePressEvent(QMouseEvent *event)
{
     f_point = event->pos();
}

 void Painty::mouseReleaseEvent(QMouseEvent *event)
 {
    l_point = event->pos();
    addLine();
    history->push(image);
 }

 void Painty::mouseMoveEvent(QMouseEvent *event)
 {
    l_point = event->pos();
    addLine();
 }


 void Painty::paintEvent(QPaintEvent *event)
 {
    QWidget::paintEvent(event);
    QPainter painter(this);
    painter.drawImage(0,0,image);
 }

void Painty::addLine() 
{
     image=history->top();
     QPainter paint(&image);
     paint.drawLine(f_point,l_point);
     paint.end();
     this->update();
}

I tried to debug as much as I could but all I could realize is that the line that is making the program crash is history = new QStack <QImage>; but I don't know what is wrong with it.

demonplus
  • 5,613
  • 12
  • 49
  • 68
fo fu
  • 33
  • 7
  • You probably need to define it as `history = new QStack ();` instead? But I think there is absolutely no need to store the pointer to your stack object. Just declare it as `QStack history;`. – vahancho Oct 07 '15 at 08:42
  • If you're doing graphics, you may be better off taking a look at using the [Graphics View Framework](http://doc.qt.io/qt-5/graphicsview.html) – TheDarkKnight Oct 07 '15 at 08:58
  • history = new QStack (); crashed in the same way (I already tried this one). – fo fu Oct 07 '15 at 09:05
  • You should probably draw the "ghost line" directly onto the widget, and never draw it in the QImage. Look at QWidget::paintEvent, and related examples for example under QPainter docs. – hyde Oct 07 '15 at 09:41
  • You call `addLine()` before adding any image to `history`, so `history->top()` will not give a valid pointer. – jhnnslschnr Oct 07 '15 at 09:43
  • Another thing, why is `stack` a pointer? Just have it as normal member variable... In general, when ever you see a naked pointer in C++, be wary, you may be doing something unwise (important exception is, that in Qt you often have pointers to QObject subclasses, but in these cases they should usually have parent QObject, which will delete them). – hyde Oct 07 '15 at 09:43
  • Only assignable types can be stored in Qt containers and QImage is not assignable – demonplus Oct 07 '15 at 09:44
  • @demonplus Of course it is assignable. QImage is typical Qt copy-on-write class. – hyde Oct 07 '15 at 09:45

1 Answers1

2

You need to either push the empty image to the stack at the end of the constructor, or swap the lines

addLine();
history->push(image);

in mouseReleaseEvent(...).

Otherwise at the first call to addLine() history->top() will be invalid.

jhnnslschnr
  • 404
  • 2
  • 9
  • Make sence ! But should I quit the idea of using pointers as suggested vahancho and hyde ? – fo fu Oct 07 '15 at 09:56
  • 1
    Specific quote from the docs for `QStack::top()`: *"This function assumes that the stack isn't empty."* – hyde Oct 07 '15 at 09:57
  • @fofu Yes, it would make it simpler for you, because you don't have to do the memory management yourself. And as `QStack` will allocate dynamically on the heap, there is absolutely no need for this (the c++ stack is somewhat limited, so an array like `int[100000000]` would have to be allocated dynamically as you do now.) – jhnnslschnr Oct 07 '15 at 10:02