From 057eb9f7bb30b4e090298595f01587adfa9653e0 Mon Sep 17 00:00:00 2001 From: Brendan MacLean Date: Tue, 24 Dec 2024 04:07:11 -0800 Subject: [PATCH] Skyline: Improvements to ImageComparer for challenging differences (#3298) - Added a binary diff pane for when pixels match but binary headers do not - Added AlphaColorPickerButton to control the alpha and color used for image diffs - Fixe Bitmap resolution variability with graph metafiles that was causing differences in the pHYs blocks of saved PNG files --- .../ImageComparer/AlphaColorPickerButton.cs | 211 +++++++++ .../DevTools/ImageComparer/App.config | 6 + .../DevTools/ImageComparer/GitFileHelper.cs | 2 + .../ImageComparer/ImageComparer.csproj | 3 + .../ImageComparer.sln.DotSettings | 5 + .../ImageComparerWindow.Designer.cs | 14 + .../ImageComparer/ImageComparerWindow.cs | 439 +++++++++++++----- .../Properties/Settings.Designer.cs | 24 + .../Properties/Settings.settings | 6 + .../Skyline/TestUtil/ScreenshotManager.cs | 43 +- 10 files changed, 633 insertions(+), 120 deletions(-) create mode 100644 pwiz_tools/Skyline/Executables/DevTools/ImageComparer/AlphaColorPickerButton.cs diff --git a/pwiz_tools/Skyline/Executables/DevTools/ImageComparer/AlphaColorPickerButton.cs b/pwiz_tools/Skyline/Executables/DevTools/ImageComparer/AlphaColorPickerButton.cs new file mode 100644 index 0000000000..4a307a664c --- /dev/null +++ b/pwiz_tools/Skyline/Executables/DevTools/ImageComparer/AlphaColorPickerButton.cs @@ -0,0 +1,211 @@ +/* + * Original authors: Brendan MacLean + * MacCoss Lab, Department of Genome Sciences, UW + * + * Copyright 2024 University of Washington - Seattle, WA + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +using System; +using System.Drawing; +using System.Drawing.Imaging; +using System.Windows.Forms; +using ImageComparer.Properties; + +namespace ImageComparer +{ + public class AlphaColorPickerButton : ToolStripDropDownButton + { + private Color _selectedColor; + private int _alpha; + private Timer _colorChangeTimer; + private bool _pendingColorChange; + + public event EventHandler ColorChanged; + + public Color SelectedColor + { + get => Color.FromArgb(_alpha, _selectedColor); + set + { + _selectedColor = Color.FromArgb(value.R, value.G, value.B); + _alpha = value.A; + UpdateButtonAppearance(); + TriggerColorChange(); + } + } + + public AlphaColorPickerButton() + { + _selectedColor = Settings.Default.ImageDiffColor; + _alpha = Settings.Default.ImageDiffAlpha; + + ToolTipText = @"Pick Color"; + InitializeDropDown(); + InitializeTimer(); + UpdateButtonAppearance(); + } + + private void InitializeTimer() + { + _colorChangeTimer = new Timer { Interval = 300 }; // 300ms delay + _colorChangeTimer.Tick += (s, e) => + { + _colorChangeTimer.Stop(); + if (_pendingColorChange) + { + _pendingColorChange = false; + OnColorChanged(EventArgs.Empty); + } + }; + } + + private void InitializeDropDown() + { + var predefinedColors = new[] + { + Color.Red, Color.Green, Color.Blue, Color.Yellow, + Color.Orange, Color.Purple, Color.Cyan, Color.Magenta + }; + + var panel = new Panel { Width = 8*30, Height = 120, BackColor = SystemColors.Control }; + + var colorPanel = new FlowLayoutPanel + { + Dock = DockStyle.Top, + Height = 24, + FlowDirection = FlowDirection.LeftToRight + }; + + foreach (var color in predefinedColors) + { + var colorButton = new Button + { + BackColor = color, + Width = 20, + Height = 20, + Margin = new Padding(5, 2, 5, 2), + FlatStyle = FlatStyle.Flat + }; + colorButton.Click += (s, e) => + { + _selectedColor = color; + UpdateButtonAppearance(); + OnColorChanged(EventArgs.Empty); + }; + colorPanel.Controls.Add(colorButton); + } + + var alphaLabelText = new Label + { + Text = @"Alpha:", + Dock = DockStyle.Top, + TextAlign = ContentAlignment.MiddleRight, + Width = panel.Width/2, + }; + var alphaLabelValue = new Label + { + Text = _alpha.ToString(), + Dock = DockStyle.Top, + TextAlign = ContentAlignment.MiddleRight, + Width = 25, + Margin = new Padding(0), + }; + var alphaContainer = new FlowLayoutPanel + { + Dock = DockStyle.Bottom, + Height = 20, + FlowDirection = FlowDirection.LeftToRight, + Margin = new Padding(0) + }; + alphaContainer.Controls.Add(alphaLabelText); + alphaContainer.Controls.Add(alphaLabelValue); + + var alphaTrackBar = new TrackBar + { + Minimum = 0, + Maximum = 255, + Value = _alpha, + Dock = DockStyle.Bottom + }; + alphaTrackBar.Scroll += (s, e) => + { + _alpha = alphaTrackBar.Value; + alphaLabelValue.Text = _alpha.ToString(); + UpdateButtonAppearance(); + TriggerColorChange(); + }; + + var moreColorsButton = new Button + { + Text = @"More Colors...", + Dock = DockStyle.Bottom + }; + moreColorsButton.Click += (s, e) => + { + using var dialog = new ColorDialog(); + dialog.Color = _selectedColor; + if (dialog.ShowDialog() == DialogResult.OK) + { + _selectedColor = dialog.Color; + UpdateButtonAppearance(); + OnColorChanged(EventArgs.Empty); + } + }; + + panel.Controls.Add(colorPanel); + panel.Controls.Add(alphaContainer); + panel.Controls.Add(alphaTrackBar); + panel.Controls.Add(moreColorsButton); + + var host = new ToolStripControlHost(panel) + { + AutoSize = false, + Size = panel.Size + }; + + DropDownItems.Add(host); + } + + private void UpdateButtonAppearance() + { + if (!DesignMode) + { + Image = CreateColorSwatch(SelectedColor, new Size(16, 16)); + ToolTipText = $@"Selected Color: {SelectedColor}"; + } + } + + private Image CreateColorSwatch(Color color, Size size) + { + var bitmap = new Bitmap(size.Width, size.Height, PixelFormat.Format32bppArgb); + using var g = Graphics.FromImage(bitmap); + using var brush = new SolidBrush(color); + g.FillRectangle(brush, 0, 0, size.Width, size.Height); + g.DrawRectangle(Pens.Black, 0, 0, size.Width - 1, size.Height - 1); + return bitmap; + } + + private void TriggerColorChange() + { + _pendingColorChange = true; + _colorChangeTimer.Stop(); + _colorChangeTimer.Start(); + } + + protected virtual void OnColorChanged(EventArgs e) + { + ColorChanged?.Invoke(this, e); + } + } +} diff --git a/pwiz_tools/Skyline/Executables/DevTools/ImageComparer/App.config b/pwiz_tools/Skyline/Executables/DevTools/ImageComparer/App.config index 0c408c5559..4414fc1d33 100644 --- a/pwiz_tools/Skyline/Executables/DevTools/ImageComparer/App.config +++ b/pwiz_tools/Skyline/Executables/DevTools/ImageComparer/App.config @@ -28,6 +28,12 @@ + + Red + + + 127 + \ No newline at end of file diff --git a/pwiz_tools/Skyline/Executables/DevTools/ImageComparer/GitFileHelper.cs b/pwiz_tools/Skyline/Executables/DevTools/ImageComparer/GitFileHelper.cs index b456eed5d6..a881817f35 100644 --- a/pwiz_tools/Skyline/Executables/DevTools/ImageComparer/GitFileHelper.cs +++ b/pwiz_tools/Skyline/Executables/DevTools/ImageComparer/GitFileHelper.cs @@ -52,6 +52,8 @@ public static IEnumerable GetChangedFilePaths(string directoryPath) using var reader = new StringReader(output); while (reader.ReadLine() is { } line) { + if (!line.StartsWith(" M ")) + continue; // 'git status --porcelain' format: XY path/to/file var filePath = line.Substring(3).Replace('/', Path.DirectorySeparatorChar); yield return Path.Combine(GetPathInfo(directoryPath).Root, filePath); diff --git a/pwiz_tools/Skyline/Executables/DevTools/ImageComparer/ImageComparer.csproj b/pwiz_tools/Skyline/Executables/DevTools/ImageComparer/ImageComparer.csproj index 4abf994bad..87f991fb36 100644 --- a/pwiz_tools/Skyline/Executables/DevTools/ImageComparer/ImageComparer.csproj +++ b/pwiz_tools/Skyline/Executables/DevTools/ImageComparer/ImageComparer.csproj @@ -46,6 +46,9 @@ + + Component + diff --git a/pwiz_tools/Skyline/Executables/DevTools/ImageComparer/ImageComparer.sln.DotSettings b/pwiz_tools/Skyline/Executables/DevTools/ImageComparer/ImageComparer.sln.DotSettings index ffda5b37f9..fcabaa34a1 100644 --- a/pwiz_tools/Skyline/Executables/DevTools/ImageComparer/ImageComparer.sln.DotSettings +++ b/pwiz_tools/Skyline/Executables/DevTools/ImageComparer/ImageComparer.sln.DotSettings @@ -3,4 +3,9 @@ <Policy Inspect="True" Prefix="" Suffix="" Style="aa_bb" /> <Policy Inspect="True" Prefix="" Suffix="" Style="AA_BB" /> <Policy Inspect="True" Prefix="" Suffix="" Style="AA_BB" /> + <Policy><Descriptor Staticness="Static" AccessRightKinds="Private" Description="Static readonly fields (private)"><ElementKinds><Kind Name="READONLY_FIELD" /></ElementKinds></Descriptor><Policy Inspect="True" Prefix="" Suffix="" Style="AA_BB" /></Policy> + <Policy><Descriptor Staticness="Any" AccessRightKinds="Private" Description="Constant fields (private)"><ElementKinds><Kind Name="CONSTANT_FIELD" /></ElementKinds></Descriptor><Policy Inspect="True" Prefix="" Suffix="" Style="AA_BB" /></Policy> + <Policy><Descriptor Staticness="Any" AccessRightKinds="Protected, ProtectedInternal, Internal, Public, PrivateProtected" Description="Constant fields (not private)"><ElementKinds><Kind Name="CONSTANT_FIELD" /></ElementKinds></Descriptor><Policy Inspect="True" Prefix="" Suffix="" Style="AA_BB" /></Policy> + <Policy><Descriptor Staticness="Any" AccessRightKinds="Any" Description="Enum members"><ElementKinds><Kind Name="ENUM_MEMBER" /></ElementKinds></Descriptor><Policy Inspect="True" Prefix="" Suffix="" Style="aa_bb" /></Policy> + True True \ No newline at end of file diff --git a/pwiz_tools/Skyline/Executables/DevTools/ImageComparer/ImageComparerWindow.Designer.cs b/pwiz_tools/Skyline/Executables/DevTools/ImageComparer/ImageComparerWindow.Designer.cs index 5bff2a71e1..0587cd488b 100644 --- a/pwiz_tools/Skyline/Executables/DevTools/ImageComparer/ImageComparerWindow.Designer.cs +++ b/pwiz_tools/Skyline/Executables/DevTools/ImageComparer/ImageComparerWindow.Designer.cs @@ -52,6 +52,7 @@ private void InitializeComponent() this.toolStripAutoSize = new System.Windows.Forms.ToolStripButton(); this.toolStripSeparator3 = new System.Windows.Forms.ToolStripSeparator(); this.toolStripGotoWeb = new System.Windows.Forms.ToolStripButton(); + this.toolStripPickColorButton = new ImageComparer.AlphaColorPickerButton(); ((System.ComponentModel.ISupportInitialize)(this.oldScreenshotPictureBox)).BeginInit(); ((System.ComponentModel.ISupportInitialize)(this.newScreenshotPictureBox)).BeginInit(); ((System.ComponentModel.ISupportInitialize)(this.previewSplitContainer)).BeginInit(); @@ -214,6 +215,7 @@ private void InitializeComponent() this.toolStripRevert, this.toolStripFileList, this.toolStripAutoSize, + this.toolStripPickColorButton, this.toolStripSeparator3, this.toolStripGotoWeb}); this.toolStrip.Location = new System.Drawing.Point(0, 0); @@ -306,6 +308,17 @@ private void InitializeComponent() this.toolStripGotoWeb.ToolTipText = "Goto Web (Ctrl-G)"; this.toolStripGotoWeb.Click += new System.EventHandler(this.toolStripGotoWeb_Click); // + // toolStripPickColorButton + // + this.toolStripPickColorButton.BackColor = System.Drawing.SystemColors.Control; + this.toolStripPickColorButton.DisplayStyle = System.Windows.Forms.ToolStripItemDisplayStyle.Image; + this.toolStripPickColorButton.ImageTransparentColor = System.Drawing.Color.Magenta; + this.toolStripPickColorButton.Name = "toolStripPickColorButton"; + this.toolStripPickColorButton.SelectedColor = System.Drawing.Color.FromArgb(((int)(((byte)(128)))), ((int)(((byte)(255)))), ((int)(((byte)(0)))), ((int)(((byte)(0))))); + this.toolStripPickColorButton.Size = new System.Drawing.Size(29, 22); + this.toolStripPickColorButton.ToolTipText = "Selected Color: Color [A=128, R=255, G=0, B=0] (Alpha: 128)"; + this.toolStripPickColorButton.ColorChanged += new System.EventHandler(this.toolStripPickColorButton_ColorChanged); + // // ImageComparerWindow // this.AutoScaleDimensions = new System.Drawing.SizeF(6F, 13F); @@ -362,5 +375,6 @@ private void InitializeComponent() private System.Windows.Forms.PictureBox pictureMatching; private System.Windows.Forms.ToolStripComboBox toolStripFileList; private System.Windows.Forms.ToolStripSeparator toolStripSeparator3; + private AlphaColorPickerButton toolStripPickColorButton; } } \ No newline at end of file diff --git a/pwiz_tools/Skyline/Executables/DevTools/ImageComparer/ImageComparerWindow.cs b/pwiz_tools/Skyline/Executables/DevTools/ImageComparer/ImageComparerWindow.cs index 88b3e973d5..cdb06717a0 100644 --- a/pwiz_tools/Skyline/Executables/DevTools/ImageComparer/ImageComparerWindow.cs +++ b/pwiz_tools/Skyline/Executables/DevTools/ImageComparer/ImageComparerWindow.cs @@ -20,7 +20,6 @@ using System.Collections.Generic; using System.Diagnostics; using System.Drawing; -using System.Drawing.Imaging; using System.IO; using System.Linq; using System.Net; @@ -44,9 +43,11 @@ public partial class ImageComparerWindow : Form private ScreenshotFile _fileToShow; private OldScreenshot _oldScreenshot; private OldScreenshot _newScreenshot; - private bool? _oldAndNewMatch; + private ScreenshotDiff _diff; #endregion + private RichTextBox _rtfDiff; + public ImageComparerWindow() { _oldScreenshot = new OldScreenshot(); @@ -144,7 +145,8 @@ private void FormStateChanged() if (HasBackgroundWork) { var imageSource = OldImageSource; // Get this value now - var threadUpdateScreenshots = new Thread(() => UpdateScreenshotsAsync(imageSource)); + var highlightColor = HighlightColor; + var threadUpdateScreenshots = new Thread(() => UpdateScreenshotsAsync(imageSource, highlightColor)); threadUpdateScreenshots.Start(); } } @@ -155,22 +157,57 @@ private void UpdatePreviewImages() { helpTip.SetToolTip(oldScreenshotLabel, _oldScreenshot.FileLoaded); SetPreviewSize(labelOldSize, _oldScreenshot); - SetPreviewImage(oldScreenshotPictureBox, _oldScreenshot); + SetPreviewImage(oldScreenshotPictureBox, _oldScreenshot, _diff); helpTip.SetToolTip(newScreenshotLabel, _newScreenshot.FileLoaded); SetPreviewSize(labelNewSize, _newScreenshot); SetPreviewImage(newScreenshotPictureBox, _newScreenshot); - if (!_oldAndNewMatch.HasValue) + if (_diff == null) { pictureMatching.Visible = false; labelOldSize.Left = pictureMatching.Left; + labelOldSize.ForeColor = Color.Black; } else { pictureMatching.Visible = true; labelOldSize.Left = pictureMatching.Right; - var bmpDiff = _oldAndNewMatch.Value ? Resources.Peak : Resources.NoPeak; + bool matching = !_diff.IsDiff; + var bmpDiff = matching ? Resources.Peak : Resources.NoPeak; bmpDiff.MakeTransparent(Color.White); pictureMatching.Image = bmpDiff; + if (matching) + { + labelOldSize.ForeColor = Color.Green; + } + else + { + labelOldSize.ForeColor = Color.Red; + labelOldSize.Text += _diff.DiffText; + bool imagesMatch = !_diff.SizesDiffer && !_diff.PixelsDiffer; + if (imagesMatch) + { + if (_rtfDiff == null) + { + _rtfDiff = new RichTextBox + { + Dock = DockStyle.Fill, + Font = new Font("Courier New", 10), // Use monospaced font for better alignment + ReadOnly = true, + WordWrap = false, + TabIndex = oldScreenshotPictureBox.TabIndex + }; + var controlList = oldScreenshotPictureBox.Parent.Controls; + controlList.Add(_rtfDiff); + controlList.SetChildIndex(_rtfDiff, controlList.IndexOf(oldScreenshotPictureBox)); + } + + _diff.ShowBinaryDiff(_rtfDiff); + } + + oldScreenshotPictureBox.Visible = !imagesMatch; + if (_rtfDiff != null) + _rtfDiff.Visible = imagesMatch; + } } } } @@ -184,7 +221,7 @@ private static void SetPreviewSize(Label labelSize, ScreenshotInfo screenshot) { lock (image) { - labelSize.Text = $@"{image.Width} x {image.Height}px"; + labelSize.Text = $@"{screenshot.ImageSize.Width} x {screenshot.ImageSize.Height}px"; } } } @@ -193,11 +230,12 @@ private void UpdateToolStrip() { toolStripPrevious.Enabled = toolStripFileList.SelectedIndex > 0; toolStripNext.Enabled = toolStripFileList.SelectedIndex < toolStripFileList.Items.Count - 1; - toolStripRevert.Enabled = !(_oldAndNewMatch ?? true); + // TODO: Pre-calculate diff to know whether revert is appropriate + // toolStripRevert.Enabled = (_pixelDiffCount ?? 0) != 0; toolStripGotoWeb.Enabled = _fileToShow != null; } - private void UpdateScreenshotsAsync(ImageSource oldImageSource) + private void UpdateScreenshotsAsync(ImageSource oldImageSource, Color highlightColor) { ScreenshotFile fileToShow; ScreenshotInfo oldScreenshot; @@ -206,6 +244,8 @@ private void UpdateScreenshotsAsync(ImageSource oldImageSource) lock (_lock) { fileToShow = _fileToShow; + if (!Equals(fileToShow, _fileToShow)) + return; oldFileLoaded = _oldScreenshot.FileLoaded; oldScreenshot = new ScreenshotInfo(_oldScreenshot); } @@ -222,11 +262,12 @@ private void UpdateScreenshotsAsync(ImageSource oldImageSource) if (!Equals(fileToShow, _fileToShow)) return; _oldScreenshot = new OldScreenshot(oldScreenshot, oldFileLoaded); + _diff = null; } ScreenshotInfo newScreenshot; string newFileLoaded; - bool? oldAndNewMatch = null; + ScreenshotDiff diff = null; lock (_lock) { @@ -241,29 +282,9 @@ private void UpdateScreenshotsAsync(ImageSource oldImageSource) newScreenshot = LoadScreenshot(fileToShow, ImageSource.disk); } - if (!newScreenshot.IsPlaceholder && !oldScreenshot.IsPlaceholder) + if (!newScreenshot.IsPlaceholder || !oldScreenshot.IsPlaceholder) { - Bitmap diffImage; - bool imageChanged; - lock (oldScreenshot.Image) - { - lock (newScreenshot.Image) - { - diffImage = DiffImages(oldScreenshot.Image, newScreenshot.Image); - imageChanged = !ReferenceEquals(diffImage, _oldScreenshot.Image); - } - } - - if (imageChanged || oldScreenshot.ImageSize != newScreenshot.ImageSize) - { - oldAndNewMatch = false; - } - else - { - oldAndNewMatch = true; - } - - oldScreenshot = new ScreenshotInfo(diffImage); + diff = DiffScreenshots(oldScreenshot, newScreenshot, highlightColor); } lock (_lock) @@ -272,70 +293,26 @@ private void UpdateScreenshotsAsync(ImageSource oldImageSource) return; _newScreenshot = new OldScreenshot(newScreenshot, newFileDescription); - _oldScreenshot = new OldScreenshot(oldScreenshot, oldFileDescription); - _oldAndNewMatch = oldAndNewMatch; + _diff = diff; } FormStateChangedBackground(); } - private Bitmap DiffImages(Bitmap bmpOld, Bitmap bmpNew) + private ScreenshotDiff DiffScreenshots(ScreenshotInfo oldScreenshot, ScreenshotInfo newScreenshot, Color highlightColor) { - if (bmpNew == null || bmpOld == null || bmpNew.Size != bmpOld.Size) - return bmpOld; - try { - return HighlightDifferences(bmpOld, bmpNew, Color.Red); + return new ScreenshotDiff(oldScreenshot, newScreenshot, highlightColor); } catch (Exception e) { - this.BeginInvoke((Action)(() => ShowMessageWithException( + BeginInvoke((Action)(() => ShowMessageWithException( "Failed to diff bitmaps.", e))); - return bmpOld; + return null; } } - // ReSharper disable once UnusedParameter.Local - private void ShowMessageWithException(string message, Exception e) - { - // TODO: Make exception stack trace available somehow - MessageBox.Show(this, message); - } - - public static Bitmap HighlightDifferences(Bitmap bmpOld, Bitmap bmpNew, Color highlightColor, int alpha = 128) - { - var result = new Bitmap(bmpOld.Width, bmpOld.Height); - - bool diffFound = false; - for (int y = 0; y < bmpOld.Height; y++) - { - for (int x = 0; x < bmpOld.Width; x++) - { - var pixel1 = bmpOld.GetPixel(x, y); - var pixel2 = bmpNew.GetPixel(x, y); - - if (pixel1 != pixel2) - { - var blendedColor = Color.FromArgb( - alpha, - highlightColor.R * alpha / 255 + pixel1.R * (255 - alpha) / 255, - highlightColor.G * alpha / 255 + pixel1.G * (255 - alpha) / 255, - highlightColor.B * alpha / 255 + pixel1.B * (255 - alpha) / 255 - ); - result.SetPixel(x, y, blendedColor); - diffFound = true; - } - else - { - result.SetPixel(x, y, pixel1); - } - } - } - - return diffFound ? result : bmpOld; - } - private ScreenshotInfo LoadScreenshot(ScreenshotFile file, ImageSource source) { try @@ -360,7 +337,7 @@ private ScreenshotInfo LoadScreenshot(ScreenshotFile file, ImageSource source) break; } var ms = new MemoryStream(imageBytes); - return new ScreenshotInfo(new Bitmap(ms)); + return new ScreenshotInfo(ms); } catch (Exception e) { @@ -368,37 +345,27 @@ private ScreenshotInfo LoadScreenshot(ScreenshotFile file, ImageSource source) string.Format("Failed to load a bitmap from {0}.", file.GetDescription(source)), e))); var failureBmp = Resources.DiskFailure; failureBmp.MakeTransparent(Color.White); - return new ScreenshotInfo(failureBmp, true); + return new ScreenshotInfo(failureBmp); } } - private void SetPreviewImage(PictureBox previewBox, ScreenshotInfo screenshot) + private void SetPreviewImage(PictureBox previewBox, ScreenshotInfo screenshot, ScreenshotDiff diff = null) { - var newImage = screenshot.Image; - if (newImage != null && !screenshot.IsPlaceholder) + var baseImage = diff?.HighlightedImage ?? screenshot.Image; + var newImage = baseImage; + if (baseImage != null && !screenshot.IsPlaceholder) { - lock (newImage) + lock (baseImage) { var containerSize = !toolStripAutoSize.Checked ? previewBox.Size : Size.Empty; var screenshotSize = CalcBitmapSize(screenshot, containerSize); // Always make a copy to avoid having PictureBox lock the bitmap // which can cause issues with future image diffs - newImage = new Bitmap(screenshot.Image, screenshotSize); + newImage = new Bitmap(baseImage, screenshotSize); } } previewBox.Image = newImage; - if (newImage != null && Equals(newImage.RawFormat, ImageFormat.Gif)) - { - // Unfortunately the animated progress GIF has a white background - // and it cannot be made transparent without removing the animation - previewBox.BackColor = Color.White; - } - else - { - // The oldScreenshotPictureBox never gets a white background - previewBox.BackColor = oldScreenshotPictureBox.BackColor; - } } private Size CalcBitmapSize(ScreenshotInfo screenshot, Size containerSize) @@ -668,6 +635,20 @@ private ImageSource OldImageSource } } + private Color HighlightColor + { + get => Color.FromArgb(Settings.Default.ImageDiffAlpha, + Settings.Default.ImageDiffColor.R, + Settings.Default.ImageDiffColor.G, + Settings.Default.ImageDiffColor.B); + + set + { + Settings.Default.ImageDiffAlpha = value.A; + Settings.Default.ImageDiffColor = Color.FromArgb(value.R, value.G, value.B); + } + } + private void NextOldImageSource() { OldImageSource = OLD_IMAGE_SOURCES[(Settings.Default.OldImageSource + 1) % OLD_IMAGE_SOURCES.Length]; @@ -721,16 +702,16 @@ private void toolStripNext_Click(object sender, EventArgs e) Next(); } + private void toolStripFileList_SelectedIndexChanged(object sender, EventArgs e) + { + SetPath(toolStripFileList.SelectedItem as ScreenshotFile); + } + private void toolStripRevert_Click(object sender, EventArgs e) { Revert(); } - // private void toolStripRefresh_Click(object sender, EventArgs e) - // { - // RefreshScreenshots(); - // } - private void toolStripGotoWeb_Click(object sender, EventArgs e) { GotoLink(); @@ -749,6 +730,13 @@ private void buttonImageSource_Click(object sender, EventArgs e) NextOldImageSource(); } + private void toolStripPickColorButton_ColorChanged(object sender, EventArgs e) + { + HighlightColor = toolStripPickColorButton.SelectedColor; + if (oldScreenshotPictureBox.Visible) + UpdateScreenshotsAsync(OldImageSource, HighlightColor); // On foreground thread because the screenshots are already loaded - just updating the diff + } + private void ScreenshotPreviewForm_KeyDown(object sender, KeyEventArgs e) { switch (e.KeyCode) @@ -772,6 +760,7 @@ private void ScreenshotPreviewForm_KeyDown(object sender, KeyEventArgs e) e.Handled = true; break; case Keys.Down: + case Keys.Right: if (e.Control) { Next(); @@ -779,6 +768,7 @@ private void ScreenshotPreviewForm_KeyDown(object sender, KeyEventArgs e) } break; case Keys.Up: + case Keys.Left: if (e.Control) { Previous(); @@ -876,6 +866,13 @@ public static void ForceOnScreen(Form form) form.Location = location; } + // ReSharper disable once UnusedParameter.Local + private void ShowMessageWithException(string message, Exception e) + { + // TODO: Make exception stack trace available somehow + MessageBox.Show(this, message); + } + private static Screen GetScreen(int x, int y) { return Screen.FromPoint(new Point(x, y)); @@ -934,32 +931,39 @@ public string GetDescription(ImageSource source) private class ScreenshotInfo { - public ScreenshotInfo(Bitmap image = null, bool isPlaceholder = false) + protected ScreenshotInfo() + { + } + + public ScreenshotInfo(Bitmap image) { Image = image; ImageSize = image?.Size ?? Size.Empty; - IsPlaceholder = isPlaceholder; + } + + public ScreenshotInfo(MemoryStream ms) + : this(new Bitmap(ms)) + { + Memory = ms; } public ScreenshotInfo(ScreenshotInfo info) { Image = info.Image; + Memory = info.Memory; ImageSize = info.ImageSize; - IsPlaceholder = info.IsPlaceholder; } - public Bitmap Image { get; private set; } - public Size ImageSize { get; private set; } - public bool IsPlaceholder { get; private set; } + public Bitmap Image { get; } + public MemoryStream Memory { get; } + public Size ImageSize { get; } + public bool IsPlaceholder => Memory == null; } private class OldScreenshot : ScreenshotInfo { - public OldScreenshot(Bitmap image = null, bool isPlaceholder = false, string fileLoaded = null, ImageSource source = ImageSource.disk) - : base(image, isPlaceholder) + public OldScreenshot() { - FileLoaded = fileLoaded; - Source = source; } public OldScreenshot(ScreenshotInfo info, string fileLoaded, ImageSource source = ImageSource.disk) @@ -980,9 +984,212 @@ public bool IsCurrent(ScreenshotFile screenshot, ImageSource currentSource) } } - private void toolStripFileList_SelectedIndexChanged(object sender, EventArgs e) + private class ScreenshotDiff { - SetPath(toolStripFileList.SelectedItem as ScreenshotFile); + private readonly Size _sizeOld; + private readonly byte[] _memoryOld; + private readonly Size _sizeNew; + private readonly byte[] _memoryNew; + + public ScreenshotDiff(ScreenshotInfo oldScreenshot, ScreenshotInfo newScreenshot, Color highlightColor) + { + _sizeOld = oldScreenshot.ImageSize; + _memoryOld = oldScreenshot.Memory?.ToArray(); + _sizeNew = newScreenshot.ImageSize; + _memoryNew = newScreenshot.Memory?.ToArray(); + + if (!SizesDiffer) + { + lock (oldScreenshot.Image) + { + lock (newScreenshot.Image) + { + CalcHighlightImage(oldScreenshot.Image, newScreenshot.Image, highlightColor); + } + } + } + } + + public bool IsDiff => SizesDiffer || PixelsDiffer || BytesDiffer; + public bool SizesDiffer => !Equals(_sizeOld, _sizeNew); + public bool PixelsDiffer => PixelCount != 0; + public bool BytesDiffer => _memoryOld.Length != _memoryNew.Length || !_memoryOld.SequenceEqual(_memoryNew); + public Bitmap HighlightedImage { get; private set; } + public int PixelCount { get; private set; } + + public string DiffText + { + get + { + if (SizesDiffer) + return $@" ({_sizeNew.Width - _sizeOld.Width} x {_sizeNew.Height - _sizeOld.Height}px)"; + if (PixelsDiffer) + return $@" ({PixelCount} pixels)"; + if (BytesDiffer) + { + if (_memoryOld.Length != _memoryNew.Length) + return $" ({_memoryNew.Length - _memoryOld.Length} bytes)"; + int startIndex = 0, diffCount = 0; + for (int i = 0; i < _memoryOld.Length; i++) + { + if (_memoryOld[i] != _memoryNew[i]) + { + if (diffCount == 0) + startIndex = i; + diffCount++; + } + } + + return $@" (at {startIndex}, diff {diffCount} bytes)"; + } + return string.Empty; + } + } + + private void CalcHighlightImage(Bitmap bmpOld, Bitmap bmpNew, Color highlightColor) + { + var result = new Bitmap(bmpOld.Width, bmpOld.Height); + var alpha = highlightColor.A; + + PixelCount = 0; + for (int y = 0; y < bmpOld.Height; y++) + { + for (int x = 0; x < bmpOld.Width; x++) + { + var pixel1 = bmpOld.GetPixel(x, y); + var pixel2 = bmpNew.GetPixel(x, y); + + if (pixel1 != pixel2) + { + var blendedColor = Color.FromArgb( + alpha, + highlightColor.R * alpha / 255 + pixel1.R * (255 - alpha) / 255, + highlightColor.G * alpha / 255 + pixel1.G * (255 - alpha) / 255, + highlightColor.B * alpha / 255 + pixel1.B * (255 - alpha) / 255 + ); + result.SetPixel(x, y, blendedColor); + PixelCount++; + } + else + { + result.SetPixel(x, y, pixel1); + } + } + } + + HighlightedImage = PixelCount > 0 ? result : bmpOld; + } + + public void ShowBinaryDiff(RichTextBox richTextBox) + { + ShowDiff(richTextBox, _memoryOld, _memoryNew); + + } + private static void ShowDiff(RichTextBox richTextBox, byte[] array1, byte[] array2) + { + richTextBox.Clear(); + + // Determine the longer array length + int maxLength = Math.Max(array1.Length, array2.Length); + int linesShown = 0; + for (int i = 0; i < maxLength; i += 16) + { + if (ArraysMatch(i, 16, array1, array2)) + { + continue; + } + + ShowLineDiff(richTextBox, i, 16, array1, array2, Color.Red, "<<"); + ShowLineDiff(richTextBox, i, 16, array2, array1, Color.Green, ">>"); + + if (++linesShown >= 4) + { + // Stop after 4 lines + AppendText(richTextBox, "...", Color.Black); + break; + } + } + } + + private static bool ArraysMatch(int startIndex, int len, byte[] array1, byte[] array2) + { + for (int i = startIndex; i < startIndex + len; i++) + { + if (i >= array1.Length && i >= array2.Length) + return true; + if (i >= array1.Length || i >= array2.Length) + return false; + if (array1[i] != array2[i]) + return false; + } + return true; + } + + private static void ShowLineDiff(RichTextBox richTextBox, int startIndex, int lineLen, byte[] arrayShow, byte[] arrayCompare, Color color, string prefix) + { + AppendText(richTextBox, $"{prefix} {startIndex:X8} ", Color.Black); // address + + for (int n = 0; n < lineLen; n++) + { + if (n == lineLen/2) + AppendText(richTextBox, " ", Color.Black); + + int i = startIndex + n; + if (i < arrayShow.Length && i < arrayCompare.Length && arrayShow[i] == arrayCompare[i]) + { + // Bytes are the same, display them in black + AppendText(richTextBox, $"{arrayShow[i]:X2} ", Color.Black); + } + else if (i < arrayShow.Length) + { + // Bytes differ + AppendText(richTextBox, $"{arrayShow[i]:X2} ", color); + } + else + { + // Bytes missing + AppendText(richTextBox, "-- ", color); + } + } + AppendText(richTextBox, " ", Color.Black); + for (int n = 0; n < lineLen; n++) + { + int i = startIndex + n; + if (i < arrayShow.Length && i < arrayCompare.Length && arrayShow[i] == arrayCompare[i]) + { + // Bytes are the same, display them in black + AppendText(richTextBox, GetChar(arrayShow[i]), Color.Black); + } + else if (i < arrayShow.Length) + { + // Bytes differ + AppendText(richTextBox, GetChar(arrayShow[i]), color); + } + else + { + // Bytes missing + AppendText(richTextBox, "^", color); + } + } + AppendText(richTextBox, Environment.NewLine, Color.Black); + } + + /// + /// If a byte is printable ASCII the character as text is returned otherwise a period. + /// + private static string GetChar(byte b) + { + return (b >= 0x20 && b <= 0x7E ? (char)b : '.').ToString(); + } + + private static void AppendText(RichTextBox richTextBox, string text, Color color) + { + richTextBox.SelectionStart = richTextBox.TextLength; + richTextBox.SelectionLength = 0; + richTextBox.SelectionColor = color; + richTextBox.AppendText(text); + richTextBox.SelectionColor = richTextBox.ForeColor; + } } } } diff --git a/pwiz_tools/Skyline/Executables/DevTools/ImageComparer/Properties/Settings.Designer.cs b/pwiz_tools/Skyline/Executables/DevTools/ImageComparer/Properties/Settings.Designer.cs index 8fdd2c08a2..eb73990702 100644 --- a/pwiz_tools/Skyline/Executables/DevTools/ImageComparer/Properties/Settings.Designer.cs +++ b/pwiz_tools/Skyline/Executables/DevTools/ImageComparer/Properties/Settings.Designer.cs @@ -94,5 +94,29 @@ public string LastOpenFolder { this["LastOpenFolder"] = value; } } + + [global::System.Configuration.UserScopedSettingAttribute()] + [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] + [global::System.Configuration.DefaultSettingValueAttribute("Red")] + public global::System.Drawing.Color ImageDiffColor { + get { + return ((global::System.Drawing.Color)(this["ImageDiffColor"])); + } + set { + this["ImageDiffColor"] = value; + } + } + + [global::System.Configuration.UserScopedSettingAttribute()] + [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] + [global::System.Configuration.DefaultSettingValueAttribute("127")] + public int ImageDiffAlpha { + get { + return ((int)(this["ImageDiffAlpha"])); + } + set { + this["ImageDiffAlpha"] = value; + } + } } } diff --git a/pwiz_tools/Skyline/Executables/DevTools/ImageComparer/Properties/Settings.settings b/pwiz_tools/Skyline/Executables/DevTools/ImageComparer/Properties/Settings.settings index 9377e3d52a..d078793d16 100644 --- a/pwiz_tools/Skyline/Executables/DevTools/ImageComparer/Properties/Settings.settings +++ b/pwiz_tools/Skyline/Executables/DevTools/ImageComparer/Properties/Settings.settings @@ -20,5 +20,11 @@ + + Red + + + 127 + \ No newline at end of file diff --git a/pwiz_tools/Skyline/TestUtil/ScreenshotManager.cs b/pwiz_tools/Skyline/TestUtil/ScreenshotManager.cs index 47fd2467e6..f32f28ab30 100644 --- a/pwiz_tools/Skyline/TestUtil/ScreenshotManager.cs +++ b/pwiz_tools/Skyline/TestUtil/ScreenshotManager.cs @@ -1,4 +1,23 @@ -using System; +/* + * Original author: Rita Chupalov , + * MacCoss Lab, Department of Genome Sciences, UW + * + * Copyright 2020 University of Washington - Seattle, WA + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +using System; using System.Drawing; using System.Drawing.Imaging; using System.Globalization; @@ -222,11 +241,27 @@ public override Bitmap Take() var emf = _zedGraphControl.MasterPane.GetMetafile(); var bmp = new Bitmap(emf.Width, emf.Height); bmp.SetResolution(emf.HorizontalResolution, emf.VerticalResolution); - using var g = Graphics.FromImage(bmp); - g.DrawImage(emf, 0, 0); + using (var g = Graphics.FromImage(bmp)) + { + g.DrawImage(emf, 0, 0); + } + // Need to use a consistent resolution or PNGs will differ depending on the monitor they were rendered on + // You can get PNGs that are pixel for pixel identical but the PNGs will differ in the pHYs blocks + // Original resolution on ASUS 28" monitors - a modern laptop had much higher DPI + // Hex values taken from binary editor for original files + bmp.SetResolution(DpmToDpi(0x0C13), DpmToDpi(0x0C5F)); return bmp; } + /// + /// Converts dots per meter (DPM) found in pHYs blocks in PNG files to DPI used in Bitmap resolution. + /// + /// A dots per meter integer + /// A dots per inch (DPM/39.37) value + private float DpmToDpi(int dpm) + { + return (float)(dpm / 39.37); + } } public ScreenshotManager([NotNull] SkylineWindow skylineWindow, string tutorialPath) @@ -451,7 +486,7 @@ private void SaveToFile(string filePath, Bitmap bmp) if (dirPath != null && !Directory.Exists(dirPath)) Directory.CreateDirectory(dirPath); - bmp.Save(filePath); + bmp.Save(filePath, ImageFormat.Png); } } }