Important: Please read the Qt Code of Conduct - https://forum.qt.io/topic/113070/qt-code-of-conduct

How do I properly serialize and deserialize a QList class in QT using QDatastream?



  • I am trying to serialize a custom class Layer* and read it back using QDataStream. Now, Layer is an abstract class with virtual method which is inherited by different kinds of layers: RasterLayer, TextLayer, AdjustmentLayer etc.

    I have a QList<Layer*> layers which keeps track of all the layers, and any adjustments made to a layer are updated in the list. I need to serialize and deserialize the QList to its original state and restore the properties of the individual layers (of different types).

    Here is layer.h:

    #ifndef LAYER_H
    #define LAYER_H
    
    #include <QString>
    #include <QImage>
    #include <QDebug>
    #include <QListWidgetItem>
    #include <QGraphicsItem>
    #include <QPixmap>
    
    class Layer : public QListWidgetItem
    {
    
    public:
    
        enum LayerType{
            RASTER,
            VECTOR,
            TEXT,
            ADJUSTMENT
        };
    
        Layer(QString name, LayerType type);
    
        ~Layer();
        inline void setName(QString &name) { _name = name; }
        inline QString getName() { return _name; }
        inline LayerType getType() { return _type; }
    
        virtual void setSceneSelected(bool select) = 0;
        virtual void setLayerSelected(bool select) = 0;
        virtual void setZvalue(int z) = 0;
        virtual void setParent(QGraphicsItem *parent) = 0;
    
    protected:
        QString _name;
        LayerType _type;
    };
    
    #endif // LAYER_H
    

    This is extended by a RasterLayer class:

    #ifndef RASTERLAYER_H
    #define RASTERLAYER_H
    
    #include <QGraphicsPixmapItem>
    #include <QPainter>
    #include <QGraphicsScene>
    
    #include "layer.h"
    
        class RasterLayer : public Layer, public QGraphicsPixmapItem
        {
        public:
            RasterLayer(const QString &name, const QImage &image);
            RasterLayer();
            ~RasterLayer();
        
            void setLocked(bool lock);
            void setSceneSelected(bool select);
            void setLayerSelected(bool select);
            void setZvalue(int z);
            void setParent(QGraphicsItem *parent);
            inline QPixmap getPixmap() const { return pixmap(); }
            inline QPointF getPos() const { return QGraphicsPixmapItem::pos(); }
            inline void setLayerPos(QPointF pos) { setPos(pos);}
            inline void setLayerPixmap(QPixmap pixmap) { setPixmap(pixmap); }
        
            friend QDataStream& operator<<(QDataStream& ds, RasterLayer *&layer)
            {
                ds << layer->getPixmap() << layer->getName() << layer->getPos();
                return ds;
            }
        
            friend QDataStream& operator>>(QDataStream& ds, RasterLayer *layer)
            {
                QString name;
                QPixmap pixmap;
                QPointF pos;
        
                ds >> pixmap >> name >> pos;
        
                layer->setName(name);
                layer->setPixmap(pixmap);
                layer->setPos(pos);
        
                return ds;
            }
        
        protected:
            void paint(QPainter *painter,
                       const QStyleOptionGraphicsItem *option,
                       QWidget *widget);
        
        private:
            QImage _image;
        };
        
        #endif // RASTERLAYER_H
    

    I am currently trying to test serialization-deserialization of a RasterLayer like this:

    QFile file(fileName);
    
    file.open(QIODevice::WriteOnly);
    QDataStream out(&file);
    
    Layer *layer = paintWidget->getItems().at(1);
    // Gets the second element in the list
    
    RasterLayer *raster = dynamic_cast<RasterLayer*> (layer);
    out << raster;
    file.close();
    

    Now, as you can see here, I am specifically casting Layer* to a RasterLayer*to be serialized, and this works since I have worked on only one type of layer till now. So my first question is:

    How do I generalize this serialization process to all types of layers?

    Every type of layer will have a different way of serialization since each hold different properties. Also, the casting here feels like a bit of code smell and a possible bad design choice. So, having something like serialize the entire list of layers calling their corresponding overloaded operators would be the expected scenario.

    My second question is:

    How do I deserialize the data properly?
    Here's how I am currently serializing an individual RasterLayer:

    QFile newFile(fileName);
    newFile.open(QIODevice::ReadOnly);
    QDataStream in(&newFile);
    
    RasterLayer *layer2 = new RasterLayer;
    in >> layer2;
    paintWidget->pushLayer(layer2);
    ui->layerView->updateItems(paintWidget->getItems());
    

    Firstly, I don't think serializing to a pointer is something I should be doing in this case, but I am not sure what else to do or how to do better yet. Secondly, the deserialization works here, but it doesn't quite do what I would be expecting it to do. Although I am using setters in the overloaded operators, it's really not updating the layer properly. I need to call the constructor to make a new layer out of it.

    I have tried this: https://stackoverflow.com/questions/2570679/serialization-with-qt but I am not quite sure how to have a Layer* convert it to a Layer, serialize it, deserialize it and then convert it back to Layer*.
    So I need to add a third step:

    RasterLayer *layer3 = new RasterLayer(layer2->getName(), layer2->getPixmap().toImage());
    layer3->setPos(layer2->pos());
    

    and then push layer3 to the list to actually make it work. According to this post: https://stackoverflow.com/a/23697747/6109408, I really shouldn't be doing a new RasterLayer... inside the operator overloading function (or else I will fry in hell), and I am following the first suggestion given there, which isn't very much working in my case and I don't know the right way to do it.

    Also, how do I deserialize this for a general QList of Layer*s instead of having to create new specific layer instances and injecting them with deserialized data? Although this is similar: https://stackoverflow.com/questions/23202667/serialize-a-class-with-a-qlist-of-custom-classes-as-member-using-qdatastream#23219659, the answers weren't clear enough for me to understand.

    I have had an idea about an intermediate value holder class that I will use to serialize all sorts of Layers and let that create and inject the parameters depending upon the type of Layer it is, but I am not sure if that will work.

    Thanks for helping me out.



  • @twodee said in How do I properly serialize and deserialize a QList class in QT using QDatastream?:

    RasterLayer *raster = dynamic_cast<RasterLayer*> (layer);
    out << raster;

    Now, as you can see here, I am specifically casting Layer* to a RasterLayer*to be serialized, and this works since I have worked on only one type of layer till now

    While you are waiting for someone who can really help with your overall problem....

    Although I am not a C++-er, I don't think that dynamic cast makes a blind bit of difference. You might just as well have left it at:

    out << layer;

    As far as I know, dynamic_cast<> just does a run-time check on the type and returns the same pointer if it is of the specified class or nullptr if not. It changes nothing. And out << variable; just calls the variable's actual serialization operator, regardless of type.

    You might like to check this. If I'm right, you won't be able to use anything in that approach in the way you thought it worked. And if I'm wrong you can tell me so!



  • @JonB said in How do I properly serialize and deserialize a QList class in QT using QDatastream?:

    You might like to check this. If I'm right, you won't be able to use anything in that approach in the way you thought it worked. And if I'm wrong you can tell me so!

    Hi, thanks for your input. I tried this out and for some reason, if I directly use Layer nothing gets serialized (maybe a NULL value does) and none of the data gets written to the file. I have tried this earlier on, but gave it a try again swapping the datatypes of the overloaded operator from RasterLayer with Layer*, but it still doesn't seem to do the trick. I was hoping it would, but I think I am missing something here.



  • @twodee
    You seem to be talking about changing between pointers or not, that's not what I meant. And I trust you used your layer variable and not your Layer type (note the capitalizations, you must get that right, I did mean literally out << layer;). I'm surprised at your finding as now I don't understand how the cast could make any difference at runtime, but I'm not a C++ expert so we'll leave it at that. Sorry.



  • @JonB Yes, I did use my layer variable, I did comment out the raster part and that didn't seem to work. My findings look a bit odd to me as well, thanks for trying. :D



  • The normal way to do it:

    • create 2 pure virtual protected members in Layer that takes a QDataStream& argument and saves/loads the layer from it
    • create QDataStream stream operators for Layer (not Layer*) that do nothing but calling those protected methods
    • serialise something that will tell you what kind of Layer it will be (something like QVariant::userType does)
    • serialise directly by dereferencing a Layer* no need to cast anything

    RasterLayer *layer2 = new RasterLayer; in >> *layer2; is perfectly valid, as long as paintWidget owns the allocated memory



  • @VRonin Hi, thanks for responding. I am getting this error which I usually can avoid using a const somewhere: error: passing ‘const RasterLayer’ as ‘this’ argument discards qualifiers [-fpermissive] : setName(name);

    As you can see in the layer.h, it is an inline method inline void setName(QString &name) { _name = name; } , what should I be doing in this case?



  • Looks like you are deserialising on a const method. What is the code around that call?



  • This post is deleted!


  • @VRonin I figured the serialization out, now I have to do something like out << *layer; to get it serialized. But how will that allow me to serialize a QList<Layer*> to be serialized as a whole?



  • @twodee
    Read through threadin this forum https://forum.qt.io/topic/58701/how-to-serialize-deserialize-a-qlist-myclass

    As per http://doc.qt.io/qt-5/datastreamformat.html, QList<T> can be de/serialized. You need to provide the de/serialization for your class T, which is what you have been working on here.



  • This post is deleted!


  • As the post said it's about one line with the operator new, could you tell me what that line is?

     QList<Layer*> layers = paintWidget->getItems(); 
    out << layers;
    

    This doesn't seem to do the trick. I have been scratching my head for too long, sorry if I am being stupid.

    Also for some reason overloading QDataStream& operator<<(QDataStream& ds, const Layer *layer) doesn't work anymore, which did work earlier.



  • out << qint32(layers.size());
    for(Layer* layer : qAsConst(layers))
    out << qint32(layer->type()) << *layer;
    

    qint32 size;
    qint32 type;
    in >> size;
    while(size-- >0 ){
    in >> type;
    Layer* layer = nullptr;
    switch(type){
    case Raster:
    layer = new RasterLayer;
    break;
    default:
    Q_UNREACHABLE();
    }
    in >> *layer;
    }


  • @VRonin
    I don't want to muddy the waters for the OP here, but may I ask: why do you explicitly serialize the layers list yourself in a loop? I thought that the point of the link I mentioned, http://doc.qt.io/qt-5/datastreamformat.html, is that it shows:

    The QDataStream allows you to serialize some of the Qt data types. The table below lists the data types that QDataStream can serialize

    QList<T>	
        The number of items (quint32)
        The items (T)
    

    so why can't you just out << layers ?

    Is this because you want to know where layer->type() is in the serialization, so that you can look at it during deserialization and do your own newing? If there were no sub-classing going on in the list elements then you wouldn't need to do that and could just do the list directly? Could it then just be deserialized with in >> layers? Or will deserializing never do any newing of elements for you?



  • @JonB That would imply having datastream operators acting on pointers and that's dangerous:

    • What if you pass a null pointer
    • What if you pass a dangling pointer
    • Who own the memory allocated by the pointer? the operator?


  • @VRonin
    So are you saying: "Yes, you can de/serialize QList<T> directly as per that link, but while that's fine for simple types it's not suitable for pointers"?

    I may be confusing myself. In my C# we don't have "pointers" and we just de/serialize lists directly without a care. Deserializing does whatever newing is necessary behind the scenes.


  • Moderators

    @JonB said in How do I properly serialize and deserialize a QList class in QT using QDatastream?:

    I may be confusing myself. In my C# we don't have "pointers" and we just de/serialize lists directly without a care.

    I beg to differ!

    you can use pointers in c# and manage your memory by hand, but you have to explicitly tell the compiler to allow it with -unsafe, IIRC

    Quick google search:
    https://www.tutorialspoint.com/csharp/csharp_unsafe_codes.htm



  • take this example:

    // QList<int*> m_ownerList;
    m_ownerList.append(new int(5));
    m_ownerList.append(new int(3));
    
    m_myInt1 = new  int(5);
    m_myInt2 = new  int(3);
    m_nonOwnerList = QList<int*>{{m_myInt1 ,m_myInt2} };
    

    What should QDataStream& operator>>(QDataStream& , const QList<int*> ) do? free the memory already allocated or not?



  • @J.Hilk
    But I would never want to use pointers in C# or manage memory by hand, that's (more than) half the point of using C#!?

    Maybe there's a misunderstanding. I don't use C# with Qt (I use Python). I'm just familiar with C# compared to C++. I am trying to understand @VRonin's explanation of de/serializing this QList<T>, where he is saying he does it explicitly to manage pointers, when I know I would just de/serialize a list from C# without iterating the elements myself, and trying to understand why.



  • @VRonin Hi, thanks for the extended example. I have come up with a working solutin that looks similar to yours, could you let me know if this is a good way to deal with it?

    QDataStream& operator >>(QDataStream& stream, QList<Layer*>& layers){
        layers.clear();
        int size;
        int type;
    
        stream>>size;
    
        QString name;
        QPixmap pixmap;
    
        Layer* layer = nullptr;
        for(int i =0; i<size; ++i){
    
            stream >> name >> type;
            switch (type) {
            case Layer::RASTER:
                stream >> pixmap;
                layer = new RasterLayer(name, pixmap.toImage());
                break;
            default:
                Q_UNREACHABLE();
                break;
            }
            layer->read(stream);
    
            layers.push_back(layer);
        }
        return stream;
    }
    


  • The very first line of your code is a memory leak. This is exactly my point. Don't handle memory inside serialising/deserialising it's not the right place



  • I felt wrong in doing so too as I mentioned in my original question, but I still am not sure where would I be creating the new RasterLayer in my case. Could you give me some clues on how to go about it?



  • Memory owned by an object should be created and freed by a method of said object:

    You can create a wrapper class around QList<Layer*> that will take care of allocating/freeing the memory inside and create safe datastream operator for this wrapper


  • Moderators

    @twodee said in How do I properly serialize and deserialize a QList class in QT using QDatastream?:

    I felt wrong in doing so too as I mentioned in my original question, but I still am not sure where would I be creating the new RasterLayer in my case. Could you give me some clues on how to go about it?

    Firstly, use the factory as you're using polymorphism, that'd be the way to go here. And secondly delegate the deserialization to the object through a virtual function; the whole reading. It's a good idea, since you're dealing with polymorphic objects, to also employ some kind of stack-based memory management here, to make sure you don't leak memory.
    You can see how I did it here and draw some inspiration:
    https://github.com/VSRonin/ChatExample/blob/commonlib/QtSimpleChat/chatmessage.h
    https://github.com/VSRonin/ChatExample/blob/commonlib/QtSimpleChat/chatmessage.cpp

    I am trying to understand @VRonin's explanation of de/serializing this QList<T>, where he is saying he does it explicitly to manage pointers, when I know I would just de/serialize a list from C# without iterating the elements myself, and trying to understand why.

    'Cause when you have allocations and deallocations in different parts of the code, i.e. not declaring the objects' lifetimes clearly you're begging for trouble. C++ is lower level (thank god) than C#, so it's up to the proficiency of the programmer to actually choose the best and most versatile implementation. It may seem elitist, but we don't want the language to be something to be worked around whenever there's something falling outside of the standard set of problems to solve. In Java/C# you start by assuming the programmer is irresponsible and can't handle the problem of finding a good way to manage the memory. This may prevent a lot of errors, but also hits hard on the efficiency ...



  • @VRonin So, if I create a function for the wrapper around QList<Layer*> class (let's call it LayerList) that creates the correct layers depending upon the type (essentially acting as a factory), then I can go to the >> stream and instead of doing a new RasterLayer() I will call the LayerList::createLayer() method, in which case the memory is created and freed by the object owning the layers. Is my idea correct?


  • Moderators

    You have a two-layered problem here.

    1. How to manage the memory
    2. How to do (de)serialization

    Start backwards, ensure you read and write the object correctly. You can do "virtual shift" operators as a syntactic sugar as well by delegating:

    virtual bool Layer::serialize(QDataStream &) const;
    virtual bool Layer::deserialize(QDataStream &);
    
    // Then just delegate from the base class (need to be `friend`s to the base class):
    QDataStream & operator >> (QDataStream & stream, Layer & layer)
    {
        layer.deserialize(stream);
        return stream;
    }
    
    QDataStream & operator << (QDataStream & stream, const Layer & layer)
    {
        layer.serialize(stream);
        return stream;
    }
    

    This should more or less take care of task 2. Don't forget to use the virtuals up the hirearchy - i.e. each derived class would call the base class' (de)serialize to get the data of the base read/written. This way you don't have code duplication and you keep the idea that each class is responsible for its own data.

    As for 1), you should ideally go with some kind of clever pointer. In the example above I used QSharedData and QSharedDataPointer, but you can also use QSharedPointer (which provides external reference counting). In principle I'd always opt for the first option, as the refcount is internal to the object a reference is held to. Using QSharedDataPointer would mean also that the base class has to derive from QSharedData, just as the example I sourced above. Whenever you have that it's a breeze, as you don't have to worry when the deallocation is happening and you are left only with one thing to do - read an integer and pass it to the static function of your Layer class that'd create the correct object based on that integer and return a base pointer to it. After all that the stream operator would look really simple:

    class LayerPointer; //< This is your stack-based memory management class
    
    QDataStream & operator >> (QDataStream & stream, QList<LayerPointer> & layers)
    {
        layers.clear();
    
        int size;
        stream >> size;
        layers.reserve(size);
    
        for(int i = 0; i < size; i++)  {
            stream >> type;
            LayerPointer layer = Layer::create(type); //< Factory creation
            stream >> *layer; //< The "virtual stream operator", will call the proper implementation
    
            layers.append(layer);
        }
    
        return stream;
    }
    


  • @kshegunov So for 1) to work, all I have to do is a create a LayerPointer class that extends QSharedData or is there something else I would have to add? I have never dealt with this type of situation before. Also, I have already solved 2) the way you suggested, so I am currently focusing on 1).


  • Moderators

    See the chat example above, it's exactly what you have to do. There the ChatMessage is the base class that derives from the QSharedData and there's one typedef for the pointer: typedef QSharedDataPointer<ChatMessage> ChatMessagePointer;. Also you should read this, to have an idea what's happening behind the scenes. That's pretty much it.
    Then serialization and deserialization is done in fromJson and toJson (the messages come from JSON objects), you can easily adapt that to a data stream.


Log in to reply