Some changes

[skynet1248]

Hi I like this bittorrent client and I would be grateful if author may see these improvements / changes.

Old:

New:


Project Diff from QtCreator:

Code: Select all

Index: src/properties/propertieswidget.cpp
===================================================================
--- src/properties/propertieswidget.cpp	(wersja 5198)
+++ src/properties/propertieswidget.cpp	(kopia robocza)
@@ -365,7 +365,7 @@
           std::vector<int> avail;
           h.piece_availability(avail);
           pieces_availability->setAvailability(avail);
-          avail_average_lbl->setText(QString::number(h.status().distributed_copies, 'f', 1));
+          avail_average_lbl->setText(QString::number(h.status().distributed_copies, 'f', 3));
         } else {
           showPiecesAvailability(false);
         }
Index: src/properties/downloadedpiecesbar.h
===================================================================
--- src/properties/downloadedpiecesbar.h	(wersja 5198)
+++ src/properties/downloadedpiecesbar.h	(kopia robocza)
@@ -61,65 +61,25 @@
       pixmap = pix;
     } else {
       const qulonglong nb_pieces = pieces.size();
-      // Reduce the number of pieces before creating the pixmap
-      // otherwise it can crash when there are too many pieces
-      const uint w = width();
-      if(nb_pieces > w) {
-        const uint ratio = floor(nb_pieces/(double)w);
-        libtorrent::bitfield scaled_pieces(ceil(nb_pieces/(double)ratio), false);
-        libtorrent::bitfield scaled_downloading(ceil(nb_pieces/(double)ratio), false);
-        uint scaled_index = 0;
-        for(qulonglong i=0; i<nb_pieces; i+= ratio) {
-          bool have = true;
-          for(qulonglong j=i; j<qMin(i+ratio, nb_pieces); ++j) {
-            if(!pieces[i]) { have = false; break; }
-          }
-          if(have) {
-            scaled_pieces.set_bit(scaled_index);
+      QPixmap pix = QPixmap(nb_pieces, 1);
+      //pix.fill();
+      QPainter painter(&pix);
+      QPen penDownloaded(QColor(0x2b, 0x2b, 0xff));
+      for(uint i=0; i<nb_pieces; ++i) {
+        if(pieces[i]) {
+          painter.setPen(penDownloaded);
+        } else {
+          if(downloading_pieces[i]) {
+            // really hard to see yellow
+            painter.setPen(Qt::green);
           } else {
-            bool downloading = false;
-            for(qulonglong j=i; j<qMin(i+ratio, nb_pieces); ++j) {
-              if(downloading_pieces[i]) { downloading = true; break; }
-            }
-            if(downloading)
-              scaled_downloading.set_bit(scaled_index);
+            painter.setPen(Qt::white);
           }
-          ++scaled_index;
         }
-        QPixmap pix = QPixmap(scaled_pieces.size(), 1);
-        //pix.fill();
-        QPainter painter(&pix);
-        for(uint i=0; i<scaled_pieces.size(); ++i) {
-          if(scaled_pieces[i]) {
-            painter.setPen(Qt::blue);
-          } else {
-            if(scaled_downloading[i]) {
-              painter.setPen(Qt::yellow);
-            } else {
-              painter.setPen(Qt::white);
-            }
-          }
-          painter.drawPoint(i,0);
-        }
-        pixmap = pix;
-      } else {
-        QPixmap pix = QPixmap(pieces.size(), 1);
-        //pix.fill();
-        QPainter painter(&pix);
-        for(uint i=0; i<pieces.size(); ++i) {
-          if(pieces[i]) {
-            painter.setPen(Qt::blue);
-          } else {
-            if(downloading_pieces[i]) {
-              painter.setPen(Qt::yellow);
-            } else {
-              painter.setPen(Qt::white);
-            }
-          }
-          painter.drawPoint(i,0);
-        }
-        pixmap = pix;
+        painter.drawPoint(i,0);
       }
+      // Qt::SmoothTransformation looks bad, TODO: write better SmoothTransformation loop
+      pixmap = pix.scaledToWidth(width(), Qt::FastTransformation);
     }
     update();
   }
Index: src/properties/pieceavailabilitybar.h
===================================================================
--- src/properties/pieceavailabilitybar.h	(wersja 5198)
+++ src/properties/pieceavailabilitybar.h	(kopia robocza)
@@ -37,6 +37,7 @@
 #include <QColor>
 #include <numeric>
 #include <math.h>
+#include <algorithm>
 
 #define BAR_HEIGHT 18
 
@@ -53,50 +54,26 @@
   }
 
   void setAvailability(const std::vector<int>& avail) {
-    qreal average = 0;
     if(avail.empty()) {
       // Empty bar
       QPixmap pix = QPixmap(1, 1);
       pix.fill();
       pixmap = pix;
     } else {
-      // Look for maximum value
       const qulonglong nb_pieces = avail.size();
-      average = std::accumulate(avail.begin(), avail.end(), 0)/(double)nb_pieces;
-      // Reduce the number of pieces before creating the pixmap
-      // otherwise it can crash when there are too many pieces
-      const uint w = width();
-      if(nb_pieces > w) {
-        const qulonglong ratio = floor(nb_pieces/(double)w);
-        std::vector<int> scaled_avail;
-        scaled_avail.reserve(ceil(nb_pieces/(double)ratio));
-        for(qulonglong i=0; i<nb_pieces; i+= ratio) {
-          /*qulonglong j = i;
-          qulonglong sum = avail[i];
-          for(j=i+1; j<qMin(i+ratio, nb_pieces); ++j) {
-            sum += avail[j];
-          }
-          scaled_avail.push_back(sum/(qMin(ratio, nb_pieces-i)));*/
-          // XXX: Do not compute the average to save cpu
-          scaled_avail.push_back(avail[i]);
-        }
-        QPixmap pix = QPixmap(scaled_avail.size(), 1);
-        //pix.fill();
+      // Look for maximum value
+      int max = *std::max_element(avail.begin(), avail.end());
+      QPixmap pix = QPixmap(nb_pieces, 1);
+      if(max == 0) {
+        pix.fill();
+      } else{
         QPainter painter(&pix);
-        for(qulonglong i=0; i < scaled_avail.size(); ++i) {
-          painter.setPen(getPieceColor(scaled_avail[i], average));
-          painter.drawPoint(i,0);
-        }
-        pixmap = pix;
-      } else {
-        QPixmap pix = QPixmap(nb_pieces, 1);
-        //pix.fill();
-        QPainter painter(&pix);
         for(qulonglong i=0; i < nb_pieces; ++i) {
-          painter.setPen(getPieceColor(avail[i], average));
-          painter.drawPoint(i,0);
+          uint rg = 0xff - (0xff * avail[i]/max);
+          painter.setPen(QColor(rg, rg, 0xff));
+          painter.drawPoint(i, 0);
         }
-        pixmap = pix;
+        pixmap = pix.scaledToWidth(width(), Qt::FastTransformation);
       }
     }
     update();
@@ -113,17 +90,6 @@
     QPainter painter(this);
     painter.drawPixmap(rect(), pixmap);
   }
-
-  QColor getPieceColor(int avail, qreal average) {
-    if(!avail) return Qt::white;
-    //qDebug("avail: %d/%d", avail, max_avail);
-    qreal fraction = 100.*average/avail;
-    if(fraction < 100)
-      fraction *= 0.8;
-    else
-      fraction *= 1.2;
-    return QColor(Qt::blue).lighter(fraction);
-  }
 };
 
 #endif // PIECEAVAILABILITYBAR_H
Index: src/properties/proptabbar.cpp
===================================================================
--- src/properties/proptabbar.cpp	(wersja 5198)
+++ src/properties/proptabbar.cpp	(kopia robocza)
@@ -32,6 +32,7 @@
 #include <QPushButton>
 #include <QSpacerItem>
 #include <QKeySequence>
+#include <QPalette>
 
 #include "proptabbar.h"
 #include "iconprovider.h"
@@ -40,8 +41,10 @@
 #define DEFAULT_BUTTON_CSS "QPushButton {border: 1px solid rgb(85, 81, 91);border-radius: 3px;padding: 2px; margin-left: 8px; margin-right: 8px;}"
 #define SELECTED_BUTTON_CSS "QPushButton {border: 1px solid rgb(85, 81, 91);border-radius: 3px;padding: 2px;background-color: rgb(255, 208, 105); margin-left: 8px; margin-right: 8px;}"
 #else
-#define DEFAULT_BUTTON_CSS "QPushButton {border: 1px solid rgb(85, 81, 91);border-radius: 3px;padding: 2px; margin-left: 3px; margin-right: 3px;}"
-#define SELECTED_BUTTON_CSS "QPushButton {border: 1px solid rgb(85, 81, 91);border-radius: 3px;padding: 2px;background-color: rgb(255, 208, 105); margin-left: 3px; margin-right: 3px;}"
+#define DEFAULT_BUTTON_CSS "QPushButton {border: 1px solid #888;border-radius: 2px;padding: 4px; margin-left: 0px; margin-right: 3px;}"
+#define SELECTED_BUTTON_CSS "QPushButton {border: 1px solid #888;border-radius: 2px;padding: 4px;background-color: rgb(112, 168, 222); margin-left: 0px; margin-right: 3px;}"
+#define SELECTED_BUTTON_CSS2a "QPushButton {border: 1px solid #888;border-radius: 2px;padding: 4px;background-color: rgb("
+#define SELECTED_BUTTON_CSS2b "); margin-left: 0px; margin-right: 3px;}"
 #endif
 
 const int BTN_ICON_SIZE = 16;
@@ -50,7 +53,7 @@
   QHBoxLayout(parent), m_currentIndex(-1)
 {
   m_btnGroup = new QButtonGroup(this);
-  setContentsMargins(5, 4, 5, 2);
+  setContentsMargins(0, 4, 5, 7);
   // General tab
   QPushButton *main_infos_button = new QPushButton(IconProvider::instance()->getIcon("document-properties"), tr("General"), parent);
   main_infos_button->setShortcut(QKeySequence(QString::fromUtf8("Alt+P")));
@@ -118,7 +121,10 @@
     emit visibilityToggled(true);
   }
   // Select the new button
-  m_btnGroup->button(index)->setStyleSheet(SELECTED_BUTTON_CSS);
+//  m_btnGroup->button(index)->setStyleSheet(SELECTED_BUTTON_CSS);
+  QColor hC = m_btnGroup->button(index)->palette().highlight().color();
+  QString b = QString(SELECTED_BUTTON_CSS2a) + QString::number(hC.red()) + QString(", ") + QString::number(hC.green()) + QString(", ") + QString::number(hC.blue()) + QString(SELECTED_BUTTON_CSS2b);
+  m_btnGroup->button(index)->setStyleSheet(b);
   m_currentIndex = index;
   // Emit the signal
   emit tabChanged(index);

[christophe.dumez]

Thanks! I'm currently reviewing your patch for inclusion.

I like the CSS part but I'd rather use simply:
#define SELECTED_BUTTON_CSS "QPushButton {border: 1px solid #888;border-radius: 2px; padding: 4px;background-color: palette(highlight); margin-left: 0px; margin-right: 3px;}"

Some hidden Qt stylesheet feature (background-color: palette(highlight))

Regarding the piece bar code optimization, I'm need to have a closer look but I'm pretty sure you did not understand the purpose of allocating a small QPixmap. It is not simply for efficiency, but actually to avoid an overflow with big torrents.

QPixmap takes in parameter integers for width and height. In your case, the width is the number of pieces in the torrent.
However, MAX INT is 32767.

A torrent piece size can be quite small, for example 64kb.
32767*64 = 2097088 Kb = ~2 Gb
Thus, your code will crash if a torrent has a size greater than 2Gb and a piece size <= 64kb (for example).

I will still have a look at this part of your patch and see what I can safely include.

[skynet1248]

>> Some hidden Qt stylesheet feature (background-color: palette(highlight))
damn smart

About QPixmap I also testing alternative algorithm with loop uses to calculate hmm completion per pixel[now I checking if there are no loses pieces].

[christophe.dumez]

I have applied part of your patch in SVN.

[skynet1248]

I add scaling pieces and downloading_pieces to width() on vectors.
Ideal option should be updated only changed pieces, but it could be complicated.

Question if is possible in qBittorrent send my IP to tracker?
Because I have forwarded some ports on other IP address, other clients see me from my normal IP address and I have not direct connection.

ps. sorry for my english

Code: Select all

Index: src/properties/downloadedpiecesbar.h
===================================================================
--- src/properties/downloadedpiecesbar.h	(wersja 5210)
+++ src/properties/downloadedpiecesbar.h	(kopia robocza)
@@ -37,6 +37,7 @@
 #include <QPixmap>
 #include <libtorrent/bitfield.hpp>
 #include <math.h>
+#include <vector>
 
 #define BAR_HEIGHT 18
 
@@ -53,6 +54,54 @@
     setFixedHeight(BAR_HEIGHT);
   }
 
