-
Notifications
You must be signed in to change notification settings - Fork 100
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Skyline: Improvements to tutorial screenshots based on testing #3283
Skyline: Improvements to tutorial screenshots based on testing #3283
Conversation
brendanx67
commented
Dec 17, 2024
- Created ScreenshotProcessingExtensions to hold bitmap processing functions
- Added diff coloring to the old image in ScreenshotPreviewForm
- Added labels and a picture-box to ScreenshotPreviewForm for improved diffing
- Improved code to decide when to use the skyline icon as Form.Icon
- New DigitalRune.Windows.Docking.dll with better LArrow cursor
- Hid Source tab in DocumentGridViewContext for screenshots in debug builds
- Initial attempt to freeze the AllChromatogramsGraph for a screenshot
- Added function to ImportFastaControl to scroll FASTA file path text to show the file
- Hid the GPF checkbox in ImportResultsDIAControl in screenshot mode for 24.1 screenshots
- Better normalization of language names in SkylineTester and TestRunner
- Show context menu selection in s-09 for TestHiResMetabolomicsTutorial
- Fixed SkylineTester issue with test selection when "Tutorials" starts selected
- Fixed column widths in library build tab in DIA/SWATH tutorial for screenshot
- Fixed forcing SkylineWindow on screen in DIA/SWATH tutorial during docking screenshots before docking starts
- Fixed waits in auto-screenshot mode to avoid blurry screenshots
- Fixed a number issues with border calculations between normal Windows forms, floating dockable forms, and docked dockable forms in Document and non-Document states.
- Fixed screenshot numbers in URLs to match the what is currently in https://skyline.ms/tutorials/24-1/
- Fixed some necessary border clean-up in cases where image processing is implemented
- Created ScreenshotProcessingExtensions to hold bitmap processing functions - Added diff coloring to the old image in ScreenshotPreviewForm - Added labels and a picture-box to ScreenshotPreviewForm for improved diffing - Improved code to decide when to use the skyline icon as Form.Icon - New DigitalRune.Windows.Docking.dll with better LArrow cursor - Hid Source tab in DocumentGridViewContext for screenshots in debug builds - Initial attempt to freeze the AllChromatogramsGraph for a screenshot - Added function to ImportFastaControl to scroll FASTA file path text to show the file - Hid the GPF checkbox in ImportResultsDIAControl in screenshot mode for 24.1 screenshots - Better normalization of language names in SkylineTester and TestRunner - Show context menu selection in s-09 for TestHiResMetabolomicsTutorial - Fixed SkylineTester issue with test selection when "Tutorials" starts selected - Fixed column widths in library build tab in DIA/SWATH tutorial for screenshot - Fixed forcing SkylineWindow on screen in DIA/SWATH tutorial during docking screenshots before docking starts - Fixed waits in auto-screenshot mode to avoid blurry screenshots - Fixed a number issues with border calculations between normal Windows forms, floating dockable forms, and docked dockable forms in Document and non-Document states. - Fixed screenshot numbers in URLs to match the what is currently in https://skyline.ms/tutorials/24-1/ - Fixed some necessary border clean-up in cases where image processing is implemented
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice, looks good to me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me, but why is there code that locks on Bitmap objects? Are Bitmap objects somehow not thread-safe?
When you need to take a screenshot of a progress indicator with a specific progress value, I wonder whether it would have been easier to tell the thing to draw itself with a particular progress value, instead of waiting until it actually gets to that number.
icon1.Save(ms1); | ||
icon2.Save(ms2); | ||
|
||
return StructuralComparisons.StructuralEqualityComparer.Equals(ms1.ToArray(), ms2.ToArray()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I usually use "SequenceEqual" as in:
ms1.ToArray().SequenceEqual(ms2.ToArray())
Yeah. We modify the pixels of the bitmaps directly in our diff code, and even when I switched to having the UI thread make a copy of the Bitmap and never giving the bitmap directly to PictureBox, the diff code was throwing exceptions in GetPixel() or SetPixel() that said the object was in use. This fixed the problem. Maybe I could have the diff code always make a copy and never SetPixel() directly on the Bitmap itself, but it wasn't clear whether I could even have two threads requesting GetPixel() from the same bitmap at the same time. I don't know how threadsafe Bitmaps are for reading vs reading-while-writing |
Oops. The code already does create a new copy every time. I guess that means the answer to your question is, yes, Bitmaps are not threadsafe even for GetPixel() calls on two threads at the same time. I definitely got "in use" exceptions until I added the locks. |
- save TestUtilSettings on every continue to more consistently save ScreenshotPreviewForm settings - use IEnumerable<T>.SequencesEqual() as suggested by Nick