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

WPF- OxyPlot doesn't render inside a Popup (fix included) #1796

Closed
Tetedeiench opened this issue Oct 5, 2021 · 6 comments · Fixed by #1797
Closed

WPF- OxyPlot doesn't render inside a Popup (fix included) #1796

Tetedeiench opened this issue Oct 5, 2021 · 6 comments · Fixed by #1797

Comments

@Tetedeiench
Copy link
Contributor

Doing my best to format it, as I'm not a git expert by any means.

Steps to reproduce

  1. Put a plotview inside a WPF Popup
  2. Display the popup
  3. See a white graph

Platform: Windows
.NET version: 4.6.1 and others

Expected behaviour

The graph should appear normally inside a popup

Actual behaviour

The graph appears Empty

Why does it happen ?

In PlotViewBase , you're checking if the graph is part of the visual tree by checking if the ancestor is a window :

       /// <summary>
        /// Gets a value indicating whether the <see cref="PlotViewBase"/> is connected to the visual tree.
        /// </summary>
        /// <returns><c>true</c> if the PlotViewBase is connected to the visual tree; <c>false</c> otherwise.</returns>
        private bool IsInVisualTree()
        {
            DependencyObject dpObject = this;
            while ((dpObject = VisualTreeHelper.GetParent(dpObject)) != null)
            {
                if (dpObject is Window)
                {
                    return true;
                }
            }

            return false;
        }

In the case of a WPF popup, the ancestor is a Popuproot :
image

The fix

We need to check if the ancestor is also a Popuproot. If so, it's in the visual tree. But as the popuproot is an internal type, you can't check against it easily. The best way I found is to check if the logical parent of the PopupRoot is a Popup :

        /// <summary>
        /// Gets a value indicating whether the <see cref="PlotViewBase"/> is connected to the visual tree.
        /// </summary>
        /// <returns><c>true</c> if the PlotViewBase is connected to the visual tree; <c>false</c> otherwise.</returns>
        private bool IsInVisualTree()
        {
            DependencyObject dpObject = this;
            while ((dpObject = VisualTreeHelper.GetParent(dpObject)) != null)
            {
                if (dpObject is Window)
                {
                    return true;
                }

                //Check if the logical parent is a popup. If so, we found the popuproot
                var logicalRoot = LogicalTreeHelper.GetParent(dpObject);
                if (logicalRoot is Popup)
                {
                    // popup root found here
                    return true;
                }
            }

            return false;
        }

This isn't enough, as in PlotView, you're getting the window parent from the Visual tree using this call :

        /// <summary>
        /// Returns a reference to the window object that hosts the dependency object in the visual tree.
        /// </summary>
        /// <returns> The host window from the visual tree.</returns>
        private Window GetAncestorWindowFromVisualTree(DependencyObject startElement)
        {
            DependencyObject parent = startElement;
            while (!(parent is Window))
            {
                if (parent == null) { break; }
                parent = VisualTreeHelper.GetParent(parent);
            }
            return parent as Window ?? Window.GetWindow(this);
        }

I find this Window check superfluous, as if you're there, you have already checked you're in the visual tree. You can thus just walk the visual tree and get the root here, no check needed, and you only need a Visual type in the call, not a Window type :

        private Visual GetAncestorVisualFromVisualTree(DependencyObject startElement)
        {
            
            DependencyObject child = startElement;
            DependencyObject parent = VisualTreeHelper.GetParent(child);
            while (parent != null)
            {
                child = parent;
                parent = VisualTreeHelper.GetParent(child);
            }

            return child is Visual visualChild ? visualChild : Window.GetWindow(this);
        }

This makes Oxyplot work flawlessly in a popup.

Feel free to correct me and point any mistakes !

Following writing all this, I'll try to a PR - not sure I'll succeed though :D

Tetedeiench added a commit to Tetedeiench/oxyplot that referenced this issue Oct 5, 2021
Tetedeiench added a commit to Tetedeiench/oxyplot that referenced this issue Oct 5, 2021
update changelog

Fix "WPF- OxyPlot doesn't render inside a Popup" (oxyplot#1796)
@Tetedeiench
Copy link
Contributor Author

Did my best with the PR !

Please note I didn't touch the SKIA part - some changes may be required there as well.

@VisualMelon
Copy link
Contributor

Thanks for reporting this and providing a fix; I'll try to look it over tomorrow if I have time!

@Tetedeiench
Copy link
Contributor Author

Oxyplot has helped me so much it's a pleasure to help :) I just hope my first PR meets your requirements, as it's my first :)

I didn't include any tests for this, as I really don't know how you can test that. Sorry !

@Tetedeiench
Copy link
Contributor Author

I just thought you could walk the visual tree once ( in "IsInVisualTree"), store a reference to the root you found there, and reuse it later on for your DPI check.

This will save you a walk of the tree, and is probably a nice optimization.

Too late I guess !

@VisualMelon
Copy link
Contributor

Too late I guess !

You can always update the PR by pushing additional commits (or indeed, force pushing different commits altogether).

This will save you a walk of the tree, and is probably a nice optimization.

In this case, however, without evidence that it's a performance or correctness concern I would suggest keeping it simple.

@Tetedeiench
Copy link
Contributor Author

Tetedeiench commented Oct 5, 2021

I did implement it for kicks - the gain is minimal, as walking the visual tree is indeed pretty cheap. It's a 50 to 100 ticks gain per redraw (StopWatch.ElapsedTicks to be precise). Which is... basically nothing.

I did not submit it as per your comment :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants