From QGraphicsSceneContextMenuEvent* event to QGraphicsSceneMouseEvent* ev

Please_Help_me_D

@mrjj aaah I see
Ok, I follow your advice :)

By the way one message before you wrote the line:

QGraphicsSceneMouseEvent* event = new QGraphicsSceneMouseEvent

As far as I know when I use the "new" key word then I should care about deleting this variable (that is allocated on the heap) to prevent memory leaks.
Would it be better if I simply write:

QGraphicsSceneMouseEvent* event // no "new" word: does that mean that I should not delete it after all?

Is there a difference? I'm trying to avoid allocating memory on the heap.

mrjj

Hi
super. I hope its easy to refactor.

Lets talk about

QGraphicsSceneMouseEvent* event = new QGraphicsSceneMouseEvent

-As far as I know when I use the "new" key word then I should care about deleting this variable (that is allocated on the -heap) to prevent memory leaks.
This is 100% correct for plain C++. With Qt its differnt as Parents will delete it children so when you
insert a button to a UI Form. the form will take of deleting the button. (when its deleted itself)

However, in this case you are right. If we new one our self there we would have to clean it up
by ourself, since we did not use the normal system with PostEvent or anything like that,
we own it and
we must clean it.

Would it be better if I simply write:

QGraphicsSceneMouseEvent* event // no "new" word: does that mean that I should not delete it after all?

That would only declare it and then point to some random location in memory.
We call them dangling pointers and they give crashes if used.
But yes you dont have to clean the declaration. Only if you allocated it with new.

This is a very important difference so let just talk a moment more

Class *a; // this is just the saying 'a' can point to the type Class and currently it points to something random.
Class *a=nullptr; // can point to the type Class and currently, it points to nothing.

a = new Class; // here we actually set to a real object. often called an instance.

so
Class *a;// declare it
a = new Class; // allocate it

we could maybe have gotten away with
this->mouseDoubleClickEvent(nullptr);
if the event variable was not used inside mouseDoubleClickEvent
but that might become bad later if you change the code and did use the event now and forgot you called it
with null from right click.

Please_Help_me_D

@mrjj Thank you very much for explanation!
Very important to know this

mrjj

@Please_Help_me_D
Hi
no problem.
yes its very important as its also easy to forget with member variables.

say you have class

class MyWidget : QWidget {
FancyWidget *widget;
};

then in some place you must have

widget = new FancyWidget

before you use it anywhere ! - as else its a crash.

widget->DoSomething()

will crash hard if you only have the declaration and forgot allocation.

Therefore its often good to always do
class MyWidget : QWidget {
FancyWidget *widget=nullptr;
};

so its very clear we forgot to allocate it when looking in debugger as
its then ZERO.
If not assigned zero, it just points to some random location and just looks odd but its hard to tell
if its allocated or not.

Please_Help_me_D

