[Development] Some Qt3D feedback
sean.harmer at kdab.com
Mon Jun 8 15:18:33 CEST 2015
thanks for the feedback!
On Monday 08 Jun 2015 01:11:23 Stephen Kelly wrote:
> Congrats to Paul, Sean and others working on getting this module in a
> releasable state for Qt 5.5!
> I have not reviewed the code, but I found some items to raise:
> 1) The include/Qt3DCore/Window file doesn't have a Q prefix.
> as every other header does. Should probably be Qt3DWindow.
Right, this actually needs removing and something temporary putting in place
in the examples for now. The window class doesn't do much at all but there is
a need for more full featured integration points. My current thoughts on what
we should aim for are:
1) A convenience window to be used with Qt3D that has a standard camera and
handles aspect ratio changes etc and also uses a default framegraph (that can
be changed by the user). QML and C++ versions.
2) Qt3D Widget to embed a Qt3D scene into a QWidget hierarchy. Likely
analogous to QOpenGLWidget but with the above Qt3D plumbing.
3) Embed Qt3D into a Qt Quick 2 scene. Already available via the Scene3D Qt
Quick 2 element (provided by Qt3D module).
4) Use Qt Quick within Qt3D as e.g. a texture provider so that it is possible
to use a Qt Quick 2 render within the 3D world.
I'll try to tidy up and move the existing Window thing to the examples later
today or tomorrow.
> 2) A private header is included in a public header:
> include/Qt3DCore$ grep private/ *.h
> qaspectjobmanager.h:#include <Qt3DCore/private/qt3dcore_global_p.h>
> This is concerning - Don't we have a unit test preventing that?
I can't find that. Is that from the 5.5 branch?
> 3) The cmake unit tests don't pass.
> It is easily fixable, but does this mean that the cmake tests are not run
> for this module? That is concerning.
> Are unit tests run for this module in CI at all?
Hmm I thought they were, but obviously not. I'm looking at making the test
> 4) Private dependencies
> git grep -w -e QT --and -e private
> shows a bunch of content. Shouldn't they be added to QT_PRIVATE instead?
> 5) Qt3D namespace
> This is the first time that all classes in a library are in a namespace.
> Previously only enums (in various modules) and free functions (in
> QtConcurrent) have been put in namespaces.
> In QtConcurrent, the module name also appears in the header file, but that
> is not followed by Qt3D libraries.
> Given that Qt has never put classes in a namespace like this, is there
> something to be consistent about here?
> 6) QParameter is a very generic name
> I realise it is in a namespace, but still...
> Qt3DParamter might be better *and* more consistent. Similar applies to other
It's precisely because of these kinds of issues that we decided to use
namespaces in Qt3D rather than the poor-man's prefix name spacing.
If it's required to not use namespaces to be part of the Qt project then we
can of course change it. However, I would argue against doing so, especially
in the light of being able to use some more modern C++ features in upcoming QT
versions. Name spaces are supported everywhere these days so why not just use
them, especially in a new add-on module?
> 7) Unneeded Q_DECLARE_METATYPE
> Using the macro for QObject derived types is not needed. I saw
> Q_DECLARE_METATYPE(Qt3D::QNode *)
> and similar for QParameter and then stopped searching.
> 8) Docs in headers.
> I saw one method documented in qparameter.h instead of in the cpp and I
> didn't look for more.
The docs need a lot of work. I have some time set aside for doc writing this
week and will do a clean up pass for such issues.
Dr Sean Harmer | sean.harmer at kdab.com | Managing Director UK
KDAB (UK) Ltd, a KDAB Group company
Tel. +44 (0)1625 809908; Sweden (HQ) +46-563-540090
Mobile: +44 (0)7545 140604
KDAB - Qt Experts
More information about the Development