diff --git a/pwiz_tools/Shared/Common/DataBinding/Controls/Editor/FilterTab.resx b/pwiz_tools/Shared/Common/DataBinding/Controls/Editor/FilterTab.resx index 2e0372ad8c..5fed4860f8 100644 --- a/pwiz_tools/Shared/Common/DataBinding/Controls/Editor/FilterTab.resx +++ b/pwiz_tools/Shared/Common/DataBinding/Controls/Editor/FilterTab.resx @@ -139,7 +139,7 @@ 0 - 314, 307 + 313, 307 0 @@ -156,6 +156,12 @@ 0 + + True + + + GrowAndShrink + True @@ -190,10 +196,10 @@ Right - 314, 0 + 313, 0 - 56, 307 + 57, 307 1 @@ -247,7 +253,7 @@ 0, 0 - 475, 307 + 483, 307 0 @@ -256,7 +262,7 @@ dataGridViewFilter - System.Windows.Forms.DataGridView, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 + pwiz.Common.Controls.CommonDataGridView, pwiz.Common, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null splitContainerFilter.Panel2 @@ -274,16 +280,16 @@ Magenta - 29, 20 + 21, 20 Delete - 475, 0 + 483, 0 - 32, 307 + 24, 307 1 diff --git a/pwiz_tools/Skyline/Controls/Databinding/PivotReplicateAndIsotopeLabelWidget.ja.resx b/pwiz_tools/Skyline/Controls/Databinding/PivotReplicateAndIsotopeLabelWidget.ja.resx index 82cf946456..0e32c2461b 100644 --- a/pwiz_tools/Skyline/Controls/Databinding/PivotReplicateAndIsotopeLabelWidget.ja.resx +++ b/pwiz_tools/Skyline/Controls/Databinding/PivotReplicateAndIsotopeLabelWidget.ja.resx @@ -127,7 +127,7 @@ - 138, 3 + 157, 3 117, 17 @@ -186,8 +186,14 @@ 6, 13 + + True + + + GrowAndShrink + - 259, 22 + 277, 23 PivotReplicateAndIsotopeLabelWidget diff --git a/pwiz_tools/Skyline/Controls/Databinding/PivotReplicateAndIsotopeLabelWidget.resx b/pwiz_tools/Skyline/Controls/Databinding/PivotReplicateAndIsotopeLabelWidget.resx index a313ef93a8..583f272c8e 100644 --- a/pwiz_tools/Skyline/Controls/Databinding/PivotReplicateAndIsotopeLabelWidget.resx +++ b/pwiz_tools/Skyline/Controls/Databinding/PivotReplicateAndIsotopeLabelWidget.resx @@ -127,7 +127,7 @@ - 138, 3 + 157, 3 117, 17 @@ -186,8 +186,14 @@ 6, 13 + + True + + + GrowAndShrink + - 259, 22 + 277, 23 PivotReplicateAndIsotopeLabelWidget diff --git a/pwiz_tools/Skyline/Controls/Databinding/PivotReplicateAndIsotopeLabelWidget.zh-CHS.resx b/pwiz_tools/Skyline/Controls/Databinding/PivotReplicateAndIsotopeLabelWidget.zh-CHS.resx index ff1b7eac69..e39f301db0 100644 --- a/pwiz_tools/Skyline/Controls/Databinding/PivotReplicateAndIsotopeLabelWidget.zh-CHS.resx +++ b/pwiz_tools/Skyline/Controls/Databinding/PivotReplicateAndIsotopeLabelWidget.zh-CHS.resx @@ -127,7 +127,7 @@ - 138, 3 + 157, 3 117, 17 @@ -186,8 +186,14 @@ 6, 13 + + True + + + GrowAndShrink + - 259, 22 + 277, 23 PivotReplicateAndIsotopeLabelWidget diff --git a/pwiz_tools/Skyline/Controls/GroupComparison/VolcanoPlotFormattingDlg.cs b/pwiz_tools/Skyline/Controls/GroupComparison/VolcanoPlotFormattingDlg.cs index 36d0870f6b..4b42c79011 100644 --- a/pwiz_tools/Skyline/Controls/GroupComparison/VolcanoPlotFormattingDlg.cs +++ b/pwiz_tools/Skyline/Controls/GroupComparison/VolcanoPlotFormattingDlg.cs @@ -390,7 +390,52 @@ private T CreateDataGridViewColumn(bool readOnly, string dataPropertyName, st return column; } - public BindingList GetCurrentBindingList() + public MatchRgbHexColor AddRow(MatchRgbHexColor match = null) + { + if (IsLastRowEmpty) + { + // Testing for screenshots needs to be able to activate + // the form which puts to focus on the grid and adds an + // empty row to the binding list. This was causing tests + // to fail when they tried to change the binding list + // with a dirty row. Silently removing the row below fixes + // the issue. + _bindingList.RaiseListChangedEvents = false; + try + { + regexColorRowGrid1.DataGridView.CancelEdit(); + _bindingList.RemoveAt(_bindingList.Count - 1); + } + finally + { + _bindingList.RaiseListChangedEvents = true; + } + } + + if (match == null) + return _bindingList.AddNew(); + + _bindingList.Add(match); + return match; + } + + private bool IsLastRowEmpty => Equals(_bindingList.LastOrDefault(), MatchRgbHexColor.EMPTY); + + public PointSymbol GetRowPointSymbol(int rowIndex) + { + return _bindingList[rowIndex].PointSymbol; + } + + public void SetRowPointSymbol(int rowIndex, PointSymbol pointSymbol) + { + _bindingList[rowIndex].PointSymbol = pointSymbol; + } + + /// + /// Used by the but not for general use. + /// Extend the testing interface if you need more. + /// + BindingList ColorGrid.IColorGridOwner.GetCurrentBindingList() { return _bindingList; } diff --git a/pwiz_tools/Skyline/Model/GroupComparison/MatchRgbHexColor.cs b/pwiz_tools/Skyline/Model/GroupComparison/MatchRgbHexColor.cs index 31d087ab32..3768c2ede3 100644 --- a/pwiz_tools/Skyline/Model/GroupComparison/MatchRgbHexColor.cs +++ b/pwiz_tools/Skyline/Model/GroupComparison/MatchRgbHexColor.cs @@ -50,6 +50,8 @@ public enum PointSymbol [XmlRoot(XML_ROOT)] public class MatchRgbHexColor : RgbHexColor, ICloneable { + public static readonly MatchRgbHexColor EMPTY = new MatchRgbHexColor(); + public const string XML_ROOT = "format_detail"; private string _expression; private bool _labeled; diff --git a/pwiz_tools/Skyline/SkylineTester/TabTutorials.cs b/pwiz_tools/Skyline/SkylineTester/TabTutorials.cs index 737751f5dc..1afec00d27 100644 --- a/pwiz_tools/Skyline/SkylineTester/TabTutorials.cs +++ b/pwiz_tools/Skyline/SkylineTester/TabTutorials.cs @@ -18,7 +18,6 @@ */ using System; using System.Collections.Generic; -using System.IO; using System.Text; using System.Windows.Forms; @@ -66,10 +65,8 @@ public override bool Run() pauseSeconds = 0; args.Append(" pause=").Append(pauseSeconds); } - args.Append(" screenshotlist=\""); - args.Append(Path.Combine(MainWindow.RootDir, "ScreenShotForms.txt")); - args.Append("\" test="); - args.Append(String.Join(",", testList)); + args.Append(" test="); + args.Append(string.Join(",", testList)); MainWindow.AddTestRunner(args.ToString()); MainWindow.RunCommands(); diff --git a/pwiz_tools/Skyline/TestFunctional/PeakAreaRelativeAbundanceGraphTest.cs b/pwiz_tools/Skyline/TestFunctional/PeakAreaRelativeAbundanceGraphTest.cs index a71546e043..e03b46c8a0 100644 --- a/pwiz_tools/Skyline/TestFunctional/PeakAreaRelativeAbundanceGraphTest.cs +++ b/pwiz_tools/Skyline/TestFunctional/PeakAreaRelativeAbundanceGraphTest.cs @@ -90,12 +90,12 @@ private void TestFormattingDialog() Assert.AreEqual(Skyline.Controls.GroupComparison.GroupComparisonResources .VolcanoPlotFormattingDlg_VolcanoPlotFormattingDlg_Protein_Expression_Formatting, formattingDlg.Text); - var row = formattingDlg.GetCurrentBindingList().AddNew(); + var row = formattingDlg.AddRow(); Assert.IsNotNull(row); row.Expression = new MatchExpression("QE", new[] { MatchOption.PeptideSequence }).ToString(); row.PointSymbol = PointSymbol.Diamond; row.Color = Color.FromArgb(Color.Indigo.ToArgb()); - row = formattingDlg.GetCurrentBindingList().AddNew(); + row = formattingDlg.AddRow(); Assert.IsNotNull(row); row.Expression = new MatchExpression("GQ", new[] { MatchOption.PeptideSequence }).ToString(); row.PointSymbol = PointSymbol.Triangle; @@ -158,11 +158,11 @@ private void TestFormattingDialog() formattingDlg = ShowDialog(pane.ShowFormattingDialog); RunUI(() => { - Assert.AreEqual(PointSymbol.Diamond, formattingDlg.GetCurrentBindingList()[0].PointSymbol); - formattingDlg.GetCurrentBindingList()[0].PointSymbol = PointSymbol.Plus; + Assert.AreEqual(PointSymbol.Diamond, formattingDlg.GetRowPointSymbol(0)); + formattingDlg.SetRowPointSymbol(0, PointSymbol.Plus); }); WaitForGraphs(); - RunUI(() => formattingDlg.GetCurrentBindingList()[0].PointSymbol =PointSymbol.Diamond); + RunUI(() => formattingDlg.SetRowPointSymbol(0, PointSymbol.Diamond)); WaitForGraphs(); OkDialog(formattingDlg, formattingDlg.OkDialog); } diff --git a/pwiz_tools/Skyline/TestFunctional/VolcanoPlotFormattingTest.cs b/pwiz_tools/Skyline/TestFunctional/VolcanoPlotFormattingTest.cs index 76d2ddebb8..265e02f32d 100644 --- a/pwiz_tools/Skyline/TestFunctional/VolcanoPlotFormattingTest.cs +++ b/pwiz_tools/Skyline/TestFunctional/VolcanoPlotFormattingTest.cs @@ -314,13 +314,13 @@ private void VerifyMatchExpressions(FoldChangeVolcanoPlot volcanoPlot, MatchExpr var createExprDlg = ShowDialog(() => { - var bindingList = formattingDlg.GetCurrentBindingList(); - Assert.AreEqual(initialRowCount + index, bindingList.Count); + int rows = formattingDlg.ResultList.Count; + Assert.AreEqual(initialRowCount + index, rows); - bindingList.Add(new MatchRgbHexColor(string.Empty, exprInfo.ExpectedPointsInfo.Labeled, + formattingDlg.AddRow(new MatchRgbHexColor(string.Empty, exprInfo.ExpectedPointsInfo.Labeled, exprInfo.ExpectedPointsInfo.Color, exprInfo.ExpectedPointsInfo.PointSymbol, exprInfo.ExpectedPointsInfo.PointSize)); - formattingDlg.ClickCreateExpression(bindingList.Count - 1); + formattingDlg.ClickCreateExpression(rows); }); RunUI(() => @@ -456,11 +456,10 @@ private void TestMatchExpressionListDlg(FoldChangeVolcanoPlot volcanoPlot) var formattingDlg = ShowDialog(volcanoPlot.ShowFormattingDialog); var createExprDlg = ShowDialog(() => { - var bindingList = formattingDlg.GetCurrentBindingList(); - bindingList.Add(new MatchRgbHexColor(string.Empty, exprInfo.ExpectedPointsInfo.Labeled, + formattingDlg.AddRow(new MatchRgbHexColor(string.Empty, exprInfo.ExpectedPointsInfo.Labeled, exprInfo.ExpectedPointsInfo.Color, exprInfo.ExpectedPointsInfo.PointSymbol, exprInfo.ExpectedPointsInfo.PointSize)); - formattingDlg.ClickCreateExpression(bindingList.Count - 1); + formattingDlg.ClickCreateExpression(formattingDlg.ResultList.Count - 1); }); var matchExprListDlg = ShowDialog(createExprDlg.ClickEnterList); RunUI(() => diff --git a/pwiz_tools/Skyline/TestPerf/DiaSwathTutorialTest.cs b/pwiz_tools/Skyline/TestPerf/DiaSwathTutorialTest.cs index 6eda86d9bf..e2e4ad2185 100644 --- a/pwiz_tools/Skyline/TestPerf/DiaSwathTutorialTest.cs +++ b/pwiz_tools/Skyline/TestPerf/DiaSwathTutorialTest.cs @@ -90,6 +90,8 @@ public class AnalysisValues public int[] DiffPeptideCounts; public int UnpolishedProteins; public int? PolishedProteins; + public double? FoldChangeProteinsMax; + public double? FoldChangeProteinsMin; public string FastaPath => IsWholeProteome @@ -275,6 +277,7 @@ public void TestQeData(bool fullSet) }, DiffPeptideCounts = new[] { 139, 47, 29, 52 }, UnpolishedProteins = 9, + FoldChangeProteinsMax = 2, }; } } @@ -1124,10 +1127,7 @@ protected override void DoTest() var formattingDlg = ShowDialog(volcanoPlot.ShowFormattingDialog); ApplyFormatting(formattingDlg, "ECOLI", "128, 0, 255"); var createExprDlg = ShowDialog(() => - { - var bindingList = formattingDlg.GetCurrentBindingList(); - formattingDlg.ClickCreateExpression(bindingList.Count - 1); - }); + formattingDlg.ClickCreateExpression(formattingDlg.ResultList.Count - 1)); PauseForScreenShot("Create Expression form", screenshotPage++); OkDialog(createExprDlg, createExprDlg.OkDialog); @@ -1212,20 +1212,17 @@ protected override void DoTest() RestoreViewOnScreen(31); barGraph = WaitForOpenForm(); - if (IsPauseForScreenShots) + WaitForBarGraphPoints(barGraph, _analysisValues.PolishedProteins ?? targetProteinCount); + RunUIForScreenShot(() => { - WaitForBarGraphPoints(barGraph, _analysisValues.PolishedProteins ?? targetProteinCount); - RunUI(() => - { - var yScale = barGraph.ZedGraphControl.GraphPane.YAxis.Scale; - yScale.MinAuto = yScale.MaxAuto = false; - yScale.Min = -2.4; - yScale.Max = 2.2; - yScale.MajorStep = 1; - yScale.MinorStep = 0.2; - yScale.Format = "0.#"; - }); - } + var yScale = barGraph.ZedGraphControl.GraphPane.YAxis.Scale; + yScale.MinAuto = yScale.MaxAuto = false; + yScale.Max = _analysisValues.FoldChangeProteinsMax ?? 2.2; + yScale.Min = _analysisValues.FoldChangeProteinsMin ?? -2.4; + yScale.MajorStep = 1; + yScale.MinorStep = 0.2; + yScale.Format = "0.#"; + }); PauseForGraphScreenShot("By Condition:Bar Graph - proteins", barGraph); RunQValueSummaryTest(); @@ -1370,9 +1367,9 @@ private void ApplyFormatting(VolcanoPlotFormattingDlg formattingDlg, string matc { RunUI(() => { - var bindingList = formattingDlg.GetCurrentBindingList(); var color = RgbHexColor.ParseRgb(rgbText).Value; - bindingList.Add(new MatchRgbHexColor("ProteinName: " + matchText, false, color, PointSymbol.Circle, PointSize.normal)); + formattingDlg.AddRow(new MatchRgbHexColor("ProteinName: " + matchText, + false, color, PointSymbol.Circle, PointSize.normal)); }); } diff --git a/pwiz_tools/Skyline/TestPerf/DiaUmpireTutorialTest.cs b/pwiz_tools/Skyline/TestPerf/DiaUmpireTutorialTest.cs index e620a35b11..a98d0a1f82 100644 --- a/pwiz_tools/Skyline/TestPerf/DiaUmpireTutorialTest.cs +++ b/pwiz_tools/Skyline/TestPerf/DiaUmpireTutorialTest.cs @@ -40,7 +40,6 @@ using pwiz.Skyline.Model; using pwiz.Skyline.Model.AuditLog; using pwiz.Skyline.Model.DocSettings; -using pwiz.Skyline.Model.GroupComparison; using pwiz.Skyline.Model.Irt; using pwiz.Skyline.Model.Results; using pwiz.Skyline.Properties; @@ -854,16 +853,6 @@ private void RestoreViewOnScreenNoSelChange(int pageName) } } - private void ApplyFormatting(VolcanoPlotFormattingDlg formattingDlg, string matchText, string rgbText) - { - RunUI(() => - { - var bindingList = formattingDlg.GetCurrentBindingList(); - var color = RgbHexColor.ParseRgb(rgbText).Value; - bindingList.Add(new MatchRgbHexColor("ProteinName: " + matchText, false, color, PointSymbol.Circle, PointSize.normal)); - }); - } - private void ValidateTargets(ref int[] targetCounts, int proteinCount, int peptideCount, int precursorCount, int transitionCount, string propName) { if (IsRecordMode) diff --git a/pwiz_tools/Skyline/TestRunner/Program.cs b/pwiz_tools/Skyline/TestRunner/Program.cs index d11ecd39f1..3583e9cb29 100644 --- a/pwiz_tools/Skyline/TestRunner/Program.cs +++ b/pwiz_tools/Skyline/TestRunner/Program.cs @@ -250,7 +250,7 @@ public LeakTracking Min(LeakTracking lastDeltas) "parallelmode=off;workercount=0;waitforworkers=off;keepworkerlogs=off;checkdocker=on;workername;queuehost;workerport;workertimeout;alwaysupcltpassword;" + "coverage=off;dotcoverexe=jetbrains.dotcover.commandlinetools\\2023.3.3\\tools\\dotCover.exe;" + "maxsecondspertest=-1;" + - "demo=off;showformnames=off;showpages=off;status=off;buildcheck=0;screenshotlist;" + + "demo=off;showformnames=off;showpages=off;status=off;buildcheck=0;" + "quality=off;pass0=off;pass1=off;pass2=on;" + "perftests=off;" + "retrydatadownloads=off;" + diff --git a/pwiz_tools/Skyline/TestUtil/PauseAndContinueForm.cs b/pwiz_tools/Skyline/TestUtil/PauseAndContinueForm.cs index 92b9598295..c64b5362ed 100644 --- a/pwiz_tools/Skyline/TestUtil/PauseAndContinueForm.cs +++ b/pwiz_tools/Skyline/TestUtil/PauseAndContinueForm.cs @@ -42,6 +42,7 @@ public partial class PauseAndContinueForm : Form, IPauseTestController private string _description; private string _linkUrl; private string _imageUrl; + private string _fileToShow; private string _fileToSave; private bool _showMatchingPage; // Information for taking a screenshot @@ -69,7 +70,7 @@ public PauseAndContinueForm(ScreenshotManager screenshotManager) _screenshotManager = screenshotManager; _ownerForm = FindOwnerForm(); - _currentMode = TestUtilSettings.Default.ShowPreview + _currentMode = screenshotManager != null && TestUtilSettings.Default.ShowPreview ? PauseAndContinueMode.PREVIEW_SCREENSHOT : PauseAndContinueMode.PAUSE_AND_CONTINUE; } @@ -99,7 +100,8 @@ private void ShowInternal(string description, int screenshotNum = 0, bool showMa { _screenshotNum = screenshotNum; _description = description; - _fileToSave = _screenshotManager?.ScreenshotFile(screenshotNum); + _fileToShow = _screenshotManager?.ScreenshotSourceFile(screenshotNum); + _fileToSave = _screenshotManager?.ScreenshotDestFile(screenshotNum); _linkUrl = _screenshotManager?.ScreenshotUrl(screenshotNum); _imageUrl = _screenshotManager?.ScreenshotImgUrl(screenshotNum); _showMatchingPage = showMatchingPages; @@ -107,14 +109,20 @@ private void ShowInternal(string description, int screenshotNum = 0, bool showMa _fullScreen = fullScreen; _processShot = processShot; - // TODO: Put this back to allow keyboard to work as expected in paused Skyline - //RunUI(SkylineWindow, () => SkylineWindow.UseKeysOverride = false); //determine if this is needed + // Allow full manual interaction with Skyline once in pause mode + // Testing will have UseKeysOverride set to true to prevent accidental + // keyboard interaction with the running test. + RunUI(Program.MainWindow, () => Program.MainWindow.UseKeysOverride = false); lock (_pauseLock) { RunUI(_ownerForm, RefreshAndShow); Monitor.Wait(_pauseLock, timeout ?? -1); } - //RunUI(SkylineWindow, () => SkylineWindow.UseKeysOverride = true); + RunUI(Program.MainWindow, () => Program.MainWindow.UseKeysOverride = true); + + // Record that the screenshot happened in the console output to make + // it easier to set a starting screenshot in a subsequent test. + Console.WriteLine(_description); } private static Form FindOwnerForm() @@ -139,7 +147,7 @@ private void SwitchToPreview() Hide(); - _screenshotPreviewForm.Show(false); + _screenshotPreviewForm.ShowOrUpdate(); } private void EnsurePreviewForm() @@ -197,8 +205,7 @@ private void RefreshAndShow() { EnsurePreviewForm(); - // TODO: This location used to force a 1 second delay. Is that really necessary? If so, why should be commented here - _screenshotPreviewForm.Show(false); + _screenshotPreviewForm.ShowOrUpdate(); } if (_showMatchingPage) @@ -341,6 +348,7 @@ void IPauseTestController.ShowPauseForm() public string Description => _description; public string LinkUrl => _linkUrl; public string ImageUrl => _imageUrl; + public string FileToShow => _fileToShow; public string FileToSave => _fileToSave; public Control ScreenshotControl => _screenshotForm; public bool FullScreen => _fullScreen; diff --git a/pwiz_tools/Skyline/TestUtil/ScreenshotManager.cs b/pwiz_tools/Skyline/TestUtil/ScreenshotManager.cs index 1b9e021e91..47dedc2b1d 100644 --- a/pwiz_tools/Skyline/TestUtil/ScreenshotManager.cs +++ b/pwiz_tools/Skyline/TestUtil/ScreenshotManager.cs @@ -21,7 +21,8 @@ namespace pwiz.SkylineTestUtil public class ScreenshotManager { private readonly SkylineWindow _skylineWindow; - private readonly string _tutorialPath; + private readonly string _tutorialSourcePath; + private readonly string _tutorialDestPath; public class PointFactor { @@ -136,11 +137,10 @@ private enum DeviceCap public static PointFactor GetScalingFactor() { - Graphics g = Graphics.FromHwnd(IntPtr.Zero); + using var g = Graphics.FromHwnd(IntPtr.Zero); IntPtr desktop = g.GetHdc(); int LogicalScreenHeight = GetDeviceCaps(desktop, (int)DeviceCap.VERTRES); int PhysicalScreenHeight = GetDeviceCaps(desktop, (int)DeviceCap.DESKTOPVERTRES); - float ScreenScalingFactor = PhysicalScreenHeight / (float)LogicalScreenHeight; return new PointFactor(ScreenScalingFactor); // 1.25 = 125% @@ -148,9 +148,11 @@ public static PointFactor GetScalingFactor() private abstract class SkylineScreenshot { - /** - * Factory method - */ + /// + /// Returns a new instance of the right type of screenshot + /// + /// A control to create a screenshot for + /// True if it should be fullscreen public static SkylineScreenshot CreateScreenshot(Control control, bool fullScreen = false) { if (control is ZedGraphControl zedGraphControl) @@ -162,6 +164,7 @@ public static SkylineScreenshot CreateScreenshot(Control control, bool fullScree return new ActiveWindowShot(control, fullScreen); } } + public abstract Bitmap Take(); } @@ -179,25 +182,28 @@ public ActiveWindowShot(Control activeWindow, bool fullscreen) [NotNull] public override Bitmap Take() { - Rectangle shotFrame = GetWindowRectangle(_activeWindow, _fullscreen); - Bitmap bmCapture = new Bitmap(shotFrame.Width, shotFrame.Height, PixelFormat.Format32bppArgb); - Graphics graphCapture = Graphics.FromImage(bmCapture); - bool captured = false; - while (!captured) + var shotRect = GetWindowRectangle(_activeWindow, _fullscreen); + var bmpCapture = new Bitmap(shotRect.Width, shotRect.Height, PixelFormat.Format32bppArgb); + using var g = Graphics.FromImage(bmpCapture); + while (!CaptureFromScreen(g, shotRect)) + { + Thread.Sleep(1000); // Try again in one second - remote desktop may be minimized + } + return bmpCapture; + } + + private static bool CaptureFromScreen(Graphics g, Rectangle shotRect) + { + try + { + g.CopyFromScreen(shotRect.Location, + Point.Empty, shotRect.Size); + return true; + } + catch (Exception) { - try - { - graphCapture.CopyFromScreen(shotFrame.Location, - Point.Empty, shotFrame.Size); - captured = true; - } - catch (Exception) - { - Thread.Sleep(1000); // Try again in one second - remote desktop may be minimized - } + return false; } - graphCapture.Dispose(); - return bmCapture; } } @@ -210,13 +216,11 @@ public ZedGraphShot(ZedGraphControl zedGraphControl) } public override Bitmap Take() { - Metafile emf = (_zedGraphControl.MasterPane.GetMetafile()); - Bitmap bmp = new Bitmap(emf.Width, emf.Height); + var emf = _zedGraphControl.MasterPane.GetMetafile(); + var bmp = new Bitmap(emf.Width, emf.Height); bmp.SetResolution(emf.HorizontalResolution, emf.VerticalResolution); - using (Graphics g = Graphics.FromImage(bmp)) - { - g.DrawImage(emf, 0, 0); - } + using var g = Graphics.FromImage(bmp); + g.DrawImage(emf, 0, 0); return bmp; } @@ -225,7 +229,8 @@ public override Bitmap Take() public ScreenshotManager([NotNull] SkylineWindow pSkylineWindow, string tutorialPath) { _skylineWindow = pSkylineWindow; - _tutorialPath = GetAvailableTutorialPath(tutorialPath); + _tutorialDestPath = tutorialPath; + _tutorialSourcePath = GetAvailableTutorialPath(tutorialPath); } private string GetAvailableTutorialPath(string tutorialPath) @@ -246,7 +251,7 @@ private string GetAvailableTutorialPath(string tutorialPath) public string ScreenshotUrl(int screenshotNum) { - if (string.IsNullOrEmpty(_tutorialPath)) + if (string.IsNullOrEmpty(_tutorialSourcePath)) return null; return GetTutorialUrl("index.html") + "#s-" + PadScreenshotNum(screenshotNum); } @@ -260,17 +265,22 @@ public string ScreenshotImgUrl(int screenshotNum) private string GetTutorialUrl(string filePart) { - if (string.IsNullOrEmpty(_tutorialPath)) + if (string.IsNullOrEmpty(_tutorialSourcePath)) return null; - var fileUri = new Uri(Path.Combine(_tutorialPath, filePart)).AbsoluteUri; + var fileUri = new Uri(Path.Combine(_tutorialSourcePath, filePart)).AbsoluteUri; const string tutorialSearch = "/Tutorials/"; int tutorialIndex = fileUri.IndexOf(tutorialSearch, StringComparison.Ordinal); return "https://skyline.ms/tutorials/" + SCREENSHOT_URL_FOLDER + "/" + fileUri.Substring(tutorialIndex + tutorialSearch.Length); } - public string ScreenshotFile(int screenshotNum) + public string ScreenshotSourceFile(int screenshotNum) + { + return !string.IsNullOrEmpty(_tutorialSourcePath) ? $"{Path.Combine(_tutorialSourcePath, "s-" + PadScreenshotNum(screenshotNum))}.png" : null; + } + + public string ScreenshotDestFile(int screenshotNum) { - return !string.IsNullOrEmpty(_tutorialPath) ? $"{Path.Combine(_tutorialPath, "s-" + PadScreenshotNum(screenshotNum))}.png" : null; + return !string.IsNullOrEmpty(_tutorialDestPath) ? $"{Path.Combine(_tutorialDestPath, "s-" + PadScreenshotNum(screenshotNum))}.png" : null; } private static string PadScreenshotNum(int screenshotNum) @@ -289,6 +299,15 @@ public bool IsOverlappingScreenshot(Rectangle bounds) return !Rectangle.Intersect(skylineRect, bounds).IsEmpty; } + /// + /// Returns the bounds of the area reserved for the screenshot. + /// Currently, the bounds of the SkylineWindow is returned, which + /// is imperfect because there is no guarantee that the screenshot + /// will not be taken outside those bounds. If we knew the true + /// bounds of the screenshot at this point, we would probably want + /// the union of them and the Skyline bounds, since avoiding covering + /// the SkylineWindow is still useful. + /// public Rectangle GetScreenshotBounds() { return (Rectangle)SkylineWindow.Invoke((Func)(() => SkylineWindow.Bounds)); @@ -315,7 +334,11 @@ public static void ActivateScreenshotForm(Control screenshotControl) var form = FormUtil.FindParentOfType
(screenshotControl)?.ParentForm; if (form != null) { - RunUI(form, () => form.Activate()); + RunUI(form, () => + { + FormEx.ForceOnScreen(form); // Make sure the owning form is fully on screen + form.Activate(); + }); } else { @@ -346,8 +369,8 @@ public Bitmap TakeShot(Control activeWindow, bool fullScreen = false, string pat { activeWindow ??= _skylineWindow; - //check UI and create a blank shot according to the user selection - SkylineScreenshot newShot = SkylineScreenshot.CreateScreenshot(activeWindow, fullScreen); + // Check UI and create a blank shot according to the user selection + var newShot = SkylineScreenshot.CreateScreenshot(activeWindow, fullScreen); Bitmap shotPic = newShot.Take(); if (processShot != null) @@ -375,7 +398,7 @@ public Bitmap TakeShot(Control activeWindow, bool fullScreen = false, string pat SaveToFile(pathToSave, shotPic); } - //Have to do it this way because of the limitation on OLE access from background threads. + // Have to do it this way because of the limitation on OLE access from background threads. var clipThread = new Thread(() => Clipboard.SetImage(shotPic)); clipThread.SetApartmentState(ApartmentState.STA); clipThread.Start(); @@ -441,6 +464,5 @@ private void SaveToFile(string filePath, Bitmap bmp) bmp.Save(filePath); } } - } diff --git a/pwiz_tools/Skyline/TestUtil/ScreenshotPreviewForm.cs b/pwiz_tools/Skyline/TestUtil/ScreenshotPreviewForm.cs index c145ff19a3..c2969ed5ed 100644 --- a/pwiz_tools/Skyline/TestUtil/ScreenshotPreviewForm.cs +++ b/pwiz_tools/Skyline/TestUtil/ScreenshotPreviewForm.cs @@ -43,6 +43,7 @@ public interface IPauseTestController string Description { get; } string LinkUrl { get; } string ImageUrl { get; } + string FileToShow { get; } string FileToSave { get; } Control ScreenshotControl { get; } bool FullScreen { get; } @@ -71,6 +72,7 @@ public partial class ScreenshotPreviewForm : Form private string _description; private string _linkUrl; private string _imageUrl; + private string _fileToShow; private string _fileToSave; private ScreenshotValues _screenshotValues; private Bitmap _oldScreenshot; @@ -99,7 +101,6 @@ public NextScreenshotProgress(int currentNum, int stopNum) public ScreenshotPreviewForm(IPauseTestController pauseTestController, ScreenshotManager screenshotManager) { - _pauseTestController = pauseTestController; _screenshotManager = screenshotManager; @@ -119,13 +120,14 @@ public ScreenshotPreviewForm(IPauseTestController pauseTestController, Screensho splitBar.Visible = false; // Unfortunately there is not enough information about the image sizes to - // the the starting location right here, but this is better than using the Windows default + // the starting location right here, but this is better than using the Windows default StartPosition = FormStartPosition.Manual; var savedLocation = TestUtilSettings.Default.PreviewFormLocation; if (!TestUtilSettings.Default.ManualSizePreview) { - // CONSIDER: Only do this if the stored location is on the same screen as Skyline - Location = GetBestLocation(); + Location = Equals(_screenshotManager.GetScreenshotScreen(), Screen.FromPoint(savedLocation)) + ? GetBestLocation() + : savedLocation; } else { @@ -147,8 +149,7 @@ public ScreenshotPreviewForm(IPauseTestController pauseTestController, Screensho /// /// To be called by the when switching modes or entering new pause. /// - /// True when test UI may need time to stabilize before a screenshot - public void Show(bool delayForScreenshot) + public void ShowOrUpdate() { lock (_lock) { @@ -156,11 +157,12 @@ public void Show(bool delayForScreenshot) _description = _pauseTestController.Description; _linkUrl = _pauseTestController.LinkUrl; _imageUrl = _pauseTestController.ImageUrl; + _fileToSave = _pauseTestController.FileToSave; _screenshotValues = new ScreenshotValues(_pauseTestController.ScreenshotControl, - _pauseTestController.FullScreen, _pauseTestController.ProcessShot, delayForScreenshot); - if (!Equals(_fileToSave, _pauseTestController.FileToSave)) + _pauseTestController.FullScreen, _pauseTestController.ProcessShot); + if (!Equals(_fileToShow, _pauseTestController.FileToShow)) { - _fileToSave = _pauseTestController.FileToSave; + _fileToShow = _pauseTestController.FileToShow; _fileLoaded = null; } @@ -224,7 +226,7 @@ public void WaitForCompletion() private bool HasBackgroundWork { get { lock(_lock) { return !IsLoaded || (!IsWaiting && !IsScreenshotTaken); } } } private bool IsComplete { get { lock (_lock) { return IsLoaded && !IsWaiting && IsScreenshotTaken; } } } // CONSIDER: This doesn't really cover the case where the current thing is loaded but not from the current source - private bool IsLoaded { get { lock (_lock) { return Equals(_fileLoaded, _fileToSave) || Equals(_fileLoaded, _imageUrl); } } } + private bool IsLoaded { get { lock (_lock) { return Equals(_fileLoaded, _fileToShow) || Equals(_fileLoaded, _imageUrl); } } } private bool IsWaiting { get { lock (_lock) { return _nextScreenshotProgress is { IsReadyForScreenshot: false }; } } } private bool IsScreenshotTaken { get { lock (_lock) { return _screenshotTaken; } } } @@ -376,7 +378,7 @@ private void UpdateNextTextBox(TextControl textBox, EnableControl label) else { int nextScreenshot = _screenshotNum + 1; - bool nextExists = File.Exists(_screenshotManager.ScreenshotFile(nextScreenshot)); + bool nextExists = File.Exists(_screenshotManager.ScreenshotSourceFile(nextScreenshot)); textBox.Text = nextExists ? nextScreenshot.ToString() : END_TEST_TEXT; textBox.Enabled = nextExists; @@ -484,20 +486,20 @@ private void UpdateScreenshotsAsync(bool showWebImage) Assume.IsTrue(InvokeRequired); // Expecting this to run on a background thread. Use ActionUtil.RunAsync() Bitmap oldScreenshot; - string fileToSave, imageUrl, fileLoaded; + string fileToShow, imageUrl, fileLoaded; lock (_lock) { - fileToSave = _fileToSave; + fileToShow = _fileToShow; fileLoaded = _fileLoaded; imageUrl = showWebImage ? _imageUrl : null; oldScreenshot = _oldScreenshot; } - string fileToLoad = imageUrl ?? fileToSave; + string fileToLoad = imageUrl ?? fileToShow; if (!Equals(fileLoaded, fileToLoad)) { - oldScreenshot = LoadScreenshot(fileToSave, imageUrl); + oldScreenshot = LoadScreenshot(fileToShow, imageUrl); fileLoaded = fileToLoad; } @@ -546,20 +548,18 @@ private void UpdateScreenshotsAsync(bool showWebImage) private struct ScreenshotValues { - public static readonly ScreenshotValues Empty = new ScreenshotValues(null, false, null, false); + public static readonly ScreenshotValues Empty = new ScreenshotValues(null, false, null); - public ScreenshotValues(Control control, bool fullScreen, Func processShot, bool delay) + public ScreenshotValues(Control control, bool fullScreen, Func processShot) { Control = control; FullScreen = fullScreen; ProcessShot = processShot; - Delay = delay; } public Control Control { get; } public bool FullScreen { get; } public Func ProcessShot { get; } - public bool Delay { get; } } private Bitmap TakeScreenshot(ScreenshotValues values) @@ -568,9 +568,6 @@ private Bitmap TakeScreenshot(ScreenshotValues values) { return Resources.noscreenshot; } - // TODO: Is this value necessary anymore. - if (values.Delay) - Thread.Sleep(1000); var control = values.Control; try @@ -783,7 +780,7 @@ private void Continue() return; } - string nextScreenshotFile = _screenshotManager.ScreenshotFile(nextShot); + string nextScreenshotFile = _screenshotManager.ScreenshotSourceFile(nextShot); if (!File.Exists(nextScreenshotFile)) { helper.ShowTextBoxError(textBoxNext, "Invalid {0} value {1}. Screenshot file {2} does not exist.", null, nextShot, nextScreenshotFile); @@ -823,7 +820,7 @@ private void ResetAndFocusNext(int nextValue) /// The last number found to exist without interruption in the series int FindLastShot(int startFrom) { - while (File.Exists(_screenshotManager.ScreenshotFile(startFrom + 1))) + while (File.Exists(_screenshotManager.ScreenshotSourceFile(startFrom + 1))) startFrom++; return startFrom; } @@ -836,7 +833,8 @@ private void IncrementScreenshot() // Since it is not yet known what the description will be use ... and the link to the next screenshot // CONSIDER: Make this a ScreenShotInfo class? _description = _screenshotManager.ScreenshotDescription(_screenshotNum, "..."); - _fileToSave = _screenshotManager.ScreenshotFile(_screenshotNum); + _fileToShow = _screenshotManager.ScreenshotSourceFile(_screenshotNum); + _fileToSave = _screenshotManager.ScreenshotDestFile(_screenshotNum); _imageUrl = _screenshotManager.ScreenshotImgUrl(_screenshotNum); _linkUrl = _screenshotManager.ScreenshotUrl(_screenshotNum); _screenshotTaken = false; @@ -849,7 +847,7 @@ private void ContinueInternal() { Hide(); } - else if(File.Exists(_screenshotManager.ScreenshotFile(_screenshotNum))) + else if(File.Exists(_screenshotManager.ScreenshotSourceFile(_screenshotNum))) { FormStateChanged(); } @@ -877,7 +875,7 @@ private void PauseAtNextScreenshot() private bool SaveScreenshot() { - if (!FileEx.IsWritable(_fileToSave)) + if (File.Exists(_fileToSave) && !FileEx.IsWritable(_fileToSave)) { PreviewMessageDlg.Show(this, TextUtil.LineSeparate(string.Format("The file {0} is locked.", _fileToSave), "Check that it is not open in another program such as TortoiseIDiff.")); @@ -885,6 +883,10 @@ private bool SaveScreenshot() } try { + string screenshotDir = Path.GetDirectoryName(_fileToSave) ?? string.Empty; + Assume.IsFalse(string.IsNullOrEmpty(screenshotDir)); // Because ReSharper complains about possible null + Directory.CreateDirectory(screenshotDir); + _newScreenshot.Save(_fileToSave); return true; } @@ -1178,6 +1180,28 @@ private void ScreenshotPreviewForm_KeyDown(object sender, KeyEventArgs e) e.Handled = true; } break; + case Keys.C: + if (e.Control) + { + if (e.Shift) + CopyBitmap(_oldScreenshot); + else + CopyBitmap(_newScreenshot); + e.Handled = true; + } + break; + } + } + + private void CopyBitmap(Bitmap bitmap) + { + try + { + Clipboard.SetImage(bitmap); + } + catch (Exception e) + { + PreviewMessageDlg.ShowWithException(this, "Failed clipboard operation.", e); } } diff --git a/pwiz_tools/Skyline/TestUtil/TestFunctional.cs b/pwiz_tools/Skyline/TestUtil/TestFunctional.cs index 6f02f0f586..1cfc07849c 100644 --- a/pwiz_tools/Skyline/TestUtil/TestFunctional.cs +++ b/pwiz_tools/Skyline/TestUtil/TestFunctional.cs @@ -1338,14 +1338,6 @@ public bool IsRecordAuditLogForTutorials private PauseAndContinueForm _pauseAndContinueForm; - private PauseAndContinueForm PauseAndContinueFormInstance - { - get - { - return _pauseAndContinueForm ??= new PauseAndContinueForm(ScreenshotManager); - } - } - public void BeginDragDisplay(Control dockableForm, double xProportion, double yProportion) { if (!IsPauseForScreenShots) @@ -1755,7 +1747,9 @@ protected ZedGraphControl FindZedGraph(Control graphContainer) } /// - /// Type that indicates a full-screen screenshot should be taken + /// Type that indicates a full-screen screenshot should be taken. + /// A null Type and a null Control were already reserved for a + /// screenshot of the Skyline form. /// public class ScreenForm : IFormView { @@ -1795,15 +1789,16 @@ private void PauseForScreenShotInternal(string description, Type formType = null if (IsAutoScreenShotMode) { - Thread.Sleep(1000); // Wait for UI to settle down - Necessary? + // Thread.Sleep(500); // Wait for UI to settle down - Necessary? ScreenshotManager.ActivateScreenshotForm(screenshotForm); - var fileToSave = _shotManager.ScreenshotFile(ScreenshotCounter); + var fileToSave = _shotManager.ScreenshotDestFile(ScreenshotCounter); _shotManager.TakeShot(screenshotForm, fullScreen, fileToSave, processShot); } else { bool showMatchingPages = IsShowMatchingTutorialPages || Program.ShowMatchingPages; - PauseAndContinueFormInstance.Show(description, ScreenshotCounter, showMatchingPages, timeout, screenshotForm, fullScreen, processShot); + _pauseAndContinueForm ??= new PauseAndContinueForm(ScreenshotManager); + _pauseAndContinueForm.Show(description, ScreenshotCounter, showMatchingPages, timeout, screenshotForm, fullScreen, processShot); } } else