Fix NullReference in SkiaSharp WPF renderer if UIElement has no Prese… #1799

albtaeler · 2021-10-08T06:50:12Z

…ntationSource (#1798)

Fixes #1798 .

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:

Return/use the default values if the PresentationSource or the ancestor visual is null:

@oxyplot/admins

VisualMelon · 2021-10-10T07:51:55Z

Thanks for putting this together. I can't take a proper look now, but hopefully I'll be able to get back to you on this in the next few days.

albtaeler · 2021-11-09T13:50:23Z

I force pushed a version with the fix (default values if the ancestor visual is null) for the canvas renderer, too.

…ntationSource (oxyplot#1798)

albtaeler · 2021-11-24T14:29:01Z

Another NullReferenceException appeared during tests of our application with the latest Oxyplot version.
I had to add this check in Plotview.cs SkElement_PaintSurface().

        private void SkElement_PaintSurface(object sender, SKPaintSurfaceEventArgs e)
        {
            if (this.plotPresenter == null || this.renderContext == null)
            {
                return;
            }

Hopefully you @VisualMelon or another maintainer will have time to review it soon.

VisualMelon

All looks good. Some duplication in #1820; I suggest we merge this and then work out if there is anything in #1820 that is missing.

VisualMelon · 2021-12-24T09:57:22Z

Merged now so that we can look at #1820 properly, and because this was checked with the WPF and has 3 issues requesting the fix.

Thanks again @albtaeler for this and #1797

HavenDV · 2022-02-09T23:25:02Z

Source/OxyPlot.Wpf/PlotView.cs

        protected override double UpdateDpi()
        {
            var scale = base.UpdateDpi();
-            this.RenderContext.DpiScale = scale;


Was this really planned? Slightly inconsistent with the rest of the PR and its description.
P.S. I periodically push new changes to my fork - https://github.com/HavenDV/H.OxyPlot

Thanks for spotting this.

For Skia/WPF it's fine I believe as it's handled at a different level, but does look like a regression for non-Skia/WPF which is a bit hard to spot, but the most obvious thing is that axis lines are fuzzy when the scale isn't 1.

@albtaeler do you recall if there was a particular reason for removing this, or did it just look redundant?

I can't remember a specific reason. Maybe it was a merge accident, or for redundantency reasons. But I think you're right, it's still needed for the non-Skia/WPF renderer.

@albtaeler thanks for taking another look; feel free to open a PR putting this back in; hopefully I'll get around to it eventually otherwise.

HavenDV · 2022-02-09T23:47:37Z

Source/OxyPlot.Wpf.Shared/PlotViewBase.cs

        /// </summary>
        protected void Render()
        {
-            if (this.plotPresenter == null || this.renderContext == null || !(this.isInVisualTree = this.IsInVisualTree()))


This raises questions too - it cannot throw a NullReferenceException. If it's just a refactoring - then the behavior is changed.

This was mentioned in #1798 (comment) but looking briefly now it does look a bit odd.

I'll try to find time to take another look at this (I should have some tests lying around somewhere....); my WPF knowledge is pretty limited, so I'd welcome any input from you and @albtaeler.

Didn't see the comment. Considering it, it's all right. The only question is - did this check play any role in performance?

I believe the only reason for this check was that this.TransformToAncestor() fails with a NullReferenceException without my changes. As mentioned in #1798 (comment) I changed this behavior, because rendering it's not possible if it isn't in the VisualTree, but sometimes needed.
I could not see any performance changes. It is important that the property this.isInVisualTree = this.IsInVisualTree(); is set.

I was confused at this point only because I don't see a place where IsInVisualTree() could throw a NullReferenceException.

albtaeler force-pushed the Issue1798 branch from cf4b573 to 8e0445e Compare October 19, 2021 15:31

albtaeler force-pushed the Issue1798 branch from 8e0445e to 1091bff Compare November 9, 2021 13:24

albtaeler mentioned this pull request Nov 9, 2021

OxyPlot SkiaSharp renderer throws a null reference exception if the rendered UIElement has no PresentationSource #1798

Closed

Fix NullReference in SkiaSharp WPF renderer if UIElement has no Prese…

bfe472d

…ntationSource (oxyplot#1798)

albtaeler force-pushed the Issue1798 branch from 1091bff to bfe472d Compare November 24, 2021 13:52

VisualMelon approved these changes Dec 23, 2021

View reviewed changes

This was referenced Dec 23, 2021

Fix rendering in ElementHost with SkiaSharp #1820

Merged

InvalidOperationException when PlotView ancestry changes #1834

Closed

VisualMelon merged commit 79b7d24 into oxyplot:develop Dec 24, 2021

HavenDV reviewed Feb 9, 2022

View reviewed changes

albtaeler deleted the Issue1798 branch February 11, 2022 09:41

VisualMelon mentioned this pull request Feb 12, 2022

Fix WPF Dpi Regression #1849

Merged

4 tasks

Fix NullReference in SkiaSharp WPF renderer if UIElement has no Prese… #1799

Fix NullReference in SkiaSharp WPF renderer if UIElement has no Prese… #1799

Uh oh!

Conversation

albtaeler commented Oct 8, 2021

Checklist

Changes proposed in this pull request:

Uh oh!

VisualMelon commented Oct 10, 2021

Uh oh!

albtaeler commented Nov 9, 2021

Uh oh!

albtaeler commented Nov 24, 2021

Uh oh!

VisualMelon left a comment

Choose a reason for hiding this comment

Uh oh!

VisualMelon commented Dec 24, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HavenDV Feb 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

VisualMelon commented Dec 24, 2021 •

edited

Loading

HavenDV Feb 10, 2022 •

edited

Loading