+  // least common multiple
+  qlonglong lcm(qlonglong a, qlonglong b)
+  {
+    if (a < 0 || b < 0)
+      return NULL;
+    qlonglong addedA = a;
+    qlonglong addedB = b;
+    for(;;)
+    {
+      if(a < b)
+        a += addedA;
+      else if(a > b)
+        b += addedB;
+      else if (a < 0 || b < 0) // secure for infinite
+        return NULL;
+      else if (a == b)
+        return a;
+    }
+  }
+
+  float resizeVectorAccumulate(const libtorrent::bitfield &sourceArray, int multipier, int start, int end)
+  {
+    int result = 0;
+    for(int i = start; i < end; i++){
+      result += sourceArray[i/multipier];
+    }
+    return result/(float)multipier;
+  }
+
+  // scaling vectors works in booth directions[small -> big, big -> small] and is fast.
+  std::vector<float> resizeVector(const libtorrent::bitfield &oldArray, int newLength){
+    std::vector<float> newArray;
+
+    int _lcm = lcm(oldArray.size(), newLength);
+    int ratio_arr1_to_lcm = _lcm / oldArray.size();
+    int ratio_arr2_to_lcm = _lcm / newLength;
+
+    for (int i = 0; i < newLength; i++){
+      int start = i*ratio_arr2_to_lcm;
+      int end = (i+1)*ratio_arr2_to_lcm;
+      float newValue = resizeVectorAccumulate(oldArray, ratio_arr1_to_lcm, start, end);
+      // corection level, should be always <=1
+      newValue *= ratio_arr1_to_lcm/(float)ratio_arr2_to_lcm;
+      newArray.push_back(newValue);
+    }
+    return newArray;
+  }
+
   void setProgress(const libtorrent::bitfield &pieces, const libtorrent::bitfield &downloading_pieces) {
     if(pieces.empty()) {
       // Empty bar
@@ -60,36 +109,31 @@
       pix.fill();
       pixmap = pix;
     } else {
-      const qulonglong nb_pieces = pieces.size();
-      // Reduce the number of pieces before creating the pixmap
-      // otherwise it can crash when there are too many pieces
-      const uint w = width();
-      if(nb_pieces > w) {
-        const uint ratio = floor(nb_pieces/(double)w);
-        libtorrent::bitfield scaled_pieces(ceil(nb_pieces/(double)ratio), false);
-        libtorrent::bitfield scaled_downloading(ceil(nb_pieces/(double)ratio), false);
-        uint scaled_index = 0;
-        for(qulonglong i=0; i<nb_pieces; i+= ratio) {
-          bool have = true;
-          for(qulonglong j=i; j<qMin(i+ratio, nb_pieces); ++j) {
-            if(!pieces[i]) { have = false; break; }
-          }
-          if(have) {
-            scaled_pieces.set_bit(scaled_index);
-          } else {
-            bool downloading = false;
-            for(qulonglong j=i; j<qMin(i+ratio, nb_pieces); ++j) {
-              if(downloading_pieces[i]) { downloading = true; break; }
-            }
-            if(downloading)
-              scaled_downloading.set_bit(scaled_index);
-          }
-          ++scaled_index;
-        }
-        updatePixmap(scaled_pieces, scaled_downloading);
-      } else {
-        updatePixmap(pieces, downloading_pieces);
+
+      // bitfield resized to width()
+      std::vector<float> resized_pieces = resizeVector(pieces, width());
+      std::vector<float> resized_downloading_pieces = resizeVector(downloading_pieces, width());
+
+      QPixmap pix = QPixmap(width(), 1);
+      pix.fill();
+      QPainter painter(&pix);
+      QColor color_pieces = QColor::fromRgb(0x2b2bff);
+      QColor color_downloading_pieces = QColor::fromRgb(0x05cd00);
+      for(int i=0; i<width(); ++i) {
+        // pix.fill(); or next 2 lines
+        // painter.setPen(Qt::white);
+        // painter.drawPoint(i, 0);
+
+        // this part is not as slow as it looks,
+        // loop length is small and we do not need to carry about scaling
+        color_pieces.setAlphaF(resized_pieces[i]);
+        painter.setPen(color_pieces);
+        painter.drawPoint(i, 0);
+        color_downloading_pieces.setAlphaF(resized_downloading_pieces[i]);
+        painter.setPen(color_downloading_pieces);
+        painter.drawPoint(i, 0);
       }
+      pixmap = pix;
     }
     update();
   }

[christophe.dumez]

The button text should also be switched to highlighted-text palette. I have just fixed that in SVN. I have also made some margin fixes here and there so that everything is aligned. Thanks for your work.

[christophe.dumez]

[quote=""skynet1248""]
Question if is possible in qBittorrent send my IP to tracker?
Because I have forwarded some ports on other IP address, other clients see me from my normal IP address and I have not direct connection.
[/quote]
I have actually implemented that today in SVN/trunk, what a coincidence

[skynet1248]

[quote=""christophe.dumez""]I have actually implemented that today in SVN/trunk, what a coincidence [/quote]
Checked now: :mrgreen: Green Icon and direct connect, Yeah.

[christophe.dumez]

Are you sure your code is really faster than mine? I don't see much difference (I already simplified my code in SVN, avoiding code duplication).

Also, It looks like you still have overflow issues because you are using 32bits INTs to iterate over a vector that may contain more than INT_MAX values (this is the whole point to scaling down the vector size).

[skynet1248]

Have similar speed[mine possible bit slower now, I compiling profiler for Qt-Creator to check performance] but works with smooth.
If you create pixmap with all pieces and scale to width()[to have a smooth], my solution will be faster.

Int's 32bit can be corrected on the long or qlong* type. Sorry about that.

[skynet1248]

I increased the speed of drawing the downloadedpiecesbar again.

The changes that you have added to SVN, reduce the draw time to 1.77 million clock cycles, patch which is proposed to reduce clock cycles to 525 thousand.
I measured the function of setProgress.

Code: Select all

Index: src/properties/downloadedpiecesbar.h
===================================================================
--- src/properties/downloadedpiecesbar.h	(wersja 5229)
+++ src/properties/downloadedpiecesbar.h	(kopia robocza)
@@ -40,12 +40,16 @@
 
 #define BAR_HEIGHT 18
 
+#define COLOR_DOWNLOADED 0x0000ff
+#define COLOR_DOWNLOADING 0x00ff00
+#define COLOR_BACKGROUND 0xffffff
+
 class DownloadedPiecesBar: public QWidget {
   Q_OBJECT
   Q_DISABLE_COPY(DownloadedPiecesBar)
 
 private:
-  QPixmap pixmap;
+      QImage pixmap;
 
 
 public:
@@ -56,8 +60,8 @@
   void setProgress(const libtorrent::bitfield &pieces, const libtorrent::bitfield &downloading_pieces) {
     if(pieces.empty()) {
       // Empty bar
-      QPixmap pix = QPixmap(1, 1);
-      pix.fill();
+      QImage pix = QImage(pieces.size(), 1, QImage::Format_RGB888); // really is needed a new Pixmap?
+      pix.fill(COLOR_BACKGROUND);
       pixmap = pix;
     } else {
       const qulonglong nb_pieces = pieces.size();
@@ -69,17 +73,20 @@
         libtorrent::bitfield scaled_pieces(ceil(nb_pieces/(double)ratio), false);
         libtorrent::bitfield scaled_downloading(ceil(nb_pieces/(double)ratio), false);
         uint scaled_index = 0;
+        qulonglong len;
         for(qulonglong i=0; i<nb_pieces; i+= ratio) {
           bool have = true;
-          for(qulonglong j=i; j<qMin(i+ratio, nb_pieces); ++j) {
-            if(!pieces[i]) { have = false; break; }
+          len = qMin(i+ratio, nb_pieces);
+          for(qulonglong j=i; j<len; ++j) {
+            if(!pieces.get_bit(i)) { have = false; break; }
           }
           if(have) {
             scaled_pieces.set_bit(scaled_index);
           } else {
             bool downloading = false;
-            for(qulonglong j=i; j<qMin(i+ratio, nb_pieces); ++j) {
-              if(downloading_pieces[i]) { downloading = true; break; }
+            len = qMin(i+ratio, nb_pieces);
+            for(qulonglong j=i; j<len; ++j) {
+              if(downloading_pieces.get_bit(i)) { downloading = true; break; }
             }
             if(downloading)
               scaled_downloading.set_bit(scaled_index);
@@ -95,7 +102,7 @@
   }
 
   void clear() {
-    pixmap = QPixmap();
+    pixmap = QImage();
     update();
   }
 
@@ -103,25 +110,23 @@
   void paintEvent(QPaintEvent *) {
     if(pixmap.isNull()) return;
     QPainter painter(this);
-    painter.drawPixmap(rect(), pixmap);
+    painter.drawImage(rect(), pixmap);
   }
 
 private:
   void updatePixmap(const libtorrent::bitfield &pieces, const libtorrent::bitfield &downloading_pieces) {
-    QPixmap pix = QPixmap(pieces.size(), 1);
-    //pix.fill();
-    QPainter painter(&pix);
-    for(uint i=0; i<pieces.size(); ++i) {
-      if(pieces[i]) {
-        painter.setPen(Qt::blue);
+    QImage pix = QImage(pieces.size(), 1, QImage::Format_RGB888);
+    uint len = pieces.size();
+    for(uint i=0; i<len; ++i) {
+      if(pieces.get_bit(i)) {
+        pix.setPixel(i, 0, COLOR_DOWNLOADED);
       } else {
-        if(downloading_pieces[i]) {
-          painter.setPen(Qt::green);
+        if(downloading_pieces.get_bit(i)) {
+          pix.setPixel(i, 0, COLOR_DOWNLOADING);
         } else {
-          painter.setPen(Qt::white);
+          pix.setPixel(i, 0, COLOR_BACKGROUND);
         }
       }
-      painter.drawPoint(i,0);
     }
     pixmap = pix;
   }