{"thread":{"id":"5956","subject":"[PATCH 1/2] Simpler way to draw commit graph","startedAt":"2006-10-19T14:13:30Z","lastAt":"2006-10-20T11:46:33Z","messageCount":2,"participants":["Josef Weidendorfer","Marco Costalba"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"29232","messageId":"200610191613.31119.Josef.Weidendorfer@gmx.de","threadId":"5956","inReplyTo":null,"subject":"[PATCH 1/2] Simpler way to draw commit graph","fromName":"Josef Weidendorfer","fromEmail":"josef.weidendorfer@gmx.de","sentAt":"2006-10-19T14:13:30Z","receivedAt":"2006-10-19T14:13:30Z","isPatch":true,"sender":{"key":"josef.weidendorfer@gmx.de","avatar":null},"body":"For drawing the commit graph, previously every item got a\npixmap created and set with item->setPixmap(), which is\ndrawn by the standard implementation of QListView::paintCell().\n\nInstead, this commit implements drawing of the graph\ndirectly in our own ListView::paintCell(). This gets rid of\na lot of complex code to reset the pixmap of invisible items\nwhich was needed in large repositories before to not allocate\nhuge amounts of memory.\n\nAs we directly draw only the visible cells, it has no\ninfluence on performance (especially, as we got rid of\npixmaps of invisible items before, and most often had\nto draw the graph anyway).\n\nSigned-off-by: Josef Weidendorfer <Josef.Weidendorfer@gmx.de>\n---\n\nHi Marco,\n\ncurrently, when drawing branch/tag labels in the commit graph,\nQGit shows in the graph small white spaces. This is because\nthese lines are a little higher than the rest, and the\npregenerated graph pixmaps only have a given height.\n\nIn order to solve this, I looked at the code, and do not understand\none thing: Why are you creating pixmaps for the graph, and do\ndraw directly in paintCell() ?\n\nThis patch does exactly this, and the next one does cleanup\nof code which is not used afterwards.\n\nIf you like, I can comeup with a patch to directly draw the lines\nwhich would get rid of the original problem.\n\nJosef\n\n src/listview.cpp |   57 ++++++++++++++++++++++++++++++++++++++++++++++++++++++\n src/listview.h   |    1 +\n 2 files changed, 58 insertions(+), 0 deletions(-)\n\ndiff --git a/src/listview.cpp b/src/listview.cpp\nindex cef1c2a..418836b 100644\n--- a/src/listview.cpp\n+++ b/src/listview.cpp\n@@ -448,6 +448,56 @@ void ListViewItem::setDiffTarget(bool b)\n \trepaint();\n }\n \n+void ListViewItem::paintGraph(const Rev& c, QPainter *p, const QColorGroup &cg, int width)\n+{\n+    // Copied from QListViewItem::paintCell\n+    QListView *lv = listView();\n+    if ( !lv ) return;\n+\n+    const BackgroundMode bgmode = lv->viewport()->backgroundMode();\n+    const QColorGroup::ColorRole crole\n+\t= QPalette::backgroundRoleFromMode( bgmode );\n+    \n+    if ( isSelected() && lv->allColumnsShowFocus() )\n+\tp->fillRect( 0, 0, width, height(), cg.brush( QColorGroup::Highlight ) );\n+    else\n+\tp->fillRect( 0, 0, width, height(), cg.brush( crole ) );\n+\t\n+    // Copy from getGraph(), modified to directly draw into cell\n+    const QValueVector<int>& lanes(c.lanes);\n+    uint laneNum = lanes.count();\n+    int pw = pms[0]->width();\n+    int mergeLane = -1;\n+    for (uint i = 0; i < laneNum; i++)\n+\tif (isMerge(lanes[i])) {\n+\t    mergeLane = i;\n+\t    break;\n+\t}\n+\n+    for (uint i = 0; i < laneNum; i++) {\n+\t\n+\tint ln = lanes[i], idx;\n+\tif (ln == EMPTY)\n+\t    continue;\n+\t\n+\tif (ln == CROSS)\n+\t    idx = COLORS_NUM * (NOT_ACTIVE - 1);\n+\telse\n+\t    idx = COLORS_NUM * (ln - 1);\n+\t\n+\tint col = (   isHead(ln) || isTail(ln) || isJoin(ln)\n+\t\t      || ln == CROSS_EMPTY) ? mergeLane : i;\n+\t\n+\tidx += col % COLORS_NUM;\n+\tp->drawPixmap(i * pw, 0, *pms[idx]);\n+\tif (ln == CROSS) {\n+\t    idx = COLORS_NUM * (CROSS - 1) + mergeLane % COLORS_NUM;\n+\t    p->drawPixmap(i * pw, 0, *pms[idx]);\n+\t}\n+    }\n+}\n+\n+\n void ListViewItem::paintCell(QPainter* p, const QColorGroup& cg,\n                              int column, int width, int alignment) {\n \tQColorGroup _cg(cg);\n@@ -457,12 +507,19 @@ void ListViewItem::paintCell(QPainter* p\n \tif (!populated)\n \t\tsetupData(c);\n \n+#if 1\n+\tif (column == GRAPH_COL) {\n+\t        paintGraph(c, p, _cg, width);\n+\t\treturn;\n+\t}\n+#else\n \t// pixmap graph, separated from setupData to allow deleting\n \tif (!pixmap(GRAPH_COL)) {\n \t\tQPixmap* pm = getGraph(c);\n \t\tsetPixmap(GRAPH_COL, *pm);\n \t\tdelete pm;\n \t}\n+#endif\n \t// adjust for annotation id column presence\n \tint mycolumn = (fh) ? column : column + 1;\n \ndiff --git a/src/listview.h b/src/listview.h\nindex 672ed7d..25de935 100644\n--- a/src/listview.h\n+++ b/src/listview.h\n@@ -33,6 +33,7 @@ public:\n \n private:\n \tvoid setupData(const Rev& c);\n+\tvoid paintGraph(const Rev& c, QPainter *p, const QColorGroup &cg, int width);\n \tQPixmap* getGraph(const Rev& c);\n \tvoid addTextPixmap(SCRef text, const QColor& color, bool bold = false);\n \tQPixmap* doAddTextPixmap(SCRef text, const QColor& color, int col, bool bold);\n-- \n1.4.3.rc2.gf8ffb\n"},{"id":"29339","messageId":"e5bfff550610200446o35bac985n3d520066fdbae2bb@mail.gmail.com","threadId":"5956","inReplyTo":"200610191613.31119.Josef.Weidendorfer@gmx.de","subject":"Re: [PATCH 1/2] Simpler way to draw commit graph","fromName":"Marco Costalba","fromEmail":"mcostalba@gmail.com","sentAt":"2006-10-20T11:46:33Z","receivedAt":"2006-10-20T11:46:33Z","isPatch":true,"sender":{"key":"mcostalba@gmail.com","avatar":null},"body":"On 10/19/06, Josef Weidendorfer <Josef.Weidendorfer@gmx.de> wrote:\n> For drawing the commit graph, previously every item got a\n> pixmap created and set with item->setPixmap(), which is\n> drawn by the standard implementation of QListView::paintCell().\n>\n> Instead, this commit implements drawing of the graph\n> directly in our own ListView::paintCell(). This gets rid of\n> a lot of complex code to reset the pixmap of invisible items\n> which was needed in large repositories before to not allocate\n> huge amounts of memory.\n>\n> As we directly draw only the visible cells, it has no\n> influence on performance (especially, as we got rid of\n> pixmaps of invisible items before, and most often had\n> to draw the graph anyway).\n>\n> Signed-off-by: Josef Weidendorfer <Josef.Weidendorfer@gmx.de>\n> ---\n>\n\nIt looks sane. Thanks, I will apply this week-end.\n\n>\n> In order to solve this, I looked at the code, and do not understand\n> one thing: Why are you creating pixmaps for the graph, and do\n> draw directly in paintCell() ?\n>\n\nThe code to create pixmaps is older then the one to remove not visible pixmaps.\nWhen I added the latter I missed the opportunity to reformat exsisting code.\n\n> This patch does exactly this, and the next one does cleanup\n> of code which is not used afterwards.\n>\n> If you like, I can comeup with a patch to directly draw the lines\n> which would get rid of the original problem.\n>\n\nYes, please.\n\n\nThanks\nMarco\n"}]}