@mrjj after all I got that I should avoid using form like `FancyWidget *widget;'
But can I think that this form:

FancyWidget *widget=nullptr; // pointer to zero

may succesfully replace:

FancyWidget *widget=new FancyWidget ; // object instance

no matter FancyWidget is a Qt clas or a C++ class? And as a result using the form FancyWidget *widget=nullptr; allows us to forget about deleting the pointer variable widget after all?

mrjj

@Please_Help_me_D
Hi
Using the syntax
FancyWidget *widget=nullptr; // pointer to zero

Only makes it easier to detect when we forget to allocated.

We must always allocated before we can use the widget/object/class

so we must have
widget=new FancyWidget ; // this allocated the member variable widget.

however, if you type

FancyWidget *widget=new FancyWidget ; // this will make new local variable and not use the member one.

Regarding calling delete.
Most classes that are based on QObject/QWidget will be auto cleaned if inserted into any other widget/layout or Form.
https://doc.qt.io/qt-5/objecttrees.html
This is an ownership system.

But this only for Qt classes.

If you have your own that is not based on QWidget, but just plain c++ class, then yes you must
call delete on it by yourself.

Please_Help_me_D

@mrjj very thank you!
now I'm grand-master at pointers and memory allocation :D

Please_Help_me_D

@mrjj Hi,
I started to work with destructor on practice.
I have QwtScaleWidget* (from QWT library) that was created in MyClass as:

QwtScaleWidget* scaleWidget = new QwtScaleWidget;

In destructor of MyClass I'm trying to completely remove QwtScaleWidget* to prevent memory leaks. But QwtScaleWidget also has its own destructor. When I try to write:

MyClass::~MyClass()
{
    delete ui;
    delete scaleWidget; // here is an exception
}

Debugger throws me an exception when I close the QDialog with MyClass.
Does that mean that QwtScaleWidget destructructor begins to work when I close the QDialog with it?
I also cant get access to the scaleWidget parameters in MyClass's Destructor (I looked to the source code of QWT). And if so seems to me that QwtScaleWidget destructor started to work sooner then any of line (command) in Destructor of MyClass

jsulm

@Please_Help_me_D said in From QGraphicsSceneContextMenuEvent* event to QGraphicsSceneMouseEvent* ev:

Debugger throws me an exception

What exception exactly?
Don't you shadow scaleWidget here:

QwtScaleWidget* scaleWidget = new QwtScaleWidget;

?
You're declaring a LOCAL variable with same name in this line and probably trying to delete another one (class member) in destructor which was never initialised.

Please_Help_me_D

@jsulm exception is on the picture below. Earlier here on the forum I wrote about scaleWidget but in reality its name colorBarWidget (variable of a class colorBarWidget that is inherited form QwtScaleWidget)
And actually colorBarWidget is created as:

colorBarWidget = new ColorBarWidget();

So colorBarWidget doesn't have any relationship to shadowing variable.

But I have ColorBarWidgetMain *colorBarWidgetMain that is a parameter of a class and I need to store colorBarWidgetMain as a private variable. So I can't understand how to avoid shadowing in this case. In header file I have:

class ColorBarEditorForm : public QDialog
{
    Q_OBJECT

public:
    explicit ColorBarEditorForm(QWidget *parent, ColorBarWidgetMain *colorBarWidgetMain); // colorBarWidgetMain should be variable of the class available to all method of this class
    ~ColorBarEditorForm();

private:
    Ui::ColorBarEditorForm *ui;

private:
    void showEventHelper();
    QGraphicsScene *scene;
    ColorBarWidget *colorBarWidget;
    ColorBarGraphicsWidget *colorBarGraphicsWidget;
    ColorBarWidgetMain *colorBarWidgetMain;
    QColorDialog *colorDialog;
    QwtLinearColorMap* colorMap;

private slots:
    void setPickerColor();
    void on_pushButton_clicked();

protected:
      void showEvent(QShowEvent *ev);
};

In .cpp file:

ColorBarEditorForm::ColorBarEditorForm(QWidget *parent, ColorBarWidgetMain *colorBarWidgetMain) // here is a warning message about colorBarWidgetMain shadowing

How to avoid shadowing?

jsulm

@Please_Help_me_D said in From QGraphicsSceneContextMenuEvent* event to QGraphicsSceneMouseEvent* ev:

How to avoid shadowing?

How do you assign/use colorBarWidgetMain parameter in ColorBarEditorForm::ColorBarEditorForm?
Do you assign it to colorBarGraphicsWidget class member? If so how?
Do you pass a valid pointer there?
Do you delete colorBarWidgetMain somewhere else and then try to delete it in ColorBarEditorForm?

Please_Help_me_D

@jsulm I assign it in ColorBarEditorForm::ColorBarEditorForm with:

this->colorBarWidgetMain = colorBarWidgetMain;

No I don't delete colorBarWidgetMain at all. I SHOULD NOT delete colorBarWidgetMain in ColorBarEditorForm. When ColorBarEditorForm is closed colorBarWidgetMain should still be alive beacause it is in the other window but all other private variables of ColorBarEditorForm should be removed to avoid memory leaks.

In case my answer is not full here is ColorBarEditorForm::ColorBarEditorForm:

ColorBarEditorForm::ColorBarEditorForm(QWidget *parent, ColorBarWidgetMain *colorBarWidgetMain) :
    QDialog(parent),
    ui(new Ui::ColorBarEditorForm)
{
    ui->setupUi(this);

    colorDialog = new QColorDialog(this);
    /* set it as our widiget, you can add it to a layout or something */
    ui->formLayout->addWidget(colorDialog);
    ui->formLayout->setWidget(0, QFormLayout::FieldRole, colorDialog);
    //this->setCentralWidget(colorDialog);
    /* define it as a Qt::Widget (SubWindow would also work) instead of a dialog */
    colorDialog->setWindowFlags(Qt::Widget);
    /* a few options that we must set for it to work nicely */
    colorDialog->setOptions(
                /* do not use native dialog */
                QColorDialog::DontUseNativeDialog
                /* you don't need to set it, but if you don't set this
                    the "OK" and "Cancel" buttons will show up, I don't
                    think you'd want that. */
                | QColorDialog::NoButtons
    );
    connect(colorDialog, &QColorDialog::currentColorChanged, this, &ColorBarEditorForm::setPickerColor);

    scene = new QGraphicsScene(this);
    scene->setBackgroundBrush(QBrush(QColor(240, 240, 240)));
    ui->graphicsView->setVerticalScrollBarPolicy(Qt::ScrollBarAlwaysOff);
    ui->graphicsView->setHorizontalScrollBarPolicy(Qt::ScrollBarAlwaysOff);

    this->colorBarWidgetMain = colorBarWidgetMain;
    colorBarWidget = new ColorBarWidget();
    colorBarWidget->scaleDraw()->setScaleDiv(colorBarWidgetMain->scaleDraw()->scaleDiv());
    QwtInterval interval = QwtInterval(0, 1);
    QwtInterval intervalTicks = QwtInterval(colorBarWidgetMain->scaleDraw()->scaleDiv().lowerBound(),
                                       colorBarWidgetMain->scaleDraw()->scaleDiv().upperBound());
    QColor color1, color2;
    QVector<QColor> color;
    QVector<double> colorStop;
    int I = static_cast<QwtLinearColorMap*>(const_cast<QwtColorMap*>(colorBarWidgetMain->colorMap()))->colorStops().size();
    for (int i = 0; i < I; i++){
        double stopVal = static_cast<QwtLinearColorMap*>(const_cast<QwtColorMap*>(colorBarWidgetMain->colorMap()))->colorStops().at(i);
        if (stopVal == 0){
            color1 = static_cast<QwtLinearColorMap*>(const_cast<QwtColorMap*>(colorBarWidgetMain->colorMap()))->color1();
        } else if (stopVal == 1){
            color2 = static_cast<QwtLinearColorMap*>(const_cast<QwtColorMap*>(colorBarWidgetMain->colorMap()))->color2();
        } else {
            color.push_back(static_cast<QwtLinearColorMap*>(const_cast<QwtColorMap*>(colorBarWidgetMain->colorMap()))->color(interval, stopVal));
            colorStop.push_back(stopVal);
        }
    }
    colorMap = new QwtLinearColorMap(color1, color2, QwtColorMap::RGB);
    for (int i = 0; i < colorStop.size(); i++){
        colorMap->addColorStop(colorStop.at(i), color.at(i));
    }
    colorBarWidget->setColorMap(intervalTicks, colorMap);

    colorBarGraphicsWidget = new ColorBarGraphicsWidget(nullptr, Qt::Widget);
    colorBarGraphicsWidget->setWidget(colorBarWidget);
    scene->addItem(colorBarGraphicsWidget);
    ui->graphicsView->setScene(scene);
}

jsulm

@Please_Help_me_D You could set a break point in ColorBarWidgetMain destructor and run through debugger to see when it is called, to make sure it is really not deleted twice.

Please_Help_me_D

@jsulm I just tried to change the places of delete ui and delete colorBarWidget:

ColorBarEditorForm::~ColorBarEditorForm()
{
    delete colorBarWidget;
    delete ui;
    for (int i = 0; i < scene->items().size(); i++){
        delete scene->items().at(i);
    }
    delete scene;
}

And now I dont see any error. Is it possible that when ui is closed a QwtScaleWidget (ColorBarWidget is inherited form QwtScaleWidget) destructor is called?