Store the floating point proportional playlist column widths directly instead of storing the integer pixel widths then recalculating the floating point values on startup, which would lead to floating point errors over time. Fixes a bug where columns would slowly change size the more times you opened and closed clementine.

src/playlist/playlistview.cpp

@@ -20,6 +20,7 @@
 #include "playlistdelegates.h"
 #include "playlistheader.h"
 #include "playlistview.h"
+#include "core/logging.h"
 
 #include <QCleanlooksStyle>
 #include <QPainter>
@@ -86,6 +87,7 @@ PlaylistView::PlaylistView(QWidget *parent)
     playlist_(NULL),
     header_(new PlaylistHeader(Qt::Horizontal, this)),
     setting_initial_header_layout_(false),
+    upgrading_from_qheaderview_(false),
     read_only_settings_(true),
     glow_enabled_(true),
     currently_glowing_(false),
@@ -214,27 +216,34 @@ void PlaylistView::LoadGeometry() {
   QSettings settings;
   settings.beginGroup(Playlist::kSettingsGroup);
 
-  if (!header_->restoreState(settings.value("state").toByteArray())) {
+  QByteArray state(settings.value("state").toByteArray());
-    header_->HideSection(Playlist::Column_Disc);
+  if (!header_->RestoreState(state)) {
-    header_->HideSection(Playlist::Column_Year);
+    // Maybe we're upgrading from a version that persisted the state with
-    header_->HideSection(Playlist::Column_Genre);
+    // QHeaderView.
-    header_->HideSection(Playlist::Column_BPM);
+    if (!header_->restoreState(state)) {
-    header_->HideSection(Playlist::Column_Bitrate);
+      header_->HideSection(Playlist::Column_Disc);
-    header_->HideSection(Playlist::Column_Samplerate);
+      header_->HideSection(Playlist::Column_Year);
-    header_->HideSection(Playlist::Column_Filename);
+      header_->HideSection(Playlist::Column_Genre);
-    header_->HideSection(Playlist::Column_Filesize);
+      header_->HideSection(Playlist::Column_BPM);
-    header_->HideSection(Playlist::Column_Filetype);
+      header_->HideSection(Playlist::Column_Bitrate);
-    header_->HideSection(Playlist::Column_DateCreated);
+      header_->HideSection(Playlist::Column_Samplerate);
-    header_->HideSection(Playlist::Column_DateModified);
+      header_->HideSection(Playlist::Column_Filename);
-    header_->HideSection(Playlist::Column_AlbumArtist);
+      header_->HideSection(Playlist::Column_Filesize);
-    header_->HideSection(Playlist::Column_Composer);
+      header_->HideSection(Playlist::Column_Filetype);
-    header_->HideSection(Playlist::Column_Rating);
+      header_->HideSection(Playlist::Column_DateCreated);
-    header_->HideSection(Playlist::Column_PlayCount);
+      header_->HideSection(Playlist::Column_DateModified);
-    header_->HideSection(Playlist::Column_SkipCount);
+      header_->HideSection(Playlist::Column_AlbumArtist);
-    header_->HideSection(Playlist::Column_LastPlayed);
+      header_->HideSection(Playlist::Column_Composer);
+      header_->HideSection(Playlist::Column_Rating);
+      header_->HideSection(Playlist::Column_PlayCount);
+      header_->HideSection(Playlist::Column_SkipCount);
+      header_->HideSection(Playlist::Column_LastPlayed);
 
-    header_->moveSection(header_->visualIndex(Playlist::Column_Track), 0);
+      header_->moveSection(header_->visualIndex(Playlist::Column_Track), 0);
-    setting_initial_header_layout_ = true;
+      setting_initial_header_layout_ = true;
+    } else {
+      upgrading_from_qheaderview_ = true;
+    }
   }
 
   // New columns that we add are visible by default if the user has upgraded
@@ -272,7 +281,7 @@ void PlaylistView::SaveGeometry() {
 
   QSettings settings;
   settings.beginGroup(Playlist::kSettingsGroup);
-  settings.setValue("state", header_->saveState());
+  settings.setValue("state", header_->SaveState());
   settings.setValue("state_version", kStateVersion);
 }
 
@@ -861,7 +870,11 @@ void PlaylistView::ReloadSettings() {
   s.beginGroup(Playlist::kSettingsGroup);
   glow_enabled_ = s.value("glow_effect", true).toBool();
   background_enabled_ = s.value("bg_enabled", true).toBool();
-  header_->SetStretchEnabled(s.value("stretch", true).toBool());
+
+  if (setting_initial_header_layout_ || upgrading_from_qheaderview_) {
+    header_->SetStretchEnabled(s.value("stretch", true).toBool());
+    upgrading_from_qheaderview_ = false;
+  }
 
   if (currently_glowing_ && glow_enabled_ && isVisible())
     StartGlowing();
@@ -887,7 +900,6 @@ void PlaylistView::SaveSettings() {
   QSettings s;
   s.beginGroup(Playlist::kSettingsGroup);
   s.setValue("glow_effect", glow_enabled_);
-  s.setValue("stretch", header_->is_stretch_enabled());
   s.setValue("column_alignments", QVariant::fromValue(playlist_->column_alignments()));
   s.setValue("bg_enabled", background_enabled_);
 

src/playlist/playlistview.h

@@ -152,6 +152,7 @@ class PlaylistView : public QTreeView {
   Playlist* playlist_;
   PlaylistHeader* header_;
   bool setting_initial_header_layout_;
+  bool upgrading_from_qheaderview_;
   bool read_only_settings_;
 
   bool background_enabled_;

src/widgets/stretchheaderview.cpp

@@ -16,15 +16,16 @@
 */
 
 #include "stretchheaderview.h"
+#include "core/logging.h"
 
 #include <QDataStream>
-#include <QtDebug>
 
 #include <algorithm>
 #include <cmath>
 #include <numeric>
 
 const int StretchHeaderView::kMinimumColumnWidth = 10;
+const int StretchHeaderView::kMagicNumber = 0x502c950f;
 
 StretchHeaderView::StretchHeaderView(Qt::Orientation orientation, QWidget* parent)
   : QHeaderView(orientation, parent),
@@ -47,8 +48,9 @@ void StretchHeaderView::NormaliseWidths(const QList<int>& sections) {
   if (!stretch_enabled_)
     return;
 
-  float total_sum = std::accumulate(column_widths_.begin(), column_widths_.end(), 0.0);
+  const ColumnWidthType total_sum =
-  float selected_sum = total_sum;
+      std::accumulate(column_widths_.begin(), column_widths_.end(), 0.0);
+  ColumnWidthType selected_sum = total_sum;
 
   if (!sections.isEmpty()) {
     selected_sum = 0.0;
@@ -57,8 +59,8 @@ void StretchHeaderView::NormaliseWidths(const QList<int>& sections) {
         selected_sum += column_widths_[i];
   }
 
-  if (total_sum != 0.0 && !qFuzzyCompare(total_sum, 1.0f)) {
+  if (total_sum != 0.0 && !qFuzzyCompare(total_sum, 1.0)) {
-    const float mult = (selected_sum + (1.0 - total_sum)) / selected_sum;
+    const ColumnWidthType mult = (selected_sum + (1.0 - total_sum)) / selected_sum;
     for (int i=0 ; i<column_widths_.count() ; ++i) {
       if (sections.isEmpty() || sections.contains(i))
         column_widths_[i] *= mult;
@@ -70,10 +72,10 @@ void StretchHeaderView::UpdateWidths(const QList<int>& sections) {
   if (!stretch_enabled_)
     return;
 
-  float total_w = 0.0;
+  ColumnWidthType total_w = 0.0;
 
   for (int i=0 ; i<column_widths_.count() ; ++i) {
-    const float w = column_widths_[i];
+    const ColumnWidthType w = column_widths_[i];
     int pixels = w * width();
 
     if (pixels != 0 && total_w - int(total_w) > 0.5)
@@ -168,7 +170,7 @@ void StretchHeaderView::SectionResized(int logical, int, int new_size) {
 
   if (in_mouse_move_event_) {
     // Update this section's proportional width
-    column_widths_[logical] = float(new_size) / width();
+    column_widths_[logical] = ColumnWidthType(new_size) / width();
 
     // Find the visible sections to the right of the section that's being resized
     int visual = visualIndex(logical);
@@ -199,7 +201,7 @@ void StretchHeaderView::SetStretchEnabled(bool enabled) {
     // Initialise the list of widths from the current state of the widget
     column_widths_.resize(count());
     for (int i=0 ; i<count() ; ++i) {
-      column_widths_[i] = float(sectionSize(i)) / width();
+      column_widths_[i] = ColumnWidthType(sectionSize(i)) / width();
     }
 
     // Stretch the columns to fill the widget
@@ -210,7 +212,7 @@ void StretchHeaderView::SetStretchEnabled(bool enabled) {
   emit StretchEnabledChanged(enabled);
 }
 
-void StretchHeaderView::SetColumnWidth(int logical, float width) {
+void StretchHeaderView::SetColumnWidth(int logical, ColumnWidthType width) {
   if (!stretch_enabled_)
     return;
 
@@ -229,3 +231,68 @@ void StretchHeaderView::AssertMinimalColumnWidth(int logical) {
     resizeSection(logical, kMinimumColumnWidth);
   }
 }
+
+bool StretchHeaderView::RestoreState(const QByteArray& data) {
+  QDataStream s(data);
+  s.setVersion(QDataStream::Qt_4_6);
+
+  int magic_number = 0;
+  s >> magic_number;
+
+  if (magic_number != kMagicNumber || s.atEnd()) {
+    return false;
+  }
+
+  QList<int> pixel_widths;
+  QList<int> visual_indices;
+
+  s >> stretch_enabled_;
+  s >> pixel_widths;
+  s >> visual_indices;
+  s >> column_widths_;
+
+  const int persisted_column_count =
+      qMin(qMin(visual_indices.count(), pixel_widths.count()), column_widths_.count());
+
+  // Set column visible state, visual indices and, if we're not in stretch mode,
+  // pixel widths.
+  for (int i=0 ; i<count() && i<persisted_column_count ; ++i) {
+    setSectionHidden(i, pixel_widths[i] <= kMinimumColumnWidth);
+    moveSection(visualIndex(visual_indices[i]), i);
+
+    if (!stretch_enabled_) {
+      resizeSection(i, pixel_widths[i]);
+    }
+  }
+
+  if (stretch_enabled_) {
+    // In stretch mode, we've already set the proportional column widths so apply
+    // them now.
+    UpdateWidths();
+  }
+
+  return true;
+}
+
+QByteArray StretchHeaderView::SaveState() const {
+  QByteArray ret;
+  QDataStream s(&ret, QIODevice::WriteOnly);
+
+  QList<int> pixel_widths;
+  QList<int> visual_indices;
+
+  for (int i=0 ; i<count() ; ++i) {
+    pixel_widths << sectionSize(i);
+    visual_indices << logicalIndex(i);
+  }
+
+  s.setVersion(QDataStream::Qt_4_6);
+  s << kMagicNumber;
+
+  s << stretch_enabled_;
+  s << pixel_widths;
+  s << visual_indices;
+  s << column_widths_;
+
+  return ret;
+}

src/widgets/stretchheaderview.h

@@ -26,31 +26,61 @@ class StretchHeaderView : public QHeaderView {
 public:
   StretchHeaderView(Qt::Orientation orientation, QWidget* parent = 0);
 
+  typedef double ColumnWidthType;
+
   static const int kMinimumColumnWidth;
+  static const int kMagicNumber;
 
   void setModel(QAbstractItemModel* model);
 
+  // Serialises the proportional and actual column widths.  Use these instead
+  // of QHeaderView::restoreState and QHeaderView::saveState to persist the
+  // proportional values directly and avoid floating point errors over time.
+  bool RestoreState(const QByteArray& data);
+  QByteArray SaveState() const;
+
+  // Hides a section and resizes all other sections to fill the gap.  Does
+  // nothing if you try to hide the last section.
   void HideSection(int logical);
+
+  // Shows a section and resizes all other sections to make room.
   void ShowSection(int logical);
+
+  // Calls either HideSection or ShowSection.
   void SetSectionHidden(int logical, bool hidden);
-  void SetColumnWidth(int logical, float width);
+
+  // Sets the width of the given column and resizes other columns appropriately.
+  // width is the proportion of the entire width from 0.0 to 1.0.
+  void SetColumnWidth(int logical, ColumnWidthType width);
 
   bool is_stretch_enabled() const { return stretch_enabled_; }
 
 public slots:
+  // Changes the stretch mode.  Enabling stretch mode will initialise the
+  // proportional column widths from the current state of the header.
   void ToggleStretchEnabled();
   void SetStretchEnabled(bool enabled);
 
 signals:
+  // Emitted when the stretch mode is changed.
   void StretchEnabledChanged(bool enabled);
 
 protected:
+  // QWidget
   void mouseMoveEvent(QMouseEvent* e);
   void resizeEvent(QResizeEvent* event);
 
 private:
+  // If the width of the given column is less than a sensible threshold, resize
+  // it to make it bigger.  Workaround for a QHeaderView oddity that means a
+  // column can be visible but with a width of 0.
   void AssertMinimalColumnWidth(int logical);
+
+  // Scales column_widths_ values so the total is 1.0.
   void NormaliseWidths(const QList<int>& sections = QList<int>());
+
+  // Resizes the actual columns to make them match the proportional values
+  // in column_widths_.
   void UpdateWidths(const QList<int>& sections = QList<int>());
 
 private slots:
@@ -58,7 +88,7 @@ private slots:
 
 private:
   bool stretch_enabled_;
-  QVector<float> column_widths_;
+  QVector<ColumnWidthType> column_widths_;
 
   bool in_mouse_move_event_;
 };