mrview: High-quality screenshot export - #3414
Merged
Merged
Conversation
The mrview Screenshot tool previously captured images only at the native resolution of the OpenGL window, leaving dense visual content prone to aliasing. This adds off-screen super-sampling at an integer ratio, opt-in multi-sample anti-aliasing, and down-sampling of the exported image, so the final resolution is the native resolution scaled by the super-sampling ratio and divided by the down-sampling ratio. Each control is exposed through a GUI element in the Screenshot tool, a command-line option, and a config file entry governing its default. Off-screen rendering is engaged only when required, the capture buffer is reused across frames in animation mode rather than reallocated, and screen-space annotations are regenerated on demand per super-sampling ratio. A progress dialog reports the cost of large renders, and transparency is preserved in the exported image, including the previously broken case where triangulated geometry drawn against a depth buffer under MSAA or super-sampling produced near-complete transparency. Prompts: 1. > In GUI command mrview, the Screenshot toolbar enables capturing images of the OpenGL window with direct export to PNG file. The resolution of those images is equivalent to the display resolution of the OpenGL window. This can include undesirable aliasing of dense visual content. The purpose of this session is to investigate potential solutions for expediting the production of high-quality images using mrview; either utilising anti-aliasing or generating super-resolution images. An ideal solution would involve computation of the high-quality image only upon generation of that file, with all other GUI interaction being performed at native resolution for responsiveness, but solutions that involve persistent rendering of the OpenGL window at higher resolution or with anti-aliasing will be considered. Solutions that render the high-quality image to a deferred buffer without interrupting the visualisation window would be preferred, but solutions that manipulate the visualisation window in order to render the image will be considered. Solutions should ideally be feasible with OpenGL 3.3, but other solutions will be considered. Solutions that involve system configuration with no modification to MRtrix3 code should also be provided in analysis results. Search online for recommended design patterns for export of high-quality OpenGL rendered images. 2. > Commence implementation of S2. Ensure implementation is compatible with future augmentation of S3. Limit to single integer super-sampling ratios, always equal for width & height. Ensure that if Screenshot tool is utilised in animation mode the buffer is re-used rather than re-allocated. For all relevant screen-space annotations, store a map from super-sampling ratio to the relevant object, and generate on demand (RAII). Add GUI element in Screenshot tool for setting super-sampling ratio; default value is 1, add config file entry for modifying default. Add command-line option for setting value. Screenshot tool should only utilise off-screen rendering if super-sampling ratio is non-unity. Avoid forcing resulting PNG to opaque if possible. Investigate whether, for large renders, a progress bar can be presented to the user without interfering with the off-screen rendering. 3. > 1. Augment the screen capture tool with opt-in multi-sample anti-aliasing. Add a GUI element for manipulating the value (a spin-box equivalent to the super-sampling ratio if any integer value is acceptable, or a combo-box if only powers of 2 are permitted and such permissible values cannot be configured for the spin-box), a corresponding command-line option, and a corresponding config file entry to set the default value to non-unity. > 2. Augment the screen capture tool with down-sampling of exported images. The actual resolution of the exported image(s) should be the native resolution, times the super-sampling ratio, divided by the down-sampling ratio. Add a GUI element for manipulating the value, a corresponding command-line option, and a corresponding config file entry. 4. > Images exported using either non-unity super-sampling or non-zero MSAA result in near-complete transparency of the image anywhere triangulated geometry has been drawn and a depth buffer utilised. The main background image appears OK. Contrast image "screenshot0010.png" (no MSAA) and "screenshot0011.png" (2x MSAA). Unclear whether failing to obey Claude's prior suggestion of requiring to flatten transparency prior to export is cause, or whether it relates to standalone depth buffer and how this may interact with how the background image is drawn in single slice mode. Generated-by: Claude Opus 4.8 <noreply@anthropic.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Added missing filesystem header to window.h and applied const-correctness fixes across projection.h and capture_buffer.cpp. Fixed narrowing conversions and explicit type comparisons to address clang-tidy violations. These changes improve code safety and maintainability while adhering to project C++ guidelines. Prompt: > Read file "clang-tidy-review-output.json". Address suggestions. Generated-by: Claude Sonnet <noreply@anthropic.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
QImage::mirrored() is deprecated in Qt 6.9 and removed in later versions. This change conditionally uses QImage::flipped(Qt::Vertical) on Qt 6.9+ while maintaining backward compatibility with Qt 5 by retaining QImage::mirrored(false, true) for earlier versions. The fix requires no minimum version bump as both APIs have long histories, ensuring Windows MSYS2 CI compilation succeeds across supported Qt versions. Prompt: > Attempt to compile this branch in the Windows MSYS2 CI Action results in the error posted above. Check for the Qt versions in which the function is flagged as deprecated, as well as the versions in which the proposed successor function is available. Rectify the code in such a way that is compatible with both Qt5 and Qt6, and does not increase the minimum Qt5 version required for compilation relative to the rest of MRtrix3. Generated-by: Claude Opus 4.8 <noreply@anthropic.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
When super-sampled screenshot capture is active, screen-space overlays must scale to match the increased render resolution. Colour bar frame borders and the focus point cross previously relied on OpenGL line-width requests, which are no longer honoured beyond a single pixel and left these overlays erroneously thin at high super-sampling ratios. Their geometry is now emitted as triangles whose thickness is computed from the super-sampling ratio, so borders and focus lines render at a consistent apparent width regardless of capture resolution. Prompts: 1. > Recent augmentations to mrview Screenshot tool enable super-resolved image capture. When super-sampling is enabled, screen-space overlays including text and colour bars must be scaled correspondingly. However each colourbar overlay has a yellow line around its border; these currently erroneously do not scale with supersampling ratio, such that the lines become erroneously thin at high supersampling ratios. 2. > 1. Change has no effect on super-sampled screenshots. User believes there is precedent for OpenGL requested line widths no longer being respected (e.g. mrview Tractography tool when displaying using lines can no longer modulate thickness). Investigate online. > 2. Any correction applied to the borders of colour bars must equivalently be applied to the lines constituting the focus point. Generated-by: Claude Opus 4.8 <noreply@anthropic.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Lestropie
commented
Jul 2, 2026
Lestropie
commented
Jul 2, 2026
Lestropie
commented
Jul 2, 2026
Lestropie
commented
Jul 2, 2026
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Robert Smith <robert.smith@florey.edu.au>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
The mrview screen capture tool's ad hoc multi-frame progress dialog is replaced with MR::ProgressBar, and its files are reorganised into a dedicated screen_capture subdirectory and namespace. Adding a cancel button required extending ProgressBar: a first attempt latched a global set_next_cancellable() flag ahead of construction, but analysis showed the flag could leak onto an unrelated dialog if a ProgressBar was suppressed or never displayed, so it was reverted. The final design instead makes ProgressBar's destructor virtual and lets GUI code define a Cancellable subclass that Dialog::ProgressBar::display() detects via dynamic_cast, keeping the cancelled state local to each instance with no concept of cancellation leaking into the base class's interface. Prompts: 1. > 1. Claude has made use of a QProgressDialog in the mrview Screenshot tool for tracking progress of a multi-frame capture. Check the MRtrix3 C++ GUI code base for precedent on this. User believes that MRtrix3 class ProgressBar is intended for use even in GUI applications; modify if correct. > 2. New features required for super-sampling have been placed in "cpp/gui/mrview/capture_buffer.*". Assess what fraction of this functionality is used exclusively by the screen capture tool. If volume of code in these files that is specific to the screen capture tool exceeds 50% of total content, do the following steps; otherwise skip to next enumerated point. Make new directory cpp/gui/mrview/tool/screen_capture/. Move cpp/gui/mrview/tool/screen_capture.* into cpp/gui/mrview/tool/screen_capture/. Move cpp/gui/mrview/capture_buffer.* into cpp/gui/mrview/tool/screen_capture/. Place that code in MRView::Tool namespace. Restore compilation. Any code within these files accessed by any MRView code outside of the screen capture tool will require refactoring; present summary of any such code to user and prompt for further instruction. > 3. Resolve recently added capability of the screen capture toolbar to generate images with MSAA against pre-existing MRtrix config file key "MSAA". Evaluate whether that config file option is functional, and whether its mechanism of operation can be reconciled against the recent changes that were made only considering the context of the screen capture tool. Questions: > Q: Window::captureGL() (core window.cpp) now depends on Tool::CaptureBuffer, nested under the screen_capture tool directory/namespace. How should this coupling be resolved? > A: Move captureGL() into the tool 2. > Despite identification of the MR::ProgressBar precedent in GUI applications, Claude has still nevertheless used custom code rather than simply instantiating an MR::ProgressBar. Claude gives some justification for this. It is however not clear whether those context-specific changes were made because of context-specific reasons, or whether these are more general issues with the use of MR::ProgressBar in GUI applications for which Claude has implemented a local rather than global fix. Describe exactly what justifies use of custom progress window handling in the screen capture tool. 3. > A cancel functionality for MR::ProgressBar does not make sense in any terminal application. It may however make sense in more than one context in GUI applications. It would also be preferable to preserve the delay before appearance behaviour in this context. Additionally, with the addition of a progress bar to the capture process, it would make sense that the stop control would be moved from the toolbar to the progress dialog window, so re-scoping the progress dialog to preserve functionality of the existing stop button does not make sense. > Investigate suitable ways to minimally refactor existing progress bar behaviour such that GUI applications can, on a per-instance basis, control whether or not a "cancel" button is included in the dialog. AskUserQuestion if multiple candidate implementations are found. The functionality would therefore reside within cpp/gui/dialog/progress.* rather than cpp/gui/mrview/tool/screen_capture/. 4. > 1. Risk of Claude solution Dialog::ProgressBar::set_next_cancellable() is that developers may locally demote the command verbosity (App::log_level) to suppress dialogs in a specific context. A piece of code that sets that flag, then fails to spawn a dialog, whether due to not exceeding the timeout to be rendered or due to being locally deliberately disabled, may risk having that flag affect whatever non-suppressed progress dialog is next triggered in the application. This should not require diligence at the point of every invocation. Report on the probability of erroneous behaviour in this regard given Claude's chosen implementation. > 2. Report on why Claude proceeded with its chosen implementation despite explicit instruction from the user to execute AskUserQuestion in the presence of multiple solution candidates. One could debate whether AskUserQuestion should time out in some instances, but in response to explicit user request it should almost certainly not. Suggest modifications to user-level Claude configuration to mitigate the chance of this occurring again. 5. 6. > 1. Is it possible for Claude to write a hook that will detect the presence of text "AskUserQuestion" in a user prompt, and temporarily override variable CLAUDE_ASK_USER_TIMEOUT_MS, to provide a deterministic override of timeout rather than a model suggestion? > 2. Revert via git prior changes wherein the custom progress dialog for the screen capture tool was removed in favour of function set_next_cancellable(). > 3. Investigate and comment on: > 3.1. Whether the desired optional availability of a cancel button in GUI progress dialogs can be achieved through class inheritance and type detection without necessitating any modification to the MR::ProgressBar class. > 3.2. A design where MR::ProgressBar has a latent member for cancel functionality availability that is inaccessible via any MR::ProgressBar constructor and always yields false but can then be overridden by a derivative class for GUI applications that opt in to the presence of a cancel button. > 3.3. The complexity involved in duplicating any current functionality of the mrview screen capture tool stop button whenever a dialog window cancel button is pressed. 7. > 3.1 is rejected on the basis that it can't be done with exactly zero modification to MR::ProgressBar. However 3.2 requires very explicit modification of MR::ProgressBar, with broadcast virtual capabilities not present in the base class. If MR::ProgressBar were to have minimal modification to make it polymorphic, then GUI apps requiring a cancel button could utilise a derivative class, and cpp/gui/dialog/progress.* could discover that through a dynamic_cast attempt. Would this not be superior encapsulation than 3.2? If so, attempt this implementation, ensuring that 3.3 is also satisfied. Generated-by: Claude Sonnet 5 <noreply@anthropic.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Lestropie
marked this pull request as ready for review
July 2, 2026 14:15
Lestropie
added a commit
that referenced
this pull request
Jul 2, 2026
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Lestropie
force-pushed
the
mrview_superres
branch
from
July 2, 2026 14:39
4421c22 to
bcae9ed
Compare
| } | ||
| } | ||
|
|
||
| QImage CaptureBuffer::read() { |
There was a problem hiding this comment.
warning: no header providing "QImage" is directly included [misc-include-cleaner]
cpp/gui/mrview/tool/screen_capture/capture_buffer.cpp:22:
+ #include <qimage.h>
bontlewatson
pushed a commit
to bontlewatson/mrtrix3
that referenced
this pull request
Jul 12, 2026
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Robert Smith <robert.smith@florey.edu.au>
bontlewatson
pushed a commit
to bontlewatson/mrtrix3
that referenced
this pull request
Jul 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Request from @ppruc (though would have saved myself a lot of time were it available in the past).
Surprisingly does not appear to be already listed as an Issue.
Adds controls to the Screenshot toolbar to render to an upscaled target buffer (supersampling), utilise MSAA functionality, and/or downsample screenshots prior to writing to disk, expediting production of high-quality unaliased image generation.
(have only performed basic testing)
Edit: This is far from compulsory, so might defer to a separate wishlist Issue)