WpfPlot: Show plot in design mode by swharden · Pull Request #1048 · ScottPlot/ScottPlot

swharden · 2021-05-21T03:09:09Z

addresses #1043

swharden · 2021-05-21T03:09:20Z

FormsPlot and WpfPlot now both show plots at design time.

If there is a problem with System.Drawing.Common, it may crash Visual Studio ☠️

wpf.mp4

Add method to gently crash if there is a problem with System.Drawing.Common rendering and return a text error message #1048

swharden · 2021-05-21T03:53:57Z

I added a render test inside a try/catch block that should identify render errors and display them as text in the control at design time

ScottPlot/src/ScottPlot/Drawing/GDI.cs

Lines 21 to 47 in 382e985

    
           /// <summary> 
        
           /// Test if the system can properly render graphics using System.Drawing.Common. 
        
           /// If an exception occurs this will return a string description, otherwise it returns null. 
        
           /// </summary> 
        
           public static string DrawingTest() 
        
           { 
        
               try 
        
               { 
        
                   var bmp = new Bitmap(10, 10); 
        
                   using var gfx = GDI.Graphics(bmp); 
        
                   gfx.Clear(Color.Magenta); 
        
                   gfx.DrawLine(Pens.Black, 0, 0, 10, 10); 
        
                   using var stream = new System.IO.MemoryStream(); 
        
                   bmp.Save(stream, ImageFormat.Bmp); 
        
                   byte[] bytes = stream.ToArray(); 
        
                   int expectedBitmapSize = 454; 
        
                   if (bytes.Length != expectedBitmapSize) 
        
                       throw new InvalidOperationException($"test image was {bytes.Length} bytes (expected {expectedBitmapSize})"); 
        
               } 
        
               catch (Exception ex) 
        
               { 
        
                   return "ScottPlot Render Error:" + Environment.NewLine + ex.ToString(); 
        
               } 
        
               return null; 
        
           }

swharden · 2021-05-21T04:11:13Z

WpfPlot and FormsPlot now do this as the first thing in their initializers:

string renderErrorMessage = ScottPlot.Drawing.GDI.DrawingTest();
if (renderErrorMessage != null)
{
    InitializeComponent();
    /* display the error message in a textbox */
    return;
}

@bclehmann do you think this is worth adding to AvaPlot?

bclehmann · 2021-05-21T23:05:05Z

src/ScottPlot/Drawing/GDI.cs

+                byte[] bytes = stream.ToArray();
+                int expectedBitmapSize = 454;
+                if (bytes.Length != expectedBitmapSize)
+                    throw new InvalidOperationException($"test image was {bytes.Length} bytes (expected {expectedBitmapSize})");


Throwing something in order to immediately catch it is something of an antipattern. It's using exception handling as flow control instead of an if/else.

This method should probably either return a boolean to indicate success/failure, or just let any exceptions bubble up to the caller.

If the actual error message is important then it should throw an exception which is caught by the caller and the error message surfaced to the user. Alternatively, you can return false to indicate failure and then set an error field somewhere (like in many C APIs), or return a struct with a success field set to false and some extra information (like in many AJAX libraries).

I don't normally like the C way of doing things because it's used everywhere to get around the lack of exception support, however it does have some benefits:

It does not obligate the caller to handle any exceptions

Thus it's a pattern often used in AJAX when it doesn't matter if the request fails (or when failures are already handled by some middleware which will retry)

Performance and compatibility with embedded systems (not really a consideration here)

Returning a struct like this would probably be the best option, provided you don't want to let the exception bubble up to the caller. That choice depends on whether you think the caller ought to be obligated to handle the issue or die.

struct TestResult { bool success; string errorMessage; }

bclehmann · 2021-05-21T23:11:23Z

@bclehmann do you think this is worth adding to AvaPlot?

Yeah, I think it looks easy and helpful for a lot of users.

I don't really like how GDI.DrawingTest works, I think there's a better way to indicate a failure. Mostly from an API clarity perspective. And maybe issues like this must be handled by the caller under the threat of crashing due to an unhandled exception (or simply allow the application to crash, sometimes that is the best solution).

swharden · 2021-05-21T23:38:38Z

I think there's a better way to indicate a failure.

Hey @bclehmann, I'm a bit confused by your objection/suggestion. Could you rephrase your concern or proposed solution to help me understand it better?

