On Apr 2, 2007, at 3:27 AM, dcurtis3@users.sourceforge.net wrote:
Log Message:
Label rendering engine added. Needs work. Possibly using an overlay?
I don’t think we need an overlay, but I wonder if the color needs to
be set, font size, etc. I also added a wireframe view in case the
atoms were obscuring the labels.
Cheers,
-Geoff
Am Montag, 2. April 2007 14:24 schrieb Geoffrey Hutchison:
I don’t think we need an overlay, but I wonder if the color needs to
be set, font size, etc. I also added a wireframe view in case the
atoms were obscuring the labels.
I had a look at this code:
===============================================
const QList<Primitive *> *list;
Is there a reason why you didn’t choose the Qt-style code which is in my
opinion much easier to read and less error-prone:
==================================================
const QList<Primitive*> list = queue().primitiveList(Primitive::AtomType);
foreach( Primitive * i , list ){
render((Atom *) i);
}
===================================================
On Apr 2, 2007, at 8:38 AM, Carsten Niehaus wrote:
Is there a reason why you didn’t choose the Qt-style code which is
in my
opinion much easier to read and less error-prone:
No reason except that I was in a hurry to get to a meeting this
morning and just moved the old WireframeEngine code. 
Yes, it’s better to use the Qt-style. I’ll do that in the future.
Thanks,
-Geoff
Am Montag, 2. April 2007 14:48 schrieben Sie:
Is there a reason why you didn’t choose the Qt-style code which is
in my
opinion much easier to read and less error-prone:No reason except that I was in a hurry to get to a meeting this
morning and just moved the old WireframeEngine code.Yes, it’s better to use the Qt-style. I’ll do that in the future.
On further inspection I noticed
const QList<Primitive > primitiveList(enum Primitive::Type type) const;
I really wonder why you don’t simply return a QList…
QList<Primitive *> primitiveList(enum Primitive::Type type) const;
Also, in some destructure you do something like this:
PrimitiveQueue::~PrimitiveQueue() {
for( int i = 0; iqueue.size(); i++ ) {
delete d->queue[i];
}
delete d;
}
As queue is a QList<> you can as well write:
PrimitiveQueue::~PrimitiveQueue() {
qDeleteAll(queue);
delete d;
}
That is much easier to type and read, IMHO…
Carsten
(Mon, Apr 02, 2007 at 03:00:28PM +0200) Carsten Niehaus cniehaus@gmx.de:
Am Montag, 2. April 2007 14:48 schrieben Sie:
Is there a reason why you didn’t choose the Qt-style code which is
in my
opinion much easier to read and less error-prone:No reason except that I was in a hurry to get to a meeting this
morning and just moved the old WireframeEngine code.Yes, it’s better to use the Qt-style. I’ll do that in the future.
Agreed. As much as i try to conform to Qt naming, i get lost in
“helper” functionality i’m not familiar with.
On further inspection I noticed
const QList<Primitive > primitiveList(enum Primitive::Type type) const;
I really wonder why you don’t simply return a QList…
Yeah, i thought about this as i was working today. We really need to
return a const QList<Primitive *>& (reference). We just want to make
sure that no copy occurs. Also, i dynamically allocate in
PrimitiveQueue and i think that can be fixed.
QList<Primitive *> primitiveList(enum Primitive::Type type) const;
Also, in some destructure you do something like this:
PrimitiveQueue::~PrimitiveQueue() {
for( int i = 0; iqueue.size(); i++ ) {
delete d->queue[i];
}
delete d;
}As queue is a QList<> you can as well write:
PrimitiveQueue::~PrimitiveQueue() {
qDeleteAll(queue);
delete d;
}That is much easier to type and read, IMHO…
Carsten
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net’s Techsay panel and you’ll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
Avogadro-devel mailing list
Avogadro-devel@lists.sourceforge.net
avogadro-devel List Signup and Options
(Mon, Apr 02, 2007 at 02:38:08PM +0200) Carsten Niehaus cniehaus@gmx.de:
Am Montag, 2. April 2007 14:24 schrieb Geoffrey Hutchison:
I don’t think we need an overlay, but I wonder if the color needs to
be set, font size, etc. I also added a wireframe view in case the
atoms were obscuring the labels.I had a look at this code:
===============================================
const QList<Primitive *> *list;list = queue().primitiveList(Primitive::AtomType);
for( int i=0; isize(); i++ ) {
render((Atom *)(*list)[i]);
}Is there a reason why you didn’t choose the Qt-style code which is in my
opinion much easier to read and less error-prone:==================================================
const QList<Primitive*> list = queue().primitiveList(Primitive::AtomType);
foreach( Primitive * i , list ){
render((Atom *) i);
}===================================================
After i went and updated all the loops, i was curious if there is a
speed decrease based off this. The Q_FOREACH actually allocated an
iterator type of class meaning at loop time we must allocate a new class
and use a bunch of function calls to iterate. However, this is all
inlined so maybe there is no problem. I just know in the QList
documentation they do a for(int i=0…) loop. shrug