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

From: Marco Costalba <mcostalba@gmail.com>
Date: Oct 20, 2006, 11:46 UTC
Message-ID: <e5bfff550610200446o35bac985n3d520066fdbae2bb@mail.gmail.com>
In-Reply-To: <200610191613.31119.Josef.Weidendorfer@gmx.de>

On 10/19/06, Josef Weidendorfer <Josef.Weidendorfer@gmx.de> wrote:

Show 18 quoted lines

> 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.

Show 5 quoted lines

>
> 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.

Show 6 quoted lines

> 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