From 3ec0df4b5f9bf82918e0b783f547d2bc560f5ccf Mon Sep 17 00:00:00 2001 From: Eskil Abrahamsen Blomfeldt Date: Wed, 13 May 2020 07:32:19 +0200 Subject: [PATCH] RHI: Store texture handle as 64-bit int When storing a void* pointer to the texture handle, we had to ensure that the variable would exist until the build phase, which is error prone and caused errors in QQuickWidget because we copied the texture ID from the FBO into a local variable before passing it into QQuickWindow::setRenderTarget(). The reason for using a void* was that we cannot know the width of the handles in the different backends, but we do know that they are 64-bit at maximum, so instead of storing potentially dangling pointers, we just make it a 64-bit integer and cast it back and forth in the backends. Task-number: QTBUG-78638 Change-Id: I7951e24351ddb209045ab6197d81eb1290b4da67 Reviewed-by: Laszlo Agocs --- src/gui/rhi/qrhi.cpp | 14 +++++--------- src/gui/rhi/qrhi_p.h | 2 +- src/gui/rhi/qrhid3d11.cpp | 8 ++++---- src/gui/rhi/qrhigles2.cpp | 8 ++++---- src/gui/rhi/qrhimetal.mm | 8 ++++---- src/gui/rhi/qrhivulkan.cpp | 8 ++++---- tests/auto/gui/rhi/qrhi/tst_qrhi.cpp | 12 ++++-------- tests/manual/rhi/texuploads/texuploads.cpp | 2 +- 8 files changed, 27 insertions(+), 35 deletions(-) diff --git a/src/gui/rhi/qrhi.cpp b/src/gui/rhi/qrhi.cpp index 6a118d20c80..e92479bb4dc 100644 --- a/src/gui/rhi/qrhi.cpp +++ b/src/gui/rhi/qrhi.cpp @@ -2261,16 +2261,12 @@ QRhiResource::Type QRhiRenderBuffer::resourceType() const /*! \variable QRhiTexture::NativeTexture::object - \brief a pointer to the native object handle. + \brief 64-bit integer containing the native object handle. - With OpenGL, the native handle is a GLuint value, so \c object is then a - pointer to a GLuint. With Vulkan, the native handle is a VkImage, so \c - object is a pointer to a VkImage. With Direct3D 11 and Metal \c - object is a pointer to a ID3D11Texture2D or MTLTexture pointer, respectively. - - \note Pay attention to the fact that \a object is always a pointer - to the native texture handle type, even if the native type itself is a - pointer. + With OpenGL, the native handle is a GLuint value, so \c object can then be + cast to a GLuint. With Vulkan, the native handle is a VkImage, so \c + object can be cast to a VkImage. With Direct3D 11 and Metal \c + object contains a ID3D11Texture2D or MTLTexture pointer, respectively. */ /*! diff --git a/src/gui/rhi/qrhi_p.h b/src/gui/rhi/qrhi_p.h index e63a828bfc6..71064cfc6ac 100644 --- a/src/gui/rhi/qrhi_p.h +++ b/src/gui/rhi/qrhi_p.h @@ -777,7 +777,7 @@ public: }; struct NativeTexture { - const void *object; + quint64 object; int layout; }; diff --git a/src/gui/rhi/qrhid3d11.cpp b/src/gui/rhi/qrhid3d11.cpp index 2f7974944c2..97294669f76 100644 --- a/src/gui/rhi/qrhid3d11.cpp +++ b/src/gui/rhi/qrhid3d11.cpp @@ -2937,14 +2937,14 @@ bool QD3D11Texture::build() bool QD3D11Texture::buildFrom(QRhiTexture::NativeTexture src) { - auto *srcTex = static_cast(src.object); - if (!srcTex || !*srcTex) + ID3D11Texture2D *srcTex = reinterpret_cast(src.object); + if (srcTex == nullptr) return false; if (!prepareBuild()) return false; - tex = *srcTex; + tex = srcTex; if (!finishBuild()) return false; @@ -2960,7 +2960,7 @@ bool QD3D11Texture::buildFrom(QRhiTexture::NativeTexture src) QRhiTexture::NativeTexture QD3D11Texture::nativeTexture() { - return {&tex, 0}; + return {quint64(tex), 0}; } ID3D11UnorderedAccessView *QD3D11Texture::unorderedAccessViewForLevel(int level) diff --git a/src/gui/rhi/qrhigles2.cpp b/src/gui/rhi/qrhigles2.cpp index d8c64c7408f..373b66df893 100644 --- a/src/gui/rhi/qrhigles2.cpp +++ b/src/gui/rhi/qrhigles2.cpp @@ -3776,14 +3776,14 @@ bool QGles2Texture::build() bool QGles2Texture::buildFrom(QRhiTexture::NativeTexture src) { - const uint *textureId = static_cast(src.object); - if (!textureId || !*textureId) + const uint textureId = uint(src.object); + if (textureId == 0) return false; if (!prepareBuild()) return false; - texture = *textureId; + texture = textureId; specified = true; QRHI_RES_RHI(QRhiGles2); @@ -3799,7 +3799,7 @@ bool QGles2Texture::buildFrom(QRhiTexture::NativeTexture src) QRhiTexture::NativeTexture QGles2Texture::nativeTexture() { - return {&texture, 0}; + return {texture, 0}; } QGles2Sampler::QGles2Sampler(QRhiImplementation *rhi, Filter magFilter, Filter minFilter, Filter mipmapMode, diff --git a/src/gui/rhi/qrhimetal.mm b/src/gui/rhi/qrhimetal.mm index c5afdf63ebc..5d94b481a63 100644 --- a/src/gui/rhi/qrhimetal.mm +++ b/src/gui/rhi/qrhimetal.mm @@ -2592,14 +2592,14 @@ bool QMetalTexture::build() bool QMetalTexture::buildFrom(QRhiTexture::NativeTexture src) { - void * const * tex = (void * const *) src.object; - if (!tex || !*tex) + id tex = id(src.object); + if (tex == 0) return false; if (!prepareBuild()) return false; - d->tex = (id) *tex; + d->tex = tex; d->owns = false; @@ -2615,7 +2615,7 @@ bool QMetalTexture::buildFrom(QRhiTexture::NativeTexture src) QRhiTexture::NativeTexture QMetalTexture::nativeTexture() { - return {&d->tex, 0}; + return {quint64(d->tex), 0}; } id QMetalTextureData::viewForLevel(int level) diff --git a/src/gui/rhi/qrhivulkan.cpp b/src/gui/rhi/qrhivulkan.cpp index e67b249f9cc..1591829cac3 100644 --- a/src/gui/rhi/qrhivulkan.cpp +++ b/src/gui/rhi/qrhivulkan.cpp @@ -5570,14 +5570,14 @@ bool QVkTexture::build() bool QVkTexture::buildFrom(QRhiTexture::NativeTexture src) { - auto *img = static_cast(src.object); - if (!img || !*img) + VkImage img = VkImage(src.object); + if (img == 0) return false; if (!prepareBuild()) return false; - image = *img; + image = img; if (!finishBuild()) return false; @@ -5595,7 +5595,7 @@ bool QVkTexture::buildFrom(QRhiTexture::NativeTexture src) QRhiTexture::NativeTexture QVkTexture::nativeTexture() { - return {&image, usageState.layout}; + return {quint64(image), usageState.layout}; } void QVkTexture::setNativeLayout(int layout) diff --git a/tests/auto/gui/rhi/qrhi/tst_qrhi.cpp b/tests/auto/gui/rhi/qrhi/tst_qrhi.cpp index d3524a39b79..41057a15ab1 100644 --- a/tests/auto/gui/rhi/qrhi/tst_qrhi.cpp +++ b/tests/auto/gui/rhi/qrhi/tst_qrhi.cpp @@ -519,9 +519,8 @@ void tst_QRhi::nativeTexture() #ifdef TST_VK case QRhi::Vulkan: { - auto *image = static_cast(nativeTex.object); + auto image = VkImage(nativeTex.object); QVERIFY(image); - QVERIFY(*image); QVERIFY(nativeTex.layout >= 1); // VK_IMAGE_LAYOUT_GENERAL QVERIFY(nativeTex.layout <= 8); // VK_IMAGE_LAYOUT_PREINITIALIZED } @@ -530,27 +529,24 @@ void tst_QRhi::nativeTexture() #ifdef TST_GL case QRhi::OpenGLES2: { - auto *textureId = static_cast(nativeTex.object); + auto textureId = uint(nativeTex.object); QVERIFY(textureId); - QVERIFY(*textureId); } break; #endif #ifdef TST_D3D11 case QRhi::D3D11: { - auto *texture = static_cast(nativeTex.object); + auto *texture = reinterpret_cast(nativeTex.object); QVERIFY(texture); - QVERIFY(*texture); } break; #endif #ifdef TST_MTL case QRhi::Metal: { - void * const * texture = (void * const *)nativeTex.object; + auto texture = (void *)nativeTex.object; QVERIFY(texture); - QVERIFY(*texture); } break; #endif diff --git a/tests/manual/rhi/texuploads/texuploads.cpp b/tests/manual/rhi/texuploads/texuploads.cpp index a6b7d87d3e7..b6500c3fc3f 100644 --- a/tests/manual/rhi/texuploads/texuploads.cpp +++ b/tests/manual/rhi/texuploads/texuploads.cpp @@ -241,7 +241,7 @@ void Window::customRender() if (nativeTexture.object) { #if defined(Q_OS_MACOS) || defined(Q_OS_IOS) if (graphicsApi == Metal) { - qDebug() << "Metal texture: " << *(void**)nativeTexture.object; + qDebug() << "Metal texture: " << nativeTexture.object; // Now could cast to id and do something with // it, keeping in mind that copy operations are only done // in beginPass, while rendering into a texture may only