Uploaded image for project: 'Qt'
  1. Qt
  2. QTBUG-24043

Resource handling needs refactoring

    XMLWordPrintable

Details

    • Bug
    • Resolution: Done
    • P1: Critical
    • Qt3D 1.0
    • Qt3D TP2
    • Qt3D
    • None

    Description

      Currently Qt3D still has remnants of the old Qt4 QGLSharedResourceGuard that went through a few name changes and went into Qt. It is/was used for managing GL resources, especially textures, but also things like the extension resolver.

      This is completely broken in Qt5, relies on copied private headers and has been responsible for several recent bugs. It will continue to cause problems until it is fixed. At present because of this we have some memory leaks and instability.

      At the core the problem is to call this function reliably and at the right time:

      static void qt_gl_destroyTextureId(GLuint id)
      {
          glDeleteTextures(1, &id);
      }
      

      It has to be called when the QOpenGLContext which the GLuint id was created against is current, and in the same thread as that context.

      The function has to be called when the texture was no longer needed. The resource guard thing tried to hookup a cleanup handler calling this function, onto the destroyed signal from the context. So the resources could be consumed during the whole lifetime of the application - not good if resources are continually created.

      Also supposedly when the QGLTexture itself got destroyed the cleanup would happen. This is flawed because a) the texture being destroyed might not be soon enough - you might want to keep the texture around with its image attached in CPU side, but free it from the GPU. Also if the textures are attached to some big structure they might stick around after the context is deleted, in which case the destroyed signal would fire, and all the resources would be cleaned up - only if the textures were not shared with another context. But if there was no other context, alright go and clean them up. Try to make one of the contexts it is shared with current then call the above. Very brittle, sensitive to ordering problems, and has much more complexity than is generally needed.

      Also it is completely broken in the threaded case since the context probably does not exist in the thread where the signal of the destroyed texture lived. Trying to queue a destroy handler and for when the gl context can be made current is even more brittle than above.

      The only sane approach is to put the onus back on to the application developer via a QGLTexture2D::cleanupResources() function, which calls the above explicitly. If the function is called when the appropriate context is not current, or the function is not running in the context's thread a warning is printed. When a gl resource is assigned a flag is set, and its cleared when the cleanupResources() is called. If the flag is not cleared when the destructor is called, a warning is printed. This is currently what the SceneGraph is doing with warnings about leaking resources being printed - presumably this should only happen in debug mode. This all means lots of warnings which enables the application programmer to detect if resources are being cleaned up, and it is only the application programmer who can know when the current context is available.

      This allows the app developer fine grained control over the lifetime of the gl resources. It is also more efficient for things like LOD or culling algorithms where QGLTexture objects may be kept around but flushed out of GPU ram while they are not required for rendering.

      Note that similar changes are required for QGLTextureCube and any other controlled resources.

      Attachments

        Issue Links

          No reviews matched the request. Check your Options in the drop-down menu of this sections header.

          Activity

            People

              sergey.dubitskiy Sergey (closed Nokia identity) (Inactive)
              sarasmit Sarah Smith (closed Nokia identity) (Inactive)
              Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Gerrit Reviews

                  There are no open Gerrit changes