[PATCH 1/2] Simpler way to draw commit graph

[PATCH 1/2] Simpler way to draw commit graph

[Josef Weidendorfer]

For drawing the commit graph, previously every item got a
pixmap created and set with item->setPixmap(), which is
drawn by the standard implementation of QListView::paintCell().

Instead, this commit implements drawing of the graph
directly in our own ListView::paintCell(). This gets rid of
a lot of complex code to reset the pixmap of invisible items
which was needed in large repositories before to not allocate
huge amounts of memory.

As we directly draw only the visible cells, it has no
influence on performance (especially, as we got rid of
pixmaps of invisible items before, and most often had
to draw the graph anyway).

Signed-off-by: Josef Weidendorfer <Josef.Weidendorfer@gmx.de>
---

Hi Marco,

currently, when drawing branch/tag labels in the commit graph,
QGit shows in the graph small white spaces. This is because
these lines are a little higher than the rest, and the
pregenerated graph pixmaps only have a given height.

In order to solve this, I looked at the code, and do not understand
one thing: Why are you creating pixmaps for the graph, and do
draw directly in paintCell() ?

This patch does exactly this, and the next one does cleanup
of code which is not used afterwards.

If you like, I can comeup with a patch to directly draw the lines
which would get rid of the original problem.

Josef

 src/listview.cpp |   57 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/listview.h   |    1 +
 2 files changed, 58 insertions(+), 0 deletions(-)

diff --git a/src/listview.cpp b/src/listview.cpp
index cef1c2a..418836b 100644
--- a/src/listview.cpp
+++ b/src/listview.cpp
@@ -448,6 +448,56 @@ void ListViewItem::setDiffTarget(bool b)
 	repaint();
 }
 
+void ListViewItem::paintGraph(const Rev& c, QPainter *p, const QColorGroup &cg, int width)
+{
+    // Copied from QListViewItem::paintCell
+    QListView *lv = listView();
+    if ( !lv ) return;
+
+    const BackgroundMode bgmode = lv->viewport()->backgroundMode();
+    const QColorGroup::ColorRole crole
+	= QPalette::backgroundRoleFromMode( bgmode );
+    
+    if ( isSelected() && lv->allColumnsShowFocus() )
+	p->fillRect( 0, 0, width, height(), cg.brush( QColorGroup::Highlight ) );
+    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();
+    int mergeLane = -1;
+    for (uint i = 0; i < laneNum; i++)
+	if (isMerge(lanes[i])) {
+	    mergeLane = i;
+	    break;
+	}
+
+    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]);
+	}
+    }
+}
+
+
 void ListViewItem::paintCell(QPainter* p, const QColorGroup& cg,
                              int column, int width, int alignment) {
 	QColorGroup _cg(cg);
@@ -457,12 +507,19 @@ 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;
 
diff --git a/src/listview.h b/src/listview.h
index 672ed7d..25de935 100644
--- a/src/listview.h
+++ b/src/listview.h
@@ -33,6 +33,7 @@ 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);
-- 
1.4.3.rc2.gf8ffb

Re: [PATCH 1/2] Simpler way to draw commit graph

[Marco Costalba]

On 10/19/06, Josef Weidendorfer <Josef.Weidendorfer@gmx.de> wrote:
> For drawing the commit graph, previously every item got a
> pixmap created and set with item->setPixmap(), which is
> drawn by the standard implementation of QListView::paintCell().
>
> Instead, this commit implements drawing of the graph
> directly in our own ListView::paintCell(). This gets rid of
> a lot of complex code to reset the pixmap of invisible items
> which was needed in large repositories before to not allocate
> huge amounts of memory.
>
> As we directly draw only the visible cells, it has no
> influence on performance (especially, as we got rid of
> pixmaps of invisible items before, and most often had
> to draw the graph anyway).
>
> Signed-off-by: Josef Weidendorfer <Josef.Weidendorfer@gmx.de>
> ---
>

It looks sane. Thanks, I will apply this week-end.

>
> In order to solve this, I looked at the code, and do not understand
> one thing: Why are you creating pixmaps for the graph, and do
> draw directly in paintCell() ?
>

The code to create pixmaps is older then the one to remove not visible pixmaps.
When I added the latter I missed the opportunity to reformat exsisting code.

> This patch does exactly this, and the next one does cleanup
> of code which is not used afterwards.
>
> If you like, I can comeup with a patch to directly draw the lines
> which would get rid of the original problem.
>

Yes, please.


Thanks
Marco