Should I use QDialog widget to draw line?

lansing

Sorry I got mixed up with the suggestion here to always use QGraphicsScene to draw. I will use QGraphicsView to see how it goes.

Bonnie

@lansing
Just pick from those two methods (QGraphicsScene + QGraphicsView / QWidget + QPainter), they are totally different.
You shouldn't "paint" by yourself if you use QGraphicsScene + QGraphicsView.
Use QGraphicsScene::addLine to add the line, the painting part will be finished by themselves.

lansing

@Bonnie

Oh, I am looking to draw like in photoshop. So I should be using QWidget + QPainter?

Bonnie

@lansing
Depending on how complicated it will be.
If you only want to draw like "press, drag, release, done", then you may use QWidget + QPainter.
But you need to save the lines you draw as member variables (the Scribble example save them to a QImage).
If you want move or do other transform operations to the drawn lines, you need to use QGraphicsScene + QGraphicsView.
You may also look at this example: Diagram Scene Example
It have some line drawing thing in a QGraphicsScene (but in this example the line will be deleted when releasing the mouse)

lansing

I have followed the scribble example to use QWidget and QPainter to draw line. I have used QPixmap to hold the line drawing instead of QImage from the example. The drawing works fine until I centered the QPixmap in paintEvent(), my drawing wasn't centered along with the QPixmap.

void Canvas::mousePressEvent(QMouseEvent *a_pEvent)
{
    if (a_pEvent->button() == Qt::LeftButton) {
        m_drawing = true;
        m_lastPoint =  a_pEvent->pos();
    }
}

void Canvas::mouseMoveEvent(QMouseEvent *a_pEvent)
{
    if ((a_pEvent->buttons() & Qt::LeftButton) && m_drawing) {
        drawPointTo(a_pEvent->pos());        
    }
}

void Canvas::paintEvent(QPaintEvent *event)
{
    QPainter painter(this);
    /* center image */
    painter.drawPixmap((rect().bottomRight() - m_drawingPixmap.rect().bottomRight()) / 2, m_drawingPixmap); 
    painter.end();
}

void Canvas::drawPointTo(const QPointF &a_endPoint)
{
    QPainter painter(&m_drawingPixmap);
    painter.setPen(QPen(Qt::black, 2));
    painter.drawLine(m_lastPoint, a_endPoint);
    update();
    m_lastPoint = a_endPoint;
}

Bonnie

@lansing
I think that's not hard to understand.
If your pixmap's top left isn't the same with the widget, then they have different coordinate system.
You need to map your points' coordinates from the widget to the pixmap when saving.

lansing

@Bonnie

How do I do that? I did a few trials and errors but still couldn't get it right.

So I ended up subclassing a QLabel class and added it to the canvas, and then set the pixmap and move all the paintEvent, mouse event functions to this label class.

Do I not need to do this?

Bonnie

@lansing
If you feel the mapping difficult I think it is no problem to put a centered "painting QWidget" inside your canvas.
But what's wrong with just subclass a QWidget and add that to the canvas?
Do you have a reason that you must use a QLabel?

lansing

@Bonnie

The QLabel is the "centered Qwidget" you talked of. Because it can hold a QPixmap for the line drawing, have all the mouse event functions and paintEvent, as well as able to retrieve the position.

canvas

Canvas::Canvas(QWidget *a_pParent): QWidget(a_pParent),  
{
    m_drawingLabel = new DrawingLabel(this);
    QHBoxLayout *layout = new QHBoxLayout();
    layout->addWidget(m_drawingLabel );
    setLayout(layout);
}

void Canvas::drawFrame(const QPixmap &a_drawingPixmap)
{
    // center the Qlabel when loading in the pixmap
    int x = (rect().width() - a_drawingPixmap.width()) / 2 ;
    int y = (rect().height() - a_drawingPixmap.height()) /2;
    m_drawingLabel ->setGeometry (x, y, a_drawingPixmap.width(), a_drawingPixmap.height());

    m_drawingLabel ->setDrawingPixmap(a_drawingPixmap);
}

qlabel

void DrawingLabel::paintEvent(QPaintEvent *event)
{
    QPainter painter(this);
    painter.drawPixmap(0,0,m_drawingPixmap);
    painter.end();
}

void DrawingLabel::setDrawingPixmap(const QPixmap & a_pixmap)
{
    m_drawingPixmap= a_pixmap;
    update();
}

void DrawingLabel::mousePressEvent(QMouseEvent *a_pEvent)
{
    if (a_pEvent->button() == Qt::LeftButton) {
        m_drawing = true;
        m_lastPoint = a_pEvent->pos();
    }
}
...
mousePressEvent()
mouseReleaseEvent()
drawPointTo()

Bonnie

@lansing
I mean I don't think it is necessary to use a QLabel as the centered widget since you don't seem to use any QLabel functions.
Why can't you just subclass a QWidget as the "DrawingLabel"?

And also, why are you setting the geometry while you are using a layout?
That don't make any sense.

lansing

@Bonnie

I can change to that, I just need some wrapper that can do all the things I mentioned above.

The layout was used to add the QLabel to the canvas.

