Skip to content
  • Categories
  • Recent
  • Tags
  • Popular
  • Users
  • Groups
  • Search
  • Get Qt Extensions
  • Unsolved
Collapse
Brand Logo
  1. Home
  2. Qt Development
  3. General and Desktop
  4. How stupid is QImage implemented??
Forum Updated to NodeBB v4.3 + New Features

How stupid is QImage implemented??

Scheduled Pinned Locked Moved Unsolved General and Desktop
22 Posts 7 Posters 6.8k Views 5 Watching
  • Oldest to Newest
  • Newest to Oldest
  • Most Votes
Reply
  • Reply as topic
Log in to reply
This topic has been deleted. Only users with topic management privileges can see it.
  • QT-static-prgmQ QT-static-prgm

    I thought i use Qt and not my own implementation to load an tga file. Now i notice that:

    from 5 loadings of the same file:
    QImage: ~428 ms
    My implementation: ~116,4 ms

    So i wonder that my way is more efficient than Qt.

    IDK who made those implementation, but if Qt is interested in my one you can use it.

    jsulmJ Offline
    jsulmJ Offline
    jsulm
    Lifetime Qt Champion
    wrote on last edited by jsulm
    #5

    @QT-static-prgm You shouldn't use words like "stupid" - you insinuate that Qt developers are stupid, this is rude. Especially without knowing why QImage is implemented in the way it is working. Don't you think there might be good reasons for that ( @Chris-Kawa pointed out some of them)?

    https://forum.qt.io/topic/113070/qt-code-of-conduct

    1 Reply Last reply
    8
    • BjornWB Offline
      BjornWB Offline
      BjornW
      wrote on last edited by
      #6

      The burn ^_^

      1 Reply Last reply
      1
      • kshegunovK Offline
        kshegunovK Offline
        kshegunov
        Moderators
        wrote on last edited by kshegunov
        #7

        As a side question, where's TARGA loading implemented in Qt? I peeked at the source yesterday but couldn't find neither a implementation in the QImageReader class nor a relevant plugin.

        Read and abide by the Qt Code of Conduct

        1 Reply Last reply
        0
        • M Offline
          M Offline
          mtrch
          wrote on last edited by
          #8

          As a side question, where's TARGA loading implemented in Qt?

          It is in QtImageFormats module/subrepo (not qtbase): http://code.qt.io/cgit/qt/qtimageformats.git/tree/src/plugins/imageformats/tga

          kshegunovK 1 Reply Last reply
          2
          • M mtrch

            As a side question, where's TARGA loading implemented in Qt?

            It is in QtImageFormats module/subrepo (not qtbase): http://code.qt.io/cgit/qt/qtimageformats.git/tree/src/plugins/imageformats/tga

            kshegunovK Offline
            kshegunovK Offline
            kshegunov
            Moderators
            wrote on last edited by
            #9

            I wanted to see what's so different, but I guess I didn't look quite far enough to find it. Thanks for the link!

            Read and abide by the Qt Code of Conduct

            1 Reply Last reply
            0
            • QT-static-prgmQ QT-static-prgm

              here you go : https://git.rwth-aachen.de/carstenf/OpenGL/blob/master/QtMeshViewer/Header/tga.h

              ? Offline
              ? Offline
              A Former User
              wrote on last edited by
              #10

              @QT-static-prgm Thx!

              1 Reply Last reply
              0
              • QT-static-prgmQ Offline
                QT-static-prgmQ Offline
                QT-static-prgm
                wrote on last edited by
                #11

                @Chris-Kawa yes I know the code is not perfect. I used it in an older version, before I used qt. When I switched to qt I used qimage but it does not support all my tga files, so the fastest solution was to copy over my old code in case that qimage failed. And then I noticed that my code was much faster. I'll look into your comments about my code and gonna rework it, since it is the bottleneck of my program.

                @jsulm I was just surprised that my imperfect code was that faster than the qt solution. You are right that stupid is the wrong word for it and I'm sorry for that choose. I hope that no one feels hurt because of that. It was not my intention. You guys from qt are doing a great job and I'm always impressed that for every problem I get, the solution is an already implemented qt function (calculating transform matrix for normal is in qt so much easier then in glm.)

                @kshegunov I'd be interested in the different, too. If you find it, please let me know. As I mentioned above there are some tga files even not loaded by qimage, but my code did.

                @Wieland you're welcome. I'm pretty sure I'm gonna improve this further, so you may wanna stay tuned on that link. You can get the future updates that way.

                1 Reply Last reply
                1
                • Chris KawaC Offline
                  Chris KawaC Offline
                  Chris Kawa
                  Lifetime Qt Champion
                  wrote on last edited by
                  #12

                  Looking at the Qt's code it seems it plays it just a little too safe. Apart from the necessary bounds and header integrity checking it reads the data byte by byte and verifies each and every one of those reads. That's most likely what's killing it. That implementation could use some love too.

                  1 Reply Last reply
                  2
                  • QT-static-prgmQ Offline
                    QT-static-prgmQ Offline
                    QT-static-prgm
                    wrote on last edited by QT-static-prgm
                    #13

                    i could improve the code using quint, qfile,.. and by handling the unsuccessful tries an other way. This decreases the time from ~142,5 to ~102,4.
                    Now i used scanline (thanks for the tipp) and now i'm somewhere around 89. But the images are no longer loaded correctly.

                    Maybe someone of you find the problem:

                    The Colors are wrong and it looks a bit wrong positioned, too.

                    Here the old code:

                    for (unsigned int y = 0; y < ui32Height; y++)
                    {
                    	for (unsigned int x = 0; x < ui32Width; x++)
                    	{
                    		int valr = vui8Pixels.at(y * ui32Width * ui32BpP / 8 + x * ui32BpP / 8 + 2);
                    		int valg = vui8Pixels.at(y * ui32Width * ui32BpP / 8 + x * ui32BpP / 8 + 1);
                    		int valb = vui8Pixels.at(y * ui32Width * ui32BpP / 8 + x * ui32BpP / 8);
                    		int vala = 255;
                    		if (ui32BpP == 32)
                    			vala = vui8Pixels.at(y * ui32Width * ui32BpP / 8 + x * ui32BpP / 8 + 3);
                    
                    		QColor value(valr, valg, valb, vala);
                    
                    		img.setPixel(x, ui32Width - 1 - y, value.rgba());
                    	}
                    }
                    

                    and here the new one

                    for (unsigned int y = 0; y < ui32Height; y++)
                    {
                    	QRgb* imgLine = (QRgb*)img.scanLine(y);
                    	for (unsigned int x = 0; x < ui32Width; x++)
                    	{
                    		int valr = vui8Pixels.at(y * ui32Width * ui32BpP / 8 + x * ui32BpP / 8 + 2);
                    		int valg = vui8Pixels.at(y * ui32Width * ui32BpP / 8 + x * ui32BpP / 8 + 1);
                    		int valb = vui8Pixels.at(y * ui32Width * ui32BpP / 8 + x * ui32BpP / 8);
                    		int vala = 255;
                    		if (ui32BpP == 32)
                    			vala = vui8Pixels.at(y * ui32Width * ui32BpP / 8 + x * ui32BpP / 8 + 3);
                    
                    		QColor value(valr, valg, valb, vala);
                    
                    		imgLine[x] = value.rgba();
                    	}
                    }
                    

                    ==EDIT==

                    i fixed it. For some reason the picture was mirrored and R and B was swapped. now it just takes ~78MS vs Qt 428MS o.O
                    https://git.rwth-aachen.de/carstenf/OpenGL/blob/master/QtMeshViewer/Header/tga.h

                    if you have further ideas about how to improve, let me know.

                    kshegunovK 1 Reply Last reply
                    0
                    • QT-static-prgmQ QT-static-prgm

                      i could improve the code using quint, qfile,.. and by handling the unsuccessful tries an other way. This decreases the time from ~142,5 to ~102,4.
                      Now i used scanline (thanks for the tipp) and now i'm somewhere around 89. But the images are no longer loaded correctly.

                      Maybe someone of you find the problem:

                      The Colors are wrong and it looks a bit wrong positioned, too.

                      Here the old code:

                      for (unsigned int y = 0; y < ui32Height; y++)
                      {
                      	for (unsigned int x = 0; x < ui32Width; x++)
                      	{
                      		int valr = vui8Pixels.at(y * ui32Width * ui32BpP / 8 + x * ui32BpP / 8 + 2);
                      		int valg = vui8Pixels.at(y * ui32Width * ui32BpP / 8 + x * ui32BpP / 8 + 1);
                      		int valb = vui8Pixels.at(y * ui32Width * ui32BpP / 8 + x * ui32BpP / 8);
                      		int vala = 255;
                      		if (ui32BpP == 32)
                      			vala = vui8Pixels.at(y * ui32Width * ui32BpP / 8 + x * ui32BpP / 8 + 3);
                      
                      		QColor value(valr, valg, valb, vala);
                      
                      		img.setPixel(x, ui32Width - 1 - y, value.rgba());
                      	}
                      }
                      

                      and here the new one

                      for (unsigned int y = 0; y < ui32Height; y++)
                      {
                      	QRgb* imgLine = (QRgb*)img.scanLine(y);
                      	for (unsigned int x = 0; x < ui32Width; x++)
                      	{
                      		int valr = vui8Pixels.at(y * ui32Width * ui32BpP / 8 + x * ui32BpP / 8 + 2);
                      		int valg = vui8Pixels.at(y * ui32Width * ui32BpP / 8 + x * ui32BpP / 8 + 1);
                      		int valb = vui8Pixels.at(y * ui32Width * ui32BpP / 8 + x * ui32BpP / 8);
                      		int vala = 255;
                      		if (ui32BpP == 32)
                      			vala = vui8Pixels.at(y * ui32Width * ui32BpP / 8 + x * ui32BpP / 8 + 3);
                      
                      		QColor value(valr, valg, valb, vala);
                      
                      		imgLine[x] = value.rgba();
                      	}
                      }
                      

                      ==EDIT==

                      i fixed it. For some reason the picture was mirrored and R and B was swapped. now it just takes ~78MS vs Qt 428MS o.O
                      https://git.rwth-aachen.de/carstenf/OpenGL/blob/master/QtMeshViewer/Header/tga.h

                      if you have further ideas about how to improve, let me know.

                      kshegunovK Offline
                      kshegunovK Offline
                      kshegunov
                      Moderators
                      wrote on last edited by
                      #14

                      @QT-static-prgm said in How stupid is QImage implemented??:

                      QRgb* imgLine = (QRgb*)img.scanLine(y);
                      

                      This is still a reinterpret cast, so you may run into problems on OSX vs Linux/Windows (see Chris's 3rd or 4th point on byte ordering).

                      QColor value(valr, valg, valb, vala);
                      

                      Don't convert into color (which is much more complex), rather use the qRgba() function.

                      Read and abide by the Qt Code of Conduct

                      1 Reply Last reply
                      1
                      • Chris KawaC Offline
                        Chris KawaC Offline
                        Chris Kawa
                        Lifetime Qt Champion
                        wrote on last edited by Chris Kawa
                        #15

                        If you want to go full speed ahead I would do a couple things:

                        • further specialize the algorithm for 24 and 32 bit
                        • explicitly handle the endianness so you don't have to go through it on a byte by byte level
                        • avoid additional allocations and buffers. Do everything in-place.

                        Here's a sample for the uncompressed 32bit little endian variant:

                        if (ui32BpP == 32) {
                            int lineWidth = ui32Width * ui32BpP / 8;
                            QImage img(ui32Width, ui32Height, QImage::Format_RGBA8888);
                            for (int i = ui32Height - 1; i >= 0; --i)
                                file.read(reinterpret_cast<char*>(img.scanLine(i)), lineWidth);
                        
                        #ifdef Q_LITTLE_ENDIAN
                               return qMove(img).rgbSwapped(); //qMove makes it use in-place swap instead of a copy
                        #elif Q_BIG_ENDIAN
                               swapTheBytesInOtherWay(img);
                               return img;
                        #endif
                        }
                        

                        For 24 bit image you can use QImage::Format_RGB888 internal format so you can do it as fast.
                        I got another 4x speedup compared to your version. You could probably optimize the rgb swap further with some clever SSE intrinsics.

                        1 Reply Last reply
                        4
                        • QT-static-prgmQ Offline
                          QT-static-prgmQ Offline
                          QT-static-prgm
                          wrote on last edited by
                          #16

                          Gonna try this tomorrow. But I think swapping r and b won't be the problem. But tga holds the data bgra and qrgb is defined argb so it's not done by swapping just r and b. A needs to be moved from back to front

                          1 Reply Last reply
                          0
                          • Chris KawaC Offline
                            Chris KawaC Offline
                            Chris Kawa
                            Lifetime Qt Champion
                            wrote on last edited by
                            #17

                            @QT-static-prgm said in How stupid is QImage implemented??:

                            But tga holds the data bgra and qrgb is defined argb so it's not done by swapping just r and b. A needs to be moved from back to front

                            It doesn't matter how QRgb stores the data. I'm not using it.
                            tga stores the bytes in little endian ARGB so the actual order of bytes in the file is [0]BB [1]GG [2]RR [3]AA.
                            I'm using internal format Format_RGBA8888 and on little endian platform this means the actual bytes in memory are stored as [0]RR [1]GG [2]BB [3]AA. As you can see to convert one to the other swapping R and B is enough.
                            If you want to go through QRgb then yes, you need extra work to put the bytes in right order, but it's unnecessary here and makes it confusing.

                            1 Reply Last reply
                            2
                            • QT-static-prgmQ Offline
                              QT-static-prgmQ Offline
                              QT-static-prgm
                              wrote on last edited by
                              #18

                              wow, thank you. i'm always confused by those format things.

                              This is the final code. it can decide whether it is 32 or 24 and chooses the correct internal format.
                              i don't really wanna touch the compressed part, since it is not relevant for me, but maybe i'll speed it up in the future.

                              if (ui32PicType == 2 && (ui32BpP == 24 || ui32BpP == 32))
                              {
                              	img = QImage(ui32Width, ui32Height, ui32BpP == 32 ? QImage::Format_RGBA8888 : QImage::Format_RGB888);
                              	int lineWidth = ui32Width * ui32BpP / 8;
                              	for (int i = ui32Height - 1; i >= 0; --i)
                              		file.read(reinterpret_cast<char*>(img.scanLine(i)), lineWidth);
                              }
                              

                              This code makes ~3-4ms!!!!

                              1 Reply Last reply
                              0
                              • QT-static-prgmQ Offline
                                QT-static-prgmQ Offline
                                QT-static-prgm
                                wrote on last edited by
                                #19

                                @Chris-Kawa Any ideas how to read directly to QString??

                                Before i had something like this:

                                struct{
                                    char name[5];
                                    ...
                                }
                                
                                ...
                                m_file.read(F2V(tmp_header->name[0]), sizeof(tmp_header->name) - 1);
                                
                                

                                so i have my struct with an array of the size 5 and read in a string of the size 4 (last need to be the \0 character)

                                Now i'd like to change it to this:

                                struct{
                                    QString name;
                                    ...
                                }
                                
                                ...
                                tmp_header->name.reserve(4);
                                m_file.read(F2V(tmp_header->name), sizeof(tmp_header->name) );
                                
                                

                                not sure if it needs to be 4 or 5, but non of them do their work. Already tried QBytearray, but this didn't worked either.

                                1 Reply Last reply
                                0
                                • Chris KawaC Offline
                                  Chris KawaC Offline
                                  Chris Kawa
                                  Lifetime Qt Champion
                                  wrote on last edited by Chris Kawa
                                  #20

                                  QString uses variable length Unicode representation with 16bit codepoints. Since you've got an 8bit string you can't read directly into a QString. You'll need to convert.
                                  The easiest way to do it is:

                                  tmp_header->name = QString::fromUtf8(m_file.read(4));     //if your string is Utf-8 or ASCII
                                  //or
                                  tmp_header->name = QString::fromLatin1(m_file.read(4));   //if your string is Latin-1 or ASCII
                                  

                                  This variant of QFile::read will allocate new QByteArray every time so if you want to avoid that you can create one yourself and reuse it:

                                  QByteArray arr(4, 0);
                                  
                                  m_file.read(arr.data(), 4);
                                  tmp_header->name = QString::fromUtf8(arr);
                                  

                                  Btw. Many of Qt data classes like QByteArray or QString are non-trivial and use implicit sharing so just directly casting them to the data type pointer is an error (I assume your F2V does something like that?). You'll get a pointer that points to some internal implementation detail not to the data. Qt's storage classes have access methods (usually called data() and constData()) that give you pointers to the underlying storage.

                                  1 Reply Last reply
                                  1
                                  • QT-static-prgmQ Offline
                                    QT-static-prgmQ Offline
                                    QT-static-prgm
                                    wrote on last edited by
                                    #21

                                    Define F2V(value) reinterpret_cast<char*>(&value) is just an "shortcut" that makes the code more readable in my opinion.

                                    1 Reply Last reply
                                    0
                                    • QT-static-prgmQ Offline
                                      QT-static-prgmQ Offline
                                      QT-static-prgm
                                      wrote on last edited by
                                      #22

                                      @Chris-Kawa i tried those variants:

                                       struct{
                                          QByteArray name = QByteArray(4,0);
                                          ...
                                      }
                                      ...
                                      m_file.read(tmp_header->name.data(), 4);
                                      

                                      ~90ms

                                       struct{
                                          QString name;
                                          ...
                                      }
                                      ...
                                      tmp_header->name = QString::fromUtf8(m_file.read(4));     
                                      

                                      ~100ms

                                       struct{
                                          QString name;
                                          ...
                                      }
                                      ...
                                      char tmpName[5] = { 0 };
                                      m_file.read(F2V(tmpName[0]), sizeof(tmpName) -1);
                                      tmp_header->name = QString(tmpName);
                                      

                                      ~20ms

                                      So both are not fast enough ;)

                                      1 Reply Last reply
                                      0

                                      • Login

                                      • Login or register to search.
                                      • First post
                                        Last post
                                      0
                                      • Categories
                                      • Recent
                                      • Tags
                                      • Popular
                                      • Users
                                      • Groups
                                      • Search
                                      • Get Qt Extensions
                                      • Unsolved