From d2ce65f85792b5f56d3d1b201965fe0625a33d89 Mon Sep 17 00:00:00 2001 From: Filip Leonarski Date: Sun, 26 Jul 2026 21:11:42 +0200 Subject: [PATCH] Viewer: magnifier displays the frame the main view already rendered The magnifier and the main view are two views of the same image at different position and zoom, but the magnifier ran the whole pipeline again on its own copy: it wrapped the same int32 buffer in a SimpleImage, converted it to float, coloured every pixel and kept its own full-size QImage. That is a second conversion and two extra full-detector buffers (20 MB at 2.8 Mpx, 138 MB at 18 Mpx) to feed a 320x320 window. Separate producing a frame from displaying one: - JFJochImage keeps the rendered frame in a shared_ptr (the pointer is stable for the widget's lifetime; only the contents change, so the existing buffer reuse is unaffected), publishes it via Frame() and announces new pixels with frameRendered(). - JFJochImageItem holds that shared_ptr instead of a reference to a member of its owner, which also removes a lifetime coupling. - JFJochFollowerImage is a small read-only view of such a frame with its own zoom and centre. It shows only the image: overlays, ROI tools and per-pixel labels belong to the view that owns the data. - The magnifier becomes one of those, fed from frameRendered(). Consequences beyond the saving: the magnifier now agrees with the main view on colour map, contrast and HDR mode, which it never did -- it was wired to neither, so it always drew with its own defaults. And the visibility guard added in 6d1af4921 is gone: there is no longer any per-frame work to skip, so nothing needs guarding. That guard was a workaround for this design. Stepping 30 frames with the magnifier open: 5550 -> 4810 ms CPU, which is what it costs with the magnifier closed (4770 ms) -- it is now free either way. Verified: main image panel and a drag-pan stay pixel-identical to the pre-refactor binary (AE=0); the magnifier follows the cursor, updates on a new frame, and now tracks a colour-map change. Co-Authored-By: Claude Opus 5 (1M context) --- viewer/CMakeLists.txt | 2 + viewer/JFJochViewerWindow.cpp | 7 ++- .../image_viewer/JFJochDiffractionImage.cpp | 1 + viewer/image_viewer/JFJochFollowerImage.cpp | 60 +++++++++++++++++++ viewer/image_viewer/JFJochFollowerImage.h | 32 ++++++++++ viewer/image_viewer/JFJochImage.cpp | 33 ++++++---- viewer/image_viewer/JFJochImage.h | 23 ++++--- viewer/windows/JFJochMagnifierWindow.cpp | 57 +++--------------- viewer/windows/JFJochMagnifierWindow.h | 28 ++++----- 9 files changed, 154 insertions(+), 89 deletions(-) create mode 100644 viewer/image_viewer/JFJochFollowerImage.cpp create mode 100644 viewer/image_viewer/JFJochFollowerImage.h diff --git a/viewer/CMakeLists.txt b/viewer/CMakeLists.txt index a6cc9e78..9c311a5b 100644 --- a/viewer/CMakeLists.txt +++ b/viewer/CMakeLists.txt @@ -89,6 +89,8 @@ ADD_EXECUTABLE(jfjoch_viewer jfjoch_viewer.cpp JFJochViewerWindow.cpp JFJochView windows/JFJochLicenseWindow.h image_viewer/JFJochImage.cpp image_viewer/JFJochImage.h + image_viewer/JFJochFollowerImage.cpp + image_viewer/JFJochFollowerImage.h windows/JFJoch2DAzintImageWindow.cpp windows/JFJoch2DAzintImageWindow.h widgets/JFJochViewerROIResult.cpp diff --git a/viewer/JFJochViewerWindow.cpp b/viewer/JFJochViewerWindow.cpp index 0c323170..82fade34 100644 --- a/viewer/JFJochViewerWindow.cpp +++ b/viewer/JFJochViewerWindow.cpp @@ -395,8 +395,11 @@ JFJochViewerWindow::JFJochViewerWindow(QWidget *parent, bool dbus, const QString viewer, &JFJochDiffractionImage::centerOnSpot); // --- Magnifier --- - connect(this, &JFJochViewerWindow::imageReady, - magnifierWindow, &JFJochHelperWindow::imageLoaded); + // The magnifier shows the frame the main view has already rendered - the same pixels with + // its own zoom and centre - so it neither converts nor colours anything itself. + connect(viewer, &JFJochImage::frameRendered, magnifierWindow, [viewer, magnifierWindow] { + magnifierWindow->setFrame(viewer->Frame()); + }); connect(viewer, &JFJochImage::hoverScenePos, magnifierWindow, &JFJochMagnifierWindow::centerAt); diff --git a/viewer/image_viewer/JFJochDiffractionImage.cpp b/viewer/image_viewer/JFJochDiffractionImage.cpp index 34ee23c7..5f7b3576 100644 --- a/viewer/image_viewer/JFJochDiffractionImage.cpp +++ b/viewer/image_viewer/JFJochDiffractionImage.cpp @@ -910,6 +910,7 @@ void JFJochDiffractionImage::loadImage(std::shared_ptr } else { image.reset(); W = 0; H = 0; + ClearFrame(); // followers (magnifier) must not keep showing the old frame if (scene()) scene()->clear(); resetScenePointers(); diff --git a/viewer/image_viewer/JFJochFollowerImage.cpp b/viewer/image_viewer/JFJochFollowerImage.cpp new file mode 100644 index 00000000..83d92126 --- /dev/null +++ b/viewer/image_viewer/JFJochFollowerImage.cpp @@ -0,0 +1,60 @@ +// SPDX-FileCopyrightText: 2026 Filip Leonarski, Paul Scherrer Institute +// SPDX-License-Identifier: GPL-3.0-only + +#include "JFJochFollowerImage.h" +#include "JFJochImage.h" + +#include + +#include +#include + +JFJochFollowerImage::JFJochFollowerImage(QWidget *parent) : QGraphicsView(parent) { + setScene(new QGraphicsScene(this)); + setTransformationAnchor(QGraphicsView::AnchorViewCenter); + setHorizontalScrollBarPolicy(Qt::ScrollBarAlwaysOff); + setVerticalScrollBarPolicy(Qt::ScrollBarAlwaysOff); + setTransform(QTransform::fromScale(zoom_, zoom_)); +} + +void JFJochFollowerImage::SetFrame(std::shared_ptr frame) { + const bool same_pointer = (frame_ == frame); + const bool same_size = frame_ && frame && frame_->size() == frame->size(); + frame_ = std::move(frame); + + if (!frame_ || frame_->isNull()) { + scene()->clear(); + item_ = nullptr; + return; + } + + // The producer reuses one buffer, so normally only the pixels changed and the item stays. + if (!item_ || !same_pointer) { + scene()->clear(); + item_ = new JFJochImageItem(frame_); + scene()->addItem(item_); + } + + if (!same_size) { + item_->refresh(); + scene()->setSceneRect(0, 0, frame_->width(), frame_->height()); + } + + viewport()->update(); +} + +void JFJochFollowerImage::CenterAt(QPointF scenePos) { + if (!frame_ || frame_->isNull()) + return; + centerOn(scenePos); +} + +void JFJochFollowerImage::wheelEvent(QWheelEvent *event) { + constexpr double step = 1.15; + zoom_ *= (event->angleDelta().y() > 0) ? step : 1.0 / step; + zoom_ = std::clamp(zoom_, 1.0, 200.0); + + const QPointF center = mapToScene(viewport()->rect().center()); + setTransform(QTransform::fromScale(zoom_, zoom_)); + centerOn(center); +} diff --git a/viewer/image_viewer/JFJochFollowerImage.h b/viewer/image_viewer/JFJochFollowerImage.h new file mode 100644 index 00000000..0c484f79 --- /dev/null +++ b/viewer/image_viewer/JFJochFollowerImage.h @@ -0,0 +1,32 @@ +// SPDX-FileCopyrightText: 2026 Filip Leonarski, Paul Scherrer Institute +// SPDX-License-Identifier: GPL-3.0-only + +#pragma once + +#include + +#include +#include + +class JFJochImageItem; + +// A second view of a frame that a JFJochImage has already rendered: the same pixels, with its own +// zoom and centre. Nothing is converted or coloured here and there is no second full-size buffer, +// so following the main view costs a pointer assignment per frame rather than a whole render. +// +// It shows only the image. Overlays, ROI tools and per-pixel value labels belong to the view that +// owns the data; a magnifier does not need them. +class JFJochFollowerImage : public QGraphicsView { + Q_OBJECT + + JFJochImageItem *item_ = nullptr; + std::shared_ptr frame_; + double zoom_ = 12.0; + + void wheelEvent(QWheelEvent *event) override; +public: + explicit JFJochFollowerImage(QWidget *parent = nullptr); + + void SetFrame(std::shared_ptr frame); + void CenterAt(QPointF scenePos); +}; diff --git a/viewer/image_viewer/JFJochImage.cpp b/viewer/image_viewer/JFJochImage.cpp index ef165b21..26172d87 100644 --- a/viewer/image_viewer/JFJochImage.cpp +++ b/viewer/image_viewer/JFJochImage.cpp @@ -20,7 +20,7 @@ #include QRectF JFJochImageItem::boundingRect() const { - return QRectF(0, 0, img_.width(), img_.height()); + return img_ ? QRectF(0, 0, img_->width(), img_->height()) : QRectF(); } QPainterPath JFJochImageItem::opaqueArea() const { @@ -31,12 +31,12 @@ QPainterPath JFJochImageItem::opaqueArea() const { } void JFJochImageItem::paint(QPainter *painter, const QStyleOptionGraphicsItem *, QWidget *) { - if (img_.isNull()) + if (!img_ || img_->isNull()) return; // QGraphicsPixmapItem defaults to Qt::FastTransformation and turned this hint off before // drawing; keep doing that, so zoomed-in detector pixels stay sharp squares. painter->setRenderHint(QPainter::SmoothPixmapTransform, false); - painter->drawImage(0, 0, img_); + painter->drawImage(0, 0, *img_); } void JFJochImageItem::refresh() { @@ -402,7 +402,7 @@ void JFJochImage::contextMenuEvent(QContextMenuEvent *event) { QAction *fitAct = menu.addAction(tr("Fit image to view")); QAction *clearRoiAct = menu.addAction(tr("Clear ROI")); - const bool hasImage = (W > 0 && H > 0 && !qimg_buffer_.isNull()); + const bool hasImage = (W > 0 && H > 0 && !frame_->isNull()); copyImageAct->setEnabled(hasImage); copyWithOverlayAct->setEnabled(hasImage && scene()); saveImageAct->setEnabled(hasImage); @@ -462,7 +462,7 @@ QImage JFJochImage::renderToImage(bool with_overlay) { p.end(); } else { // The underlying rendered image (no overlay) - img = qimg_buffer_; + img = *frame_; } // Ensure 1:1 pixel ratio and 96 DPI metadata to avoid rescaling in consumer apps img.setDevicePixelRatio(1.0); @@ -473,7 +473,7 @@ QImage JFJochImage::renderToImage(bool with_overlay) { } void JFJochImage::copyImageToClipboard() { - if (W == 0 || H == 0 || qimg_buffer_.isNull()) return; + if (W == 0 || H == 0 || frame_->isNull()) return; setClipboardAsJpegAndImage(renderToImage(false), 95); emit writeStatusBar(tr("Image copied to clipboard"), 2000); @@ -487,7 +487,7 @@ void JFJochImage::copyImageWithOverlayToClipboard() { } void JFJochImage::saveImageToFile(bool with_overlay) { - if (W == 0 || H == 0 || qimg_buffer_.isNull()) return; + if (W == 0 || H == 0 || frame_->isNull()) return; if (with_overlay && !scene()) return; const QString caption = with_overlay ? tr("Save image with overlay as JPEG") @@ -762,13 +762,13 @@ void JFJochImage::ColorRow(size_t y, const PixelColorMap &map, QRgb *out) const } void JFJochImage::RenderImage() { - if (qimg_buffer_.width() != int(W) || qimg_buffer_.height() != int(H)) - qimg_buffer_ = QImage(int(W), int(H), QImage::Format_RGB32); + if (frame_->width() != int(W) || frame_->height() != int(H)) + *frame_ = QImage(int(W), int(H), QImage::Format_RGB32); // Take the data pointer once, here: scanLine() is non-const, so calling it from the // workers below would have each of them detach the (possibly shared) buffer in parallel. - uchar *const bits = qimg_buffer_.bits(); - const qsizetype stride = qimg_buffer_.bytesPerLine(); + uchar *const bits = frame_->bits(); + const qsizetype stride = frame_->bytesPerLine(); const PixelColorMap map = MakeColorMap(); @@ -781,6 +781,13 @@ void JFJochImage::RenderImage() { }); image_dirty_ = true; + emit frameRendered(); +} + +void JFJochImage::ClearFrame() { + *frame_ = QImage(); + image_dirty_ = true; + emit frameRendered(); } void JFJochImage::centerOnSpot(QPointF point) { @@ -875,7 +882,7 @@ void JFJochImage::writePixelLabels() { textItem->setFont(font); // Read the colour back from the rendered image rather than keeping a // full-size mirror of it around for the few pixels that get a label. - const QRgb pxl = qimg_buffer_.pixel(x, y); + const QRgb pxl = frame_->pixel(x, y); if (luminance(rgb{.r = static_cast(qRed(pxl)), .g = static_cast(qGreen(pxl)), .b = static_cast(qBlue(pxl))}) > 128.0) @@ -912,7 +919,7 @@ void JFJochImage::updateOverlay() { // dirty, which forces a full repaint of the viewport, so only do it when the image // really changed - not on every pan and zoom. if (!image_item_) { - image_item_ = new JFJochImageItem(qimg_buffer_); + image_item_ = new JFJochImageItem(frame_); image_item_->setZValue(0); scene()->addItem(image_item_); image_dirty_ = false; diff --git a/viewer/image_viewer/JFJochImage.h b/viewer/image_viewer/JFJochImage.h index 2da38b3d..b0b1da6d 100644 --- a/viewer/image_viewer/JFJochImage.h +++ b/viewer/image_viewer/JFJochImage.h @@ -4,6 +4,7 @@ #pragma once #include +#include #include #include @@ -55,13 +56,13 @@ struct PixelColorMap { } }; -// Draws the rendered frame straight out of JFJochImage::qimg_buffer_. A QGraphicsPixmapItem -// would mean converting the whole image into a QPixmap on every recolour, which costs one -// extra allocation and a full pass over the pixels. +// Draws a rendered frame. A QGraphicsPixmapItem would mean converting the whole image into a +// QPixmap on every recolour, which costs one extra allocation and a full pass over the pixels. +// The frame is held by shared_ptr so that several views can show the same pixels. class JFJochImageItem : public QGraphicsItem { - const QImage &img_; + std::shared_ptr img_; public: - explicit JFJochImageItem(const QImage &img) : img_(img) {} + explicit JFJochImageItem(std::shared_ptr img) : img_(std::move(img)) {} QRectF boundingRect() const override; QPainterPath opaqueArea() const override; void paint(QPainter *painter, const QStyleOptionGraphicsItem *option, QWidget *widget) override; @@ -119,7 +120,10 @@ protected: float background = 0.0; ColorScale color_scale; std::vector image_fp; - QImage qimg_buffer_; // reusable image buffer — avoids 64MB alloc per frame + // The rendered RGB frame. Reused across frames (no per-frame allocation) and held by + // shared_ptr so follower views can display the very same pixels - see JFJochFollowerImage. + // The pointer itself is stable for the lifetime of the widget; only its contents change. + std::shared_ptr frame_ = std::make_shared(); // Persistent image item — never destroyed/recreated on overlay update JFJochImageItem *image_item_ = nullptr; @@ -189,7 +193,8 @@ protected: // instead of queueing a full recolour per event. void ScheduleRenderImage(); bool render_pending_ = false; - bool image_dirty_ = false; // qimg_buffer_ changed since the item was last refreshed + bool image_dirty_ = false; // frame_ changed since the item was last refreshed + void ClearFrame(); // drop the pixels and tell followers // Set while a pan/zoom moves the scrollbars, so onScroll() does not rebuild the overlay // once per scrollbar; the gesture rebuilds it once itself. bool suppress_overlay_update_ = false; @@ -220,6 +225,8 @@ signals: void roiCalculated(ROIMessage &output); void viewportChanged(QTransform transform, QPointF center); void hoverScenePos(QPointF scenePos); + // A new frame has been rendered into Frame(). Follower views repaint on this. + void frameRendered(); private slots: void onScroll(int value); public slots: @@ -240,4 +247,6 @@ public slots: public: explicit JFJochImage(QWidget *parent = nullptr); double GetScaleFactor() const; + // The rendered frame, for views that want to show the same pixels without redoing the work + [[nodiscard]] std::shared_ptr Frame() const { return frame_; } }; \ No newline at end of file diff --git a/viewer/windows/JFJochMagnifierWindow.cpp b/viewer/windows/JFJochMagnifierWindow.cpp index 198072bf..52e6da7b 100644 --- a/viewer/windows/JFJochMagnifierWindow.cpp +++ b/viewer/windows/JFJochMagnifierWindow.cpp @@ -2,65 +2,24 @@ // SPDX-License-Identifier: GPL-3.0-only #include "JFJochMagnifierWindow.h" -#include "../image_viewer/JFJochSimpleImage.h" -#include "../SimpleImage.h" - -#include -#include +#include "../image_viewer/JFJochFollowerImage.h" JFJochMagnifierWindow::JFJochMagnifierWindow(QWidget *parent) : JFJochHelperWindow(parent) { setWindowTitle("Magnifier"); - m_image = new JFJochSimpleImage(this); - m_image->setZoom(m_magnification); + m_image = new JFJochFollowerImage(this); setCentralWidget(m_image); resize(320, 320); } -void JFJochMagnifierWindow::imageLoaded(std::shared_ptr image) { - m_pending_image = std::move(image); - // The window is closed most of the time, and rendering a close-up nobody is looking at costs - // a full conversion and recolour of the whole detector image on every frame. - if (!isVisible()) { - m_pending_dirty = true; - return; - } - ApplyPendingImage(); -} - -void JFJochMagnifierWindow::showEvent(QShowEvent *event) { - JFJochHelperWindow::showEvent(event); - if (m_pending_dirty) - ApplyPendingImage(); -} - -void JFJochMagnifierWindow::ApplyPendingImage() { - m_pending_dirty = false; - const std::shared_ptr &image = m_pending_image; - - if (!image) { - m_have_image = false; - m_image->setImage(nullptr); - return; - } - - const double scale = m_have_image ? m_image->GetScaleFactor() : m_magnification; - const QPointF center = m_have_image - ? m_image->mapToScene(m_image->viewport()->rect().center()) - : QPointF(image->Dataset().experiment.GetXPixelsNum() * 0.5, - image->Dataset().experiment.GetYPixelsNum() * 0.5); - - const auto &exp = image->Dataset().experiment; - auto si = std::make_shared(); - si->image = CompressedImage(image->Image(), exp.GetXPixelsNum(), exp.GetYPixelsNum()); - m_image->setImage(si); - m_image->applyViewport(QTransform::fromScale(scale, scale), center); - m_have_image = true; +void JFJochMagnifierWindow::setFrame(std::shared_ptr frame) { + // Just a pointer assignment plus an update() that a hidden window never acts on, so this + // needs no visibility guard: there is nothing expensive left to skip. + m_image->SetFrame(std::move(frame)); } void JFJochMagnifierWindow::centerAt(QPointF scenePos) { - if (!m_have_image || !isVisible()) + if (!isVisible()) return; - double scale = m_image->GetScaleFactor(); - m_image->applyViewport(QTransform::fromScale(scale, scale), scenePos); + m_image->CenterAt(scenePos); } diff --git a/viewer/windows/JFJochMagnifierWindow.h b/viewer/windows/JFJochMagnifierWindow.h index 5676155a..346be210 100644 --- a/viewer/windows/JFJochMagnifierWindow.h +++ b/viewer/windows/JFJochMagnifierWindow.h @@ -3,35 +3,27 @@ #pragma once +#include + #include "JFJochHelperWindow.h" #include -class JFJochSimpleImage; +class JFJochFollowerImage; +class QImage; -// ADXV-style magnifier: a small window showing a high-zoom close-up of the main -// image that follows the cursor. Fed the original image (converted to a -// SimpleImage) and re-centered on each hover position. +// ADXV-style magnifier: a small window showing a high-zoom close-up of the main image that +// follows the cursor. It displays the frame the main view has already rendered, so it does no +// conversion or colouring of its own and always agrees with the main view on colour map, +// contrast and HDR mode. class JFJochMagnifierWindow : public JFJochHelperWindow { Q_OBJECT - JFJochSimpleImage *m_image; - double m_magnification = 12.0; - bool m_have_image = false; - - // Building the close-up converts and colours the whole detector image, so it is only done - // while the window is actually up. The frame is remembered either way; holding the - // shared_ptr also keeps alive the buffer the SimpleImage points into. - std::shared_ptr m_pending_image; - bool m_pending_dirty = false; - void ApplyPendingImage(); - - void showEvent(QShowEvent *event) override; + JFJochFollowerImage *m_image; public: explicit JFJochMagnifierWindow(QWidget *parent = nullptr); - void imageLoaded(std::shared_ptr image) override; - public slots: + void setFrame(std::shared_ptr frame); void centerAt(QPointF scenePos); };