feat: SG-43947: Present through Vulkan for 10-bit output on Linux and Windows - #1427
Draft
cedrik-fuoco-adsk wants to merge 40 commits into
Draft
cedrik-fuoco-adsk wants to merge 40 commits into
cedrik-fuoco-adsk wants to merge 40 commits into
Conversation
cedrik-fuoco-adsk
force-pushed
the
SG-43947-vulkan-presentation
branch
6 times, most recently
from
September 29, 2026 14:57
9315785 to
e871428
Compare
…osted by VulkanView Bridge from AcademySoftwareFoundation#1319 to the restructured viewport this branch builds on. The Vulkan viewport becomes a native VulkanWindow (QWindow) embedded by a VulkanView container through createWindowContainer(), mirroring GLWindow/GLView, in place of the WA_NativeWindow/WA_PaintOnScreen QWidget. The GL interop negotiation and presentation-path record from AcademySoftwareFoundation#1319 are re-ported onto VulkanWindow by "fix(vulkan): port the Vulkan presentation interop negotiation" later in this series. Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
…t probe and the viewport Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
VulkanWindow::render() presented either the session output device or the main viewport, never both. Unlike the GL path, where QOpenGLWidget composites the control widget after paintGL regardless, the Vulkan viewport only appears via an explicit present -- so once presentation mode was on the main window sat on a stale frame. Present the viewport unconditionally, then additionally present a distinct output device. Also guard render() on m_initialized: initialize() runs from exposeEvent(), but resizeEvent() also calls requestUpdate(), so an UpdateRequest can reach the present path before the surface and swapchain exist. And present once from exposeEvent() for a doc-less window, which is a passive presentation output whose render() returns early at !session and would otherwise stay blank until the next main-view frame. Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
… output The presentation output went through DesktopVideoDevice's OpenGL ScreenView even when the main view was already presenting 10-bit through Vulkan, so the second display was capped at 8 bits. Add a DesktopVideoDevice subclass that delivers the frame through a Vulkan swapchain instead. It reuses the base class's frame handoff wholesale -- transfer()/transfer2() composite into m_viewDevice->defaultFBO() exactly as they do for the ScreenView, every stereo mode included -- and overrides only the window lifecycle and the present, since Vulkan has no QOpenGLWidget auto-composite. The output window is a top-level VulkanView constructed with a null doc. That null doc is what makes it passive: VulkanWindow::render() returns at !session, requestGLFallback() returns at !m_doc, and presentationAllowed() guards on it, so a presentation surface never drives the session nor drags the main window into a GL fallback. Supporting changes: - QTVulkanVideoDevice::fboID() reports the offscreen FBO once it exists. DesktopVideoDevice::transfer() returns early while the view device reports 0, which is how the first composite is deferred until the target exists -- without the override it would never composite. - A doc-less VulkanView keys its device name on the view rather than the doc, which would otherwise collide across screens. - DesktopVideoDevice::shouldUseVulkanPresentation() is the single GL-vs-Vulkan rule, used both by createDesktopVideoDevices() and by the RvDocument constructor so the main view and the presentation output cannot disagree on the backend. Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
DesktopVideoDevice's constructor calls addDefaultDataFormats(), whose depth defaults to 8, so a VulkanDesktopVideoDevice reported "RGB8" in Preferences > Video, in the top-view toolbar's device table and in humanReadableID() while presenting through a 10-bit swapchain. This was only ever a labelling bug, not truncation: depth is used solely to build the description string, and DesktopDataFormat forwards to DataFormat(const std::string&), which sets iformat = RGBA16F. That iformat is what picks the render target, so the presentation path is RGBA16F -> GL_RGB10_A2 shared texture -> A2B10G10R10 swapchain throughout. The number shown to the user was simply wrong. Two consequences worth knowing: - The persisted preference is safe. RvPreferences stores "dataFormat" as an index and setVideoDeviceStateFromSettings reads it back with toInt(); addDefaultDataFormats appends the same six stereo modes in the same order at any depth, so a user's stereo choice survives a GL <-> Vulkan device rebuild. - -presentData matches by description string, so "-presentData RGB8" no longer selects anything on a Vulkan presentation device, and a display profile saved against a VideoAndDataFormatID ending in RGB8 will not be found once the device reports RGB10. The coarser ModuleNameID and DeviceNameID profiles are unaffected. Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
…kend changes The presentation devices were built once at startup and only ever had their share device re-pointed afterwards, so their backend was frozen at whatever it was at launch. After a main-view backend transition the second display went black. Add DesktopVideoModule::rebuildDevices(), which re-evaluates the GL-vs-Vulkan rule and rebuilds the per-screen devices to match. It is a no-op returning false when the backend has not changed, so an unrelated depth change does not put a teardown transient on the second display; on a real change it closes each open device before destroying it, so no swapchain or ScreenView resources leak. It deliberately does not touch the session output device, because the devices it destroys may be referenced as that output. Add RvApplication::rebuildDesktopVideoDevices() to orchestrate: rebuild, re-bind the share device afterwards (so it never writes to an about-to-be-destroyed device), and re-open the presentation output, re-resolved by name through Options::presentDevice rather than by a pointer the rebuild may have invalidated. The critical piece is refreshing the session graph on a real rebuild. The DisplayGroupIPNodes built at startup by setPhysicalDevices still held the destroyed device pointers, so a later setOutputVideoDevice(newDevice) -> connectDisplayGroup -> findDisplayGroupByDevice(newDevice) matched nothing and silently no-op'd. That is the reported 10 -> 8 -> 10 -> enable-presentation black-screen repro, and it has to run even while presentation is off, since the pointers can change then and only be bound as the output later. Both hand-rolled setShareDevice loops in RvDocument now call through this instead. Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
…t change Selecting 10-bit while running on the OpenGL GLView ran the display output through rebuildGLView() at 10 bits. OpenGL cannot present 10-bit on the affected hardware (Mesa GLX, Windows WGL negotiation), so the rebuild failed validity, popped a misleading "Display Configuration is Invalid" dialog and zeroed the preference on the way out. The backend was otherwise fixed at window construction, so a depth change needed a restart. setDisplayOutput() now persists the requested depth to both Options and QSettings before doing anything else -- it never zeroes the preference -- and then applies it: - 10-bit while on GLView: promote the live view with swapGLViewToVulkan() when the probe says 10-bit can be presented, or show an honest "this display cannot present 10-bit output" dialog and stay at 8-bit when it cannot. The GL rebuild path is never reached for a 10-bit request on these platforms. - 8-bit or default while Vulkan is live: fall back with fallbackVulkanToGLView(), which rebuilds GLView from the depth just persisted. That direction is always available, so it needs no restart. swapGLViewToVulkan() is the forward mirror of fallbackVulkanToGLView(). It commits optimistically: VulkanWindow creates its surface and swapchain from exposeEvent(), so initialization cannot be verified synchronously. m_vulkanView is assigned before show() because the existing backstop -- requestGLFallback() -> fallbackVulkanToGLView() -- is guarded on it, and render() no-ops until initialized, so nothing presents to an uninitialized swapchain. Worst case is a brief blank frame during the swap. m_glView is set to null on the Vulkan path because backend-neutral code across RvDocument keys the active backend on (!m_glView). The old GLView is lazy-deleted on a timer, mirroring rebuildGLView, since deleting it inline while the swap is settling can dump core. Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
…View Disabling presentation mode aborted with ASSERT failure in Rv::VulkanView: "Called object is not of the correct type (class destructor may have already run)", qobjectdefs_impl.h:121 VulkanDesktopVideoDevice::close() deletes its VulkanView directly. The ~VulkanView body runs first, then ~QWidget destroys the widget's own QWidgetWindow and its children -- both of which emit destroyed() at a point where the VulkanView sub-object no longer exists. Qt's assertObjectType dynamic_casts the receiver before invoking a member slot, that cast fails, and Q_ASSERT_X aborts. A presentation output view is what makes this reachable. It is top-level, so window() returns the view itself and watchParentWindow() connects parentWindowDestroyed to its *own* QWidgetWindow, which dies with it. The main view watches the enclosing document window, which outlives it, so it never delivered the signal during destruction. Fix both ends. The destructor severs the watched-window connection and the viewport-window destroyed lambda before the base destructors run -- the lambda would not assert, being a functor, but would write m_vulkanWindow through a dangling this. And watchParentWindow() no longer watches anything when the view is its own top level: the point of that machinery is surviving Qt replacing the *enclosing* window, and a standalone output window has no enclosing tree to lose. Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
With 10-bit presentation enabled, an annotation stroke drew its first point and then stopped following the pointer. 10-bit alone was fine. Session::askForRedraw() redraws the control device *and* the output device, and it runs from arbitrary places -- including a mouse-motion handler, mid-stroke, with the main view's GL context current. VulkanDesktopVideoDevice::redraw() presented from there, and presenting means QTVulkanVideoDevice::syncBuffers(), which makes its own offscreen context current and never restores the previous one. The stroke's first point landed, the context was then silently swapped out from under the paint code, and every subsequent GL call went to the presentation device's context. It also ran a full vsync-blocking swapchain present per motion event. The GL output path never had this problem: its syncBuffers() is a QOpenGLWidget update(), which schedules a composite without touching the current context. Make redraw()/redrawImmediately() do nothing. The present is already driven in-frame: askForRedraw() redraws the control device too, which schedules VulkanWindow::render(), which renders, composites into this device via the inherited transfer(), and presents it through syncBuffers(). A doc-less presentation window also presents once on expose, so it is never left blank. Restore the viewport's context in VulkanWindow::render() after presenting the output device, for the same reason -- otherwise postRender() and everything after the frame run with the presentation device's context current. Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
A doc-less VulkanView is a passive presentation output owned by a VulkanDesktopVideoDevice: it is composited into and presented by that device and must never take part in input handling. Giving it focus is actively harmful -- a second focusable top-level fights the main window for activation, and the resulting WindowActivate storm starves the event loop, which was observed as annotation strokes never receiving their drag events. Build a passive output with no focus, no focus proxy and no event widget, so QTVulkanVideoDevice never creates a translator for it and VulkanWindow::event() bails at !hasTranslator() -- the window is inert by construction rather than relying on each handler to notice it has no document. WA_ShowWithoutActivating keeps show() from stealing activation, and WindowDoesNotAcceptFocus keeps the window manager from handing it back later. Also correct the reasoning in VulkanDesktopVideoDevice::redraw(): it claimed Session::askForRedraw() reaches it, which it cannot, because askForRedraw() casts to TwkGLF::GLVideoDevice while every desktop output device derives from the sibling GLBindableVideoDevice. The empty override still matters, because DesktopVideoDevice::syncBuffers() does reach redraw(). Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
…ug gpu Presentation-mode latency could not be attributed from the outside: the frame loop, the GPU, the swapchain and the Qt event loop all plausibly explain "the annotation trails the cursor", and guessing between them wasted several rounds. Measure them instead. VulkanWindow::render() now accumulates an averaged breakdown every 60 frames -- session->render(), viewport present split into fence-wait vs acquire, output present, postRender, and the loop period -- plus the pointer side: per-event handler cost, eventToRender (input to start-of-render), and eventToRetire. eventToRetire is the one that matters and the one nothing measured before: the age of the pointer event a frame answered, closed out when that frame's GPU work retires, sampled with a non-blocking vkGetFenceStatus poll per slot per frame. It is end-to-end interactive latency. eventToRender covers only the input half, and stayed flat at ~4.5ms across every configuration while the feel changed completely, which is precisely why it explained nothing. GLWindow gets the comparable subset so the two backends can be read side by side. It has no mainPresent term by construction: QOpenGLWindow swaps the control surface after paintGL returns, so that cost lands outside the measured region. QTVulkanVideoDevice reports its present path (GPU-interop vs CPU-fallback) per device and on every transition, not latched on the first frame -- the first syncBuffers() can run before that window's Vulkan is initialised, and latching there reported CPU-fallback for a device that then spent its whole life on interop. Also records, above useOptimalTilingForInterop(), that OPTIMAL tiling was measured on AMD (RADV, Mesa) and renders tile-pattern garbage, because GL is radeonsi there and GL_OPTIMAL_TILING_EXT carries no cross-driver layout guarantee. All of it is gated on ImageRenderer::debugGpu(). Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
Annotation trailed the cursor whenever presentation mode was on. The cause was not throughput: measured on a 4K presentation output, OpenGL and Vulkan ran presentation mode at the same frame interval (24-44ms vs 23-41ms), and only the Vulkan one felt like it lagged. What differed was how old the displayed pixels were. With two frames in flight the CPU runs a frame ahead, so the screen answers input from two frames back -- some 60ms at 30fps. The OpenGL path is effectively one deep, because paintGL() draws and Qt's following swap waits on that same work. Giving up the second frame costs no measurable frame rate here, since the loop is GPU-bound either way: fenceWait was 11-25ms in presentation mode against 0.004ms without it. So block on the frame's own fence after its present is queued, rather than letting the next frame's start-of-frame wait absorb it two frames later. The present is queued first so the driver still gets the frame as early as possible; this only stops the CPU running ahead. The passive presentation output is excluded. Measured after: eventToRetire settled at eventToRender plus one frame interval, with no queue left behind it. RV_VULKAN_MAX_FRAMES_IN_FLIGHT=2 restores the previous behaviour, for a machine where the CPU has enough independent work to be worth overlapping. The effective depth is reported in the -debug gpu frame line, because a silent knob cost a wasted test round. Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
…rmat QTVulkanVideoDevice allocated its offscreen FBO as GL_RGBA16F_ARB for every device. The control viewport needs that depth -- session->render() composites the whole main view into it across blended passes -- but a passive presentation output never does. It is only ever a blit destination for the inherited transfer()/transfer2()/fillWithTexture(), which hand over an already-composited frame. Keeping it at 16F there cost two full passes' worth of bandwidth at a 3840x2160 output: transfer() wrote 8 bytes/px, some 66MB, and syncBuffers() read all of it back to convert down to the 10-bit shared image. Matching the shared image's format halves both, and the conversion happens once, in a blit that was already going to run. The output is 10-bit either way, so no precision is lost that the present did not already discard. Measured on a 4K presentation output: the per-frame GPU wait dropped from 23ms to 5.5-16ms, frame interval from 33-45ms to 18-29ms, and eventToRetire from 38-49ms to 22-33ms -- parity with presentation off. Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
The OpenGL presentation path never lets the second display gate the viewport's loop: DesktopVideoDevice::syncBuffers() is a coalesced QOpenGLWidget::update() on an empty paintGL() that Qt drops when it falls behind. The Vulkan path queued its output work unconditionally, every frame, and at a 4K output that work is what the control viewport ends up waiting for in vkWaitForFences. Gate the output present on whether this device's own GPU work has caught up, rather than on whether the swapchain is full -- the latter never fires, because a loop slower than the display always leaves the queue room. canPresentNow() is checked from syncBuffers() ahead of any GL work, so a skipped frame costs nothing at all instead of costing the full-resolution blit, and no GL semaphore has been signalled yet so there is nothing to rebalance. The blocking paths inside presentSharedImage()/presentPixelData() keep a second, later skip for safety, which does have to drain the semaphore pair. A skipped present must be retried or the output is left on the frame before the one just composited, with nothing else coming back for it: the control viewport only renders when the session asks. Qt's update() is a dirty flag it must eventually honour, so parity needs the same guarantee -- requestBestEffortRetry() posts a coalesced UpdateRequest that render() serves from its passive-output branch, and a staleness timer forces a blocking frame through if the output has gone unpresented for more than 100ms. Note this does not currently fire on the hardware it was developed against: with a one-deep control pipeline the viewport waits for its own GPU work each frame, which gives the output's fences time to retire, so the gate passes every time and outputPresent never reaches zero. It is kept as the protection for the reverse balance -- a slower output display, a heavier output scene, or a faster control GPU -- which also means its skip and starvation paths are untested in practice. Isolated in its own commit so it can be dropped with a single revert. Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
…nd swap
Switching between 8- and 10-bit reset the display transfer function from
sRGB to None, discarding any assigned display profile with it.
Two causes, in sequence.
IPGraph::deviceChanged() adopted newDevice->physicalDevice()
unconditionally. VideoDevice's constructor seeds m_physicalDevice with
the device itself, and a viewport device only learns the monitor it sits
on when it first renders, via setAbsolutePosition() ->
deviceFromPosition(). Its one caller is
Session::setControlVideoDevice(), which during a backend swap runs on a
view that has never rendered -- so physicalDevice() was still that view,
and the display group's device.name was rewritten from the monitor
("Dell Inc. DELL U2725QE DP-1") to the viewport's own name ("RV Main
Window (Vulkan)/0x..."). Only adopt a physical device the new device
actually knows; the monitor has not changed, only the object drawing to
it.
RvApplication::rebuildDesktopVideoDevices() then called
IPGraph::setPhysicalDevices() to refresh stale device pointers, but that
is the startup routine: it deletes every DisplayGroupIPNode and rebuilds
them, and a new display group comes with a new colorPipeline holding
default contents. So the group was discarded and its colour state with
it. Add IPGraph::refreshPhysicalDevices(), which re-points existing
groups at the rebuilt devices by (module name, device name) -- the same
key display profiles are stored under, and stable across a rebuild
because createDesktopVideoDevices() names every screen from its QScreen
regardless of backend. Groups are created or deleted only for devices
that genuinely appeared or vanished.
It also clears a group's output device when that pointer is not among
the new devices. findDisplayGroupByDevice() compares raw pointers, so a
dangling one can alias a freshly allocated device at the same address
and return the wrong group. The control device is kept, being alive and
never in the module list.
Both changes are needed: the guard alone still lost the group to the
rebuild, and the refresh alone could not match a group whose name had
already been rewritten.
Note this touches IPCore paths shared with SDI/AJA output and
multi-monitor setups, which were not exercised here. The guard assumes a
device reporting itself as its own physical device carries no monitor
information -- true for viewport devices, and deviceChanged() only ever
runs on the control device today, but a real physical device
legitimately is its own physical device.
Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
QTVulkanVideoDevice now compares the GL physical-device UUID against the Vulkan physical device via VulkanWindow::physicalDeviceMatchesUUID(). If the UUIDs do not match (or cannot be queried), it falls back to the CPU pack-and-upload path instead of exporting GL memory to a different GPU. The result is cached and can be reset. VulkanWindow now creates a Vulkan 1.1 instance so vkGetPhysicalDeviceProperties2 and device UUIDs are available, and tightens physical-device selection to require a graphics+present queue family, VK_KHR_swapchain, and a 10-bit surface format when applicable. RvDocument's Vulkan-to-OpenGL fallback now preserves the requested display depth except when recovering from a 10-bit Vulkan failure, in which case it explicitly falls back to 8-bit OpenGL with clearer log messages. MuUICommands::colorAtCursor now reads the cursor pixel through the active GLVideoDevice with glReadPixels instead of requiring a GLView and QImage, making it backend-agnostic. Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
With no context current, glGetError() on Windows returns GL_INVALID_OPERATION for every call, for as long as nothing is current. TWK_GLDEBUG reported that as a GL error at each instrumented site that followed, so one missing context surfaced as a dozen copies of itself, attributed to whichever innocent line checked next. A missing context during presentation teardown showed up as an error inside makeCurrent(), a frame late and in the wrong place. twkGlAnyContextIsCurrent() answers the question directly. QOpenGLContext::currentContext() only knows about contexts Qt made current, and TwkGLFFBO's FBOVideoDevice binds its own natively, so trusting Qt alone would claim "no context" while a perfectly good one is current and would suppress the real errors the macro exists to print. glGetString() settles the cases Qt cannot see, and is only reached when Qt says no. twkGlPrintError() now checks that first and, when nothing is current, reports once per episode at the first site to notice, resetting when a context returns so a later episode is not swallowed. Both are declared outside the NDEBUG guard. TWK_GLDEBUG still compiles out in release -- polling glGetError() at every instrumented site is a debug-only cost -- but "no current GL context" is not instrumentation. It fires only when GL work cannot land, which is a fault in a release build too, and the callers that need to say so are compiled in both. Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
glDeleteFramebuffers() and friends are silent no-ops with no context current: the C++ object goes away, the driver's does not, and nothing says so. Teardown paths are where this bites, because they run from destructors and event callbacks rather than from inside a render, so nothing has arranged a context for them. Fixing that per call site does not converge -- each one found reveals the next, because nothing in the code states the invariant. This scope states it once. Open it at the top of anything that deletes GL objects and the question stops being the caller's problem. It resolves a context in three steps: already current, so do nothing -- the common case, a pointer compare, safe to put on paths that also run mid-render; else makeCurrent() on a supplied device, which is cheaper and is the context the objects were most likely created under; else a process-lifetime fallback. Destruction restores what was current, which -- because it only acquires when nothing was -- means making nothing current again. The fallback is what lets this work in destructors that have already had their device pointers cleared out from under them, which is exactly where the problem lives. It insists on QOpenGLContext::globalShareContext(): GL names belong to a share group, so deleting an FBO under a context outside the group that created it does nothing at all, and a non-sharing fallback would look like a fix while behaving like the bug. It is created once and never destroyed, because the paths needing it run while the application object is being torn down, so any owner freeing it at static-destruction time would free it either too early to be useful or after QGuiApplication has gone. If no context can be resolved it reports once and does nothing. It never throws and never aborts: a teardown helper must not be the reason a process fails to exit. Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
Leaving presentation mode resizes the main view, which reaches Session::deviceSizeChanged() from the resize rather than from a render, so no context is current. It flushed the renderer's entire ImageFBO pool into nothing: the C++ objects went away, the driver's did not, and the only sign was a GL_ERROR from ~GLFBO after the fact. Open a GLContextScope at the owner instead of at each caller. ImageFBOManager::flushImageFBOs() and destroyImageFBO() cover every path that reaches them, present and future, so the hand-placed makeCurrent() in Session::clearVideoDeviceCaches() and the context restore in ~RvDocument go away rather than accumulating. Session passes the device it already knows -- deviceSizeChanged() its argument, clearVideoDeviceCaches() the control device -- which is cheaper than the fallback and is the context the objects were created under. ImageRenderer needs the same at three more points. Device::clearFBOs() deletes the FBO ring buffer, clearState() deletes the program cache immediately after flushing the pool -- so the pool's own scope closing would leave a gap -- and ~ImageRenderer covers both plus the program cache object. Session clears the renderer's device pointers before destroying it, so by then it has nothing to ask and the scope falls back to its own context; that case is the reason the fallback exists. ~GLFBO keeps a backstop. With no context current it reports once, naming the destruction site, and skips the GL calls rather than pretending the names were released. It asks only for an FBO that owns GL names: the GLFBO(const GLVideoDevice*) constructor builds a handle onto whatever the device has bound, with no id and no PBO, so destroying one issues nothing and needs no context. Checking the context before checking for work reported a leak that cannot happen on the ordinary path where a device outlives its window. It reports rather than asserts. A leaked FBO is worth a line of output; it is not worth aborting a shutdown that would otherwise have completed, least of all in the debug build someone is using to diagnose that shutdown. Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
UninitPBOPools() never released a PBO. PBOWrap::uninitPBOPool() only flipped an initialized flag, leaving gPoolToGPU and gPoolFromGPU -- both file-scope statics -- to delete their buffers from ~GLPixelBufferObjectPool at static destruction, after main() has returned. Qt is gone by then and no context can be obtained on any platform, so every glDeleteBuffers() in there was a silent no-op and the driver reclaimed the memory with the process. Give the pool a clear() that does the release, and call it from uninitPBOPool() so it happens while the application is still up. clear() empties the containers and resets the accounting as well as deleting, because the destructor still runs later and would otherwise walk the same entries a second time. UninitPBOPools() is called from main() once the event loop has returned, so the views and their contexts are already gone and nothing is current. A GLContextScope supplies the fallback context -- still available there, since the QApplication outlives the call -- which also covers the GLSyncObject fences deleted alongside each buffer. Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
RvConsoleWindow installs its cout/cerr redirect under
#if defined(NDEBUG) || defined(PLATFORM_WINDOWS)
and took it down under
#if defined(NDEBUG) || !defined(PLATFORM_WINDOWS)
A Windows debug build is the one combination where those disagree, so
there the redirect went in and never came out. ConsoleBuf stayed on
cout and cerr with m_console pointing at the destroyed window.
main() deletes RvApplication before finalizePython(), and Py_Finalize's
garbage collection can still write -- a ResourceWarning from an
unclosed socket, for one. That write reached ConsoleBuf, followed
m_console into freed memory, and locked a QMutex whose bits happened to
read "contended". Nothing ever releases it, so RV hung on the way out
and had to be killed. It also meant any late shutdown output was lost.
Guard on having installed the redirect rather than on a second
attempt at the same #if. m_stdoutBuf and m_stderrBuf are non-null only
if the install ran, and processLastTextBuffer() nulls them if it got
there first, so this is correct in every build and safe to run twice.
Delete the ConsoleBuf too, so nothing is left pointing at the window.
Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
RV never calls quit(). It relies entirely on Qt's quitOnLastWindowClosed, and QApplicationPrivate::shouldQuit() counts every visible top-level widget carrying WA_QuitOnClose, which is on by default. So any auxiliary window left visible when the session window goes stops exec() from ever returning: the process stays up with a stray dialog on screen and has to be killed. The console reached that state on its own. RvConsoleWindow::processTextBuffer() calls show() and raise() for any line the show-on preference considers interesting, and at showOn=3 processLine() returns true for every line. It runs from a queued event, so it lands after ~RvDocument has closed the console, reopening it as the last visible window while the rest of shutdown is still producing output. Two changes, because either alone leaves a hole. WA_QuitOnClose is cleared on the console, the preferences dialog and the profile manager, so a log or settings window can never hold the process open however it came to be visible -- including a user deliberately leaving one open. And RvApplication::isShuttingDown(), set in ~RvDocument before the last document closes those windows, gates the auto-show so no console appears on the way out. The attribute is applied to all three because they are closed in the same ~RvDocument block and have exactly the same exposure; fixing only the one that was observed would leave two identical bugs behind. Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
detachAudioOutputDevice() hands work to the audio thread with Qt::BlockingQueuedConnection three times over -- emitStopDevice(), emitStopAudio(), and the delete of the output objects -- and none of them checked that the thread could run it. A BlockingQueuedConnection blocks until the target's event loop dispatches the call, so a thread that never started, whose createAudioOutput() failed, or that has already left exec() never releases the caller. detachAudioOutputDevice() then never reaches its own quit() and wait(). Calling from the audio thread itself would deadlock outright. canBlockOnAudioThread() answers that before each handoff: the thread is running, it has an event dispatcher, and it is not us. Skipping the stops costs nothing when it is false, since a thread not running its loop is not playing either. The delete becomes best-effort rather than all-or-nothing. It is still marshalled when there is a loop, which is what keeps Qt6 debug builds from tripping QObject::~QObject()'s cross-thread assertion on the QIODevice that QWindowsAudioSink parents. When there is not, the now idempotent deleteAudioOutputObjects() runs after wait() has returned and the thread is finished. A Qt warning on the way out beats never getting out. wait() is bounded and reports once if the bound is reached, so a wedged audio thread degrades to a slow exit rather than no exit. This is not the hang reported on Windows -- a captured stack put that in RvConsoleWindow -- but it is the same failure waiting to happen, and it is not reachable by inspection alone. Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
front() on an empty container is undefined behaviour, and a hard assert
("front() called on empty vector") in an MSVC debug build. Callers pair
data()/rawData() with size(), so reporting the absence of storage with a
null pointer makes a zero-length copy out of or into a cleared property
a no-op rather than a crash.
Release builds already returned null here for any property that never
held a value, so this only makes the existing behaviour well defined.
Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
PackageManager deleted m_globalSettingsP without clearing it. globalSettings() only allocates when that pointer is null, so every later caller got a reference to freed memory and died dereferencing the destroyed QSettings inside it. RvDocument reaches this while closing the last document, so anything that saves settings after that point -- RvConsoleWindow::done() closing the console dialog, for one -- crashed on the way out. Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
When createAudioOutput() fails the render thread returns without reaching exec(), so no event loop ever runs on it. detachAudioOutputDevice() cannot marshal the deletion back onto that thread afterwards -- its BlockingQueuedConnection has no loop to run on -- so whatever was allocated before the failure was never freed. Release it here, on the thread that owns it, before returning. Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
Two faults in DesktopVideoDevice::queryColorProfile().
The search for a window on the target screen walked every top-level
QWindow in the process, and QWindow::winId() creates the platform window
when there is none. Every QQuickWidget -- so every QWebEngineView panel,
Live Review among them -- owns a parentless offscreen QQuickWindow that
Qt is explicit must never be created ("Do not call create() on
offscreenWindow", qquickwidget.cpp). Handing it a platform window trips
Q_ASSERT(!d->offscreenWindow->handle()) at the end of
QQuickWidget::createFramebufferObject() and aborts RV the moment that
panel is first shown. Consider only windows that are already realized:
any of them on that screen reports the same monitor profile.
The ICC lookup below it sized its path buffer from an unchecked length,
freed a new[] allocation with scalar delete, used UrlCreateFromPath's
output without checking it succeeded, and dereferenced
cmsOpenProfileFromFile's result without a null test -- it returns null
when the path the driver reported is gone or unreadable -- and never
closed the profile. Use std::vector, take the DWORD the API actually
wants, and check both calls.
Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
…indow The presentation output's ScreenView was a top-level QOpenGLWidget. In Qt 6 that is composited through its own top-level window's RHI backing store and takes that window's GL context as its share parent, not the application's global share context -- so it can land in a private share group. transfer() wraps the renderer's output FBO colour texture in a local FBO, and a texture is only visible across contexts in the same group: in the wrong one glIsTexture() is false for a live texture, every transfer() is refused and the second display stays black. Which group it landed in varied run to run, which is what made the black presentation output intermittent. Split the class. ScreenWindow is a QOpenGLWindow, which takes the context to share with as a constructor argument -- the only point at which sharing can be established. ScreenView becomes a plain QWidget container around it via createWindowContainer, so the top-level is not forced onto the OpenGL RHI backend. This mirrors GLView/GLWindow, which is the main view and demonstrably sits in the renderer's group. PartialUpdateBlit keeps a backing FBO -- transfer() requires fboID() to be non-zero -- and does not clear before paintGL(). initializeGL() no longer calls context()->setShareContext(): that only takes effect on the next create(), and the context already exists by then, so it never did what it looked like it did. It verifies the share group and reports a mismatch instead. Two consequences of the new surface: - open() makes the share device current before copying its surface format. A QOpenGLWindow creates its context lazily, so right after a main-view backend swap the new main view's context does not exist yet and glShareContext() is null -- which is another way to end up outside the renderer's group. - Nothing primes the context in open() any more. A PartialUpdateBlit window's makeCurrent() binds the backing FBO that Qt only creates on the first paint, so calling it before then dereferences a null FBO inside Qt and takes the process down. transfer(), transfer2() and makeCurrent() gate on fboID() for the same reason and skip the frame; show() drives the expose that creates the FBO. Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
FBOs are not shared between contexts but textures are, so transfer() and transfer2() keep a per-context clone of the renderer's output FBO wrapped around its colour texture. The cache was keyed on the source pointer and trusted forever. Two ways that goes wrong: - The renderer deletes and reallocates those FBOs (ImageRenderer::Device::clearFBOs, ImageFBOManager::newImageFBO), so the same address comes back as a different FBO. - The borrowed texture can be dead by the time it is attached, which leaves the clone incomplete without any call failing outright. Nothing ever invalidated a cached incomplete clone, so a single transient error turned into a permanently black output that every later frame blitted from. Route both paths through cloneForSource(), which re-verifies a cached clone against the source's texture, target and size, discards an incomplete clone rather than caching it, and returns null so the caller skips the frame. On the first refusal it reports whether the texture is gone or merely in another share group, which are otherwise indistinguishable from the outside. GLFBO::isComplete() is the non-throwing completeness test that needs, for callers assembling an FBO from attachments they do not own. releaseFBOClones() drops the cache with a context current, and clearCaches()/unbind() now go through it. TWK_GLDEBUG after glBlitFramebuffer so an incomplete framebuffer is attributed to the blit instead of to the next frame's makeCurrent(). Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
Quitting out of presentation mode logged a long tail of GL_INVALID_OPERATION starting in ~GLFBO, and left the presentation output black for the rest of the session. All of it is GL teardown running with no context current: once none is, glGetError() keeps returning that same error, so one lost context is worth a great many messages, none of them near the cause. - QTGLVideoDevice::makeCurrent() did nothing at all when the platform surface was gone but the QOpenGLContext was not -- which is the state Qt leaves the device in while shutting down, since the native window is destroyed before the C++ object. Keep a QOffscreenSurface, created while the window is still healthy, and bind the context to that instead: GL deletion needs a current context, not a visible one. The handle() test also belongs in the outer condition rather than nested inside it, where a live window with a dead surface fell through every branch silently. The genuinely unreachable case now says so once. - DesktopVideoDevice::close() deleted the view before m_viewDevice. ~GLVideoDevice deletes the device's GL text context, and ~GLTextContext deletes the FTGL fonts, which delete GL textures -- all of it against a context the view had just taken with it, so the textures leaked on every presentation-mode toggle, not only at exit. Release the FBO clones first (that needs the view's context), then the device, then the view, then hand the main view's context back. - VulkanDesktopVideoDevice::close() deliberately does not chain to the base, so it has to release the FBO clones itself -- before setViewDevice(nullptr) takes away the device it needs to make a context current. - RvApplication dropped the session's output device only after close()ing it. Unbind first: ImageRenderer::setOutputDevice() calls unbind() on the outgoing device, and it has to run while that context is alive. It then restores the main view's context, because close() leaves nothing current and DesktopVideoDevice's own restore goes through its share device, which is null whenever the main view is Vulkan. That is where the bogus "Could not retrieve OpenGL version. Make sure you have installed the Nvidia drivers." came from: queryGLIntoContainer() reading GL_VERSION with no context. - ImageRenderer::setOutputDevice() falls back to the control device's context. m_outputDevice.glDevice is a dynamic_cast to GLVideoDevice and is null for every GLBindableVideoDevice output -- presentation, AJA, NDI -- because the two are siblings, not base and derived. The control context is the right fallback: it owns the FBOs being cleared, and the rest of the function already depends on it further down. Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
DesktopVideoModule::rebuildDevices() re-derived the GL-vs-Vulkan decision from shouldUseVulkanPresentation(), which reads the persisted display-depth preference. That is the requested intent, not the backend the main view is actually running: a 10-bit request that fell back to GL at runtime keeps its 10-bit intent on purpose. A presentation output built on the opposite backend to the viewport is a black second display. Pass the backend down from RvDocument instead. It is the only place that knows which widget actually exists now, and it calls rebuildDesktopVideoDevices() from each of the three swap paths anyway. shouldUseVulkanPresentation() stays for the initial build, when there is no main view to ask, and now says so. Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
RvDocument's "OpenGL is already live" branch wrote neither Options nor QSettings, and returns early whenever the GL context already has the requested depth. The 10-bit and Vulkan-live branches above it both write those first. Selecting 8-bit from 10-bit therefore did nothing to the very state other subsystems read back as "the requested display depth", leaving Options claiming 10-bit for the rest of the session. Write the depth before anything below can return early. Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
Three sizing faults, all of which surface once the output lives on a screen whose devicePixelRatio differs from the one its window was created on. - The swapchain-recreate test compared the caller's requested size against m_vkSwapchainExtent. createSwapchain() takes its extent from capabilities.currentExtent, so it cannot be driven to match a request the surface disagrees with: any caller off by even a pixel recreated the swapchain on every frame, forever and silently, since both surface-format reports are latched. Compare against the surface's current extent instead. - The grow-only shared image sized its headroom from the primary screen. Use this window's own screen: a presentation output lives on a second display, and the primary is both the wrong one and, when it is the smaller of the two, useless as headroom. - The present copy and blit took their destination extent from the shared image, which is sized from the caller's request. A stale devicePixelRatio inflating that -- a 3840x2160 output asking for 5760x3240 -- wrote outside the swapchain image, which is invalid usage and so undefined contents or a faulted submit rather than a visible error. vkCmdCopyImage cannot scale, so clamp it to the overlap; vkCmdBlitImage can, so fill the swapchain from the used sub-region. Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
Fixes from a C++ review of this branch. - twkGlAnyContextIsCurrent() probed with glGetString(), which is undefined with no context current and can crash on macOS. Ask the platform instead (CGLGetCurrentContext / wglGetCurrentContext); Linux keeps glGetString(), which GLVND answers with null. - When the audio thread does not exit within the bounded wait, stop deleting objects it still owns and leak the thread, detached from its parent, instead of destroying a running QThread (fatal in Qt6). - presentPixelData() compared the swapchain against the requested size rather than the surface extent, recreating it every frame on a mismatch and copying past the swapchain image. Share the surface check with getSharedImageInfo() and clamp the copy. - Check every staging buffer create/allocate/bind/map result and null freed handles, so a failed map no longer writes through an uninitialised pointer and an early return cannot double free. - A failed submit after the fence reset left the fence unsignaled, so the next wait on that slot hung forever. Re-signal it with an empty submit that also consumes the acquire semaphore, then fall back to GL. Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
Follow-up to the C++ review of this branch. - Order interop tiling candidates by vendor preference: OPTIMAL first on NVIDIA, LINEAR first elsewhere. Mesa reports OPTIMAL as exportable but renders tile garbage with it, so trying it first everywhere regressed RADV. Drop the now redundant legacy-heuristic log line. - ~QTVulkanVideoDevice cleans every slot whenever its context exists; slot 1 imports could previously leak and pin the Vulkan memory. - Document that GLContextScope keeps an already-current context, which is right for shared objects but not for FBOs. - Store exported fds/handles in the slot record as soon as they are obtained, so a later failure in getSharedImageInfo() closes them. - Delete copy operations on QTVulkanVideoDevice and QTGLVideoDevice, and close an existing view in VulkanDesktopVideoDevice::open(). - rebuildDesktopVideoDevices() takes the initiating document's session instead of the active document's. - Extract DesktopVideoDevice::tenBitDisplayRequested() and persist the display depth once in setDisplayOutput(). - Make the report-once flags atomic, guard a negative screen index, catch by const reference, fix narrowing and -Wparentheses, and initialise the new RvDocument members in declaration order. - Remove dead code (unused instance setup, duplicated kMaxStaleSeconds, VulkanBuildProbe.cpp), give findMemoryType internal linkage with an unsigned shift, and use include guards in the new RvCommon headers. Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
The fourth parameter of UrlCreateFromPath is a DWORD dwFlags (reserved, must be 0), not a pointer. The NULL -> nullptr sweep in b750556 turned the original NULL (which MSVC defines as 0) into nullptr, which does not convert to DWORD and fails the Windows build with C2664. Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
…k into VulkanWindow - VulkanWindow/VulkanView/RvDocument/VulkanDesktopVideoDevice: move members not set from ctor params to default member initializers. - VulkanView owns QTVulkanVideoDevice via std::unique_ptr, still reset explicitly at the same point in the destructor. - QTVulkanVideoDevice: GL context, offscreen surface, FBO and translator held by std::unique_ptr with the explicit release order kept; ~QTVulkanVideoDevice() override. - Group per-frame parallel arrays into FrameSync, StagingBuffer and SharedImage structs (VulkanWindow) and SharedGLObjects (QTVulkanVideoDevice). - File-static helpers into an anonymous namespace; findMemoryType returns std::optional; deviceProc<> helper for vkGetDeviceProcAddr; transitionImageLayout() helper for the image barriers. - std::numeric_limits instead of UINT32_MAX/UINT64_MAX; std::array for candidates, composite alpha preference and submit/present arrays; std::string_view name helpers; descriptive local names. - using instead of typedef for Timer; range-based for where the index is unused. - Remove unused RvDocument::vulkanView(), VulkanView::vulkanWindow(), VulkanView::isInitialized(), QTVulkanVideoDevice::vulkanWindow() and QTVulkanVideoDevice::eventWidget(). - Remove ImageRenderer::reportGL(), replaced by debugGpu(). Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
…nWindow - forcedTilingRequested() returns std::optional<VkImageTiling> instead of a bool plus an out-parameter - QTVulkanVideoDevice::setEventWidget uses the same ternary as the constructor Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
cedrik-fuoco-adsk
force-pushed
the
SG-43947-vulkan-presentation
branch
from
September 29, 2026 17:32
e871428 to
f612001
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
feat: SG-43947: Present through Vulkan for 10-bit output on Linux and Windows
Linked issues
Depends on #1319 (10-bit Vulkan presentation, Linux + Windows). The diff below is described relative
to that PR.
Summarize your change.
#1319 gives the main viewport a 10-bit Vulkan present path. This PR extends that path to the
presentation output (the second display), and makes the backend choice something RV can change
while it is running instead of fixing it at startup.
VulkanDesktopVideoDevicepresents the second displaythrough a Vulkan swapchain. Until now it went through the OpenGL
ScreenView, which capped it at8 bits even when the main view was already 10-bit.
devices are rebuilt onto the new backend, and the session's display groups, colour pipeline and
display profile survive the swap.
VulkanWindow(QWindow) embeddedby a
VulkanViewcontainer throughcreateWindowContainer(). This replaces feat: SG-43127: SG-43594: 10-bit Vulkan presentation (Linux + Windows) #1319'sWA_NativeWindow/WA_PaintOnScreenQWidgetand mirrorsGLWindow/GLView. It is whatmakes safe teardown and a passive, doc-less output window possible.
already existed on the GL path, but toggling presentation mode and swapping backends made them
reachable or consistently reproducible.
Describe the reason for the change.
With #1319 alone:
rebuildGLView()at 10 bits. On theaffected drivers that fails validation, shows a misleading "Display Configuration is Invalid"
dialog and resets the preference. The backend could only change after a restart.
went black.
vkDestroySwapchainKHR. Qt destroys theplatform surface before the view's destructor runs.
Describe what you have tested and on which operating system.
Tested on Linux and Windows
Add a list of changes, and note any that might need special attention during the review.
If possible, provide screenshots.