threads / patch / 5957

patch, 2 partsRemove dead code after direct graph drawing

Subject: [PATCH 2/2] Remove dead code after direct graph drawing

## tl;dr

7 messages between Oct 19, 2006 and Oct 21, 2006. Diffs are folded; open one to read it.

replies: 6people: 3as markdown or json

Josef Weidendorfer· Oct 19, 2006, 14:13 UTC · lore
Signed-off-by: Josef Weidendorfer <Josef.Weidendorfer@gmx.de>
---
Sorry, I forgot: this is on top of your public master as of today.
Josef
 src/listview.cpp |   94 +-----------------------------------------------------
 src/listview.h   |    2 -
 2 files changed, 1 insertions(+), 95 deletions(-)
Show changes to 2 files +1 −95

src/listview.cpp, src/listview.h

diff --git a/src/listview.cpp b/src/listview.cpp
index 418836b..84cbfe9 100644
--- a/src/listview.cpp
+++ b/src/listview.cpp
@@ -37,9 +37,6 @@ ListView::ListView(Domain* dm, Git* g, Q
 	connect(lv, SIGNAL(currentChanged(QListViewItem*)),
 	        this, SLOT(on_currentChanged(QListViewItem*)));
 
-	connect(lv, SIGNAL(contentsMoving(int,int)),
-	        this, SLOT(on_contentsMoving(int,int)));
-
 	connect(lv, SIGNAL(mouseButtonPressed(int,QListViewItem*,const QPoint&,int)),
 	        this, SLOT(on_mouseButtonPressed(int,QListViewItem*,const QPoint&,int)));
 
@@ -82,11 +79,6 @@ void ListView::setupListView() {
 
 void ListView::on_repaintListViews(QFont& f) {
 
-	QListViewItemIterator it(lv);
-	while (it.current()) {
-		it.current()->setPixmap(GRAPH_COL, NULL);
-		++it;
-	}
 	lv->setFont(f);
 	lv->ensureItemVisible(lv->currentItem());
 }
@@ -298,41 +290,6 @@ void ListView::on_contextMenuRequested(Q
 	emit contextMenu(((ListViewItem*)item)->sha(), POPUP_LIST_EV);
 }
 
-void ListView::on_contentsMoving(int, int newY) {
-// in case of very big archives pixmap memory can became huge.
-// so we free pixmap memory for not visible items, avoiding
-// traversing the list and using only the fast iterator
-// itemBelow() and NOT the slower itemAbove()
-
-	int ph = d->m()->gm.height();
-	int h = lv->visibleHeight();
-	if (lv->childCount() < h / ph + 10)
-		return;
-
-	int curY, dummy;
-	lv->viewportToContents(0, 0, dummy, curY);
-	int delta = newY - curY;
-	QListViewItem* item;
-	if (delta > 0)
-		item = lv->itemAt(QPoint(0, 0));
-	else {
-		item = lv->itemAt(QPoint(0, h + delta));
-		delta = -delta;
-		if (item)
-			item = item->itemBelow();
-	}
-	if (item == NULL)
-		return;
-
-	int cnt = 0;
-	delta /= ph;
-	while (cnt++ < delta) {
-		item->setPixmap(GRAPH_COL, NULL); // free pixmap memory
-		item = item->itemBelow();
-		if (item == NULL)
-			break;
-	}
-}
 
 bool ListView::eventFilter(QObject* obj, QEvent* ev) {
 // we need an event filter for:
@@ -463,7 +420,6 @@ void ListViewItem::paintGraph(const Rev&
     else
 	p->fillRect( 0, 0, width, height(), cg.brush( crole ) );
 	
-    // Copy from getGraph(), modified to directly draw into cell
     const QValueVector<int>& lanes(c.lanes);
     uint laneNum = lanes.count();
     int pw = pms[0]->width();
@@ -507,19 +463,11 @@ void ListViewItem::paintCell(QPainter* p
 	if (!populated)
 		setupData(c);
 
-#if 1
 	if (column == GRAPH_COL) {
 	        paintGraph(c, p, _cg, width);
 		return;
 	}
-#else
-	// pixmap graph, separated from setupData to allow deleting
-	if (!pixmap(GRAPH_COL)) {
-		QPixmap* pm = getGraph(c);
-		setPixmap(GRAPH_COL, *pm);
-		delete pm;
-	}
-#endif
+
 	// adjust for annotation id column presence
 	int mycolumn = (fh) ? column : column + 1;
 
@@ -655,46 +603,6 @@ const QString ListViewItem::timeDiff(uns
 	return tmp;
 }
 
-QPixmap* ListViewItem::getGraph(const Rev& c) {
-
-	const QValueVector<int>& lanes(c.lanes);
-	uint laneNum = lanes.count();
-	int pw = pms[0]->width();
-	int ph = pms[0]->height();
-	QPixmap* pm = new QPixmap(pw * laneNum, ph);
-	pm->fill(ODD_LINE_COL); // faster then drawRect()
-	int mergeLane = -1;
-	for (uint i = 0; i < laneNum; i++)
-		if (isMerge(lanes[i])) {
-			mergeLane = i;
-			break;
-		}
-	QPainter p;
-	p.begin(pm);
-	for (uint i = 0; i < laneNum; i++) {
-
-		int ln = lanes[i], idx;
-		if (ln == EMPTY)
-			continue;
-
-		if (ln == CROSS)
-			idx = COLORS_NUM * (NOT_ACTIVE - 1);
-		else
-			idx = COLORS_NUM * (ln - 1);
-
-		int col = (   isHead(ln) || isTail(ln) || isJoin(ln)
-		           || ln == CROSS_EMPTY) ? mergeLane : i;
-
-		idx += col % COLORS_NUM;
-		p.drawPixmap(i * pw, 0, *pms[idx]);
-		if (ln == CROSS) {
-			idx = COLORS_NUM * (CROSS - 1) + mergeLane % COLORS_NUM;
-			p.drawPixmap(i * pw, 0, *pms[idx]);
-		}
-	}
-	p.end();
-	return pm;
-}
 
 // ***************** GraphMetrics related methods *********************
 
diff --git a/src/listview.h b/src/listview.h
index 25de935..5a49512 100644
--- a/src/listview.h
+++ b/src/listview.h
@@ -34,7 +34,6 @@ public:
 private:
 	void setupData(const Rev& c);
 	void paintGraph(const Rev& c, QPainter *p, const QColorGroup &cg, int width);
-	QPixmap* getGraph(const Rev& c);
 	void addTextPixmap(SCRef text, const QColor& color, bool bold = false);
 	QPixmap* doAddTextPixmap(SCRef text, const QColor& color, int col, bool bold);
 	const QString timeDiff(unsigned long secs) const;
@@ -80,7 +79,6 @@ private slots:
 	void on_mouseButtonPressed(int, QListViewItem*, const QPoint&, int);
 	void on_clicked(QListViewItem*);
 	void on_onItem(QListViewItem*);
-	void on_contentsMoving(int, int newY);
 
 private:
 	void setupListView();
-- 
1.4.3.rc2.gf8ffb
Josef Weidendorfer· Oct 19, 2006, 23:11 UTC · re: Josef Weidendorfer · lore

Re: [PATCH 2/2] Remove dead code after direct graph drawing

On Thursday 19 October 2006 16:13, Josef Weidendorfer wrote:
> Signed-off-by: Josef Weidendorfer <Josef.Weidendorfer@gmx.de>
Hmmm...

Is the git mailing list the right place for qgit patches? Probably, I should have prefixed them with "qgit:" ...

Josef
Marco Costalba· Oct 20, 2006, 11:49 UTC · re: Josef Weidendorfer · lore

Re: [PATCH 2/2] Remove dead code after direct graph drawing

On 10/20/06, Josef Weidendorfer <Josef.Weidendorfer@gmx.de> wrote:
Show 6 quoted lines
> On Thursday 19 October 2006 16:13, Josef Weidendorfer wrote:
> > Signed-off-by: Josef Weidendorfer <Josef.Weidendorfer@gmx.de>
>
> Hmmm...
>
> Is the git mailing list the right place for qgit patches?
Yes, I don't see other competitors ;-)
> Probably, I should have prefixed them with "qgit:" ...
>

No problem, I should found them anyway and I don't need to manually remove "qgit" prefix before to apply to repository.

Jakub Narebski· Oct 20, 2006, 16:01 UTC · re: Marco Costalba · lore

Re: [PATCH 2/2] Remove dead code after direct graph drawing

Marco Costalba wrote:
Show 11 quoted lines
> On 10/20/06, Josef Weidendorfer <Josef.Weidendorfer@gmx.de> wrote:
>> On Thursday 19 October 2006 16:13, Josef Weidendorfer wrote:
>> > Signed-off-by: Josef Weidendorfer <Josef.Weidendorfer@gmx.de>
>>
>> Hmmm...
>>
>> Is the git mailing list the right place for qgit patches?
> 
> Yes, I don't see other competitors ;-)
> 
>> Probably, I should have prefixed them with "qgit:" ...
Or use [PATCH (qgit)] or equivalent...
 
> No problem, I should found them anyway and I don't need to manually
> remove "qgit" prefix before to apply to repository.
...which would be stripped automatically
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
Marco Costalba· Oct 21, 2006, 06:35 UTC · re: Josef Weidendorfer · lore

Re: [PATCH 2/2] Remove dead code after direct graph drawing

Josef,
 I think ther's a leak somewhere.

Checking memory use with ksysguard is see memory use going up scrolling up and down also on the same revisions list view subset.

I'm not sure it depends on your patch though.
Marco
Josef Weidendorfer· Oct 21, 2006, 15:40 UTC · re: Marco Costalba · lore

Re: [PATCH 2/2] Remove dead code after direct graph drawing

On Saturday 21 October 2006 08:35, you wrote:
> Josef,
> 
>  I think ther's a leak somewhere.

I am not really sure... I just scrolled up and down through the kernel repo with the page up/down keys (around 34K revisions).

On the first pass down, the memory increases by around 26 MB, which could by correct, as you do setupData() lazy (no idea what this function does...). On the 2nd pass up again, I only get an increase of around 2 MB. That could be other effects, as on further passes, I do not see any change with pure scrolling.

> Checking memory use with ksysguard is see memory use going up
> scrolling up and down also on the same revisions list view subset.
> 
> I'm not sure it depends on your patch though.

Can you compare with/without my patch? I have no idea what could have introduced any leak here. I do not create any new class instance / structures, but only get rid of pixmap creations/deletions.

How did you check for leaks in the past? Did you try valgrind (memcheck or massif)?

Josef
> 
> Marco
> 
> 
Marco Costalba· Oct 21, 2006, 19:33 UTC · re: Josef Weidendorfer · lore

Re: [PATCH 2/2] Remove dead code after direct graph drawing

>
> On the first pass down, the memory increases by around 26 MB,
> which could by correct, as you do setupData() lazy (no idea what
> this function does...).

Populates the list view item and calculates (only calculation not drawing) the graph of the item (see Git::updateLanes() )

> >
> > I'm not sure it depends on your patch though.
>
> Can you compare with/without my patch?
Yes. You are right, I have a memory increase also without your last patch.
>
> How did you check for leaks in the past?
> Did you try valgrind (memcheck or massif)?
>

I tried with valgrind but with little success due to a lot of noise from qt libraries. Perhaps I was not using it right.

Sorry for the noise
Marco

← back to recent threads