[PATCH 0/2] Capture and display frame thumbnails in qapitrace
Zack Rusin
zack at kde.org
Thu Mar 1 19:34:56 PST 2012
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
More information about the apitrace
mailing list