My intent is that GDI.DrawingTest() will only get called in designer mode, and only once when the control is first loaded. When a user drags a control from the toolbox and drops it onto the window, a System.Drawing support or version issue may cause it to immediately throw an exception which Visual Studio has to catch and display (e.g., #993). These can be pretty disruptive in WinForms because Visual Studio may not even display the control anymore. I think those design-time exceptions are pretty jarring and confusing for new users (especially if the program builds and runs fine when the user presses F5).

I could move DrawingTest() somewhere in the Control namespace since it's not supposed to be used for any other purpose.

Interested in your feedback!
Scott

bclehmann · 2021-05-22T00:31:30Z

Could you rephrase your concern or proposed solution to help me understand it better?

I had two concerns, the first was the throw on line 39 which looked like it was using try/catch as a control structure, but the catch can actually catch multiple issues so my conclusion was too hasty. One could argue about it as a matter of taste, but now that I have thought about it I think it's probably the better solution.

The other was how the function indicates the design-time exception. I think returning a string with an error message is counter-intuitive. The name DrawingTest implies it either passes or fails a test, which is generally shown through returning a boolean indicating whether it passed, or through throwing an exception in the event that it fails. Should the error message be necessary you can read it when you catch the exception. Also, normally returning null implies an error.

My preferred solution is just to change the return type to void and remove the catch statement. The caller catches any exceptions and displays the error message. However, I think returning a boolean (or a struct with a boolean success field) is more appropriate for a method with the word "Test" in its name.

I could move DrawingTest() somewhere in the Control namespace since it's not supposed to be used for any other purpose.

I like this idea, but I think there is a benefit to keeping it in GDI, since moving to a different graphics library would require this to be rewritten. I like the current state where nothing (that I know of) in the Control namespace is dependent on System.Drawing.Common.

bclehmann · 2021-05-22T01:05:34Z

You can see what I meant in #1052. I don't know how well I communicated my concerns.

swharden · 2021-05-22T17:10:38Z

returning a string with an error message is counter-intuitive

I'm sold! A try/catch block in the initializer of each control is a lot more straightforward, and a better design.

Thanks for expanding on your thoughts/suggestions!

I'll work in #1052 to test the AvaPlot and also add similar try/catch blocks to the other controls 👍

EDIT:

lol this came around full circle... if we are try/catching in controls, we can just test against Render() itself

if (DesignerProperties.GetIsInDesignMode(this))
{
    try
    {
        Plot.Title($"ScottPlot {Plot.Version}");
        Plot.Render();
    }
    catch (Exception e)
    {
        InitializeComponent();
        PlotImage.Visibility = System.Windows.Visibility.Hidden;
        ErrorLabel.Text = e.ToString();
        return;
    }
}

WpfPlot: Show plot in design mode

ef51b93

#1043

swharden added 2 commits May 20, 2021 23:53

Drawing.GDI.DrawingTest()

f1d9e08

Add method to gently crash if there is a problem with System.Drawing.Common rendering and return a text error message #1048

WpfPlot: design time message on render error

e27b1e0

swharden added 4 commits May 20, 2021 23:58

WpfPlot: move Backend instantiation

162dbb1

FormsPlot: design time message on render error

05e1824

improve render test error message

382e985

Update changelog.md

fd2a63c

swharden merged commit 94de364 into master May 21, 2021

swharden deleted the wpf-designer branch May 21, 2021 04:17

bclehmann reviewed May 21, 2021

View reviewed changes

swharden linked an issue May 21, 2021 that may be closed by this pull request

Improve appearance of controls in designer mode #1043

Closed

bclehmann mentioned this pull request May 22, 2021

Improve design time error messages in plot controls #1052

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WpfPlot: Show plot in design mode#1048

WpfPlot: Show plot in design mode#1048
swharden merged 7 commits intomasterfrom
wpf-designer

swharden commented May 21, 2021

Uh oh!

swharden commented May 21, 2021

Uh oh!

swharden commented May 21, 2021 •

edited

Loading

Uh oh!

swharden commented May 21, 2021

Uh oh!

bclehmann May 21, 2021

Uh oh!

bclehmann May 21, 2021

Uh oh!

bclehmann commented May 21, 2021 •

edited

Loading

Uh oh!

swharden commented May 21, 2021 •

edited

Loading

Uh oh!

bclehmann commented May 22, 2021

Uh oh!

bclehmann commented May 22, 2021

Uh oh!

swharden commented May 22, 2021 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

swharden commented May 21, 2021

Uh oh!

swharden commented May 21, 2021

Uh oh!

swharden commented May 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

swharden commented May 21, 2021

Uh oh!

bclehmann May 21, 2021

Choose a reason for hiding this comment

Uh oh!

bclehmann May 21, 2021

Choose a reason for hiding this comment

Uh oh!

bclehmann commented May 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

swharden commented May 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bclehmann commented May 22, 2021

Uh oh!

bclehmann commented May 22, 2021

Uh oh!

swharden commented May 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

swharden commented May 21, 2021 •

edited

Loading

bclehmann commented May 21, 2021 •

edited

Loading

swharden commented May 21, 2021 •

edited

Loading

swharden commented May 22, 2021 •

edited

Loading