Bonnie

@lansing
You either use a layout to keep the label centered, or set the geometry by yourself and don't use any layout.
Set geometry to a widget managed by a layout is meaningless, the layout may reset its geometry at any time.
You can use the layout like that:

layout->addWidget(m_drawingLabel, Qt::AlignCenter);

void Canvas::drawFrame(const QPixmap &a_drawingPixmap)
{
    m_drawingLabel->setFixedSize(a_drawingPixmap.size());
    m_drawingLabel->setDrawingPixmap(a_drawingPixmap);
}

Thomas Stein

This post is deleted!

lansing

I'm not spreading myself too thin right now, I'll stick to one method first until it worked well before I try other methods.

So I got it working under normal condition. I have changed the canvas to subclass a QScrollArea because I need to zoom in. I did the zoom function in the wheelEvent(). However, the problem now is with the QPixmap.scaled function, the more I zoomed in, the more laggy the program got.

void Canvas::wheelEvent(QWheelEvent *a_pEvent)
{
    QPoint delta = a_pEvent->angleDelta();
    // zooming
    if (delta.y() > 0) {
        m_zoomRatio += 0.6;
    } else {
        m_zoomRatio -= 0.6;
    }
    m_drawingLabel->setFixedSize(m_drawingPixmap.size() * m_zoomRatio);

    QSize zoomSize = m_drawingPixmap.size() * m_zoomRatio;
    m_drawingLabel->setDrawingPixmap(m_drawingPixmap.scaled(zoomSize));
}

void Canvas::drawFrame(const QPixmap &a_drawingPixmap)
{
    m_drawingPixmap = a_drawingPixmap; // save a copy for zooming
    
    m_drawingLabel->setFixedSize(a_drawingPixmap.size());
    m_drawingLabel->setDrawingPixmap(a_drawingPixmap);
}

Bonnie

Oh, if you need QScrollArea + zoom, I'm afraid QGraphics* classes are more suitable for that.
For current method, I don't know if this will take any affect, but you can try not scaling the pixmap, but the painter.
Change your paintEvent to

void DrawingLabel::paintEvent(QPaintEvent *event)
{
        QPainter painter(this);
        painter.scale(m_zoomRatio, m_zoomRatio);
        painter.drawPixmap(0,0,m_drawingPixmap);
}

And in your wheelEvent, dont' create and set new pixmap, just set the zoomed fixed size.

lansing

@Bonnie

Hi, setting the scale factor inside paintEvent() solved the lagging issue. But now I have problem mapping to the scaled position on the drawing. I tried multiplying the x, y with the same zoom ratio, but they still doesn't line up when I tried to draw. What did I do wrong?

void DrawingLabel::mousePressEvent(QMouseEvent *a_pEvent)
{
    if (a_pEvent->button() == Qt::LeftButton) {
        m_drawing = true;

        double x = double(a_pEvent->pos().x()) * m_zoomRatio;
        double y = double(a_pEvent->pos().y()) * m_zoomRatio;  
        m_lastPoint = QPointF(x, y);
    }
}

void DrawingLabel::mouseMoveEvent(QMouseEvent *a_pEvent)
{
    if ((a_pEvent->buttons() & Qt::LeftButton) && m_drawing) {
        double x = double(a_pEvent->pos().x()) * m_zoomRatio;
        double y = double(a_pEvent->pos().y()) * m_zoomRatio;
        QPointF zoomPos(x,y);

        drawPointTo(zoomPos);
    } else {
        QWidget::mouseMoveEvent(a_pEvent);
    }
}

void DrawingLabel::drawPointTo(const QPointF &a_endPoint)
{
    QPainter painter(&m_drawingPixmap);
    painter.setPen(QPen(Qt::black, 2));

    painter.drawLine(m_lastPoint, a_endPoint);
    update();
    m_lastPoint = a_endPoint;
}

Also a cosmetic problem, the drew line was also scaled with the pixmap, making it look pixelated when zoomed in, how do I make its look independent from the zoom?

Bonnie

@lansing
Well, I think you don't need to map the points in the mouse events.
Just set the painter of the pixmap to be scaled reversely.
And in order to keep the line width, set the pen width accordingly.

    QPainter painter(&m_drawingPixmap);
    painter.scale(1/m_zoomRatio, 1/m_zoomRatio);
    painter.setPen(QPen(Qt::black, 2/m_zoomRatio));
    painter.drawLine(lastPoint, endPoint);

But I feel it look not "smooth"...

If this line width issue can't be solved, then you should use back your old way, but try to optimize the wheelEvent part.
I think the lagging issue is caused by scaling be called rapidly in a short time (from several wheel events).

lansing

@Bonnie

Hi yes this doesn't work well, the drew line get softer and softer with more zoom, and it gets bigger when zoomed out. I have to revert back to the wheelEvent controlled zoom.

Oh and I just observed that the lagging issue are also presented on Photoshop too. When the zoom factor get to about 7x, zoom will get less responsive. So I'm all good now, if even Photoshop is having the same problem, that's nothing more I can do about it. I'll call this a day.

Thanks for the help.