[PATCH 0/2] Capture and display frame thumbnails in qapitrace

[Zack Rusin]

On Thursday, March 01, 2012 12:04:26 PM José Fonseca wrote:
> I'll defer to Zack comments on the gui implementation.

fwiw, i think the idea is pretty good. i really don't want the gui to do this 
by default though. i think it might be better if we just show a larger 
thumbnail in the tooltip rather than 16x16 icon in the list, because the 
latter is unlikely to mean anything to anyone, but i guess we can figure that 
out later.

besides that from the code perspective:
a) the paint method isn't right, you should never do scaling in the paint 
method, generate thumbnails before. plus please leave a little some margin 
when drawing the thumbnail,
b) you should use camel case for all new code,
c) please do not pass constant object to methods by value. also for the 
qlist<qimage> since we don't steal ownership of it i'd prefer if you passed it 
by constant reference instead of a pointer (the fact that you're leaking it 
right now is a good reason why :) ).
d) please don't abbreviate words (e.g. use snapshots instead of snaps)
e) for the changes to propagate the model needs to be notified. currently only 
calls are dynamic so we only do it for them. the way it's done is that 
ApiTrace object emits the changed(ApiTraceCall*) signal which is being 
captured by the ApiTraceModel::callChanged method which emits the 
QAbstractItemModell::dataChanged method which causes the view to update the 
given items. you should probably change the ApiTrace::changed to use 
ApiTraceEvent instead of ApiTraceCall and cast it appropriately when used.

z