Open benoitdm-oslandia opened 1 year ago
Great to see this QEP and thanks for taking time to explain the rationale behind the proposal. The way how the frame graph for 3D scenes is set up right now is indeed not ideal and it is mostly myself to be blamed for that :slightly_smiling_face:
For the proposed design, I have a couple of questions:
QgsAbstractItemRenderView
define a render pass? Does it create and return frame graph nodes, or it would use some more abstract definition of the render pass? Having some draft API would help!QgsFrameGraphManager
- this seems to be a central class to the design, would it be possible to add more details?
(It would be great to have answers to these questions added to QEP description as well)
Thanks @wonder-sk to review this QEP.
Great to see this QEP and thanks for taking time to explain the rationale behind the proposal. The way how the frame graph for 3D scenes is set up right now is indeed not ideal and it is mostly myself to be blamed for that slightly_smiling_face
You are not to be blamed!!! It is a great feature you have mainly brought alone in QGIS!
For the proposed design, I have a couple of questions:
in the text both "render pass" and "render view" terms are used - are you referring to the same thing, or they have different meaning? (if they are the same maybe better to use just one term?)
They are quite the same, but from the Qt 3D documentation they mainly use the term of view. The objects/classes in the current version are designed with the pass term and proposed version with the view term.
how does QgsAbstractItemRenderView define a render pass? Does it create and return frame graph nodes, or it would use some more abstract definition of the render pass? Having some draft API would help!
I added an UML diagram in the main QEP description as a base to discussions.
do I understand correctly that the QgsShadowRenderingFrameGraph class would get completely removed in favor of the frame graph manager class that would update the frame graph dynamically?
Render passes will be extracted, one by one, from the QgsShadowRenderingFrameGraph
class and migrated as implementation of QgsAbstractRenderView
. But we will need a class to build the whole graph and so we will keep the QgsShadowRenderingFrameGraph
class for a while then we will rename it as QgsFrameGraphManager
.
UML diagram - QgsFrameGraphManager is not present there at all...? QgsFrameGraphManager - this seems to be a central class to the design, would it be possible to add more details?
Not for now as we are still struggling with some render passes so the final design of the class is not known.
how would be render passes registered/unregistered? I guess it will be necessary to insert passes in the correct order somehow
Exactly, this is the why of the QgsFrameGraphManager
. It will also possible to define other options to build the frame graph but this is a future feature.
how would be individual render passes merged together to a frame graph? (e.g. will there be some logic to merge shared nodes?)
A QgsAbstractRenderView
implementation would have to define the root node to plug him in the main graph handled by the QgsFrameGraphManager
. As well for the input and output textures.
what objects would it own and manage? e.g. cameras? viewports? layers? textures?
Each render view owns its needed members and will provide accessors for other classes.
For example, the light camera is owned by the QgsShadowRenderView
. We may use the Qt property system to expose this inner members.
will be the new classes (e.g. QgsAbstractItemRenderView, QgsFrameGraphManager) available in python bindings for plugins to add their render passes?
We did not focus on that point but due to the recent request on the mailing list, we should think about it.
do you have any examples of what extra render passes would get added? (e.g. the ones you are planning to add later)
We have a need to push a cross section view and we think it will need a dedicated render view.
are all render passes going to be optional, or will there be render passes that would be always on, creating some kind of fixed skeleton? (e.g. forward pass for opaque objects, post-processing pass?)
This will be discussed as the way the main graph is assembled could be anything: static, programmatic, mixed, etc.
QGIS 3D FrameGraph enhancement proposal
Date 2022/11/10
Author Oslandia
maintainer @benoitdm-oslandia @ptitjano
Version QGIS 3.28
Introduction
QGIS 3D
QGIS has been offering 3D functionalities for a few years. The Qt 3D library is used to provide the main 3D widgets, the 3D drawing capabilities and primitives. As QGIS was not initially designed as a 3D viewer or editor, the 3D display is offered in a dedicated map viewer called a 3D scene.
Relates to #252
FrameGraph: principle and use
A 3D scene consists of a list of objects (Points, Maps, Buildings, Meshes, etc..) which need to be drawn and updated on each user interaction. In order to perform these operations, Qt 3D stores all the relevant information in two different graphs:
SceneGraph
= what to display (meshes, polylines, etc.)Framegraph
= how to display it (light, texture, shadow effect, etc.)The SceneGraph describes which objects need to be drawn. It stores two different information: the
Entities
(the objects) and theComponents
of each object (its material, translation or rotation operation, etc.).On the other hand, the FrameGraph describes how to display the different objects. It can be a shadow effect, a lightning, an occlusion effet, some transparency effect, etc..
In the framegraph illustration above, the 2 branches under the Viewport node are called a render pass or a render view.
In order to achieve optimal rendering performance, QGIS 3D uses its own custom frameGraph (described in the class
QgsShadowRenderingFrameGraph
).Further reading (KDAB ressources):
Current framegraph architecture
Currently, the framegraph is mainly composed of these render passes:
All theses render passes are mainly described (attributes and functions) in one class file:
QgsShadowRenderingFrameGraph
.Some objects (camera, light, textures, etc.) are common to several render passes and other objects are dedicated to a specific render pass but it is difficult to distinguish them. Also, there is no responsability separation as some render passes are directly connected (by attributes) to another one (see below).
Drawbacks
For the QGIS contributors
The main drawbacks for the 3D contributors and end user are:
The current code architecture does not ease the addition of a new render pass nor the replacement of one render pass by another implementation.
All the FrameGraph logic is located in one single file with too many complex interconnections. Combined with the current lack of documentation, this makes it hard to analyse the code, maintain or expanded it. It also becomes more and more expensive to add new features.
The current approach makes it difficult to have a clear vision of the current state of the FrameGraph: which render passes are production ready and which ones are still in a testing phase
The current implementation is substandard in terms of performance and stability. The current solution to disable a renderpass is not efficient as it only disables the first node but all the others nodes are sent to the GPU pipeline.
All theses drawbacks reduce the ability to contribute as the learning gap is too high.
Proposed Solution
Features proposal
We would like to split this monolithic class into multiple dedicated classes in order to improve its readability and the responsability separation. This would also be a good opportunity to add some documentation for all the render pass.
To do so, we will provide a base class to handle the render pass implementations (
QgsAbstractRenderView
) and a manager class to handle the final framegraph assemblying (based on the existingQgsShadowRenderingFrameGraph
). Also, it will be possible to create new test per renderview to ensure the product stability.An UML representation of the new architecture is available below:
The abstract render view is detailed below:
Actions
First of all, we will add unit tests to take a snapshot of the results obtained with the current FrameGraph. This will validate the current 3D behaviors and ensure we do not create any regression when we rewrite the
FrameGraph
.Once all the unit tests are written, we will refactor the existing render passes:
QgsShadowRenderingFrameGraph
and write a new dedicated class inheriting fromQgsAbstractRenderView
.QgsShadowRenderingFrameGraph
into a FrameGraph manager calledQgsFrameGraphManager
Its main role will be to combine all the render passes into the final FrameGraph. It will make it easier to enable or disable a render pass.All the future render passes should follow this same logic and inherit from
QgsAbstractRenderView
.Example(s)
Not Applicable
Affected Files
At least:
src/3d/CMakeLists.txt src/3d/qgs3daxis.cpp src/3d/qgs3daxis.h src/3d/qgs3daxisrenderview.cpp src/3d/qgs3daxisrenderview.h src/3d/qgs3daxissettings.h src/3d/qgs3dmapscene.cpp src/3d/qgs3dutils.cpp src/3d/qgsabstract3dengine.cpp src/3d/qgsabstractrenderview.cpp src/3d/qgsabstractrenderview.h src/3d/qgsdepthrenderview.cpp src/3d/qgsdepthrenderview.h src/3d/qgsforwardrenderview.cpp src/3d/qgsforwardrenderview.h src/3d/qgsoffscreen3dengine.cpp src/3d/qgspostprocessingentity.cpp src/3d/qgspostprocessingentity.h src/3d/qgsshadowrenderingframegraph.cpp src/3d/qgsshadowrenderingframegraph.h src/3d/qgsshadowrenderview.cpp src/3d/qgsshadowrenderview.h src/3d/qgswindow3dengine.cpp src/3d/qgswindow3dengine.h
Performance Implications
Not Applicable
Further Considerations/Improvements
Not Applicable
Backwards Compatibility
We would be iso functional with the current behaviour.
Issue Tracking and PR ID(s)
Votes