feature: IC preview on hover#363
Conversation
darktorres
left a comment
There was a problem hiding this comment.
Two critical dangling-pointer issues must be fixed before merge. All other findings are inline.
Optional improvement — shallow-load the blob for a faithful preview
The preview currently renders from m_internalElements (the simulation graph). loadBoundaryElement() replaces every Input/Output element with a proxy Node, so the preview shows unlabelled Nodes at every boundary instead of the actual buttons, switches, and LEDs the user placed.
A shallow parse of the IC blob/file would fix this without any format changes. The format is sequential (preamble → elements → connections), the blob registry lives in the preamble metadata, and IC deserialization is fully encapsulated in IC::load(). A shallow variant skips deserializeAndLoad() for nested ICs, reads only visual properties, and produces a correct one-level rendering. Result is cached via m_previewValid, so the parse cost is paid only once per hover session.
a6f8e35 to
e0abead
Compare
- Remove consecutive blank lines (GraphicElement.h) - Add missing trailing newlines (TestDanglingPointer.cpp/h) - Fix include ordering (GraphicsView.cpp, TestLanguageManager.cpp)
b8e7448 to
54d1b8d
Compare
Adds a hover-triggered preview popup so users can identify the contents of an IC without double-clicking to open its sub-circuit. Hovering an IC for one second shows a tooltip-like popup with a rendered miniature of the IC's internal circuit, dismissed on a 300 ms delay when the cursor leaves both the IC and the popup itself. # Designed circuit, not simulation graph The preview is rendered from the designed circuit — the actual buttons, switches, LEDs, and displays the user placed — rather than the post-load simulation graph in which boundary Input/Output elements are stripped and replaced with proxy `Node` junctions during `IC::loadBoundaryElement()`. The pixmap is captured eagerly at the top of `IC::processLoadedItems()`, while the freshly-deserialized `items` list still contains the original boundary elements with their connections and ports intact, then the substitution loop runs as before. The fix is strictly cheaper than a shallow re-parse of the IC blob/file: one render at load time, no re-deserialization, no format changes. Reload is automatic. `ICRegistry`'s `QFileSystemWatcher` triggers `IC::loadFile()` on every IC instance referencing a file when that file changes, which re-enters `processLoadedItems()` and regenerates the pixmap. Embedded ICs follow the same path via `loadFromBlob()`. # Popup ownership The popup is owned by `MainWindow` as a Qt child (`new ICPreviewPopup(this)` in the ctor). Qt's parent/child cleanup destroys it before `~QWidget` finishes, well before `~QApplication` runs — no singletons, no `qAddPostRoutine`, no lazy creation. `MainWindow` holds it as a `QPointer` so accesses during teardown are safe regardless of child destruction order. `IC` reaches it through a small file-scope helper that walks Application → MainWindow → popup with defensive guards on each link, but during normal operation the chain is always alive. # UX details - The popup positions itself near the cursor with a 16 px offset and is clamped to the available screen geometry so it never spills off-screen. - `hoverMoveEvent` updates the pending position while the show timer is running, so the popup appears near the *current* cursor when the delay elapses, not at the original entry point. - Cursor entering the popup itself cancels any scheduled hide, letting users move onto the preview to read it. - Double-clicking an IC (which opens its sub-circuit in a new tab) hides the popup immediately so it doesn't overlap the tab transition. - The popup chrome is a translucent dark window (OS-frameless: `Qt::ToolTip | Qt::FramelessWindowHint`) with a custom-drawn 6 px-radius border; the preview pixmap sits inside an inset frame (1 px light border, 6 px padding, near-black inset background) so the rendered circuit reads as a framed image rather than floating content. # Performance and edge cases - Circuits exceeding `MaxElementCount = 500` are skipped — the preview pixmap is left null, and `executeShow()` hides instead of showing. 500 was chosen empirically: rendering above this size exceeds 16 ms on a mid-range laptop, making the 1-second hover delay feel laggy. - Empty circuits are skipped the same way. - Pixmap rendering temporarily borrows the `items` into a throwaway `QGraphicsScene`, calls `render()`, then removes them. A `qScopeGuard` ensures the borrow is reversed even if `render()` throws — IC owns the items, so a stuck borrow at scene destruction would be a double-free. - The destination `QPixmap` is explicitly filled with the background brush before `render()`. Without the fill, `tempScene.render()`'s source→target affine can leave a 1-pixel sliver unpainted at the right/bottom edge, exposing uninitialized pixmap memory (commonly white on Windows). # Lifetime safety `ICPreviewPopup::m_pendingIC` is a `QPointer<IC>` so it auto-nulls if the IC is destroyed while a show or hide timer is pending. `IC::~IC()` additionally cancels and hides the popup if it was about to display *this* IC, so the user never sees stale content from a destroyed IC. The cached pixmap held by the popup's `QLabel` is implicitly shared with the IC's `m_previewPixmap`, so the displayed image survives the IC's destruction even though the IC pointer becomes null.
The preview will look like this: