From d48ed117795bd15b52ea74ee344a9faa9dea5dc1 Mon Sep 17 00:00:00 2001 From: phantomcake Date: Thu, 8 Oct 2026 02:09:07 -0400 Subject: [PATCH 1/8] Coin: bump for the retained-IR staleness fix Picks up the Coin change that keys SoShape's retained-IR cache on the vertex sources it was built from and adds SoGeometryDesc::retainedGeneration, so the Vulkan backend can detect a rebuild whose stream pointers were recycled by the allocator. (brianmk/coin#84) --- src/3rdParty/coin | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/3rdParty/coin b/src/3rdParty/coin index 753bdf619e9e..ca4e27f44282 160000 --- a/src/3rdParty/coin +++ b/src/3rdParty/coin @@ -1 +1 @@ -Subproject commit 753bdf619e9e6d202354de045eed1c9546036382 +Subproject commit ca4e27f44282c4596e38f510aea58d6c789010ee From 7af78052b4129ebb95edd1e580052bb7fa288391 Mon Sep 17 00:00:00 2001 From: phantomcake Date: Wed, 7 Oct 2026 23:27:24 -0400 Subject: [PATCH 2/8] Gui: parent the 3D-view context menu to a top-level window With the Vulkan page active, QWidget::createWindowContainer() makes the whole MDI chain native (the QWindowContainer detects the QAbstractScrollArea ancestor that QuarterWidget/QGraphicsView provides), so the 3D viewer gets a non-top-level native window. NavigationStyle::openPopupMenu() then created its QMenu with the GL viewport as parent; QMenu::show derives the transient parent from that widget's nativeParentWidget(), so Qt handed a non-top-level QWindow to QWindow::setTransientParent(), which warns QWidgetWindow(...) must be a top level window. once per right-click and silently drops the transient parent. Parent the menu to the view's top-level widget instead, which fixes the popup metadata for every navigation style (they all share openPopupMenu). Regression test: tests/src/Gui/RightClickContextMenu.py drives the real binary under Xvfb with the Vulkan renderer, synthesizes a right-click into the Vulkan QWindowContainer, asserts the menu pops with no warning, and includes a control that reproduces the pre-fix parent so the test detects a revert. Registered as Gui_RightClickContextMenu_Vulkan (skips 77 without a Vulkan device). --- src/Gui/Navigation/NavigationStyle.cpp | 14 +- tests/src/Gui/CMakeLists.txt | 35 +++ tests/src/Gui/RightClickContextMenu.py | 335 +++++++++++++++++++++++++ 3 files changed, 383 insertions(+), 1 deletion(-) create mode 100644 tests/src/Gui/RightClickContextMenu.py diff --git a/src/Gui/Navigation/NavigationStyle.cpp b/src/Gui/Navigation/NavigationStyle.cpp index 04852e59a75d..100131eb0604 100644 --- a/src/Gui/Navigation/NavigationStyle.cpp +++ b/src/Gui/Navigation/NavigationStyle.cpp @@ -2716,7 +2716,19 @@ void NavigationStyle::openPopupMenu(const SbVec2s& position) MenuItem view; Gui::Application::Instance->setupContextMenu("View", &view); - auto contextMenu = new QMenu(viewer->getGLWidget()); + // A QMenu is a popup (top level) window and Qt derives its transient parent + // from the parent widget's window in QMenuPrivate::transientParentWindow(). + // The GL widget is a native *child* of the viewer as soon as a Vulkan page + // exists -- QWidget::createWindowContainer() makes the viewer and all its + // ancestors native (QWindowContainerPrivate::updateUsesNativeWidgets() sees + // the QGraphicsView, and QWidget::create() propagates that upwards) -- so + // Qt's fallback QWidget::nativeParentWidget() hands it the viewer's native + // child window. QWindow::setTransientParent() rejects that with + // "QWidgetWindow(...) must be a top level window." and drops the transient + // parent. Parent the menu to the view's top level window instead, so the + // popup resolves to a real top level window and gets the transient parent + // it is supposed to have. + auto contextMenu = new QMenu(viewer->getGLWidget()->window()); MenuManager::getInstance()->setupContextMenu(&view, *contextMenu); contextMenu->setAttribute(Qt::WA_DeleteOnClose); diff --git a/tests/src/Gui/CMakeLists.txt b/tests/src/Gui/CMakeLists.txt index 6bf341a7f0af..54bc976e0904 100644 --- a/tests/src/Gui/CMakeLists.txt +++ b/tests/src/Gui/CMakeLists.txt @@ -48,3 +48,38 @@ target_link_libraries(Gui_tests_run PRIVATE if(WIN32) target_compile_definitions(Gui_tests_run PRIVATE QUARTER_DLL) endif() + +# Regression for the Vulkan-only right-click context menu warning +# ("QWidgetWindow(...) must be a top level window."). Driven through the real +# FreeCAD binary: the bug needs the Vulkan page (QWidget::createWindowContainer) +# to make the MDI chain native, which a process-local test never brings up. It +# skips (exit 77) when no Vulkan device is present. +if(BUILD_GUI AND UNIX AND NOT APPLE AND FREECAD_USE_VULKAN) + find_program(FREECAD_XVFB_RUN xvfb-run) + if(FREECAD_XVFB_RUN) + set(FREECAD_GUI_TEST_HOME "${CMAKE_BINARY_DIR}/Testing/gui-home") + file(MAKE_DIRECTORY + "${FREECAD_GUI_TEST_HOME}" + "${FREECAD_GUI_TEST_HOME}/cfg" + "${FREECAD_GUI_TEST_HOME}/data" + "${FREECAD_GUI_TEST_HOME}/cache" + ) + add_test(NAME Gui_RightClickContextMenu_Vulkan + COMMAND + "${FREECAD_XVFB_RUN}" -a -s "-screen 0 1280x1024x24" + "${CMAKE_COMMAND}" -E env + "QT_QPA_PLATFORM=xcb" + "HOME=${FREECAD_GUI_TEST_HOME}" + "XDG_CONFIG_HOME=${FREECAD_GUI_TEST_HOME}/cfg" + "XDG_DATA_HOME=${FREECAD_GUI_TEST_HOME}/data" + "XDG_CACHE_HOME=${FREECAD_GUI_TEST_HOME}/cache" + "${CMAKE_BINARY_DIR}/bin/FreeCAD" + "${CMAKE_CURRENT_SOURCE_DIR}/RightClickContextMenu.py" + ) + set_tests_properties(Gui_RightClickContextMenu_Vulkan PROPERTIES + LABELS "Qt;requires-vulkan" + SKIP_RETURN_CODE 77 + TIMEOUT 300 + ) + endif() +endif() diff --git a/tests/src/Gui/RightClickContextMenu.py b/tests/src/Gui/RightClickContextMenu.py new file mode 100644 index 000000000000..dc8e44c36ecb --- /dev/null +++ b/tests/src/Gui/RightClickContextMenu.py @@ -0,0 +1,335 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +# +# Regression test for the Vulkan-only right-click context menu warning: +# +# QWidgetWindow(...) must be a top level window. +# +# Qt derives a popup's transient parent from the parent widget's window. The +# Vulkan page is created with QWidget::createWindowContainer(), which makes the +# whole MDI chain native. QMenu(viewer->getGLWidget()) then resolved to the +# viewer's native *child* window (getGLWidget() is the QGraphicsView's +# QOpenGLWidget viewport, whose native parent is the viewer's own +# "Gui::View3DInventorViewerClassWindow"), which QWindow::setTransientParent() +# rejects. The fix parents the menu to the view's top level window +# (viewer->getGLWidget()->window()). +# +# This is driven through the real FreeCAD binary because the regression needs the +# Vulkan window hierarchy; a process-local test never brings that up. It exits +# 0 on success, 1 on failure and 77 (CTest SKIP_RETURN_CODE) when no Vulkan +# renderer/page is available, so CTest reports a skip instead of a failure. +# +# Run standalone: +# xvfb-run -a -s "-screen 0 1280x1024x24" \ +# env HOME=/tmp/fc-tests-a XDG_CONFIG_HOME=/tmp/fc-tests-a/cfg \ +# XDG_DATA_HOME=/tmp/fc-tests-a/data XDG_CACHE_HOME=/tmp/fc-tests-a/cache \ +# QT_QPA_PLATFORM=xcb build-freecad/bin/FreeCAD \ +# tests/src/Gui/RightClickContextMenu.py + +import os +import sys +import traceback + +SKIP_RETURN_CODE = 77 +TOP_LEVEL_WARNING = "must be a top level window" + +# FreeCAD redirects sys.stdout to the report view once the GUI is up, so keep a +# handle on the process stream for the CTest log. +_STDOUT = sys.__stdout__ + + +def emit(message): + try: + _STDOUT.write(message + "\n") + _STDOUT.flush() + except Exception: + pass + + +def has_vulkan_device(): + """Best-effort Vulkan device enumeration, so the test skips (77) instead of + letting the Vulkan widget come up against no driver (which currently + segfaults inside QuarterVulkanWidget on instance-creation failure).""" + import ctypes + + class VkApplicationInfo(ctypes.Structure): + _fields_ = [ + ("sType", ctypes.c_int), + ("pNext", ctypes.c_void_p), + ("pApplicationName", ctypes.c_char_p), + ("applicationVersion", ctypes.c_uint32), + ("pEngineName", ctypes.c_char_p), + ("engineVersion", ctypes.c_uint32), + ("apiVersion", ctypes.c_uint32), + ] + + class VkInstanceCreateInfo(ctypes.Structure): + _fields_ = [ + ("sType", ctypes.c_int), + ("pNext", ctypes.c_void_p), + ("flags", ctypes.c_uint32), + ("pApplicationInfo", ctypes.POINTER(VkApplicationInfo)), + ("enabledLayerCount", ctypes.c_uint32), + ("ppEnabledLayerNames", ctypes.c_void_p), + ("enabledExtensionCount", ctypes.c_uint32), + ("ppEnabledExtensionNames", ctypes.c_void_p), + ] + + try: + vk = ctypes.CDLL("libvulkan.so.1") + except OSError: + return False + + vk.vkCreateInstance.restype = ctypes.c_int + vk.vkCreateInstance.argtypes = [ + ctypes.POINTER(VkInstanceCreateInfo), + ctypes.c_void_p, + ctypes.POINTER(ctypes.c_void_p), + ] + vk.vkEnumeratePhysicalDevices.restype = ctypes.c_int + vk.vkEnumeratePhysicalDevices.argtypes = [ + ctypes.c_void_p, + ctypes.POINTER(ctypes.c_uint32), + ctypes.c_void_p, + ] + vk.vkDestroyInstance.restype = None + vk.vkDestroyInstance.argtypes = [ctypes.c_void_p, ctypes.c_void_p] + + app_info = VkApplicationInfo() + app_info.sType = 0 # VK_STRUCTURE_TYPE_APPLICATION_INFO + app_info.pApplicationName = b"freecad-right-click-regression" + app_info.apiVersion = (1 << 22) | (0 << 12) # VK_API_VERSION_1_0 + create_info = VkInstanceCreateInfo() + create_info.sType = 1 # VK_STRUCTURE_TYPE_INSTANCE_CREATE_INFO + create_info.pApplicationInfo = ctypes.pointer(app_info) + + instance = ctypes.c_void_p() + if vk.vkCreateInstance(ctypes.byref(create_info), None, ctypes.byref(instance)) != 0: + return False + count = ctypes.c_uint32(0) + result = vk.vkEnumeratePhysicalDevices(instance, ctypes.byref(count), None) + vk.vkDestroyInstance(instance, None) + return result == 0 and count.value > 0 + + +def pump(app, rounds=80): + import FreeCADGui + + for _ in range(rounds): + app.processEvents() + try: + FreeCADGui.updateGui() + except Exception: + pass + + +def find_vulkan_container(main_window): + """The QWindowContainer of the active Vulkan 3D page, or None.""" + from PySide import QtWidgets + + for widget in main_window.findChildren(QtWidgets.QWidget): + if "QuarterVulkanWidget" not in widget.metaObject().className(): + continue + for child in widget.findChildren(QtWidgets.QWidget): + if child.windowHandle() is not None: + return child + return widget + return None + + +def send_right_click(widget, position): + from PySide import QtCore, QtGui, QtWidgets + + for event_type, buttons in ( + (QtCore.QEvent.MouseButtonPress, QtCore.Qt.RightButton), + (QtCore.QEvent.MouseButtonRelease, QtCore.Qt.NoButton), + ): + event = QtGui.QMouseEvent( + event_type, + position, + widget.mapToGlobal(position), + QtCore.Qt.RightButton, + buttons, + QtCore.Qt.NoModifier, + ) + QtWidgets.QApplication.sendEvent(widget, event) + + +def run(): + import FreeCAD + import FreeCADGui + from PySide import QtCore, QtWidgets, QtGui + + messages = [] + + def handler(_mode, _context, message): + messages.append(str(message)) + + QtCore.qInstallMessageHandler(handler) + + def top_level_warnings(since): + return [m for m in messages[since:] if TOP_LEVEL_WARNING in m] + + if not has_vulkan_device(): + emit("SKIP: no Vulkan physical device") + return SKIP_RETURN_CODE + + # The Vulkan renderer is opt-in; enable it and ask for the raster Vulkan + # view mode (1) before any 3D view is created. + prefs = FreeCAD.ParamGet("User parameter:BaseApp/Preferences/View") + prefs.SetBool("UseVulkanRenderer", True) + prefs.SetInt("VulkanRenderMode", 1) + + main_window = FreeCADGui.getMainWindow() + if main_window is None: + emit("SKIP: no GUI main window") + return SKIP_RETURN_CODE + main_window.show() + + app = QtWidgets.QApplication.instance() + if app is None: + emit("SKIP: no QApplication") + return SKIP_RETURN_CODE + + document = FreeCAD.newDocument("RightClickContextMenuTest") + pump(app, 100) + + try: + view = FreeCADGui.ActiveDocument.ActiveView + except Exception: + view = None + if view is None: + emit("SKIP: no active 3D view") + return SKIP_RETURN_CODE + + if not hasattr(view, "setRenderMode"): + emit("SKIP: this build has no Vulkan render modes") + return SKIP_RETURN_CODE + view.setRenderMode(1) + + # Wait for the Vulkan page to materialize its native container. + container = None + for _ in range(60): + pump(app, 10) + container = find_vulkan_container(main_window) + if container is not None and container.windowHandle() is not None: + break + if container is None: + emit("SKIP: no Vulkan viewport page (no Vulkan device or renderer off)") + return SKIP_RETURN_CODE + + try: + graphics_view = view.graphicsView() + viewport = graphics_view.viewport() + except Exception: + graphics_view = None + viewport = None + + # Sanity-check the harness before asserting on the app path: a deliberate + # qWarning must reach the handler. + before = len(messages) + QtCore.qWarning("right-click-context-menu self test") + if not messages[before:]: + emit("FAIL: the Qt message handler captured nothing; detection is broken") + return 1 + + # ---------------------------------------------------------------- app path + # Synthesize a right-button press+release into the visible Vulkan 3D + # viewport (falling back to the Coin/GL viewport if the container does not + # accept synthesized input in this environment). + popup = None + warning_before_popup = None + for target in (container, viewport): + if target is None: + continue + active = QtWidgets.QApplication.activePopupWidget() + if active is not None: + active.close() + pump(app, 20) + before = len(messages) + send_right_click(target, target.rect().center()) + pump(app, 80) + popup = QtWidgets.QApplication.activePopupWidget() + if popup is not None: + warning_before_popup = top_level_warnings(before) + break + + if popup is None or not popup.isVisible(): + emit("FAIL: no visible QMenu popped for a right-click in the 3D viewport") + return 1 + + emit( + "right-click context menu: popped {} (visible={})".format( + popup.metaObject().className(), popup.isVisible() + ) + ) + + if warning_before_popup: + popup.close() + emit( + "FAIL: right-click produced the transient-parent warning: " + + warning_before_popup[0] + ) + return 1 + + popup.close() + pump(app, 20) + + # --------------------------------------------------- self-check (pre-fix) + # Reproduce the exact pre-fix parent: QMenu(viewer->getGLWidget()) is + # QMenu(), whose native parent is the viewer's native + # child window. If this no longer warns, the detection above is meaningless. + if viewport is None: + emit("FAIL: no QGraphicsView viewport to reproduce the pre-fix parent with") + return 1 + + before = len(messages) + pre_fix_menu = QtWidgets.QMenu(viewport) + pre_fix_menu.addAction("pre-fix parent") + pre_fix_menu.popup(QtGui.QCursor.pos()) + pump(app, 60) + pre_fix_warnings = top_level_warnings(before) + pre_fix_menu.close() + pump(app, 20) + + if not pre_fix_warnings: + emit( + "FAIL: the pre-fix parent no longer reproduces the transient-parent " + "warning; the regression check above cannot detect a revert" + ) + return 1 + emit("pre-fix parent reproduces the warning: " + pre_fix_warnings[0]) + + # And the fixed parent (the top level window) must stay silent. + before = len(messages) + fixed_menu = QtWidgets.QMenu(viewport.window()) + fixed_menu.addAction("fixed parent") + fixed_menu.popup(QtGui.QCursor.pos()) + pump(app, 60) + fixed_warnings = top_level_warnings(before) + fixed_menu.close() + pump(app, 20) + if fixed_warnings: + emit("FAIL: the fixed parent still warns: " + fixed_warnings[0]) + return 1 + + emit("OK: right-click context menu opens without a transient-parent warning") + return 0 + + +def main(): + try: + return run() + except BaseException: + traceback.print_exc(file=_STDOUT) + return 1 + finally: + try: + _STDOUT.flush() + except Exception: + pass + + +# FreeCAD executes the macro at top level (there is no __main__ guard), so run +# unconditionally and exit directly with the CTest status code: FreeCAD keeps its +# event loop running after a macro returns. +os._exit(main()) From 83320d22a6e9de0e32efc2689aa9b29618cb4ae1 Mon Sep 17 00:00:00 2001 From: phantomcake Date: Wed, 7 Oct 2026 23:27:35 -0400 Subject: [PATCH 3/8] Gui/Sketcher: commit in-tree viewport, overlay and tooling work Checkpoint of work that was sitting uncommitted in the worktree (no single topic), so the tree is no longer carrying a mixed staged/unstaged state: - Overlay and annotation nodes: OverlayManager, OverlayWidgets, So3DAnnotation, SoDatumLabel, SoBrepOverlayHelpers, ToolBarManager. - Quarter input and Vulkan surface plumbing: EventFilter hover coalescing, InputDeviceHost, QuarterWidget/QuarterVulkanWidget, VulkanViewportAdapter, View3DInventorViewer, MainWindow. - Selection and view providers: SoFCUnifiedSelection, ViewProvider, ViewProviderDocumentObject. - Sketcher tooling: CommandConstraints, CommandSketcherTools, CommandSketcherBSpline, DrawSketch* handlers, ViewProviderSketch, SketchObjectOperations. - Tests: tests/src/Gui/SelectionTest.cpp. These changes were not authored in this session; they are committed as-is to preserve them. --- src/Gui/Inventor/So3DAnnotation.cpp | 54 ++++++ src/Gui/Inventor/So3DAnnotation.h | 6 + src/Gui/MainWindow.cpp | 8 + src/Gui/OverlayManager.cpp | 61 +++++++ src/Gui/OverlayWidgets.cpp | 46 +++++ src/Gui/Quarter/EventFilter.cpp | 7 +- src/Gui/Quarter/InputDeviceHost.h | 10 ++ src/Gui/Quarter/QuarterVulkanWidget.cpp | 13 ++ src/Gui/Quarter/QuarterVulkanWidget.h | 12 ++ src/Gui/Quarter/QuarterWidget.cpp | 17 ++ src/Gui/Quarter/QuarterWidget.h | 2 + src/Gui/Selection/SoFCUnifiedSelection.cpp | 56 +++++- src/Gui/Selection/SoFCUnifiedSelection.h | 1 + src/Gui/SoDatumLabel.cpp | 15 +- src/Gui/ToolBarManager.cpp | 40 +++++ src/Gui/View3DInventorViewer.cpp | 5 + src/Gui/View3DInventorViewer.h | 2 + src/Gui/ViewProvider.h | 9 + src/Gui/ViewProviderDocumentObject.h | 16 ++ src/Gui/VulkanViewportAdapter.cpp | 43 +++-- src/Mod/Part/Gui/SoBrepOverlayHelpers.h | 16 +- .../Sketcher/App/SketchObjectOperations.cpp | 3 +- src/Mod/Sketcher/Gui/CommandConstraints.cpp | 170 ++++++++++++++---- .../Sketcher/Gui/CommandSketcherBSpline.cpp | 21 ++- src/Mod/Sketcher/Gui/CommandSketcherTools.cpp | 59 ++++-- src/Mod/Sketcher/Gui/DrawSketchController.h | 17 ++ src/Mod/Sketcher/Gui/DrawSketchHandler.cpp | 15 ++ .../Sketcher/Gui/DrawSketchHandlerOffset.h | 3 +- .../Sketcher/Gui/DrawSketchHandlerSymmetry.h | 7 +- src/Mod/Sketcher/Gui/ViewProviderSketch.cpp | 24 ++- src/Mod/Sketcher/Gui/ViewProviderSketch.h | 7 + tests/src/Gui/SelectionTest.cpp | 33 +++- 32 files changed, 719 insertions(+), 79 deletions(-) diff --git a/src/Gui/Inventor/So3DAnnotation.cpp b/src/Gui/Inventor/So3DAnnotation.cpp index 0bff944183d4..184a31217465 100644 --- a/src/Gui/Inventor/So3DAnnotation.cpp +++ b/src/Gui/Inventor/So3DAnnotation.cpp @@ -39,6 +39,12 @@ #include "So3DAnnotation.h" #include +#ifdef HAVE_COIN_IR_RENDER_ACTION +# include +# include +# include +#endif + using namespace Gui; SO_ELEMENT_SOURCE(SoDelayedAnnotationsElement); @@ -184,3 +190,51 @@ void So3DAnnotation::GLRenderOffPath(SoGLRenderAction* /* action */) { // should never render, this is a separator node } + +#ifdef HAVE_COIN_IR_RENDER_ACTION +void So3DAnnotation::IRRender(SoIRRenderAction* action) +{ + // On the OpenGL path annotations are deferred through SoDelayedAnnotationsElement + // and replayed later with the depth buffer cleared, so the subtree always draws on + // top of the scene (while still depth-testing against itself). The retained-IR + // path has no delayed replay: render the subtree inline and then promote the + // commands it recorded to the OVERLAY pass, which the raster backend draws last, + // on top. This mirrors SoFCSelection/SoRasterOverlay. Without it, for example a + // PartDesign pattern's semi-transparent removed-volume preview is depth-rejected + // by the opaque solid it is embedded in, so it never appears. + if (!action) { + return; + } + + SoDrawList& list = action->getMutableDrawList(); + const int firstCommand = list.getNumCommands(); + + inherited::IRRender(action); + + const int count = list.getNumCommands(); + if (count <= firstCommand) { + return; + } + + SoState* state = action->getState(); + const SbViewportRegion vp = SoViewportRegionElement::get(state); + const short vx = static_cast( + std::max(0, static_cast(vp.getViewportOriginPixels()[0]))); + const short vy = static_cast( + std::max(0, static_cast(vp.getViewportOriginPixels()[1]))); + const short vw = static_cast( + std::max(1, static_cast(vp.getViewportSizePixels()[0]))); + const short vh = static_cast( + std::max(1, static_cast(vp.getViewportSizePixels()[1]))); + + for (int i = firstCommand; i < count; ++i) { + SoRenderCommand& command = list.getCommand(i); + command.pass = SO_RENDERPASS_OVERLAY; + command.state.raster.scissorEnabled = TRUE; + command.state.raster.scissorX = vx; + command.state.raster.scissorY = vy; + command.state.raster.scissorWidth = vw; + command.state.raster.scissorHeight = vh; + } +} +#endif diff --git a/src/Gui/Inventor/So3DAnnotation.h b/src/Gui/Inventor/So3DAnnotation.h index 786f997d9e78..15837679a4dd 100644 --- a/src/Gui/Inventor/So3DAnnotation.h +++ b/src/Gui/Inventor/So3DAnnotation.h @@ -30,6 +30,8 @@ #include #include +class SoIRRenderAction; + namespace Gui { @@ -123,6 +125,10 @@ class GuiExport So3DAnnotation: public SoSeparator void GLRenderInPath(SoGLRenderAction* action) override; void GLRenderOffPath(SoGLRenderAction* action) override; +#ifdef HAVE_COIN_IR_RENDER_ACTION + void IRRender(SoIRRenderAction* action) override; +#endif + protected: ~So3DAnnotation() override = default; }; diff --git a/src/Gui/MainWindow.cpp b/src/Gui/MainWindow.cpp index 086959cf3960..c3023e4e5025 100644 --- a/src/Gui/MainWindow.cpp +++ b/src/Gui/MainWindow.cpp @@ -58,6 +58,7 @@ #include #include #include +#include #include @@ -1054,6 +1055,13 @@ void MainWindow::closeActiveWindow() int MainWindow::confirmSave(App::Document* doc, QWidget* parent, bool addCheckbox) { + // Test hook: FC_SKIP_UNSAVED_PROMPT=1 makes an unattended run (the fcprobe + // harness) never block on this modal dialog -- it discards unsaved changes + // instead of asking. No effect on a normal interactive session. + if (std::getenv("FC_SKIP_UNSAVED_PROMPT") != nullptr) { + return ConfirmSaveResult::Discard; + } + QMessageBox box(parent ? parent : this); box.setObjectName(QStringLiteral("confirmSave")); box.setIcon(QMessageBox::Question); diff --git a/src/Gui/OverlayManager.cpp b/src/Gui/OverlayManager.cpp index 8f301c73c3d6..22f1ec7ae1cc 100644 --- a/src/Gui/OverlayManager.cpp +++ b/src/Gui/OverlayManager.cpp @@ -42,6 +42,8 @@ #include #include +#include +#include #include #include "OverlayManager.h" @@ -1812,6 +1814,24 @@ static inline bool isNear(const QPoint& a, const QPoint& b, int tol = 16) bool OverlayManager::eventFilter(QObject* o, QEvent* ev) { + if (std::getenv("FC_DRAG_DEBUG")) { // DNDBG + const QEvent::Type _t = ev->type(); + if (_t == QEvent::MouseButtonPress || _t == QEvent::MouseMove + || _t == QEvent::MouseButtonRelease || _t == QEvent::MouseButtonDblClick) { + auto* _me = static_cast(ev); + const QPoint _g = _me->globalPosition().toPoint(); + QWidget* _wa = QApplication::widgetAt(_g); + fprintf(stderr, + "[DNDBG] ev=%d recv=%s isWidget=%d btn=%d btns=%d qbtn=%d g=(%d,%d) " + "wAt=%s dragging=%s grab=%s\n", + int(_t), o->metaObject()->className(), o->isWidgetType() ? 1 : 0, + int(_me->button()), int(_me->buttons()), int(QApplication::mouseButtons()), + _g.x(), _g.y(), _wa ? _wa->metaObject()->className() : "-", + OverlayTabWidget::_Dragging + ? OverlayTabWidget::_Dragging->metaObject()->className() : "-", + QWidget::mouseGrabber() ? QWidget::mouseGrabber()->metaObject()->className() : "-"); + } + } if (d->intercepting || !getMainWindow() || !o->isWidgetType()) { return false; } @@ -1923,12 +1943,53 @@ bool OverlayManager::eventFilter(QObject* o, QEvent* ev) return false; } if (OverlayTabWidget::_Dragging && OverlayTabWidget::_Dragging != o) { + if (std::getenv("FC_DRAG_DEBUG")) { // DNDBG + fprintf(stderr, "[DNDBG] MISMATCH dragging=%s recv=%s isWidget=%d\n", + OverlayTabWidget::_Dragging->metaObject()->className(), + o->metaObject()->className(), o->isWidgetType() ? 1 : 0); + } + // Wayland has no platform mouse grab (grabMouse() is skipped + // below), so during a drag the pointer events are delivered to + // the widget under the cursor instead of to the drag source, and + // the drag would end as soon as the cursor leaves the thin title + // bar. Forward the drag's own move/release events to the source + // so it keeps tracking. On platforms where the grab works the + // source already receives its own events (o == _Dragging), so this + // is only reached here on Wayland. + if (QGuiApplication::platformName() == QLatin1String("wayland")) { + QWidget* src = OverlayTabWidget::_Dragging; + auto* me = dynamic_cast(ev); + const bool dragMove = me && me->type() == QEvent::MouseMove + && (me->buttons() & Qt::LeftButton); + const bool dragRelease = me && me->type() == QEvent::MouseButtonRelease + && me->button() == Qt::LeftButton; + if (dragMove || dragRelease) { + if (std::getenv("FC_DRAG_DEBUG")) { // DNDBG + fprintf(stderr, "[DNDBG] FORWARD type=%d to=%s\n", + int(me->type()), src->metaObject()->className()); + } + QMouseEvent fwd(me->type(), + src->mapFromGlobal(me->globalPosition()), + me->globalPosition(), + me->button(), + me->buttons(), + me->modifiers()); + QApplication::sendEvent(src, &fwd); + return true; + } + } if (auto titleBar = qobject_cast(OverlayTabWidget::_Dragging)) { + if (std::getenv("FC_DRAG_DEBUG")) { // DNDBG + fprintf(stderr, "[DNDBG] ENDDRAG titlebar\n"); + } titleBar->endDrag(); } else if ( auto splitHandle = qobject_cast(OverlayTabWidget::_Dragging) ) { + if (std::getenv("FC_DRAG_DEBUG")) { // DNDBG + fprintf(stderr, "[DNDBG] ENDDRAG splitter\n"); + } splitHandle->endDrag(); } } diff --git a/src/Gui/OverlayWidgets.cpp b/src/Gui/OverlayWidgets.cpp index 0a3b3ade6f90..c04326d3313b 100644 --- a/src/Gui/OverlayWidgets.cpp +++ b/src/Gui/OverlayWidgets.cpp @@ -46,6 +46,8 @@ #include #include +#include +#include #include #include "OverlayWidgets.h" @@ -511,6 +513,9 @@ OverlayTabWidget::~OverlayTabWidget() } if (_Dragging == this || (_Dragging && isAncestorOf(_Dragging))) { + if (std::getenv("FC_DRAG_DEBUG")) { // DNDBG + fprintf(stderr, "[DNDBG] TABWIDGET clear _Dragging (teardown)\n"); + } _Dragging = nullptr; } } @@ -2276,6 +2281,10 @@ static inline bool isNear(const QPoint& a, const QPoint& b, int tol = 16) void OverlayTitleBar::endDrag() { + if (std::getenv("FC_DRAG_DEBUG")) { // DNDBG + fprintf(stderr, "[DNDBG] TITLEBAR endDrag isDrag=%d\n", + OverlayTabWidget::_Dragging == this ? 1 : 0); + } if (OverlayTabWidget::_Dragging == this) { OverlayTabWidget::_Dragging = nullptr; setCursor(Qt::OpenHandCursor); @@ -2290,6 +2299,11 @@ void OverlayTitleBar::endDrag() void OverlayTitleBar::mouseMoveEvent(QMouseEvent* me) { + if (std::getenv("FC_DRAG_DEBUG")) { // DNDBG + fprintf(stderr, "[DNDBG] TITLEBAR move btns=%d pending=%d isDrag=%d\n", + int(me->buttons()), mouseMovePending ? 1 : 0, + OverlayTabWidget::_Dragging == this ? 1 : 0); + } if (ignoreMouse) { if (!(me->buttons() & Qt::LeftButton)) { ignoreMouse = false; @@ -2306,6 +2320,9 @@ void OverlayTitleBar::mouseMoveEvent(QMouseEvent* me) } mouseMovePending = false; OverlayTabWidget::_Dragging = this; + if (std::getenv("FC_DRAG_DEBUG")) { // DNDBG + fprintf(stderr, "[DNDBG] TITLEBAR START DRAG\n"); + } } if (OverlayTabWidget::_Dragging != this) { @@ -2324,6 +2341,11 @@ void OverlayTitleBar::mouseMoveEvent(QMouseEvent* me) void OverlayTitleBar::mousePressEvent(QMouseEvent* me) { + if (std::getenv("FC_DRAG_DEBUG")) { // DNDBG + fprintf(stderr, "[DNDBG] TITLEBAR press btn=%d dragging=%s\n", int(me->button()), + OverlayTabWidget::_Dragging + ? OverlayTabWidget::_Dragging->metaObject()->className() : "-"); + } mouseMovePending = false; QWidget* parent = parentWidget(); if (OverlayTabWidget::_Dragging || !parent || !getMainWindow() || me->button() != Qt::LeftButton) { @@ -2377,6 +2399,10 @@ void OverlayTitleBar::mousePressEvent(QMouseEvent* me) void OverlayTitleBar::mouseReleaseEvent(QMouseEvent* me) { + if (std::getenv("FC_DRAG_DEBUG")) { // DNDBG + fprintf(stderr, "[DNDBG] TITLEBAR release btn=%d isDrag=%d\n", int(me->button()), + OverlayTabWidget::_Dragging == this ? 1 : 0); + } if (ignoreMouse) { me->ignore(); return; @@ -2681,6 +2707,10 @@ void OverlaySplitterHandle::paintEvent(QPaintEvent* e) void OverlaySplitterHandle::endDrag() { + if (std::getenv("FC_DRAG_DEBUG")) { // DNDBG + fprintf(stderr, "[DNDBG] SPLIT endDrag isDrag=%d\n", + OverlayTabWidget::_Dragging == this ? 1 : 0); + } auto tabWidget = qobject_cast(splitter()->parentWidget()); if (tabWidget) { dockWidget(); @@ -2705,6 +2735,13 @@ void OverlaySplitterHandle::keyPressEvent(QKeyEvent* ke) void OverlaySplitterHandle::mouseMoveEvent(QMouseEvent* me) { + if (std::getenv("FC_DRAG_DEBUG")) { // DNDBG + const QPoint _lp = me->pos(); + const QPoint _gp = me->globalPosition().toPoint(); + fprintf(stderr, "[DNDBG] SPLIT move dragging=%d btns=%d isDrag=%d lpos=(%d,%d) g=(%d,%d)\n", + dragging, int(me->buttons()), OverlayTabWidget::_Dragging == this ? 1 : 0, + _lp.x(), _lp.y(), _gp.x(), _gp.y()); + } if (OverlayTabWidget::_Dragging != this) { return; } @@ -2749,6 +2786,11 @@ void OverlaySplitterHandle::mouseMoveEvent(QMouseEvent* me) void OverlaySplitterHandle::mousePressEvent(QMouseEvent* me) { + if (std::getenv("FC_DRAG_DEBUG")) { // DNDBG + fprintf(stderr, "[DNDBG] SPLIT press btn=%d dragging=%s\n", int(me->button()), + OverlayTabWidget::_Dragging + ? OverlayTabWidget::_Dragging->metaObject()->className() : "-"); + } if (OverlayTabWidget::_Dragging || !getMainWindow() || me->button() != Qt::LeftButton) { return; } @@ -2782,6 +2824,10 @@ void OverlaySplitterHandle::mousePressEvent(QMouseEvent* me) void OverlaySplitterHandle::mouseReleaseEvent(QMouseEvent* me) { + if (std::getenv("FC_DRAG_DEBUG")) { // DNDBG + fprintf(stderr, "[DNDBG] SPLIT release btn=%d isDrag=%d\n", int(me->button()), + OverlayTabWidget::_Dragging == this ? 1 : 0); + } if (OverlayTabWidget::_Dragging != this || me->button() != Qt::LeftButton) { return; } diff --git a/src/Gui/Quarter/EventFilter.cpp b/src/Gui/Quarter/EventFilter.cpp index a2621f5b3a76..a06dff89d018 100644 --- a/src/Gui/Quarter/EventFilter.cpp +++ b/src/Gui/Quarter/EventFilter.cpp @@ -265,7 +265,12 @@ EventFilter::eventFilter(QObject * obj, QEvent * qevent) // obsolete when dispatched. Consume a stale pure-hover move without // picking so the queue drains to the latest position; its position is // remembered for the trailing pick that runs once the pointer stops. - if (isMove && staleMs > 0 && me->buttons() == Qt::NoButton) { + // A move that an active tool/edit handler is consuming + // (host->eventsGrabbed()) drives a live preview and must never be dropped, + // or the preview stops tracking the cursor -- only pure-hover moves are + // coalesced. + if (isMove && staleMs > 0 && me->buttons() == Qt::NoButton + && !PRIVATE(this)->host->eventsGrabbed()) { const quint64 ts = me->timestamp(); if (ts != 0) { QElapsedTimer clock; diff --git a/src/Gui/Quarter/InputDeviceHost.h b/src/Gui/Quarter/InputDeviceHost.h index c0bdcc688ca7..ba41b0e059d9 100644 --- a/src/Gui/Quarter/InputDeviceHost.h +++ b/src/Gui/Quarter/InputDeviceHost.h @@ -42,6 +42,16 @@ class QUARTER_DLL_API InputDeviceHost virtual SbVec2s inputWindowSize() const = 0; //! Deliver a translated Coin event into the interaction pipeline. virtual bool processSoEvent(const SoEvent* event) = 0; + + //! True while an active tool/edit handler is consuming the events. Such a + //! move drives a live preview and must not be discarded by pure-hover + //! coalescing. The host reports it either from the scene's modal grabber or + //! from edit mode, since some handlers (Sketcher) consume events via + //! SoEventCallback::setHandled() without ever setting a grabber. + virtual bool eventsGrabbed() const + { + return false; + } }; } // namespace Quarter diff --git a/src/Gui/Quarter/QuarterVulkanWidget.cpp b/src/Gui/Quarter/QuarterVulkanWidget.cpp index ca81a91a94d2..6fa8ed8ad018 100644 --- a/src/Gui/Quarter/QuarterVulkanWidget.cpp +++ b/src/Gui/Quarter/QuarterVulkanWidget.cpp @@ -790,6 +790,9 @@ class SIM::Coin3D::Quarter::QuarterVulkanWidgetPrivate //! an InputDeviceHost) and delivers them through eventSink. EventFilter* eventFilter = nullptr; std::function eventSink; + //! Reports whether an active tool/edit handler consumes the events (see + //! setEventGrabProbe). + std::function eventGrabProbe; // Debug-only synthetic mouse injector state (see pollInjectFile()). #ifdef FREECAD_VULKAN_DEBUG_HOOKS @@ -1317,6 +1320,11 @@ void QuarterVulkanWidget::setEventSink(std::function sink d->eventSink = std::move(sink); } +void QuarterVulkanWidget::setEventGrabProbe(std::function probe) +{ + d->eventGrabProbe = std::move(probe); +} + qreal QuarterVulkanWidget::devicePixelRatio() const { return QWidget::devicePixelRatio(); @@ -1346,6 +1354,11 @@ bool QuarterVulkanWidget::processSoEvent(const SoEvent * event) return d->eventSink ? d->eventSink(event) : false; } +bool QuarterVulkanWidget::eventsGrabbed() const +{ + return d->eventGrabProbe ? d->eventGrabProbe() : false; +} + #ifdef FREECAD_VULKAN_DEBUG_HOOKS void QuarterVulkanWidget::pollInjectFile() { diff --git a/src/Gui/Quarter/QuarterVulkanWidget.h b/src/Gui/Quarter/QuarterVulkanWidget.h index 44e73db7fb0f..5f97a6a0358b 100644 --- a/src/Gui/Quarter/QuarterVulkanWidget.h +++ b/src/Gui/Quarter/QuarterVulkanWidget.h @@ -140,6 +140,17 @@ class QuarterVulkanWidget : public QWidget, public InputDeviceHost */ void setEventSink(std::function sink); + /*! + \brief Predicate reporting whether an active tool/edit handler is + consuming the events. + + The widget holds no Coin event pipeline of its own (the hidden GL viewer + owns the `SoHandleEventAction`), so the viewport adapter supplies this + query. Hover coalescing uses it to avoid discarding a move that drives a + live preview. + */ + void setEventGrabProbe(std::function probe); + //! @name InputDeviceHost //@{ qreal devicePixelRatio() const override; @@ -147,6 +158,7 @@ class QuarterVulkanWidget : public QWidget, public InputDeviceHost QSize inputSize() const override; SbVec2s inputWindowSize() const override; bool processSoEvent(const SoEvent * event) override; + bool eventsGrabbed() const override; //@} /*! diff --git a/src/Gui/Quarter/QuarterWidget.cpp b/src/Gui/Quarter/QuarterWidget.cpp index d82fe68581dd..b921ad8e9afa 100644 --- a/src/Gui/Quarter/QuarterWidget.cpp +++ b/src/Gui/Quarter/QuarterWidget.cpp @@ -75,6 +75,7 @@ #include #include #include +#include #include #include #include @@ -793,6 +794,22 @@ QuarterWidget::getSoEventManager() const return PRIVATE(this)->soeventmanager; } +/*! + InputDeviceHost: true while a scene node is currently grabbing the translated + events (a modal interaction such as a dragger). Hover coalescing consults + this so it never drops the moves that drive a live preview. Handlers that + consume events via SoEventCallback::setHandled() without setting a grabber + (notably the Sketcher) are reported by the surface host instead -- see + VulkanViewportAdapter::setEventGrabProbe(). +*/ +bool +QuarterWidget::eventsGrabbed() const +{ + SoEventManager * manager = this->getSoEventManager(); + SoHandleEventAction * action = manager ? manager->getHandleEventAction() : nullptr; + return action && action->getGrabber() != nullptr; +} + /*! Returns a pointer to the event filter */ diff --git a/src/Gui/Quarter/QuarterWidget.h b/src/Gui/Quarter/QuarterWidget.h index a66ce2c5196c..6cde0f5c883d 100644 --- a/src/Gui/Quarter/QuarterWidget.h +++ b/src/Gui/Quarter/QuarterWidget.h @@ -179,6 +179,8 @@ class QUARTER_DLL_API QuarterWidget : public QGraphicsView, public InputDeviceHo void removeStateMachine(SoScXMLStateMachine * statemachine); bool processSoEvent(const SoEvent * event) override; + //! InputDeviceHost: true while a scene node grabs the events (active tool). + bool eventsGrabbed() const override; QSize minimumSizeHint() const override; QList transparencyTypeActions() const; diff --git a/src/Gui/Selection/SoFCUnifiedSelection.cpp b/src/Gui/Selection/SoFCUnifiedSelection.cpp index 9ebdfc8cb171..e0909df4cf0c 100644 --- a/src/Gui/Selection/SoFCUnifiedSelection.cpp +++ b/src/Gui/Selection/SoFCUnifiedSelection.cpp @@ -149,19 +149,39 @@ std::size_t choosePreferredPick(const std::vector& picked) std::size_t preferred = 0; int pickedPriority = picked.front().priority; - const void* firstOwner = picked.front().owner; + int pickedOwnerPriority = picked.front().ownerPriority; + const void* preferredOwner = picked.front().owner; bool preferredIsAnnotation = picked.front().isAnnotation; for (std::size_t i = 1; i < picked.size(); ++i) { const auto& info = picked[i]; - if (info.owner != firstOwner) { - break; + if (!info.closeToFirst) { + continue; } - if (!info.closeToFirst) { + // A pickup from an object that claims a higher pick priority wins even + // across owners: a sketch edge/face lying exactly on a solid face must + // beat the coincident solid face regardless of scene order. See + // ViewProviderDocumentObject::getPickPriority(). + if (info.ownerPriority > pickedOwnerPriority) { + preferred = i; + pickedPriority = info.priority; + pickedOwnerPriority = info.ownerPriority; + preferredOwner = info.owner; + preferredIsAnnotation = info.isAnnotation; + continue; + } + if (info.ownerPriority < pickedOwnerPriority) { continue; } + // Within the preferred owner keep the historic behaviour: only the detail + // priority (point > line > face) may change the choice, never a jump to a + // different object. + if (info.owner != preferredOwner) { + break; + } + if (info.priority > pickedPriority) { preferred = i; pickedPriority = info.priority; @@ -315,6 +335,15 @@ static bool isAnnotationPick(const SoPickedPoint* pp, const Document* doc) return false; } +// Two picked points name the same on-screen feature when they hit the same +// scene location (within a small tolerance). This happens e.g. for a sketch +// lying exactly on the solid face it is attached to; such picks must be +// resolved by priority, not treated as one object occluding another. +static bool coincidentPickedPoint(const SoPickedPoint* a, const SoPickedPoint* b) +{ + return a && b && a->getPoint().equals(b->getPoint(), 0.2F); +} + int SoFCUnifiedSelection::getPriority(const SoPickedPoint* p) { const SoDetail* detail = p->getDetail(); @@ -370,6 +399,7 @@ SelectionPickPolicy::Candidate SoFCUnifiedSelection::getPickCandidate( SelectionPickPolicy::Candidate candidate; candidate.owner = info.vpd; candidate.priority = getPriority(info.pp); + candidate.ownerPriority = info.vpd ? info.vpd->getPickPriority() : 0; candidate.isAnnotation = doc && info.pp && isAnnotationPick(info.pp, doc); candidate.hasGate = hasSelectionGate(info); candidate.passesGate = passesSelectionGate(info); @@ -418,7 +448,23 @@ std::vector SoFCUnifiedSelection::getPickedLis if (this->pcDocument && path && path->containsPath(action->getCurPath())) { vp = this->pcDocument->getViewProviderByPathFromHead(path); if (singlePick && last_vp && last_vp != vp && canFinalizeSinglePick(ret)) { - break; + // Normally the first object that produced a pick owns the single + // pick and we must not reach through it to a farther object. A + // coincident pick from an object with a higher pick priority is + // not "farther" though: a sketch lying on a solid face shares the + // picked point with the face, so keep it as a candidate and let + // the priority choice below pick the sketch. + const int newPriority = (vp && vp->isDerivedFrom(ViewProviderDocumentObject::getClassTypeId())) + ? static_cast(vp)->getPickPriority() + : 0; + const int curPriority = (!ret.empty() && ret.front().vpd) + ? ret.front().vpd->getPickPriority() + : 0; + const bool coincident = !ret.empty() && ret.front().pp + && coincidentPickedPoint(info.pp, ret.front().pp); + if (!coincident || newPriority <= curPriority) { + break; + } } } if (!vp || !vp->isDerivedFrom(ViewProviderDocumentObject::getClassTypeId())) { diff --git a/src/Gui/Selection/SoFCUnifiedSelection.h b/src/Gui/Selection/SoFCUnifiedSelection.h index 2c2d271d5321..1b712ab3fc05 100644 --- a/src/Gui/Selection/SoFCUnifiedSelection.h +++ b/src/Gui/Selection/SoFCUnifiedSelection.h @@ -57,6 +57,7 @@ struct Candidate { const void* owner {nullptr}; int priority {0}; + int ownerPriority {0}; bool closeToFirst {false}; bool isAnnotation {false}; bool hasGate {false}; diff --git a/src/Gui/SoDatumLabel.cpp b/src/Gui/SoDatumLabel.cpp index 16a641f63bbe..f24367811fd0 100644 --- a/src/Gui/SoDatumLabel.cpp +++ b/src/Gui/SoDatumLabel.cpp @@ -258,9 +258,22 @@ SoDatumLabel::SoDatumLabel() m_Root = new SoSeparator; m_Root->ref(); + // Datum/dimension labels are screen annotations that must read on top of the + // geometry they annotate. The retained IR path routes depth-disabled commands + // to its on-top pass, so disable the depth test for it; otherwise a label + // coplanar with, e.g., the sketch face z-fights with it. Keep the GL/system + // Coin path untouched: it has no on-top pass, and the Sketcher does not wrap + // datum labels in So3DAnnotation, so flipping the shared node there would + // change GL/upstream rendering and the checked-in node snapshots. (The + // companion text subgraph already renders depth-off via m_TextDepth.) m_GeometryDepth = new SoDepthBuffer; - m_GeometryDepth->test.setValue(true); m_GeometryDepth->write.setValue(false); +#ifdef HAVE_COIN_IR_RENDER_ACTION + m_GeometryDepth->test.setValue(false); + m_GeometryDepth->function.setValue(SoDepthBuffer::ALWAYS); +#else + m_GeometryDepth->test.setValue(true); +#endif m_Root->addChild(m_GeometryDepth); m_LightModel = new SoLightModel; diff --git a/src/Gui/ToolBarManager.cpp b/src/Gui/ToolBarManager.cpp index 6709e904ca2a..53616d43412c 100644 --- a/src/Gui/ToolBarManager.cpp +++ b/src/Gui/ToolBarManager.cpp @@ -33,6 +33,8 @@ #include #include +#include +#include #include @@ -181,6 +183,9 @@ ToolBar::ToolBar(QWidget* parent) void ToolBar::undock() { + if (std::getenv("FC_DRAG_DEBUG")) { // DNDBG + fprintf(stderr, "[DNDBG] TOOLBAR undock %s\n", objectName().toUtf8().constData()); + } { // We want to block only some signals - topLevelChanged should still be propagated QSignalBlocker blocker(this); @@ -298,6 +303,12 @@ void ToolBarGrip::paintEvent(QPaintEvent*) void ToolBarGrip::mouseMoveEvent(QMouseEvent* me) { + if (std::getenv("FC_DRAG_DEBUG")) { // DNDBG + const QPoint _g = me->globalPosition().toPoint(); + auto* _tb = qobject_cast(parentWidget()); + fprintf(stderr, "[DNDBG] GRIP move btns=%d g=(%d,%d) tb=%s\n", int(me->buttons()), + _g.x(), _g.y(), _tb ? _tb->objectName().toUtf8().constData() : "-"); + } auto toolbar = qobject_cast(parentWidget()); if (!toolbar) { return; @@ -937,6 +948,11 @@ bool ToolBarManager::addToolBarToArea(QObject* source, QMouseEvent* ev) if (!tb || !tb->isFloating()) { return false; } + if (std::getenv("FC_DRAG_DEBUG")) { // DNDBG + const QPoint _c = QCursor::pos(); + fprintf(stderr, "[DNDBG] TBM addToolBarToArea ev=%d tb=%s cursor=(%d,%d)\n", + int(ev->type()), tb->objectName().toUtf8().constData(), _c.x(), _c.y()); + } static QPointer tbPlaceholder; static QPointer lastArea; @@ -1156,6 +1172,30 @@ void ToolBarManager::onToggleStatusBarWidget(QWidget* widget, bool visible) bool ToolBarManager::eventFilter(QObject* source, QEvent* ev) { bool res = false; + if (std::getenv("FC_DRAG_DEBUG")) { // DNDBG + const QEvent::Type _t = ev->type(); + auto* _tb = qobject_cast(source); + if (_t == QEvent::Move || _t == QEvent::ParentChange || _t == QEvent::WindowStateChange + || _t == QEvent::Show || _t == QEvent::Hide) { + if (_tb) { + const QPoint _g0 = _tb->mapToGlobal(QPoint(0, 0)); + const QRect _gg = _tb->geometry(); + fprintf(stderr, + "[DNDBG] TBM tbEv ev=%d tb=%s floating=%d isWin=%d geom=(%d,%d,%d,%d) " + "g0=(%d,%d)\n", + int(_t), _tb->objectName().toUtf8().constData(), + _tb->isFloating() ? 1 : 0, _tb->isWindow() ? 1 : 0, _gg.x(), _gg.y(), + _gg.width(), _gg.height(), _g0.x(), _g0.y()); + } + } + if (_t == QEvent::MouseMove || _t == QEvent::MouseButtonRelease + || _t == QEvent::MouseButtonPress) { + fprintf(stderr, "[DNDBG] TBM eventFilter ev=%d src=%s tb=%s floating=%d\n", int(_t), + source->metaObject()->className(), + _tb ? _tb->objectName().toUtf8().constData() : "-", + _tb ? (_tb->isFloating() ? 1 : 0) : -1); + } + } switch (ev->type()) { case QEvent::Show: case QEvent::Hide: diff --git a/src/Gui/View3DInventorViewer.cpp b/src/Gui/View3DInventorViewer.cpp index 5a5d2bf956a3..470af71700f1 100644 --- a/src/Gui/View3DInventorViewer.cpp +++ b/src/Gui/View3DInventorViewer.cpp @@ -1914,6 +1914,11 @@ bool View3DInventorViewer::isEditingViewProvider() const return this->editViewProvider != nullptr; } +bool View3DInventorViewer::isEditingHandlerActive() const +{ + return this->editViewProvider && this->editViewProvider->isEditHandlerActive(); +} + /// return currently editing view provider ViewProvider* View3DInventorViewer::getEditingViewProvider() const { diff --git a/src/Gui/View3DInventorViewer.h b/src/Gui/View3DInventorViewer.h index 2762925dd847..771b2cc07c8d 100644 --- a/src/Gui/View3DInventorViewer.h +++ b/src/Gui/View3DInventorViewer.h @@ -311,6 +311,8 @@ class GuiExport View3DInventorViewer: public Quarter::SoQTQuarterAdaptor, void setEditingViewProvider(Gui::ViewProvider* vp, int ModNum); /// return whether a view provider is edited bool isEditingViewProvider() const override; + /// return whether the editing provider has an active move-consuming handler + bool isEditingHandlerActive() const; /// return currently editing view provider ViewProvider* getEditingViewProvider() const; /// reset from edit mode diff --git a/src/Gui/ViewProvider.h b/src/Gui/ViewProvider.h index 55e187fad7bd..05ac35916def 100644 --- a/src/Gui/ViewProvider.h +++ b/src/Gui/ViewProvider.h @@ -687,6 +687,15 @@ class GuiExport ViewProvider: public App::TransactionalObject public: virtual ViewProvider* startEditing(int ModNum = 0); bool isEditing() const; + /** Report whether an interactive tool/handler is actively consuming mouse + * moves while editing and driving a live on-view preview (e.g. a Sketcher + * DrawSketchHandler). Callers use this to avoid coalescing the moves such a + * handler needs, without disabling stale-hover coalescing for the whole + * edit session. Defaults to false. */ + virtual bool isEditHandlerActive() const + { + return false; + } void finishEditing(); virtual void setActive(bool active); diff --git a/src/Gui/ViewProviderDocumentObject.h b/src/Gui/ViewProviderDocumentObject.h index cfa98187f9a8..4a03ffe10eba 100644 --- a/src/Gui/ViewProviderDocumentObject.h +++ b/src/Gui/ViewProviderDocumentObject.h @@ -200,6 +200,22 @@ class GuiExport ViewProviderDocumentObject: public ViewProvider return false; } + /** Pick priority for geometry that is coincident with another object. + * + * Two view providers can produce a pick at the very same scene point -- most + * commonly a sketch lying exactly on the face of the solid it is attached + * to. Scene-graph order alone then decides which one the selection sees, + * which flips with the tiniest depth variation (z-fighting between picking + * the sketch and picking the face). When the candidates coincide, the one + * whose view provider returns the higher value here wins, independent of + * owner and scene order. The default (0) keeps the historic behaviour; a + * sketch returns a positive value so it is picked over the coincident face. + */ + virtual int getPickPriority() const + { + return 0; + } + void setShowable(bool enable); bool isShowable() const; diff --git a/src/Gui/VulkanViewportAdapter.cpp b/src/Gui/VulkanViewportAdapter.cpp index a02e35538f6c..126bb0513707 100644 --- a/src/Gui/VulkanViewportAdapter.cpp +++ b/src/Gui/VulkanViewportAdapter.cpp @@ -19,6 +19,7 @@ #include #include #include +#include #include #include #include @@ -129,6 +130,23 @@ VulkanViewportAdapter::VulkanViewportAdapter(QStackedWidget* stack, auto* controller = _viewer ? _viewer->getInteractionController() : nullptr; return controller && controller->processSoEvent(ev); }); + // Hover coalescing in the input EventFilter drops stale no-button moves; + // while an active tool/drag handler is driving a live preview from those + // moves it must not, or the preview stops tracking the cursor. A modal Coin + // grabber covers draggers, but a Sketcher DrawSketchHandler drives its + // preview without one, so ask the editing provider for its active handler + // (not merely "is editing": hovering during an edit session is exactly the + // workload coalescing was added to speed up, so treating all of edit mode as + // consuming would disable it for the whole session). + _vulkanViewer->setEventGrabProbe([this]() -> bool { + if (_viewer && _viewer->isEditingHandlerActive()) { + return true; + } + auto* controller = _viewer ? _viewer->getInteractionController() : nullptr; + SoEventManager* manager = controller ? controller->getSoEventManager() : nullptr; + SoHandleEventAction* action = manager ? manager->getHandleEventAction() : nullptr; + return action && action->getGrabber() != nullptr; + }); // Navigation's cursor shapes are routed to the visible surface by // View3DInventorViewer::setCursorTarget() (set in useVulkanViewport), so // no cursor mirroring is needed. The event filter on the GL widget only @@ -161,6 +179,17 @@ void VulkanViewportAdapter::syncViewer() // Re-point the camera + axis-cross decorations and re-attach the change // sensors (shared with the camera-changed fast path). this->resyncCameraAndDecorations(); + // The hidden GL viewer never renders on the Vulkan path (the Vulkan surface + // does), so the shared camera's near/far are not auto-fitted after a scene + // edit. SoRayPickAction derives its ray depth range, and the + // SoShape/SoBrepFaceSet CLIP_NEAR/CLIP_FAR bounding-box cull, from those + // planes. With a stale range a pick rejects geometry outside it, so + // hovering a solid that just appeared (e.g. a freshly padded sketch) hit + // the entity behind it -- or the solid's own far cap -- instead of the + // camera-facing face, until some later viewAll refreshed the clip. Mirror + // the GL render's auto-clip here so the pick range always matches the + // current scene. + rm->updateClippingPlanes(); // The background (solid color + gradient) is pushed by pushSettings(), the // single source of truth derived from the hidden GL viewer. syncViewer() // only re-seeds scene/camera/overlays here and lets pushSettings() refresh @@ -299,17 +328,8 @@ void VulkanViewportAdapter::resyncViewport() if (QWidget* container = _vulkanViewer->getNativeWidget()) { applySurfaceViewportToGL(container->size()); } - // SoRayPickAction derives its ray depth range from the shared camera's - // near/far planes. A GL render auto-fits those planes to the scene, but - // the display-only Vulkan path never renders through the GL viewer, so on - // a scene that appeared after the camera was last framed (e.g. the origin - // planes the sketch attachment editor enlarges over a brand-new empty - // document) the planes fall outside [near, far] and hover/click picks miss - // until a render-mode round-trip runs the GL auto-clip. Refresh the planes - // here, from the same scene bounding box the GL render would use. - if (SoRenderManager* rm = _viewer->getSoRenderManager()) { - rm->updateClippingPlanes(); - } + // syncViewer() refreshes the shared camera's near/far (the pick depth range + // the display-only Vulkan path never auto-fits) before pushing settings. syncViewer(); #endif } @@ -827,6 +847,7 @@ VulkanViewportAdapter::~VulkanViewportAdapter() // Stop delivering input to the (soon-dead) GL viewer/controller. _vulkanViewer->setEventSink(nullptr); _vulkanViewer->setRawEventTarget(nullptr); + _vulkanViewer->setEventGrabProbe(nullptr); QWidget* host = _vulkanViewer->parentWidget(); if (auto* stack = qobject_cast(host)) { stack->removeWidget(_vulkanViewer); diff --git a/src/Mod/Part/Gui/SoBrepOverlayHelpers.h b/src/Mod/Part/Gui/SoBrepOverlayHelpers.h index eb98e3f3e994..69a149b11b79 100644 --- a/src/Mod/Part/Gui/SoBrepOverlayHelpers.h +++ b/src/Mod/Part/Gui/SoBrepOverlayHelpers.h @@ -519,8 +519,20 @@ static void renderOverlayFaces( } SoLazyElement::setEmissive(state, &color); - const uint32_t packed = color.getPackedValue(0.0f); - SoLazyElement::setPacked(state, faceSet, 1, &packed, false); + // On the retained-IR path the selection/highlight is a separate overlay (the + // base is not re-materialed for cache stability), so the overlay must carry + // the shape's own transparency or an opaque overlay hides a transparent + // shape (the PartDesign pattern "selected object turns solid green" case). + // GL bakes the color into the base material and keeps its transparency + // element, so leave its overlay fallback on the historical opaque behavior. + float overlayTransparency = 0.0f; +#ifdef HAVE_COIN_IR_RENDER_ACTION + if constexpr (isIR) { + overlayTransparency = SoLazyElement::getTransparency(state, 0); + } +#endif + const uint32_t packed = color.getPackedValue(overlayTransparency); + SoLazyElement::setPacked(state, faceSet, 1, &packed, overlayTransparency > 0.0f); // setValues() does not shrink the field, so rewrite the overlay index // array to the exact size to avoid stale faces from a previous, larger diff --git a/src/Mod/Sketcher/App/SketchObjectOperations.cpp b/src/Mod/Sketcher/App/SketchObjectOperations.cpp index 342c899ccc5b..f020dd3e20a0 100644 --- a/src/Mod/Sketcher/App/SketchObjectOperations.cpp +++ b/src/Mod/Sketcher/App/SketchObjectOperations.cpp @@ -2550,7 +2550,8 @@ int SketchObject::addCopy( newconstrVals.push_back(constNew); continue; } - if (getGeometry(constr->First)->is()) { + const Part::Geometry* constrGeo = getGeometry(constr->First); + if (constrGeo && constrGeo->is()) { // Angles on a single Element are mapped to parallel // constraints in clone mode Constraint* constNew = constr->copy(); diff --git a/src/Mod/Sketcher/Gui/CommandConstraints.cpp b/src/Mod/Sketcher/Gui/CommandConstraints.cpp index 5998ce571fc0..7e047252c7ca 100644 --- a/src/Mod/Sketcher/Gui/CommandConstraints.cpp +++ b/src/Mod/Sketcher/Gui/CommandConstraints.cpp @@ -493,7 +493,12 @@ void showNoConstraintBetweenFixedGeometry(const App::DocumentObject* obj) bool isGeoConcentricCompatible(const Part::Geometry* geo) { - return (isEllipse(*geo) || isArcOfEllipse(*geo) || isCircle(*geo) || isArcOfCircle(*geo)); + // getGeometry() returns nullptr for external geometry ids (negative) and + // for out-of-range ids; the auto-constraint handler calls this while the + // selection is being built, so a nullptr here is a normal "not compatible" + // result rather than a reason to dereference null. + return geo + && (isEllipse(*geo) || isArcOfEllipse(*geo) || isCircle(*geo) || isArcOfCircle(*geo)); } // Removes point-on-object constraints made redundant with certain constraints @@ -516,7 +521,7 @@ bool removeRedundantPointOnObject(SketchObject* Obj, int GeoId1, int GeoId2, int // ONLY do this if it is a B-spline (or any other where point // on object is implied). const Part::Geometry* geom = Obj->getGeometry((*it)->Second); - if (isBSplineCurve(*geom)) + if (geom && isBSplineCurve(*geom)) cidsToBeRemoved.push_back(cid); } } @@ -1392,6 +1397,32 @@ class CmdSketcherConstraint: public Gui::Command */ std::vector> allowedSelSequences; + // The view provider whose DrawSketchHandlerGenConstraint is driving this + // command, set by the handler immediately before applyConstraint(). The + // selSeq geometry ids always belong to this sketch, which is not + // necessarily the object currently in edit: a draw handler can be invoked + // from a selection made while a different sketch (or document) owns the + // edit session. Resolving the sketch through this pointer instead of + // getInEdit() keeps the constraint on the sketch the user actually picked + // from and avoids indexing foreign ids into a smaller sketch (null + // geometry). + SketcherGui::ViewProviderSketch* drivingViewProvider = nullptr; + +public: + // The sketch to operate on: the one driving the active draw handler when + // set, otherwise whatever is currently in edit. Never null unless there + // is no sketch in edit at all. + SketcherGui::ViewProviderSketch* activeViewProvider() const + { + if (drivingViewProvider) { + return drivingViewProvider; + } + Gui::Document* doc = getActiveGuiDocument(); + return doc ? dynamic_cast(doc->getInEdit()) + : nullptr; + } + +protected: virtual void applyConstraint(std::vector&, int) {} void activated(int /*iMsg*/) override; @@ -2023,7 +2054,9 @@ class DrawSketchHandlerGenConstraint: public DrawSketchHandler for (int token : ongoingSequences) { if ((cmd->allowedSelSequences).at(token).at(seqIndex) & selectionType) { if (seqIndex == (cmd->allowedSelSequences).at(token).size() - 1) { + cmd->drivingViewProvider = sketchgui; cmd->applyConstraint(selSeq, token); + cmd->drivingViewProvider = nullptr; selSeq.clear(); resetOngoingSequences(); @@ -3342,7 +3375,8 @@ class DrawSketchHandlerDimension : public DrawSketchHandler if (availableConstraint == AvailableConstraint::SECOND) { restartCommand(QT_TRANSLATE_NOOP("Command", "Add radius constraint")); createRadiusDiameterConstrain(geoId, onSketchPos, false); - if (!isArcOfCircle(*Obj->getGeometry(geoId))) { + const Part::Geometry* geo = Obj->getGeometry(geoId); + if (!geo || !isArcOfCircle(*geo)) { //This way if key is pressed again it goes back to FIRST availableConstraint = AvailableConstraint::RESET; } @@ -4352,9 +4386,14 @@ void horVerActivated(CmdSketcherConstraint* cmd, std::string type) void horVerApplyConstraint(CmdSketcherConstraint* cmd, std::string type, std::vector& selSeq, int seqIndex) { - auto* sketchgui = - static_cast(cmd->getActiveGuiDocument()->getInEdit()); + auto* sketchgui = cmd->activeViewProvider(); + if (!sketchgui) { + return; + } Sketcher::SketchObject* Obj = sketchgui->getSketchObject(); + if (!Obj) { + return; + } switch (seqIndex) { case 0:// {Edge} @@ -4761,9 +4800,14 @@ void CmdSketcherConstrainLock::applyConstraint(std::vector& selSeq, i switch (seqIndex) { case 0:// {Vertex} // Create the constraints - SketcherGui::ViewProviderSketch* sketchgui = - static_cast(getActiveGuiDocument()->getInEdit()); + SketcherGui::ViewProviderSketch* sketchgui = activeViewProvider(); + if (!sketchgui) { + return; + } Sketcher::SketchObject* Obj = sketchgui->getSketchObject(); + if (!Obj) { + return; + } // check if the edge already has a Block constraint bool pointfixed = false; @@ -4951,10 +4995,15 @@ void CmdSketcherConstrainBlock::applyConstraint(std::vector& selSeq, case 0:// {Edge} { // Create the constraints - SketcherGui::ViewProviderSketch* sketchgui = - static_cast(getActiveGuiDocument()->getInEdit()); + SketcherGui::ViewProviderSketch* sketchgui = activeViewProvider(); + if (!sketchgui) { + return; + } auto Obj = sketchgui->getObject(); + if (!Obj) { + return; + } // check if the edge already has a Block constraint const std::vector& vals = Obj->Constraints.getValues(); @@ -5441,9 +5490,14 @@ void CmdSketcherConstrainCoincidentUnified::applyConstraintPointOnObject(std::ve return; } - SketcherGui::ViewProviderSketch* sketchgui = - static_cast(getActiveGuiDocument()->getInEdit()); + SketcherGui::ViewProviderSketch* sketchgui = activeViewProvider(); + if (!sketchgui) { + return; + } Sketcher::SketchObject* Obj = sketchgui->getSketchObject(); + if (!Obj) { + return; + } openCommand(QT_TRANSLATE_NOOP("Command", "Add point on object constraint")); bool allOK = true; @@ -5491,9 +5545,14 @@ void CmdSketcherConstrainCoincidentUnified::applyConstraintPointOnObject(std::ve void CmdSketcherConstrainCoincidentUnified::applyConstraintCoincident(std::vector& selSeq, int seqIndex) { - SketcherGui::ViewProviderSketch* sketchgui = - static_cast(getActiveGuiDocument()->getInEdit()); + SketcherGui::ViewProviderSketch* sketchgui = activeViewProvider(); + if (!sketchgui) { + return; + } Sketcher::SketchObject* Obj = sketchgui->getSketchObject(); + if (!Obj) { + return; + } int GeoId1 = selSeq.at(0).GeoId, GeoId2 = selSeq.at(1).GeoId; Sketcher::PointPos PosId1 = selSeq.at(0).PosId, PosId2 = selSeq.at(1).PosId; @@ -6248,9 +6307,14 @@ void CmdSketcherConstrainDistance::activated(int iMsg) void CmdSketcherConstrainDistance::applyConstraint(std::vector& selSeq, int seqIndex) { - SketcherGui::ViewProviderSketch* sketchgui = - static_cast(getActiveGuiDocument()->getInEdit()); + SketcherGui::ViewProviderSketch* sketchgui = activeViewProvider(); + if (!sketchgui) { + return; + } Sketcher::SketchObject* Obj = sketchgui->getSketchObject(); + if (!Obj) { + return; + } int GeoId1 = GeoEnum::GeoUndef, GeoId2 = GeoEnum::GeoUndef; Sketcher::PointPos PosId1 = Sketcher::PointPos::none, PosId2 = Sketcher::PointPos::none; @@ -6634,14 +6698,19 @@ void CmdSketcherConstrainDistanceX::activated(int iMsg) activateCoordinateDistanceConstraint(this, true); } -static void applyCoordinateDistanceConstraint(Gui::Command* cmd, +static void applyCoordinateDistanceConstraint(CmdSketcherConstraint* cmd, const std::vector& selSeq, int seqIndex, bool horizontal) { - SketcherGui::ViewProviderSketch* sketchgui = - static_cast(cmd->getActiveGuiDocument()->getInEdit()); + SketcherGui::ViewProviderSketch* sketchgui = cmd->activeViewProvider(); + if (!sketchgui) { + return; + } Sketcher::SketchObject* Obj = sketchgui->getSketchObject(); + if (!Obj) { + return; + } int GeoId1 = GeoEnum::GeoUndef, GeoId2 = GeoEnum::GeoUndef; Sketcher::PointPos PosId1 = Sketcher::PointPos::none, PosId2 = Sketcher::PointPos::none; @@ -6881,9 +6950,14 @@ void CmdSketcherConstrainParallel::applyConstraint(std::vector& selSe case 2:// {SelEdge, SelExternalEdge} case 3:// {SelExternalEdge, SelEdge} // create the constraint - SketcherGui::ViewProviderSketch* sketchgui = - static_cast(getActiveGuiDocument()->getInEdit()); + SketcherGui::ViewProviderSketch* sketchgui = activeViewProvider(); + if (!sketchgui) { + return; + } Sketcher::SketchObject* Obj = sketchgui->getSketchObject(); + if (!Obj) { + return; + } int GeoId1 = selSeq.at(0).GeoId, GeoId2 = selSeq.at(1).GeoId; @@ -7347,9 +7421,14 @@ void CmdSketcherConstrainPerpendicular::activated(int iMsg) void CmdSketcherConstrainPerpendicular::applyConstraint(std::vector& selSeq, int seqIndex) { - SketcherGui::ViewProviderSketch* sketchgui = - static_cast(getActiveGuiDocument()->getInEdit()); + SketcherGui::ViewProviderSketch* sketchgui = activeViewProvider(); + if (!sketchgui) { + return; + } Sketcher::SketchObject* Obj = sketchgui->getSketchObject(); + if (!Obj) { + return; + } int GeoId1 = GeoEnum::GeoUndef, GeoId2 = GeoEnum::GeoUndef, GeoId3 = GeoEnum::GeoUndef; Sketcher::PointPos PosId1 = Sketcher::PointPos::none, PosId2 = Sketcher::PointPos::none, @@ -8181,9 +8260,14 @@ void CmdSketcherConstrainTangent::activated(int iMsg) void CmdSketcherConstrainTangent::applyConstraint(std::vector& selSeq, int seqIndex) { - SketcherGui::ViewProviderSketch* sketchgui = - static_cast(getActiveGuiDocument()->getInEdit()); + SketcherGui::ViewProviderSketch* sketchgui = activeViewProvider(); + if (!sketchgui) { + return; + } Sketcher::SketchObject* Obj = sketchgui->getSketchObject(); + if (!Obj) { + return; + } int GeoId1 = GeoEnum::GeoUndef, GeoId2 = GeoEnum::GeoUndef, GeoId3 = GeoEnum::GeoUndef; Sketcher::PointPos PosId1 = Sketcher::PointPos::none, PosId2 = Sketcher::PointPos::none, @@ -8750,14 +8834,19 @@ static void activateRadialDimension(CmdSketcherConstraint* cmd, RadialDimension } } -static void applyRadialDimension(Gui::Command* cmd, +static void applyRadialDimension(CmdSketcherConstraint* cmd, const std::vector& selSeq, int seqIndex, RadialDimension dimension) { - SketcherGui::ViewProviderSketch* sketchgui = - static_cast(cmd->getActiveGuiDocument()->getInEdit()); + SketcherGui::ViewProviderSketch* sketchgui = cmd->activeViewProvider(); + if (!sketchgui) { + return; + } Sketcher::SketchObject* Obj = sketchgui->getSketchObject(); + if (!Obj) { + return; + } int GeoId = selSeq.at(0).GeoId; double radius = 0.0; @@ -9370,9 +9459,14 @@ void CmdSketcherConstrainAngle::activated(int iMsg) void CmdSketcherConstrainAngle::applyConstraint(std::vector& selSeq, int seqIndex) { - SketcherGui::ViewProviderSketch* sketchgui = - static_cast(getActiveGuiDocument()->getInEdit()); + SketcherGui::ViewProviderSketch* sketchgui = activeViewProvider(); + if (!sketchgui) { + return; + } Sketcher::SketchObject* Obj = sketchgui->getSketchObject(); + if (!Obj) { + return; + } int GeoId1 = GeoEnum::GeoUndef, GeoId2 = GeoEnum::GeoUndef, GeoId3 = GeoEnum::GeoUndef; Sketcher::PointPos PosId1 = Sketcher::PointPos::none, PosId2 = Sketcher::PointPos::none, @@ -9690,9 +9784,14 @@ void CmdSketcherConstrainEqual::activated(int iMsg) void CmdSketcherConstrainEqual::applyConstraint(std::vector& selSeq, int seqIndex) { - SketcherGui::ViewProviderSketch* sketchgui = - static_cast(getActiveGuiDocument()->getInEdit()); + SketcherGui::ViewProviderSketch* sketchgui = activeViewProvider(); + if (!sketchgui) { + return; + } Sketcher::SketchObject* Obj = sketchgui->getSketchObject(); + if (!Obj) { + return; + } int GeoId1 = GeoEnum::GeoUndef, GeoId2 = GeoEnum::GeoUndef; @@ -10324,9 +10423,14 @@ void CmdSketcherConstrainSymmetric::activated(int iMsg) void CmdSketcherConstrainSymmetric::applyConstraint(std::vector& selSeq, int seqIndex) { - SketcherGui::ViewProviderSketch* sketchgui = - static_cast(getActiveGuiDocument()->getInEdit()); + SketcherGui::ViewProviderSketch* sketchgui = activeViewProvider(); + if (!sketchgui) { + return; + } Sketcher::SketchObject* Obj = sketchgui->getSketchObject(); + if (!Obj) { + return; + } int GeoId1 = GeoEnum::GeoUndef, GeoId2 = GeoEnum::GeoUndef, GeoId3 = GeoEnum::GeoUndef; Sketcher::PointPos PosId1 = Sketcher::PointPos::none, PosId2 = Sketcher::PointPos::none, diff --git a/src/Mod/Sketcher/Gui/CommandSketcherBSpline.cpp b/src/Mod/Sketcher/Gui/CommandSketcherBSpline.cpp index faec07ea0c16..997603ccafc9 100644 --- a/src/Mod/Sketcher/Gui/CommandSketcherBSpline.cpp +++ b/src/Mod/Sketcher/Gui/CommandSketcherBSpline.cpp @@ -445,7 +445,12 @@ void CmdSketcherIncreaseKnotMultiplicity::activated(int iMsg) boost::uuids::uuid bsplinetag; if (!notaknot) { - bsplinetag = Obj->getGeometry(splineGeoId)->getTag(); + const Part::Geometry* spline = Obj->getGeometry(splineGeoId); + if (!spline) { + abortCommand(); + return; + } + bsplinetag = spline->getTag(); try { Gui::cmdAppObjectArgs( @@ -602,7 +607,12 @@ void CmdSketcherDecreaseKnotMultiplicity::activated(int iMsg) boost::uuids::uuid bsplinetag; if (!notaknot) { - bsplinetag = Obj->getGeometry(splineGeoId)->getTag(); + const Part::Geometry* spline = Obj->getGeometry(splineGeoId); + if (!spline) { + abortCommand(); + return; + } + bsplinetag = spline->getTag(); try { Gui::cmdAppObjectArgs( @@ -853,7 +863,12 @@ class DrawSketchHandlerBSplineInsertKnot: public DrawSketchHandler openCommand(QT_TRANSLATE_NOOP("Command", "Insert knot")); bool applied = false; - boost::uuids::uuid bsplinetag = Obj->getGeometry(GeoId)->getTag(); + const Part::Geometry* spline = Obj->getGeometry(GeoId); + if (!spline) { + abortCommand(); + return true; + } + boost::uuids::uuid bsplinetag = spline->getTag(); try { Gui::cmdAppObjectArgs(Obj, "insertBSplineKnot(%d, %lf, %d) ", GeoId, guessParam, 1); diff --git a/src/Mod/Sketcher/Gui/CommandSketcherTools.cpp b/src/Mod/Sketcher/Gui/CommandSketcherTools.cpp index 5dda04f6a509..5b36c9b6f726 100644 --- a/src/Mod/Sketcher/Gui/CommandSketcherTools.cpp +++ b/src/Mod/Sketcher/Gui/CommandSketcherTools.cpp @@ -162,7 +162,8 @@ std::vector getListOfSelectedGeoIds(bool forceInternalSelection) int geoId {}; Sketcher::PointPos PosId {}; Obj->getGeoVertexIndex(VtId, geoId, PosId); - if (isPoint(*Obj->getGeometry(geoId))) { + const Part::Geometry* geo = Obj->getGeometry(geoId); + if (geo && isPoint(*geo)) { if (geoId >= 0) { listOfGeoIds.push_back(geoId); } @@ -175,7 +176,9 @@ std::vector getListOfSelectedGeoIds(bool forceInternalSelection) const size_t loopSize = listOfGeoIds.size(); for (size_t i = 0; i < loopSize; i++) { const Part::Geometry* geo = Obj->getGeometry(listOfGeoIds[i]); - if (isEllipse(*geo) || isArcOfEllipse(*geo) || isArcOfHyperbola(*geo) || isArcOfParabola(*geo) || isBSplineCurve(*geo)) { + if (geo + && (isEllipse(*geo) || isArcOfEllipse(*geo) || isArcOfHyperbola(*geo) + || isArcOfParabola(*geo) || isBSplineCurve(*geo))) { const std::vector& constraints = Obj->Constraints.getValues(); for (const auto constr : constraints) { if (constr->Type == InternalAlignment && constr->Second == listOfGeoIds[i]) { @@ -202,8 +205,9 @@ Sketcher::SketchObject* getSketchObject() { Gui::Document* doc = Gui::Application::Instance->activeDocument(); ReleaseHandler(doc); - auto* vp = static_cast(doc->getInEdit()); - return vp->getSketchObject(); + auto* vp = doc ? dynamic_cast(doc->getInEdit()) + : nullptr; + return vp ? vp->getSketchObject() : nullptr; } // ================================================================================ @@ -231,6 +235,17 @@ bool copySelectionToClipboard(Sketcher::SketchObject* obj) std::sort(listOfGeoId.begin(), listOfGeoId.end()); listOfGeoId.erase(std::unique(listOfGeoId.begin(), listOfGeoId.end()), listOfGeoId.end()); + // Drop ids that no longer resolve (e.g. a stale external reference left in + // the selection) so the geometry list and the copiedGeoIds map stay aligned. + listOfGeoId.erase( + std::remove_if(listOfGeoId.begin(), + listOfGeoId.end(), + [obj](int geoId) { return obj->getGeometry(geoId) == nullptr; }), + listOfGeoId.end()); + if (listOfGeoId.empty()) { + return false; + } + std::vector> shapeGeometry; shapeGeometry.reserve(listOfGeoId.size()); for (auto geoId : listOfGeoId) { @@ -368,7 +383,11 @@ void CmdSketcherCut::activated(int iMsg) Gui::Document* doc = getActiveGuiDocument(); ReleaseHandler(doc); - auto* vp = static_cast(doc->getInEdit()); + auto* vp = doc ? dynamic_cast(doc->getInEdit()) + : nullptr; + if (!vp) { + return; + } openCommand(QT_TRANSLATE_NOOP("Command", "Cut in Sketcher")); vp->deleteSelected(); @@ -406,7 +425,11 @@ void CmdSketcherPaste::activated(int iMsg) Q_UNUSED(iMsg); Gui::Document* doc = getActiveGuiDocument(); ReleaseHandler(doc); - auto* vp = static_cast(doc->getInEdit()); + auto* vp = doc ? dynamic_cast(doc->getInEdit()) + : nullptr; + if (!vp) { + return; + } Sketcher::SketchObject* obj = vp->getSketchObject(); std::string data = QGuiApplication::clipboard()->text().toStdString(); @@ -1489,7 +1512,8 @@ void SketcherCopy::activate(SketcherCopy::Op op) int GeoId {}; Sketcher::PointPos PosId {}; Obj->getGeoVertexIndex(VtId, GeoId, PosId); - if (Obj->getGeometry(GeoId)->is()) { + const Part::Geometry* geom = Obj->getGeometry(GeoId); + if (geom && geom->is()) { LastGeoId = GeoId; LastPointPos = Sketcher::PointPos::start; // points to copy @@ -1507,7 +1531,8 @@ void SketcherCopy::activate(SketcherCopy::Op op) int GeoId {}; Sketcher::PointPos PosId {}; Obj->getGeoVertexIndex(VtId, GeoId, PosId); - if (!Obj->getGeometry(GeoId)->is()) { + const Part::Geometry* geom = Obj->getGeometry(GeoId); + if (geom && !geom->is()) { LastGeoId = GeoId; LastPointPos = PosId; } @@ -2095,7 +2120,8 @@ void CmdSketcherRectangularArray::activated(int iMsg) int GeoId {}; Sketcher::PointPos PosId {}; Obj->getGeoVertexIndex(VtId, GeoId, PosId); - if (Obj->getGeometry(GeoId)->is()) { + const Part::Geometry* geom = Obj->getGeometry(GeoId); + if (geom && geom->is()) { LastGeoId = GeoId; LastPointPos = Sketcher::PointPos::start; // points to copy @@ -2113,7 +2139,8 @@ void CmdSketcherRectangularArray::activated(int iMsg) int GeoId {}; Sketcher::PointPos PosId {}; Obj->getGeoVertexIndex(VtId, GeoId, PosId); - if (!Obj->getGeometry(GeoId)->is()) { + const Part::Geometry* geom = Obj->getGeometry(GeoId); + if (geom && !geom->is()) { LastGeoId = GeoId; LastPointPos = PosId; } @@ -2375,7 +2402,8 @@ void CmdSketcherRemoveAxesAlignment::activated(int iMsg) int GeoId {}; Sketcher::PointPos PosId {}; Obj->getGeoVertexIndex(VtId, GeoId, PosId); - if (Obj->getGeometry(GeoId)->is()) { + const Part::Geometry* geom = Obj->getGeometry(GeoId); + if (geom && geom->is()) { LastGeoId = GeoId; // points to copy if (LastGeoId >= 0) { @@ -2476,7 +2504,8 @@ void CmdSketcherOffset::activated(int iMsg) } const Part::Geometry* geo = Obj->getGeometry(geoId); - if (!isPoint(*geo) + if (geo + && !isPoint(*geo) && !isBSplineCurve(*geo) && !isEllipse(*geo) && !isArcOfEllipse(*geo) @@ -2645,7 +2674,11 @@ void CreateSketcherCommandsConstraintAccel() void SketcherGui::centerScale(double scaleFactor) { Gui::Document* doc = Gui::Application::Instance->activeDocument(); - auto* vp = static_cast(doc->getInEdit()); + auto* vp = doc ? dynamic_cast(doc->getInEdit()) + : nullptr; + if (!vp) { + return; + } auto scaler = DrawSketchHandlerScale::make_centerScaleAll(vp, scaleFactor, false); scaler->setSketchGui(vp); scaler->executeCommands(); diff --git a/src/Mod/Sketcher/Gui/DrawSketchController.h b/src/Mod/Sketcher/Gui/DrawSketchController.h index 497026683e41..39463a085f38 100644 --- a/src/Mod/Sketcher/Gui/DrawSketchController.h +++ b/src/Mod/Sketcher/Gui/DrawSketchController.h @@ -76,6 +76,12 @@ class OnViewParameters: public ControlAmount namespace sp = std::placeholders; +#ifdef FREECAD_USE_VULKAN +/// Default colour of the in-progress (preview) dimension labels shown while +/// drawing. White by default so the values are legible on the sketch background. +inline const SbColor previewLabelColor(1.0f, 1.0f, 1.0f); +#endif + /** @brief Class defining a generic handler controller operable with a DrawSketchControllableHandler * * @details @@ -670,6 +676,13 @@ class DrawSketchController ) .get(); + // The in-progress (preview) dimension labels start white so they stay + // legible against the sketch background. They only take the active + // (driving) colour once the user actually edits the value. +#ifdef FREECAD_USE_VULKAN + parameter->setColor(previewLabelColor); +#endif + const auto handleParameterValueChanged = [this, parameter, i](double value) { parameter->setActivatedColor(); onViewValueChanged(i, value); @@ -724,7 +737,11 @@ class DrawSketchController { onViewParameter->isSet = false; onViewParameter->hasFinishedEditing = false; +#ifdef FREECAD_USE_VULKAN + onViewParameter->setColor(previewLabelColor); +#else onViewParameter->setDeactivatedColor(); +#endif onViewParameter->setLockedAppearance(false); } diff --git a/src/Mod/Sketcher/Gui/DrawSketchHandler.cpp b/src/Mod/Sketcher/Gui/DrawSketchHandler.cpp index 77966cfdc98c..a539f4f3da0e 100644 --- a/src/Mod/Sketcher/Gui/DrawSketchHandler.cpp +++ b/src/Mod/Sketcher/Gui/DrawSketchHandler.cpp @@ -1550,6 +1550,16 @@ bool DrawSketchHandler::filterRedundantAutoConstraints( int redundantconstraintindex = redundants[index] - 1; if (redundantconstraintindex > lastsketchconstraintindex) { int removeindex = redundantconstraintindex - lastsketchconstraintindex - 1; + // The solver reports constraints by tag, and tags are only guaranteed to + // line up with the additional auto-constraints when every constraint maps + // to a single solver entry. With solver-internal constraints (e.g. the + // experimental line-midpoint rule) or otherwise non-contiguous tags this + // can fall outside the vector; erasing there would run past the end and + // crash (see the polyline OVP acceptance path). Treat it as a redundancy + // that is not an auto-constraint, like the existing branch below. + if (removeindex < 0 || removeindex >= static_cast(autoConstraints.size())) { + return false; + } autoConstraints.erase(std::next(autoConstraints.begin(), removeindex)); } else { @@ -1572,6 +1582,11 @@ bool DrawSketchHandler::filterRedundantAutoConstraints( int conflictingIndex = conflicting[index] - 1; if (conflictingIndex > lastsketchconstraintindex) { int removeindex = conflictingIndex - lastsketchconstraintindex - 1; + // Bound the erase for the same reason as the redundant loop above: the + // solver tag may not map onto a supplied auto-constraint. + if (removeindex < 0 || removeindex >= static_cast(autoConstraints.size())) { + continue; + } autoConstraints.erase(std::next(autoConstraints.begin(), removeindex)); } } diff --git a/src/Mod/Sketcher/Gui/DrawSketchHandlerOffset.h b/src/Mod/Sketcher/Gui/DrawSketchHandlerOffset.h index 472991b574e4..9e576c4c8cd4 100644 --- a/src/Mod/Sketcher/Gui/DrawSketchHandlerOffset.h +++ b/src/Mod/Sketcher/Gui/DrawSketchHandlerOffset.h @@ -993,7 +993,8 @@ class DrawSketchHandlerOffset: public DrawSketchHandlerOffsetBase } // Here we make sure that if possible the first wire is not a single line. - if (CC.size() == 1 && isLineSegment(*Obj->getGeometry(CC[0]))) { + if (CC.size() == 1 && Obj->getGeometry(CC[0]) + && isLineSegment(*Obj->getGeometry(CC[0]))) { sourceWires.push_back(wire); } else { diff --git a/src/Mod/Sketcher/Gui/DrawSketchHandlerSymmetry.h b/src/Mod/Sketcher/Gui/DrawSketchHandlerSymmetry.h index f308a75f551e..92747e72a0ee 100644 --- a/src/Mod/Sketcher/Gui/DrawSketchHandlerSymmetry.h +++ b/src/Mod/Sketcher/Gui/DrawSketchHandlerSymmetry.h @@ -111,10 +111,9 @@ class DrawSketchHandlerSymmetry: public DrawSketchHandlerSymmetryBase refGeoId = Sketcher::GeoEnum::VAxis; refPosId = Sketcher::PointPos::none; } - else if ( - (CrvId >= 0 || CrvId <= Sketcher::GeoEnum::RefExt) - && isLineSegment(*obj->getGeometry(CrvId)) - ) { // Curves + else if ((CrvId >= 0 || CrvId <= Sketcher::GeoEnum::RefExt) + && obj->getGeometry(CrvId) + && isLineSegment(*obj->getGeometry(CrvId))) { // Curves refGeoId = CrvId; refPosId = Sketcher::PointPos::none; } diff --git a/src/Mod/Sketcher/Gui/ViewProviderSketch.cpp b/src/Mod/Sketcher/Gui/ViewProviderSketch.cpp index f1dfbf5ed3cd..49139c8819b0 100644 --- a/src/Mod/Sketcher/Gui/ViewProviderSketch.cpp +++ b/src/Mod/Sketcher/Gui/ViewProviderSketch.cpp @@ -886,6 +886,14 @@ void ViewProviderSketch::setSketchMode(SketchMode mode) Gui::Application::Instance->commandManager().testActive(); } +bool ViewProviderSketch::isEditHandlerActive() const +{ + // Only an active DrawSketchHandler drives a live preview from button-less + // hover moves (STATUS_SKETCH_UseHandler). Pure hovering (STATUS_NONE) must + // still be coalesced, so it is deliberately excluded here. + return Mode == STATUS_SKETCH_UseHandler; +} + SoPickedPointList ViewProviderSketch::getPickedPointsOnRay( const SbVec2s& pos, const Gui::View3DInventorViewer* viewer @@ -2621,6 +2629,14 @@ bool ViewProviderSketch::isSelectable() const return PartGui::ViewProvider2DObject::isSelectable(); } +int ViewProviderSketch::getPickPriority() const +{ + // The sketch geometry is coplanar with the face it is attached to, so the + // two picks coincide and depth cannot separate them. Outrank the solid so + // hovering/clicking the sketch picks the sketch, not the face underneath. + return 1; +} + Base::BoundBox3d ViewProviderSketch::_getBoundingBox( const char* subname, const Base::Matrix4D* mat, @@ -3611,9 +3627,10 @@ void ViewProviderSketch::scaleBSplinePoleCirclesAndUpdateSolverAndSketchObjectGe if (GeoId >= geolistfacade.getInternalCount()) GeoId = -geolistfacade.getExternalCount(); - if ((*it)->getGeometry()->is()) {// circle + const Part::Geometry* circleGeo = (*it)->getGeometry(); + if (circleGeo && circleGeo->is()) {// circle const Part::GeomCircle* circle = - static_cast((*it)->getGeometry()); + static_cast(circleGeo); auto& gf = (*it); // BSpline weights have a radius corresponding to the weight value @@ -4892,7 +4909,8 @@ bool ViewProviderSketch::onDelete(const std::vector& subList) int GeoId; Sketcher::PointPos PosId; getSketchObject()->getGeoVertexIndex(VtId, GeoId, PosId); - if (getSketchObject()->getGeometry(GeoId)->is()) { + const Part::Geometry* vgeo = getSketchObject()->getGeometry(GeoId); + if (vgeo && vgeo->is()) { if (GeoId >= 0) delInternalGeometries.insert(GeoId); else diff --git a/src/Mod/Sketcher/Gui/ViewProviderSketch.h b/src/Mod/Sketcher/Gui/ViewProviderSketch.h index 84f166c50132..bbb0f268fe75 100644 --- a/src/Mod/Sketcher/Gui/ViewProviderSketch.h +++ b/src/Mod/Sketcher/Gui/ViewProviderSketch.h @@ -624,6 +624,12 @@ class SketcherGuiExport ViewProviderSketch: public PartGui::ViewProvider2DObject /// Is the view provider selectable bool isSelectable() const override; + /// A sketch lies exactly on the face it is attached to, so its edges and + /// internal face are coincident with that face. Claim a higher pick + /// priority than the default so the sketch is preselected/selected instead + /// of the solid when the two picks coincide (see ViewProviderDocumentObject). + int getPickPriority() const override; + /// Observer message from the Selection void onSelectionChanged(const Gui::SelectionChanges& msg) override; //@} @@ -769,6 +775,7 @@ class SketcherGuiExport ViewProviderSketch: public PartGui::ViewProvider2DObject //@{ bool setEdit(int ModNum) override; void unsetEdit(int ModNum) override; + bool isEditHandlerActive() const override; void setEditViewer(Gui::View3DInventorViewer*, int ModNum) override; void unsetEditViewer(Gui::View3DInventorViewer*) override; static void camSensCB(void* data, SoSensor*); // camera sensor callback diff --git a/tests/src/Gui/SelectionTest.cpp b/tests/src/Gui/SelectionTest.cpp index 48ca8ba01fb1..03551cc74823 100644 --- a/tests/src/Gui/SelectionTest.cpp +++ b/tests/src/Gui/SelectionTest.cpp @@ -76,12 +76,14 @@ Gui::SelectionPickPolicy::Candidate pickCandidate( int priority, bool closeToFirst, bool hasGate, - bool passesGate + bool passesGate, + int ownerPriority = 0 ) { Gui::SelectionPickPolicy::Candidate candidate; candidate.owner = owner; candidate.priority = priority; + candidate.ownerPriority = ownerPriority; candidate.closeToFirst = closeToFirst; candidate.hasGate = hasGate; candidate.passesGate = passesGate; @@ -166,3 +168,32 @@ TEST(SelectionPickPolicyTest, keepsPreferredPickWhenNoCandidatePassesGate) EXPECT_EQ(Gui::SelectionPickPolicy::choosePreferredPick(candidates), 0U); } + +// A sketch lying on a solid face produces a coincident pick with the face. +// The sketch's view provider claims a higher pick priority, so it must win even +// though the face is the first owner in the depth-sorted pick list. +TEST(SelectionPickPolicyTest, prefersHigherOwnerPriorityForCoincidentPicks) +{ + int faceOwner {}; + int sketchOwner {}; + std::vector candidates { + pickCandidate(&faceOwner, 1, true, false, true), + pickCandidate(&sketchOwner, 2, true, false, true, 1), + }; + + EXPECT_EQ(Gui::SelectionPickPolicy::choosePreferredPick(candidates), 1U); +} + +// The same object priority must not reach through a farther object: when the +// higher-priority pick does not coincide with the first pick it is ignored. +TEST(SelectionPickPolicyTest, ignoresOwnerPriorityWhenNotCoincident) +{ + int faceOwner {}; + int sketchOwner {}; + std::vector candidates { + pickCandidate(&faceOwner, 1, true, false, true), + pickCandidate(&sketchOwner, 3, false, false, true, 1), + }; + + EXPECT_EQ(Gui::SelectionPickPolicy::choosePreferredPick(candidates), 0U); +} From 860498e6dcf269843a3f752f93968619fb660f5c Mon Sep 17 00:00:00 2001 From: phantomcake Date: Sat, 10 Oct 2026 23:05:58 -0400 Subject: [PATCH 4/8] Gui/Vulkan: engage interaction LOD only on a camera pose change The interaction-LOD trigger is the camera-node sensor, and a SoNodeSensor is a node auditor: it fires for *any* camera field write, including the near/far auto-clipping rewrite that lands on ordinary frames -- a hover or preselection redraw, a selection change, a scene edit, a preference push. A plain hover moves no camera at all, yet the notification looked identical to a real move, so the reduced-detail path was latched and its 200 ms idle timer re-armed for as long as frames kept being produced. On the desk that keeps wide lines downgraded indefinitely, which is what made the Sketcher grid's every-nth division lines lose their configured width while the pointer moved. Classify each notification with cameraPoseChanged() (position, orientation, focal distance, orthographic height / perspective height-angle) and propagate the LOD only for a real pose change. Any other camera write still requests a frame, because near/far changes the projection; a camera-node swap counts as a move, so a projection-type change still engages the LOD once. Every navigation path writes one of the pose fields (orbit -> orientation, pan -> position, ortho zoom -> height, perspective zoom -> focalDistance/heightAngle), so the optimisation is unchanged for real navigation. Add an FC_VULKAN_LOD_TRACE diagnostic ([LODTRACE] cameraPose / interactionLod) so this class of bug is observable from a probe or a log. --- src/Gui/VulkanViewportAdapter.cpp | 117 ++++++++++++++++++++++++++++-- src/Gui/VulkanViewportAdapter.h | 30 +++++++- 2 files changed, 141 insertions(+), 6 deletions(-) diff --git a/src/Gui/VulkanViewportAdapter.cpp b/src/Gui/VulkanViewportAdapter.cpp index 126bb0513707..413d253b30ad 100644 --- a/src/Gui/VulkanViewportAdapter.cpp +++ b/src/Gui/VulkanViewportAdapter.cpp @@ -24,6 +24,8 @@ #include #include #include +#include +#include #include #include #include @@ -551,10 +553,104 @@ void VulkanViewportAdapter::cameraChangedCB(void* data, SoSensor* /*sensor*/) } } auto* self = static_cast(data); - self->noteCameraMoved(); + self->noteCameraChanged(); self->requestVulkanFrame(); } +void VulkanViewportAdapter::noteCameraChanged() +{ +#ifdef FREECAD_USE_VULKAN + // The camera sensor is a node auditor, so it fires for *any* camera field + // write -- and two of those are not a camera move: + // - the near/far auto-clipping rewrite (the GL viewer's + // SoRenderManager::updateClippingPlanes(), reached through syncViewer(), + // and the Vulkan manager's own mirror of it in setClippingPlanes()), + // which lands on ordinary frames: a hover/preselection redraw, a + // selection change, a scene edit, a preference push. A hover moves no + // camera at all, yet the notification looks identical to a real move. + // - a camera-node swap on a projection-type change, which re-attaches the + // sensor and is handled by resyncCameraAndDecorations(). + // Interaction LOD is a *navigation* state, so only a real pose change may + // engage it. Engaging it on the clipping rewrite kept the viewport in + // reduced quality for as long as ordinary frames were produced (every one + // of them re-armed the idle timer), which visibly dropped the width of + // every wide line -- most obviously the Sketcher grid's division lines -- + // while the pointer moved, and only restored it once the frame stream + // stopped. See also isWideLine()'s cost test, which keeps small wide-line + // sets out of the downgrade even during genuine navigation. + SoCamera* camera = nullptr; + if (_viewer) { + if (SoRenderManager* rm = _viewer->getSoRenderManager()) { + camera = rm->getCamera(); + } + } + const bool poseChanged = cameraPoseChanged(camera); + if (poseChanged && std::getenv("FC_VULKAN_LOD_TRACE") != nullptr) { + static int _n = 0; + if (_n++ < 2000) { + fprintf(stderr, + "[LODTRACE] cameraPose n=%d node=%p pos=(%.4f,%.4f,%.4f) " + "rot=(%.4f,%.4f,%.4f,%.4f) focal=%.4f height=%.4f\n", + _n, static_cast(camera), + _cameraPose.position[0], _cameraPose.position[1], + _cameraPose.position[2], + _cameraPose.orientation[0], _cameraPose.orientation[1], + _cameraPose.orientation[2], _cameraPose.orientation[3], + _cameraPose.focalDistance, _cameraPose.height); + } + } + if (poseChanged) { + noteCameraMoved(); + } +#endif +} + +bool VulkanViewportAdapter::cameraPoseChanged(SoCamera* camera) +{ + const bool sameNode = camera != nullptr && camera == _cameraPoseNode; + CameraPose pose; + bool valid = false; + if (camera) { + const SbVec3f position = camera->position.getValue(); + const SbRotation orientation = camera->orientation.getValue(); + pose.position[0] = position[0]; + pose.position[1] = position[1]; + pose.position[2] = position[2]; + orientation.getValue(pose.orientation[0], + pose.orientation[1], + pose.orientation[2], + pose.orientation[3]); + pose.focalDistance = camera->focalDistance.getValue(); + // The view volume's opening is part of the pose: the orthographic zoom + // writes `height`, while a perspective camera changes `focalDistance` + // (dolly/zoom) and `heightAngle` (field of view), so both types are + // covered for a zoom that leaves the position untouched. + if (auto* ortho = dynamic_cast(camera)) { + pose.height = ortho->height.getValue(); + } + else if (auto* perspective = dynamic_cast(camera)) { + pose.height = perspective->heightAngle.getValue(); + } + valid = true; + } + + const bool changed = !valid || !sameNode || !_cameraPoseValid + || pose.position[0] != _cameraPose.position[0] + || pose.position[1] != _cameraPose.position[1] + || pose.position[2] != _cameraPose.position[2] + || pose.orientation[0] != _cameraPose.orientation[0] + || pose.orientation[1] != _cameraPose.orientation[1] + || pose.orientation[2] != _cameraPose.orientation[2] + || pose.orientation[3] != _cameraPose.orientation[3] + || pose.focalDistance != _cameraPose.focalDistance + || pose.height != _cameraPose.height; + + _cameraPoseNode = camera; + _cameraPose = pose; + _cameraPoseValid = valid; + return changed; +} + void VulkanViewportAdapter::noteCameraMoved() { #ifdef FREECAD_USE_VULKAN @@ -563,10 +659,11 @@ void VulkanViewportAdapter::noteCameraMoved() } const VulkanViewSettings& settings = _viewer->getVulkanViewSettings(); // Interaction LOD is opt-in via the VulkanInteractionLod preference - // (default on): while the camera moves, the raster backend draws wide lines - // as plain 1px GPU lines instead of expanding every edge segment into quads - // on the CPU (the dominant navigation cost on large edge sets). When - // disabled, make sure any previously engaged state is dropped. + // (default on): while the camera moves, the raster backend draws large + // wide-line sets as plain 1px GPU lines instead of expanding every edge + // segment into quads on the CPU (the dominant navigation cost on + // edge-heavy scenes). When disabled, make sure any previously engaged + // state is dropped. if (!settings.interactionLod) { setInteractionLod(false); return; @@ -589,6 +686,16 @@ void VulkanViewportAdapter::setInteractionLod(bool active) return; } _interactionLod = active; + if (std::getenv("FC_VULKAN_LOD_TRACE") != nullptr) { + uint32_t frame = 0; +#ifdef FREECAD_USE_VULKAN + if (_vulkanViewer) { + frame = _vulkanViewer->getRenderFrameCount(); + } +#endif + fprintf(stderr, "[LODTRACE] interactionLod=%d frame=%u\n", + active ? 1 : 0, frame); + } #ifdef FREECAD_USE_VULKAN if (_vulkanViewer) { // The widget's setter requests the frame that shows the new quality. diff --git a/src/Gui/VulkanViewportAdapter.h b/src/Gui/VulkanViewportAdapter.h index 09d622c2b683..d0fb540b5040 100644 --- a/src/Gui/VulkanViewportAdapter.h +++ b/src/Gui/VulkanViewportAdapter.h @@ -13,6 +13,7 @@ class QStackedWidget; class QTimer; +class SoCamera; class SoNodeSensor; class SoSensor; @@ -232,9 +233,21 @@ class VulkanViewportAdapter : public QObject, public InteractionSurface /// drag, a seek animation and the navcube/view-home animation alike /// without depending on the GL interaction callbacks (which never fire /// for the hidden display-only GL viewer). Honours the - /// VulkanInteractionLod preference. + /// VulkanInteractionLod preference. Call only for a real pose change + /// (see noteCameraChanged()). void noteCameraMoved(); + /// Classify a camera-sensor notification and engage the interaction LOD + /// only when the camera pose actually changed. The sensor fires for any + /// camera field write, including the near/far auto-clipping rewrite that + /// lands on ordinary frames; see the implementation. + void noteCameraChanged(); + + /// Compare \a camera's pose (position, orientation, focal distance, view + /// height) with the last one seen and cache the new pose. Returns true on + /// the first call for a camera node and whenever the pose moved. + bool cameraPoseChanged(SoCamera* camera); + /// Forward the interaction-LOD state to the Vulkan widget (only on a /// change). A no-op without a Vulkan widget. void setInteractionLod(bool active); @@ -275,6 +288,21 @@ class VulkanViewportAdapter : public QObject, public InteractionSurface QTimer* _interactionTimer = nullptr; //! Current interaction-LOD state forwarded to the Vulkan widget. bool _interactionLod = false; + //! Pose signature of the camera at the last sensor notification. The + //! sensor callback compares against this before treating a notification as + //! a camera move (see noteCameraChanged()). + struct CameraPose + { + float position[3] {0.0f, 0.0f, 0.0f}; + float orientation[4] {0.0f, 0.0f, 0.0f, 1.0f}; + float focalDistance {0.0f}; + float height {0.0f}; + }; + SoCamera* _cameraPoseNode = nullptr; + CameraPose _cameraPose; + //! False until a camera pose has been recorded (the first notification for + //! a camera node counts as a move, so LOD engages rather than being missed). + bool _cameraPoseValid = false; }; } // namespace Gui From 3f4ce287b80d48f1ef00e219eb7d0d1f186a3c08 Mon Sep 17 00:00:00 2001 From: phantomcake Date: Sat, 10 Oct 2026 23:06:02 -0400 Subject: [PATCH 5/8] Gui: parent dock context menus to a top-level window Right-clicking the Report view (or the Python console) logged QWidgetWindow(0x..., name="Report viewWindow") must be a top level window. A QMenu is a popup (top level) window, and Qt derives its transient parent from the parent widget's window in QMenuPrivate::transientParentWindow(). A dock that owns a native, non-top-level window -- as it does once the viewport made its ancestors native, or after a float/re-dock cycle -- makes QWindow::setTransientParent() reject that window and drop the transient parent. Parent these popups to the containing top level window, the same way the 3D-view context menu already does (NavigationStyle::openPopupMenu()). --- src/Gui/PythonConsole.cpp | 7 ++++++- src/Gui/ReportView.cpp | 13 ++++++++++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/src/Gui/PythonConsole.cpp b/src/Gui/PythonConsole.cpp index 459f9a1e1d88..e3411c12bf55 100644 --- a/src/Gui/PythonConsole.cpp +++ b/src/Gui/PythonConsole.cpp @@ -1327,7 +1327,12 @@ void PythonConsole::overrideCursor(const QString& txt) void PythonConsole::contextMenuEvent(QContextMenuEvent* e) { - QMenu menu(this); + // Parent the popup to the top level window: the console lives in a dock, and + // a dock-owned native (non-top-level) QWidgetWindow makes Qt's + // QMenuPrivate::transientParentWindow() hand QWindow::setTransientParent() + // a window it rejects with "QWidgetWindow(...) must be a top level window." + // (see the same fix in ReportOutput::contextMenuEvent()). + QMenu menu(window()); QAction* a; bool mayPasteHere = cursorBeyond(this->textCursor(), this->inputBegin()); diff --git a/src/Gui/ReportView.cpp b/src/Gui/ReportView.cpp index f1cdc5bdc04d..fd43a6cf7075 100644 --- a/src/Gui/ReportView.cpp +++ b/src/Gui/ReportView.cpp @@ -618,7 +618,18 @@ void ReportOutput::contextMenuEvent(QContextMenuEvent* e) bool bShowOnError = ReportOutputParameter::showOnError(); bool bShowOnCritical = ReportOutputParameter::showOnCritical(); - auto menu = new QMenu(this); + // A QMenu is a popup (top level) window, and Qt derives its transient parent + // from the parent widget's window in QMenuPrivate::transientParentWindow(). + // The Report view is hosted in a dock, and as soon as that dock owns a + // native window (QWidgetWindow, e.g. after the viewport made its ancestors + // native, or after a float/re-dock cycle) the fallback + // QWidget::nativeParentWidget() hands Qt that native *child* window. + // QWindow::setTransientParent() rejects it with + // "QWidgetWindow(...) must be a top level window." and drops the transient + // parent. Parent the menu to the top level window instead, so the popup + // resolves to a real top level window (same fix as the 3D-view context menu + // in NavigationStyle::openPopupMenu()). + auto menu = new QMenu(window()); auto optionMenu = new QMenu(menu); optionMenu->setTitle(tr("Options")); menu->addMenu(optionMenu); From 856172f6cb308c0fe528afaad9c9b97e71490a5a Mon Sep 17 00:00:00 2001 From: phantomcake Date: Sat, 10 Oct 2026 23:06:05 -0400 Subject: [PATCH 6/8] Gui: don't report a filter mismatch when a file dialog is cancelled Cancelling Save As (or Open) logged FileDialog: Qt-backed nativeFileDialog returned a selected filter that wasn't in the original list, defaulting to index 0 Problem cause filter: "" QFileDialog::getSaveFileName()/getOpenFileName() return an empty QString when the user cancels, and appending that to the result list left a one-element list whose entry was empty. The cancellation test is selected.isEmpty(), so it never saw the cancellation and the filter-mismatch failsafe fired instead. Only append non-empty results for the single-file modes: a real selection is unchanged, and a cancel now takes the intended index-0 fallback silently. --- src/Gui/FileDialog.cpp | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/Gui/FileDialog.cpp b/src/Gui/FileDialog.cpp index 1e64c22bc726..b8c34b8fcc0b 100644 --- a/src/Gui/FileDialog.cpp +++ b/src/Gui/FileDialog.cpp @@ -409,7 +409,7 @@ QStringList FileDialogInternal::nativeFileDialog( QString selectedQtFilter = (selectedFilterIndex >= 0) ? qtFilterList[selectedFilterIndex] : ""; QStringList selected; if (mode == NativeFileDialogMode::OpenSingle) { - selected << QFileDialog::getOpenFileName( + const QString file = QFileDialog::getOpenFileName( parent, caption, startPath, @@ -417,6 +417,13 @@ QStringList FileDialogInternal::nativeFileDialog( &selectedQtFilter, options ); + // Cancelling returns an empty string, and appending it anyway would + // leave a one-element list whose entry is empty. The cancellation test + // below is selected.isEmpty(), so the empty entry must not be added or + // a plain cancel would be reported as a filter mismatch. + if (!file.isEmpty()) { + selected << file; + } } else if (mode == NativeFileDialogMode::OpenMultiple) { selected << QFileDialog::getOpenFileNames( @@ -429,7 +436,7 @@ QStringList FileDialogInternal::nativeFileDialog( ); } else /* (mode == NativeFileDialogMode::Save) */ { - selected << QFileDialog::getSaveFileName( + const QString file = QFileDialog::getSaveFileName( parent, caption, startPath, @@ -437,6 +444,11 @@ QStringList FileDialogInternal::nativeFileDialog( &selectedQtFilter, options ); + // See the OpenSingle case: a cancelled save dialog returns an empty + // string, which must not become an entry of `selected`. + if (!file.isEmpty()) { + selected << file; + } } selectedFilterIndex = qtFilterList.indexOf(selectedQtFilter); if (selectedFilterIndex < 0 && selected.isEmpty()) { From 1d3ae40ee69b7f0ded74ec437ef3bd0e5818d50b Mon Sep 17 00:00:00 2001 From: phantomcake Date: Sat, 10 Oct 2026 23:06:09 -0400 Subject: [PATCH 7/8] Gui/Vulkan: wake the display frame on NaviCube hover/pick changes setHilite(), the mouseMoved() hovering transition and the movable-cube drag scheduled only the GL render-manager redraw, which never reaches the display-only Vulkan widget: the cube lives in the decoration overlay, which the adapter's scene sensor does not watch. NaviCube hover highlights therefore never appeared under the Vulkan renderer (clicking worked because it moves the camera, which already requests a frame). Route these through requestRedraw(), which also calls viewer->requestNaviCubeRedraw() -> naviCubeChanged -> adapter frame. Note: this is the same fix as the standalone gui/vulkan-navicube-redraw branch (PR #148); it is repeated here because this branch is where the bug was hit. --- src/Gui/NaviCube.cpp | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/Gui/NaviCube.cpp b/src/Gui/NaviCube.cpp index 53c23024c5a4..e05fcd194af9 100644 --- a/src/Gui/NaviCube.cpp +++ b/src/Gui/NaviCube.cpp @@ -1164,7 +1164,11 @@ void NaviCubeImplementation::setHilite(PickId hilite) if (soNaviCube) { soNaviCube->hiliteId = static_cast(hiliteId); } - viewer->getSoRenderManager()->scheduleRedraw(); + // The GL render-manager redraw never reaches the display-only Vulkan + // widget (the cube lives in the decoration overlay, which the adapter's + // scene sensor does not watch), so route the wake through + // requestRedraw() -- it also asks the viewer for a display frame. + requestRedraw(false); } } @@ -1231,7 +1235,7 @@ bool NaviCubeImplementation::mouseMoved(short x, short y) if (hovering != this->hovering) { this->hovering = hovering; - viewer->getSoRenderManager()->scheduleRedraw(); + requestRedraw(false); } if (!dragStarted) { @@ -1257,7 +1261,7 @@ bool NaviCubeImplementation::mouseMoved(short x, short y) relPos[0] = std::min(std::max(newX, 0.0f), 1.0f); relPos[1] = std::min(std::max(newY, 0.0f), 1.0f); - viewer->getSoRenderManager()->scheduleRedraw(); + requestRedraw(false); } return true; } From 1e1d808d17644c26c126cf136b3db25a736042e1 Mon Sep 17 00:00:00 2001 From: phantomcake Date: Sat, 10 Oct 2026 23:06:12 -0400 Subject: [PATCH 8/8] submodules: bump coin for the wide-line interaction-LOD fix --- src/3rdParty/coin | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/3rdParty/coin b/src/3rdParty/coin index ca4e27f44282..2253a7de0659 160000 --- a/src/3rdParty/coin +++ b/src/3rdParty/coin @@ -1 +1 @@ -Subproject commit ca4e27f44282c4596e38f510aea58d6c789010ee +Subproject commit 2253a7de06594d0bd7c69237b472e7b5876582b1