Skip to content
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

Add WPF PlotView based on SkiaSharp (#1515) #1533

Merged
merged 1 commit into from
May 5, 2020
Merged

Conversation

Jonarw
Copy link
Member

@Jonarw Jonarw commented May 1, 2020

Fixes #1515.

Checklist

  • I have included examples or tests
  • I have updated the change log
  • I am listed in the CONTRIBUTORS file
  • I have cleaned up the commit history (use rebase and squash)

Changes proposed in this pull request:

  • Add PlotView based on SkiaRenderContext
  • To enable code sharing between OxyPlot.Wpf and OxyPlot.SkiaSharp.Wpf, introduce OxyPlot.Wpf.Shared. Most of the code from OxyPlot.Wpf ended up in here.
  • Update WPF example browser so it supports both Canvas and SkiaSharp renderers to allow direct optical comparison between the two.
    • Enabled DPI-aware mode for example browser, so it is possible to test the effect of different DPI settings without restart
    • Removed fps measurement from example browser (for now), as it would need some rework to also work with the SkiaSharp renderer

Notes

  • Getting pixel-perfect drawing of the bitmap rendered by SkiaSharp to the screen proved somewhat difficult. I got it to work on all configurations I can test, would be happy to hear if it works for other people as well!
  • Direct optical comparison revealed some minor inconsistencies between Canvas and SkiaSharp renderers, I will address those in separate issues
  • Should the new assemblies (OxyPlot.SkiaSharp, OxyPlot.SkiaSharp.Wpf, OxyPlot.Wpf.Shared) be signed? @objorke do you want to create the .snk files for those?

@oxyplot/admins

@objorke
Copy link
Member

objorke commented May 1, 2020

I think we should be consistent with signing.
Can we reuse the same .snk for all those libraries?

@Jonarw
Copy link
Member Author

Jonarw commented May 1, 2020

Ah I wasn't aware that you could use the same .snk for multiple assemblies. As far as I can tell so far every assembly has its own .snk file.

So if we share the .snk between some of the projects, should we share it between all of them to be consistent?

For now I will just copy the .snk files to sign each of the new projects, but that might be something to be considered in a separate issue.

@VisualMelon
Copy link
Contributor

VisualMelon commented May 2, 2020

This seems to introduce a bug into the Wpf example browser, whereby it will crash if you:

  1. Switch to the code tab
  2. Open an example that is not already open

Partial stack-trace:

System.InvalidOperationException: The specified Visual is not an ancestor of this Visual.
   at System.Windows.Media.Visual.TrySimpleTransformToAncestor(Visual ancestor, Boolean inverse, GeneralTransform& generalTransform, Matrix& simpleTransform)
   at System.Windows.Media.Visual.TransformToAncestor(Visual ancestor)
   at OxyPlot.Wpf.PlotView.UpdateDpi() in C:\Users\Murdo\source\repos\oxyplot\Source\OxyPlot.Wpf\PlotView.cs:line 113
   at OxyPlot.Wpf.PlotViewBase.RenderOverride() in C:\Users\Murdo\source\repos\oxyplot\Source\OxyPlot.Wpf.Shared\PlotViewBase.cs:line 381
   at OxyPlot.Wpf.PlotView.RenderOverride() in C:\Users\Murdo\source\repos\oxyplot\Source\OxyPlot.Wpf\PlotView.cs:line 94
   at OxyPlot.Wpf.PlotViewBase.Render() in C:\Users\Murdo\source\repos\oxyplot\Source\OxyPlot.Wpf.Shared\PlotViewBase.cs:line 373

More stack-trace

This appears to be new as of 458de6a

Everything seems to work fine if I just wrap all of PlotViewBase.RenderOverride() in a try...catch { }, but I assume there is a better way of detecting that we shouldn't render because we aren't in the visual tree.

@VisualMelon
Copy link
Contributor

Removed fps measurement from example browser (for now), as it would need some rework to also work with the SkiaSharp renderer

Could this be included, but only enabled for the Canvas renderer?

Source/OxyPlot.Wpf.Shared/PlotViewBase.Events.cs Outdated Show resolved Hide resolved
Source/OxyPlot.Wpf.Shared/PlotViewBase.Properties.cs Outdated Show resolved Hide resolved
Source/OxyPlot.Wpf.Shared/PlotViewBase.cs Outdated Show resolved Hide resolved
Source/OxyPlot.Wpf/PlotView.cs Outdated Show resolved Hide resolved
@Jonarw
Copy link
Member Author

Jonarw commented May 2, 2020

This seems to introduce a bug into the Wpf example browser

Good catch! I've addressed it and will upload a fix and test soon.

Could this be included, but only enabled for the Canvas renderer?

In principle yes, but I did not found a quick way of getting it to work consistently.
Also I did not find that fps measurement terribly useful before, as it shows the combined update+render time... It would be nice if we in the future could have a measurement that works for both Canvas and SkiaSharp and would display separate update and render times, but this would be out of the scope of this PR.

@Jonarw
Copy link
Member Author

Jonarw commented May 4, 2020

Squashed the commits.
From my side this would be ready to merge. Any other concerns?

@objorke
Copy link
Member

objorke commented May 4, 2020

Nice! I am looking forward to try this out!

@Jonarw Jonarw merged commit bbe6290 into oxyplot:develop May 5, 2020
@Jonarw Jonarw deleted the skiawpf branch May 5, 2020 12:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add WPF PlotView based on SkiaSharp
3 participants