feat: SG-42200: 10-bit Metal presentation - #1327
Draft
cedrik-fuoco-adsk wants to merge 45 commits into
Draft
cedrik-fuoco-adsk wants to merge 45 commits into
cedrik-fuoco-adsk wants to merge 45 commits into
Conversation
cedrik-fuoco-adsk
force-pushed
the
metal-10bit-macos
branch
2 times, most recently
from
July 6, 2026 14:55
f4e483d to
4c71ed7
Compare
… baseline `-debug gpu` only toggled ImageRenderer's GL reporting, and the flag was named after that one consumer. The Vulkan presentation work needs the same switch to gate its own diagnostics, so rename it to the backend-neutral debugGpu() and keep reportGL() as thin deprecated forwarders for existing callers. Under the flag, the GL viewport now reports a one-shot baseline that answers "why did I only get 8 bits per component" without a second run: the format GLView asked for, the format Qt negotiated, the context Qt actually created, the screen depth, the GL vendor/renderer/version, and -- on Linux -- which display server we are on. glDebugFormatSummary()/glDebugEnvOrUnset() live in GLView.cpp and are shared with GLWindow, which owns the post-negotiation half of that report. Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
The 10-bit presentation backend needs <vulkan/vulkan.h> and the loader, so add Vulkan to the managed dependency set rather than requiring a VULKAN_SDK environment variable or a setup-env.sh on the developer's machine. Vulkan here is Vulkan-Headers plus Vulkan-Loader, fetched from hash-pinned Khronos tarballs and modeled on glew.cmake. RV consumes only those two: every API call goes through vkGetInstanceProcAddr/vkGetDeviceProcAddr, so no validation layers or shader tools are pulled in. RV_DEPS_PREFER_INSTALLED still lets a system Vulkan win via the built-in FindVulkan module or the `vulkan` pkg-config module. FORCE_LIB pins the install dir to install/lib so the loader lands in a deterministic place across RHEL (lib64) and non-RHEL. Only Linux and Windows include it; macOS presents through Metal. Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
…idge ImageRenderer's pipeline -- renderMain, the shader cache, the paint effects -- is GL code, and rewriting it for Vulkan is not on the table. So keep rendering in GL and change only the final present: this device wraps a VulkanWindow as a TwkGLF::GLVideoDevice, so everything above it runs unchanged while the frame reaches the screen through a Vulkan swapchain that can actually carry 10 bits per component. RV renders into a GL texture that is exported to Vulkan as a shared image (external memory, plus external semaphores for the GL/Vulkan handoff), which VulkanWindow then blits into the swapchain. The device owns the GL side of that bridge: ensureGLContext() creates the context lazily and joins the Qt::AA_ShareOpenGLContexts group so FTGL font-atlas uploads land somewhere the atlas texture has storage, and the shared image is re-exported when the viewport is resized. The eventWidget is the QWindow's container QWidget, which QTTranslator needs for coordinate mapping (height/mapToGlobal) and mouse grab -- a QWindow alone cannot serve those. Not built yet; the CMake wiring lands with the integration commit. Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
VulkanWindow is the native presentation surface: it owns the instance, device, swapchain and per-frame sync, picks a 10-bit format when the surface offers one, and blits the shared GL image exported by QTVulkanVideoDevice into the swapchain. Presentation is pipelined rather than one-frame-in-flight, so a frame does not end in a blocking vkWaitForFences under FIFO vsync -- that is what kept dragging a dock splitter over the media view from starving the Qt event loop and stalling playback. Resizes reuse the exported image and recreate only the swapchain where they can. VulkanView is the host QWidget that embeds it via createWindowContainer() and owns the video device. This is deliberately the same shape as GLView/GLWindow: keeping the viewport on a native window of its own, rather than on a widget Qt composites into the top-level, keeps the main window off a render-to-texture composite path, and it leaves the two backends sharing one set of embedding and lifetime rules instead of two. VulkanBuildProbe.cpp holds one direct vkGetInstanceProcAddr reference so a Linux/Windows build proves headers and loader link availability up front instead of failing deep in the presentation code. Not built yet; the CMake wiring lands with the integration commit. Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
Builds the Vulkan sources and switches RvDocument over to creating a
VulkanView instead of a GLView on Linux and Windows, with a runtime fallback
to GLView (fallbackVulkanToGLView) when instance/device/swapchain creation
fails so a machine without a usable Vulkan driver still starts.
That fallback is why the rest of this is a spread of small changes rather than
one call site. RvDocument::view() is the GL-only accessor and is null on the
Vulkan path, so backend-neutral callers move to viewWidget() (the host QWidget)
or viewVideoDevice() (the active presentation device):
- MuUICommands/PyUICommands used view() for coordinate mapping, cursor and
focus. Wrapping a null QWidget* for Mu made the Session Manager's event
filter dereference null and crash.
- RvApplication called view()->makeCurrent() and view()->videoDevice()
unconditionally; there is no GL context to make current when presentation
is Vulkan, and the primary display group now comes from the session's
control device.
- DesktopVideoDevice is constructed with a null GL share device on this path;
it falls back to the default surface format and no explicit share context,
which Qt::AA_ShareOpenGLContexts already covers.
main.cpp keeps Qt::AA_ShareOpenGLContexts for a second reason now: with no
GLView to chain from, QTVulkanVideoDevice::ensureGLContext() joins that global
group instead.
The CMake change compiles the Vulkan sources only on Linux and Windows and
promotes WIN32_LEAN_AND_MEAN to target scope, since vulkan_win32.h includes
<windows.h> unconditionally and the legacy winsock.h that drags in collides
with the winsock2.h Qt pulls in. MuUICommands/PyUICommands guarded their GL
includes *inside* `#ifndef WIN32_LEAN_AND_MEAN`, so a target-scope define
would have skipped those includes entirely -- the `#endif` now closes the
define guard before the includes.
Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
RvSettings could fail to persist with AccessError (err: 1) on Windows for two independent reasons. QSettings writes atomically by default -- temp file plus rename -- and the rename fails if security software has RV.ini open at that instant. Writing directly to the file removes that failure point (setAtomicSyncRequired, Qt 5.13+). QSettings with IniFormat also does not create the parent directory on Windows, so the very first sync() on a clean machine fails outright. Create it before use. Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
…estroyed (#61) Fixes the SIGSEGV on quit that Bernard hit on Rocky 8 with a 10-bit monitor and the new Vulkan present path. Repros with nothing but launch RV, quit RV. ## Root cause Crash is inside the NVIDIA driver under `vkDestroySwapchainKHR` (`VulkanWindow.cpp:796`), reached from `~VulkanWindow`: ``` QEventLoop::exec -> sendPostedEvents -> QObject::event (DeferredDelete) -> ~RvDocument -> deleteChildren -> ~VulkanView -> ~QWidget -> QWindowContainer::~QWindowContainer() <- frame 15 -> ~VulkanWindow (VulkanWindow.cpp:132) <- frame 13 -> cleanupSwapchain (:796 = vkDestroySwapchainKHR) -> SIGSEGV (SEGV_MAPERR @ 0x2b) ``` Disassembling `QWindowContainer::~QWindowContainer()` in the shipped Qt 6.5.3 shows the ordering problem directly: ``` 1e8172: call QWindow::destroy()@plt <- destroys the QPlatformWindow (X drawable + VkSurfaceKHR) 1e8199: call *0x20(%rax) <- only *then* the virtual dtor -> ~VulkanWindow ``` By the time `~VulkanWindow` runs, the surface the swapchain was created against is gone, so the driver dereferences a dead object. Qt makes that `destroy()` call explicitly so a `QEvent::PlatformSurface` / `SurfaceAboutToBeDestroyed` is delivered *while the surface is still alive* — that is the documented hook for exactly this. `VulkanWindow` didn't handle the event, so it never got a chance to clean up in time. `exposeEvent()`'s `handle() != m_initializedHandle` check notices the same situation on the reparent path, but only after the fact. ## Change - New `VulkanWindow::releaseVulkanResources()`: the full teardown (shared images -> swapchain -> device), resetting to the pre-`initialize()` state. - `QEvent::PlatformSurface` handled at the **top** of `event()`, ahead of the `m_stopProcessingEvents` and null-device early-outs, since those would otherwise swallow it. - `~VulkanWindow` and `handleSurfaceLost()` both route through it. The destructor is now a backstop that is normally a no-op. - `QTVulkanVideoDevice::releaseSharedGLObjects()`, called first from `releaseVulkanResources()`, so the GL-side imported memory objects/semaphores never outlive the Vulkan memory they alias. That last one closes a **second instance of the same defect** on the live reparent path (the one your comments call out as triggered by inserting a `QWebEngineView`). Forcing a platform-window destroy+recreate aborts on the current branch with `free(): double free detected in tcache 2`. ## Verification Rocky 8, NVIDIA T1000 8GB, 10-bit monitor. Confirmed via `-debug gpu` that every run was really on `A2B10G10R10_UNORM_PACK32` with `first frame path = GPU-interop` (not the CPU fallback or the GL fallback). | Test | Before | After | |---|---|---| | launch + quit, x8 | crashes at `cleanupSwapchain:796`, stack matches the report | 8/8 clean | | launch + quit via the Quit action, x8 | 2/3 crashed | 8/8 clean | | destroy + recreate platform window (reparent path) | `free(): double free detected`, SIGABRT | clean teardown + full re-init | | `smptebars` playback + quit | — | clean, still `GPU-interop / A2B10G10R10 / 10-bit` | The crash is flaky (roughly 2 in 3 on this box), so the counts matter more than any single run. ## Notes - Nothing here changes the present path itself, only teardown ordering. - `handleSurfaceLost()` still exists and still works, but is now a late backstop rather than the primary detection — the surface event gets there first, so its `m_initialized && handle() != m_initializedHandle` condition should normally be false. - Only tested on Linux/X11. The same `QWindowContainer` ordering applies on Windows, so it should fix it there too, but I have no 10-bit Windows box to confirm on.
…ace selection A 10-bit viewport was entirely black on NVIDIA/Windows -- not just the media, but RV's own overlay geometry, so the whole GL->Vulkan frame was being lost -- while the same build was correct on AMD. createSwapchain() picked the swapchain format by matching VkFormat alone and ignored the colour space it was paired with. A surface in HDR mode advertises A2B10G10R10 more than once, once per colour space, and the enumeration order is vendor-specific: NVIDIA lists HDR10_ST2084 ahead of SRGB_NONLINEAR, AMD lists SRGB_NONLINEAR first. Taking the first format match therefore handed NVIDIA a PQ swapchain fed with RV's sRGB-encoded pixels. Under ST2084 an sRGB 0.2 lands near 2 nits and 0.5 near 90, so RV's dark viewport and its dark overlays read as black. Neither driver is at fault: the spec does not define this order. Select on the (format, colourSpace) pair, requiring SRGB_NONLINEAR, which is the only correct pairing for an sRGB renderer. A second pass accepts any colour space so the choice can never become worse than before, and warns when it has to. Alongside that, three things that let the failure be silent and black rather than degraded: * The exported shared image was never a dedicated allocation. vkGetImageMemoryRequirements2 + VkMemoryDedicatedRequirements is now consulted and VkMemoryDedicatedAllocateInfo chained when the driver asks for it, with GL told to match via GL_DEDICATED_MEMORY_OBJECT_EXT -- EXT_memory_object requires both sides to agree. AMD's Windows driver reports requiresDedicatedAllocation for this image, so the previous binding was invalid there too. The instance is now created at Vulkan 1.1, which the 2-variant query requires. RV_VULKAN_DISABLE_DEDICATED_ALLOCATION reverts it. * No GL call on the interop path was error-checked, so a rejected import left an incomplete texture, the blit was dropped, and Vulkan presented an image that was never written. The path now checks glGetError() after the import and after the wait/blit/signal, names the failing step, and demotes that device to the CPU present path for good. Errors are drained immediately before the sequence, because session->render() legitimately leaves some pending and blaming those on the interop path would demote a healthy device. * The NVIDIA OPTIMAL-tiling workaround was gated to Linux although its rationale -- one driver core behind GL and Vulkan -- holds on Windows too. Widened. The diagnostics that identify all of the above now print without -debug gpu: Vulkan device, GL vendor/renderer, chosen format *and colour space*, shared-image tiling and dedicated-allocation decision, and the present path. The surface format list prints once per window. The present-path report also replaces a first-frame-only latch that reported CPU-fallback/UNDEFINED for devices that ran on interop for the rest of the session. Verified on AMD Radeon 740M / Windows: SRGB_NONLINEAR chosen, tiling LINEAR, memory dedicated, present path GPU-interop, across an 8/10-bit backend swap and a resize. Not yet verified on NVIDIA or Linux. 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>
…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>
cedrik-fuoco-adsk
force-pushed
the
metal-10bit-macos
branch
from
September 24, 2026 16:05
33c6c4a to
90a68f7
Compare
Present the main view, and the Presentation Mode second-display output, through Metal on macOS (IOSurface + CALayer) when a 10-bit display depth is requested: the macOS counterpart of the Vulkan backend on Linux and Windows. - MetalView / QTMetalVideoDevice: a native CALayer-backed QWidget whose IOSurface is rendered by the GL-on-Metal context through IOSurface GL interop, with a GPU-blit CPU fallback (RV_METAL_FORCE_CPU_PRESENT=1 forces it for testing). Set RV_METAL_DEBUG_PRESENT to trace presents. - MetalDesktopVideoDevice: the 10-bit presentation output, a passive fullscreen MetalView whose device is the base m_viewDevice, so the inherited transfer()/transfer2() and every stereo mode work unchanged. Shares one backend-selection path with Vulkan rather than duplicating it: - DesktopVideoDevice::shouldUseNativePresentation() (was shouldUseVulkanPresentation) picks Vulkan or Metal per platform, for both the main view and the presentation output. - RvDocument routes Metal through the backend-neutral helpers (viewVideoDevice(), setActiveViewContentSize(), ...) and adds swapGLViewToMetal() / fallbackMetalToGLView() beside the Vulkan swaps, so a display-depth change applies at runtime. setDisplayOutput() dispatches to whichever native backend the platform has. - The desktop share device is retyped to TwkGLF::GLVideoDevice so the Metal main view can be it; the Vulkan path still passes null. - DesktopVideoModule::rebuildDevices() now retires the replaced devices and RvApplication::rebuildDesktopVideoDevices() purges them only after IPGraph::refreshPhysicalDevices() has re-pointed the display groups and the control device's physical device. Closing a device destroys a native window, which pumps the event loop and repaints through the graph, so destroying them first was a use-after-free. - fix(ipcore): setPhysicalDevicesInternal() deleted display groups by index while each destructor erased itself from the same vector, skipping every other group and leaving dangling device pointers. Iterate over a copy. Squashed from the metal-10bit-macos development history and rebased onto the Vulkan presentation branch. Signed-off-by: Cédrik Fuoco <cedrik.fuoco@autodesk.com>
cedrik-fuoco-adsk
force-pushed
the
metal-10bit-macos
branch
from
September 24, 2026 16:09
90a68f7 to
762e38f
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
2 participants
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-42200: 10-bit Metal presentation
Linked issues
Summarize your change
Adds a macOS-only 10-bit Metal presentation path for OpenRV when the user requests 10+2 display format and the hardware supports it. IPCore still renders through an offscreen OpenGL 2.1 context (GL-on-Metal); the new backend delivers pixels to the screen via IOSurface → CALayer instead of a
QOpenGLWidgetswap chain.The work is built on a platform-neutral view abstraction that decouples
RvDocumentfromGLView: shared code routes throughviewWidget()and the session's control video device, so alternative backends (this Metal path, and the existing Vulkan 10-bit path on Linux) can plug in without touching every call site.When 10-bit Metal is unavailable or fails at runtime, the app falls back to the legacy OpenGL
GLViewwith logging.Describe the reason for the change
OpenRV's main window has historically been a
QOpenGLWidget. True 10-bit display on macOS is unreliable through that path: requesting a 10+2 GL surface format does not reliably produce correct scanout, and aQOpenGLWidgetanywhere in the window hierarchy can force Qt onto_NSOpenGLViewBackingLayer, breaking CALayer/IOSurface presentation (including 4× tiling artifacts with device pixel ratio scaling).This PR introduces a dedicated
MetalView(nativeNSView+CALayerbacked by IOSurface) andQTMetalVideoDevice, which wraps it as aTwkGLF::GLVideoDevicesoImageRendererand the rest of IPCore continue to render into offscreen FBOs unchanged. The zero-copy path blits RGBA16F → IOSurface-backed RGB10_A2; when GL–IOSurface interop is unavailable, a GPU-blit + packed readback fallback avoids the previous per-pixel CPU pack loop (~25M float ops/frame at 4K).Additional fixes address real usability bugs on the new path: black flash on window re-expose (alt-tab / uncover), context/thread safety for upload threads, interop failure retry instead of permanently disabling zero-copy, and diagnostics overlay integration via
Qt::AA_ShareOpenGLContexts.Describe what you have tested and on which operating system
USE_METAL=ON, 10+2 display prefs: main window presents correctly, no banding vs 8-bit GL pathsupports10BitPresentation()fails or GL context creation fails →GLViewwith log messageOS tested: macOS (version / hardware — please confirm)
Add a list of changes, and note any that might need special attention during the review
Core architecture (review carefully)
RvDocumentusesQWidget* m_viewWidget+viewWidget()instead of hardcodedm_glView;MuUICommands/PyUICommandsupdated for null-safe coordinate mappingRvApplicationview(); primary display group driven by session control video deviceMetalViewWA_PaintOnScreen/WA_OpaquePaintEvent,paintEngine() → nullptr, IOSurface cache + re-present on expose, coalescedUpdateRequest,renderImmediately()for synchronous redrawQTMetalVideoDeviceglReadPixels(CPU fallback); interop failure latch with periodic retryRvDocumentMetalViewvsGLViewwhen 10+2 prefs +MetalView::supports10BitPresentation();fallbackMetalToGLView()on context failureImageRenderer/SessionKnown limitations (not blockers, but reviewers should be aware)
DesktopVideoDevicepath, not ported)