dwmkerr / sharpgl

Use OpenGL in .NET applications. SharpGL wraps all modern OpenGL features and offers a powerful scene graph to aid development.
MIT License
766 stars 300 forks source link

GC can't clean memory when wpf open gl control is removed from visual tree #59

Closed rafallopatka closed 9 years ago

rafallopatka commented 9 years ago

GC can't clean memory when Wpf OpenGlControl is removed from visual tree. This mean's that when control is removed from window, it is not destroyed by garbage collector, it is still running, and render loop is not stopped.

This is a large performance hit especially when application creates and removes control several times.

The reason of that bug is that the OpenGlControl is not unregistering events on Unload: Here's a solution.

    public OpenGLControl()
    {
        InitializeComponent();
        this.Unloaded += OnUnloaded;
        SizeChanged += new SizeChangedEventHandler(OpenGLControl_SizeChanged);
    }

    private void OnUnloaded(object sender, RoutedEventArgs routedEventArgs)
    {
        Unloaded -= OnUnloaded;
        SizeChanged -= OpenGLControl_SizeChanged;
        if(timer != null)
        {
            timer.Tick -= timer_Tick;
        }
    }
dwmkerr commented 9 years ago

Thanks for the tips, I'll work this into the next release!

robinsedlaczek commented 9 years ago

I am trying to solve this now!

rafallopatka commented 9 years ago

Hey,

After some investigation and tests in real project, i've found some improvements in my previous code.

Here it is:

public OpenGLControl() { InitializeComponent(); Unloaded += OnUnloaded; Loaded += OnLoaded; } private void OnLoaded(object sender, RoutedEventArgs routedEventArgs) { SizeChanged += OpenGLControl_SizeChanged; if (timer != null) { timer.Tick -= timer_Tick; timer.Tick += timer_Tick; // to avoid twice call, ensure timer_Tick event handler is registered only once, because it is also registred on apply template } } private void OnUnloaded(object sender, RoutedEventArgs routedEventArgs) { SizeChanged -= OpenGLControl_SizeChanged; if(timer != null) timer.Tick -= timer_Tick; }

Event registration should be done in xaml or in OnLoaded event handler not in constructor, this is because WPF calls Unloaded not only when control should be destroyed but also when parent of control changes. This can be a problem in situations when SharpGl control is placed on "switchable" container conrols such as Tab, when tab changes wpf calls unload event handler, so next time when user returns to tab which contains a SharpGl control, WPF loads back control, and then events should be registerd again.

Best regards, Rafa³ £opatka

Date: Mon, 12 Jan 2015 02:48:40 -0800 From: notifications@github.com To: sharpgl@noreply.github.com CC: rafal.lopatka@outlook.com Subject: Re: [sharpgl] GC can't clean memory when wpf open gl control is removed from visual tree (#59)

I am trying to solve this now!

Reply to this email directly or view it on GitHub.

robinsedlaczek commented 9 years ago

Thx a lot for your investigations. I'll take care of this!

robinsedlaczek commented 9 years ago

There was a problem with initialization of the OpenGL render area, especially with the first call to the code in the Resize event. So I extract a method from that code and call it in the Loaded event handler. Below the code I suggest.

You can find this code in this pull request: https://github.com/dwmkerr/sharpgl/pull/62

Would be helpful, if you can test it in your environment, @rafallopatka and give some feedback!

    /// <summary>
    /// Initializes a new instance of the <see cref="OpenGLControl"/> class.
    /// </summary>
    public OpenGLControl()
    {
        InitializeComponent();

        Unloaded += OpenGLControl_Unloaded;
        Loaded += OpenGLControl_Loaded;
    }

    /// <summary>
    /// Handles the Loaded event of the OpenGLControl control.
    /// </summary>
    /// <param name="sender">The source of the event.</param>
    /// <param name="e">The <see cref="System.Windows.RoutedEventArgs"/> Instance containing the event data.</param>
    private void OpenGLControl_Loaded(object sender, RoutedEventArgs routedEventArgs)
    {
        SizeChanged += OpenGLControl_SizeChanged;

        if (timer != null)
        {
            timer.Tick -= timer_Tick;
            timer.Tick += timer_Tick; // to avoid twice call, ensure timer_Tick event handler is registered only once, because it is also registred on apply template
        }

        UpdateOpenGLControl((int) RenderSize.Width, (int) RenderSize.Height);
    }

    /// <summary>
    /// Handles the Unloaded event of the OpenGLControl control.
    /// </summary>
    /// <param name="sender">The source of the event.</param>
    /// <param name="e">The <see cref="System.Windows.RoutedEventArgs"/> Instance containing the event data.</param>
    private void OpenGLControl_Unloaded(object sender, RoutedEventArgs routedEventArgs)
    {
        SizeChanged -= OpenGLControl_SizeChanged;

        if (timer != null)
            timer.Tick -= timer_Tick;
    }

    /// <summary>
    /// Handles the SizeChanged event of the OpenGLControl control.
    /// </summary>
    /// <param name="sender">The source of the event.</param>
    /// <param name="e">The <see cref="System.Windows.SizeChangedEventArgs"/> Instance containing the event data.</param>
    void OpenGLControl_SizeChanged(object sender, SizeChangedEventArgs e)
    {
        UpdateOpenGLControl((int) e.NewSize.Width, (int) e.NewSize.Height);
    }

    /// <summary>
    /// This method is used to set the dimensions and the viewport of the opengl control.
    /// </summary>
    /// <param name="width">The width of the OpenGL drawing area.</param>
    /// <param name="height">The height of the OpenGL drawing area.</param>
    private void UpdateOpenGLControl(int width, int height)
    {
        SizeChangedEventArgs e;
        // Lock on OpenGL.
        lock (gl)
        {
            gl.SetDimensions(width, height);

            //  Set the viewport.
            gl.Viewport(0, 0, width, height);

            //  If we have a project handler, call it...
            if (width != -1 && height != -1)
            {
                var handler = Resized;
                if (handler != null)
                    handler(this, eventArgsFast);
                else
                {
                    //  Otherwise we do our own projection.
                    gl.MatrixMode(OpenGL.GL_PROJECTION);
                    gl.LoadIdentity();

                    // Calculate The Aspect Ratio Of The Window
                    gl.Perspective(45.0f, (float) width/(float) height, 0.1f, 100.0f);

                    gl.MatrixMode(OpenGL.GL_MODELVIEW);
                    gl.LoadIdentity();
                }
            }
        }
    }
rafallopatka commented 9 years ago

Hey, I will test it, tomorrow and tell you if everything works.

Date: Mon, 12 Jan 2015 11:00:14 -0800 From: notifications@github.com To: sharpgl@noreply.github.com CC: rafal.lopatka@outlook.com Subject: Re: [sharpgl] GC can't clean memory when wpf open gl control is removed from visual tree (#59)

There was a problem with initialization of the OpenGL render area, especially with the first call to the code in the Resize event. So I extract a method from that code and call it in the Loaded event handler. Below the code I suggest.

You can find this code in this pull request: #62

Would be helpful, if you can test it in your environment, @rafallopatka and give some feedback!

/// <summary>
/// Initializes a new instance of the <see cref="OpenGLControl"/> class.
/// </summary>
public OpenGLControl()
{
    InitializeComponent();

    Unloaded += OpenGLControl_Unloaded;
    Loaded += OpenGLControl_Loaded;
}

/// <summary>
/// Handles the Loaded event of the OpenGLControl control.
/// </summary>
/// <param name="sender">The source of the event.</param>
/// <param name="e">The <see cref="System.Windows.RoutedEventArgs"/> Instance containing the event data.</param>
private void OpenGLControl_Loaded(object sender, RoutedEventArgs routedEventArgs)
{
    SizeChanged += OpenGLControl_SizeChanged;

    if (timer != null)
    {
        timer.Tick -= timer_Tick;
        timer.Tick += timer_Tick; // to avoid twice call, ensure timer_Tick event handler is registered only once, because it is also registred on apply template
    }

    UpdateOpenGLControl((int) RenderSize.Width, (int) RenderSize.Height);
}

/// <summary>
/// Handles the Unloaded event of the OpenGLControl control.
/// </summary>
/// <param name="sender">The source of the event.</param>
/// <param name="e">The <see cref="System.Windows.RoutedEventArgs"/> Instance containing the event data.</param>
private void OpenGLControl_Unloaded(object sender, RoutedEventArgs routedEventArgs)
{
    SizeChanged -= OpenGLControl_SizeChanged;

    if (timer != null)
        timer.Tick -= timer_Tick;
}

/// <summary>
/// Handles the SizeChanged event of the OpenGLControl control.
/// </summary>
/// <param name="sender">The source of the event.</param>
/// <param name="e">The <see cref="System.Windows.SizeChangedEventArgs"/> Instance containing the event data.</param>
void OpenGLControl_SizeChanged(object sender, SizeChangedEventArgs e)
{
    UpdateOpenGLControl((int) e.NewSize.Width, (int) e.NewSize.Height);
}

/// <summary>
/// This method is used to set the dimensions and the viewport of the opengl control.
/// </summary>
/// <param name="width">The width of the OpenGL drawing area.</param>
/// <param name="height">The height of the OpenGL drawing area.</param>
private void UpdateOpenGLControl(int width, int height)
{
    SizeChangedEventArgs e;
    // Lock on OpenGL.
    lock (gl)
    {
        gl.SetDimensions(width, height);

        //  Set the viewport.
        gl.Viewport(0, 0, width, height);

        //  If we have a project handler, call it...
        if (width != -1 && height != -1)
        {
            var handler = Resized;
            if (handler != null)
                handler(this, eventArgsFast);
            else
            {
                //  Otherwise we do our own projection.
                gl.MatrixMode(OpenGL.GL_PROJECTION);
                gl.LoadIdentity();

                // Calculate The Aspect Ratio Of The Window
                gl.Perspective(45.0f, (float) width/(float) height, 0.1f, 100.0f);

                gl.MatrixMode(OpenGL.GL_MODELVIEW);
                gl.LoadIdentity();
            }
        }
    }
}

Reply to this email directly or view it on GitHub.

robinsedlaczek commented 9 years ago

Cool, thx in advance! Have a nice evening!

Von meinem Windows Phone gesendet


Von: Rafałmailto:notifications@github.com Gesendet: ‎12.‎01.‎2015 20:43 An: dwmkerr/sharpglmailto:sharpgl@noreply.github.com Cc: Robin Sedlaczekmailto:robin.sedlaczek@live.de Betreff: Re: [sharpgl] GC can't clean memory when wpf open gl control is removed from visual tree (#59)

Hey, I will test it, tomorrow and tell you if everything works.

Date: Mon, 12 Jan 2015 11:00:14 -0800 From: notifications@github.com To: sharpgl@noreply.github.com CC: rafal.lopatka@outlook.com Subject: Re: [sharpgl] GC can't clean memory when wpf open gl control is removed from visual tree (#59)

There was a problem with initialization of the OpenGL render area, especially with the first call to the code in the Resize event. So I extract a method from that code and call it in the Loaded event handler. Below the code I suggest.

You can find this code in this pull request: #62

Would be helpful, if you can test it in your environment, @rafallopatka and give some feedback!

/// <summary>
/// Initializes a new instance of the <see cref="OpenGLControl"/> class.
/// </summary>
public OpenGLControl()
{
    InitializeComponent();

    Unloaded += OpenGLControl_Unloaded;
    Loaded += OpenGLControl_Loaded;
}

/// <summary>
/// Handles the Loaded event of the OpenGLControl control.
/// </summary>
/// <param name="sender">The source of the event.</param>
/// <param name="e">The <see cref="System.Windows.RoutedEventArgs"/> Instance containing the event data.</param>
private void OpenGLControl_Loaded(object sender, RoutedEventArgs routedEventArgs)
{
    SizeChanged += OpenGLControl_SizeChanged;

    if (timer != null)
    {
        timer.Tick -= timer_Tick;
        timer.Tick += timer_Tick; // to avoid twice call, ensure timer_Tick event handler is registered only once, because it is also registred on apply template
    }

    UpdateOpenGLControl((int) RenderSize.Width, (int) RenderSize.Height);
}

/// <summary>
/// Handles the Unloaded event of the OpenGLControl control.
/// </summary>
/// <param name="sender">The source of the event.</param>
/// <param name="e">The <see cref="System.Windows.RoutedEventArgs"/> Instance containing the event data.</param>
private void OpenGLControl_Unloaded(object sender, RoutedEventArgs routedEventArgs)
{
    SizeChanged -= OpenGLControl_SizeChanged;

    if (timer != null)
        timer.Tick -= timer_Tick;
}

/// <summary>
/// Handles the SizeChanged event of the OpenGLControl control.
/// </summary>
/// <param name="sender">The source of the event.</param>
/// <param name="e">The <see cref="System.Windows.SizeChangedEventArgs"/> Instance containing the event data.</param>
void OpenGLControl_SizeChanged(object sender, SizeChangedEventArgs e)
{
    UpdateOpenGLControl((int) e.NewSize.Width, (int) e.NewSize.Height);
}

/// <summary>
/// This method is used to set the dimensions and the viewport of the opengl control.
/// </summary>
/// <param name="width">The width of the OpenGL drawing area.</param>
/// <param name="height">The height of the OpenGL drawing area.</param>
private void UpdateOpenGLControl(int width, int height)
{
    SizeChangedEventArgs e;
    // Lock on OpenGL.
    lock (gl)
    {
        gl.SetDimensions(width, height);

        //  Set the viewport.
        gl.Viewport(0, 0, width, height);

        //  If we have a project handler, call it...
        if (width != -1 && height != -1)
        {
            var handler = Resized;
            if (handler != null)
                handler(this, eventArgsFast);
            else
            {
                //  Otherwise we do our own projection.
                gl.MatrixMode(OpenGL.GL_PROJECTION);
                gl.LoadIdentity();

                // Calculate The Aspect Ratio Of The Window
                gl.Perspective(45.0f, (float) width/(float) height, 0.1f, 100.0f);

                gl.MatrixMode(OpenGL.GL_MODELVIEW);
                gl.LoadIdentity();
            }
        }
    }
}

Reply to this email directly or view it on GitHub.


Reply to this email directly or view it on GitHub: https://github.com/dwmkerr/sharpgl/issues/59#issuecomment-69632110

rafallopatka commented 9 years ago

Hey,

I tested it and it works, memory can be released, and control works correctly on tabstrips - events are correctly registered and unregistered. I found only one thing to consider.

Here is a interesting fragment of code from UpdateOpenGLControl method:

if (handler != null) handler(this, eventArgsFast);

It checks if handler is not null and then it fires event, but sometimes eventArgsFast also can be null, and this can crash existing code (in projects which already use SharpGLControl ), because in event handler for resized event mostly we do things like that:

args.OpenGl.DoSomething

In my code I add if statement which checks if args is not null, but for backward compatibility for existing projects, maybe it will be better to NOT fire event if args is null.

if (handler != null && eventArgsFast != null) handler(this, eventArgsFast);

Best regards, Rafał Łopatka

Date: Mon, 12 Jan 2015 12:26:20 -0800 From: notifications@github.com To: sharpgl@noreply.github.com CC: rafal.lopatka@outlook.com Subject: Re: [sharpgl] GC can't clean memory when wpf open gl control is removed from visual tree (#59)

Cool, thx in advance! Have a nice evening!

Von meinem Windows Phone gesendet


Von: Rafałmailto:notifications@github.com

Gesendet: ‎12.‎01.‎2015 20:43

An: dwmkerr/sharpglmailto:sharpgl@noreply.github.com

Cc: Robin Sedlaczekmailto:robin.sedlaczek@live.de

Betreff: Re: [sharpgl] GC can't clean memory when wpf open gl control is removed from visual tree (#59)

Hey, I will test it, tomorrow and tell you if everything works.

Date: Mon, 12 Jan 2015 11:00:14 -0800

From: notifications@github.com

To: sharpgl@noreply.github.com

CC: rafal.lopatka@outlook.com

Subject: Re: [sharpgl] GC can't clean memory when wpf open gl control is removed from visual tree (#59)

There was a problem with initialization of the OpenGL render area, especially with the first call to the code in the Resize event. So I extract a method from that code and call it in the Loaded event handler. Below the code I suggest.

You can find this code in this pull request: #62

Would be helpful, if you can test it in your environment, @rafallopatka and give some feedback!

/// <summary>

/// Initializes a new instance of the <see cref="OpenGLControl"/> class.

/// </summary>

public OpenGLControl()

{

    InitializeComponent();

    Unloaded += OpenGLControl_Unloaded;

    Loaded += OpenGLControl_Loaded;

}

/// <summary>

/// Handles the Loaded event of the OpenGLControl control.

/// </summary>

/// <param name="sender">The source of the event.</param>

/// <param name="e">The <see cref="System.Windows.RoutedEventArgs"/> Instance containing the event data.</param>

private void OpenGLControl_Loaded(object sender, RoutedEventArgs routedEventArgs)

{

    SizeChanged += OpenGLControl_SizeChanged;

    if (timer != null)

    {

        timer.Tick -= timer_Tick;

        timer.Tick += timer_Tick; // to avoid twice call, ensure timer_Tick event handler is registered only once, because it is also registred on apply template

    }

    UpdateOpenGLControl((int) RenderSize.Width, (int) RenderSize.Height);

}

/// <summary>

/// Handles the Unloaded event of the OpenGLControl control.

/// </summary>

/// <param name="sender">The source of the event.</param>

/// <param name="e">The <see cref="System.Windows.RoutedEventArgs"/> Instance containing the event data.</param>

private void OpenGLControl_Unloaded(object sender, RoutedEventArgs routedEventArgs)

{

    SizeChanged -= OpenGLControl_SizeChanged;

    if (timer != null)

        timer.Tick -= timer_Tick;

}

/// <summary>

/// Handles the SizeChanged event of the OpenGLControl control.

/// </summary>

/// <param name="sender">The source of the event.</param>

/// <param name="e">The <see cref="System.Windows.SizeChangedEventArgs"/> Instance containing the event data.</param>

void OpenGLControl_SizeChanged(object sender, SizeChangedEventArgs e)

{

    UpdateOpenGLControl((int) e.NewSize.Width, (int) e.NewSize.Height);

}

/// <summary>

/// This method is used to set the dimensions and the viewport of the opengl control.

/// </summary>

/// <param name="width">The width of the OpenGL drawing area.</param>

/// <param name="height">The height of the OpenGL drawing area.</param>

private void UpdateOpenGLControl(int width, int height)

{

    SizeChangedEventArgs e;

    // Lock on OpenGL.

    lock (gl)

    {

        gl.SetDimensions(width, height);

        //  Set the viewport.

        gl.Viewport(0, 0, width, height);

        //  If we have a project handler, call it...

        if (width != -1 && height != -1)

        {

            var handler = Resized;

            if (handler != null)

                handler(this, eventArgsFast);

            else

            {

                //  Otherwise we do our own projection.

                gl.MatrixMode(OpenGL.GL_PROJECTION);

                gl.LoadIdentity();

                // Calculate The Aspect Ratio Of The Window

                gl.Perspective(45.0f, (float) width/(float) height, 0.1f, 100.0f);

                gl.MatrixMode(OpenGL.GL_MODELVIEW);

                gl.LoadIdentity();

            }

        }

    }

}

Reply to this email directly or view it on GitHub.


Reply to this email directly or view it on GitHub:

https://github.com/dwmkerr/sharpgl/issues/59#issuecomment-69632110

— Reply to this email directly or view it on GitHub.

robinsedlaczek commented 9 years ago

But then the event is not fired... Is that good or acceptable?

Von meinem Windows Phone gesendet


Von: Rafałmailto:notifications@github.com Gesendet: ‎13.‎01.‎2015 22:15 An: dwmkerr/sharpglmailto:sharpgl@noreply.github.com Cc: Robin Sedlaczekmailto:robin.sedlaczek@live.de Betreff: Re: [sharpgl] GC can't clean memory when wpf open gl control is removed from visual tree (#59)

Hey,

I tested it and it works, memory can be released, and control works correctly on tabstrips - events are correctly registered and unregistered. I found only one thing to consider.

Here is a interesting fragment of code from UpdateOpenGLControl method:

if (handler != null) handler(this, eventArgsFast);

It checks if handler is not null and then it fires event, but sometimes eventArgsFast also can be null, and this can crash existing code (in projects which already use SharpGLControl ), because in event handler for resized event mostly we do things like that:

args.OpenGl.DoSomething

In my code I add if statement which checks if args is not null, but for backward compatibility for existing projects, maybe it will be better to NOT fire event if args is null.

if (handler != null && eventArgsFast != null) handler(this, eventArgsFast);

Best regards, Rafał Łopatka

Date: Mon, 12 Jan 2015 12:26:20 -0800 From: notifications@github.com To: sharpgl@noreply.github.com CC: rafal.lopatka@outlook.com Subject: Re: [sharpgl] GC can't clean memory when wpf open gl control is removed from visual tree (#59)

Cool, thx in advance! Have a nice evening!

Von meinem Windows Phone gesendet


Von: Rafałmailto:notifications@github.com

Gesendet: ‎12.‎01.‎2015 20:43

An: dwmkerr/sharpglmailto:sharpgl@noreply.github.com

Cc: Robin Sedlaczekmailto:robin.sedlaczek@live.de

Betreff: Re: [sharpgl] GC can't clean memory when wpf open gl control is removed from visual tree (#59)

Hey, I will test it, tomorrow and tell you if everything works.

Date: Mon, 12 Jan 2015 11:00:14 -0800

From: notifications@github.com

To: sharpgl@noreply.github.com

CC: rafal.lopatka@outlook.com

Subject: Re: [sharpgl] GC can't clean memory when wpf open gl control is removed from visual tree (#59)

There was a problem with initialization of the OpenGL render area, especially with the first call to the code in the Resize event. So I extract a method from that code and call it in the Loaded event handler. Below the code I suggest.

You can find this code in this pull request: #62

Would be helpful, if you can test it in your environment, @rafallopatka and give some feedback!

/// <summary>

/// Initializes a new instance of the <see cref="OpenGLControl"/> class.

/// </summary>

public OpenGLControl()

{

    InitializeComponent();

    Unloaded += OpenGLControl_Unloaded;

    Loaded += OpenGLControl_Loaded;

}

/// <summary>

/// Handles the Loaded event of the OpenGLControl control.

/// </summary>

/// <param name="sender">The source of the event.</param>

/// <param name="e">The <see cref="System.Windows.RoutedEventArgs"/> Instance containing the event data.</param>

private void OpenGLControl_Loaded(object sender, RoutedEventArgs routedEventArgs)

{

    SizeChanged += OpenGLControl_SizeChanged;

    if (timer != null)

    {

        timer.Tick -= timer_Tick;

        timer.Tick += timer_Tick; // to avoid twice call, ensure timer_Tick event handler is registered only once, because it is also registred on apply template

    }

    UpdateOpenGLControl((int) RenderSize.Width, (int) RenderSize.Height);

}

/// <summary>

/// Handles the Unloaded event of the OpenGLControl control.

/// </summary>

/// <param name="sender">The source of the event.</param>

/// <param name="e">The <see cref="System.Windows.RoutedEventArgs"/> Instance containing the event data.</param>

private void OpenGLControl_Unloaded(object sender, RoutedEventArgs routedEventArgs)

{

    SizeChanged -= OpenGLControl_SizeChanged;

    if (timer != null)

        timer.Tick -= timer_Tick;

}

/// <summary>

/// Handles the SizeChanged event of the OpenGLControl control.

/// </summary>

/// <param name="sender">The source of the event.</param>

/// <param name="e">The <see cref="System.Windows.SizeChangedEventArgs"/> Instance containing the event data.</param>

void OpenGLControl_SizeChanged(object sender, SizeChangedEventArgs e)

{

    UpdateOpenGLControl((int) e.NewSize.Width, (int) e.NewSize.Height);

}

/// <summary>

/// This method is used to set the dimensions and the viewport of the opengl control.

/// </summary>

/// <param name="width">The width of the OpenGL drawing area.</param>

/// <param name="height">The height of the OpenGL drawing area.</param>

private void UpdateOpenGLControl(int width, int height)

{

    SizeChangedEventArgs e;

    // Lock on OpenGL.

    lock (gl)

    {

        gl.SetDimensions(width, height);

        //  Set the viewport.

        gl.Viewport(0, 0, width, height);

        //  If we have a project handler, call it...

        if (width != -1 && height != -1)

        {

            var handler = Resized;

            if (handler != null)

                handler(this, eventArgsFast);

            else

            {

                //  Otherwise we do our own projection.

                gl.MatrixMode(OpenGL.GL_PROJECTION);

                gl.LoadIdentity();

                // Calculate The Aspect Ratio Of The Window

                gl.Perspective(45.0f, (float) width/(float) height, 0.1f, 100.0f);

                gl.MatrixMode(OpenGL.GL_MODELVIEW);

                gl.LoadIdentity();

            }

        }

    }

}

Reply to this email directly or view it on GitHub.


Reply to this email directly or view it on GitHub:

https://github.com/dwmkerr/sharpgl/issues/59#issuecomment-69632110

— Reply to this email directly or view it on GitHub.


Reply to this email directly or view it on GitHub: https://github.com/dwmkerr/sharpgl/issues/59#issuecomment-69821484

dwmkerr commented 9 years ago

Hi Guys,

Just weighing in here because I am working on #45 which also relates to the lifecycle of the controls. We do indeed need to unregister the event handlers on UnLoaded (thanks @rafallopatka!).

One thing we should also do based on your PR Robin is to apply the same pattern to the DispatchTimer in the parent class. Ideally, we register/unregister the tick in the Loaded / Unloaded events of the parent, maintaining consistency. This means we do not need to do anything with the timer in the child class (which is nicer because the parent owns it, in this case the child is dealing with a mistake in the parent code).

Final point, eventArgsFast is really stupid, when I wrote that I should've checked MSDN (always a good thing to do) the correct pattern is to use EventArgs.Empty. This is the standard approach for events with no necessary arguments. Handlers should always assume the event args are not null, dispatchers are often in the scenario above where they might not have data to send and don't want to waste time building an object, so the static EventArgs.Empty is passed, this is the standard approach (and saves a few lines of code and is more readable :smile:).

Once you've handled this change, can you move the PR to target 2.4 and I'll merge it in, then continue with #45?

Thanks!

robinsedlaczek commented 9 years ago

Yes Dave, let's do it the way you proposed! You'll got PR later today together with the dynamic invoke stuff.

dwmkerr commented 9 years ago

Cool, we can probably have 2.4 ready to release quite soon :+1:

robinsedlaczek commented 9 years ago

Great, I am excited to see the announcements soon! :)

dwmkerr commented 9 years ago

Resolved with latest PR

rafallopatka commented 9 years ago

Thanks !😊

Wysłano z: Poczta systemu Windows

Od: Dave Kerr Wysłano: ‎środa‎, ‎28‎ ‎stycznia‎ ‎2015 ‎17‎:‎46 Do: dwmkerrsharpgl DW: Rafał Łopatka

Resolved with latest PR

— Reply to this email directly or view it on GitHub.

robinsedlaczek commented 9 years ago

You're Welcome! Thx for the Feedback!

Von meinem Windows Phone gesendet


Von: Rafałmailto:notifications@github.com Gesendet: ‎28.‎01.‎2015 19:44 An: dwmkerr/sharpglmailto:sharpgl@noreply.github.com Cc: Robin Sedlaczekmailto:robin.sedlaczek@live.de Betreff: Re: [sharpgl] GC can't clean memory when wpf open gl control is removed from visual tree (#59)

Thanks !😊

Wysłano z: Poczta systemu Windows

Od: Dave Kerr Wysłano: ‎środa‎, ‎28‎ ‎stycznia‎ ‎2015 ‎17‎:‎46 Do: dwmkerrsharpgl DW: Rafał Łopatka

Resolved with latest PR

— Reply to this email directly or view it on GitHub.


Reply to this email directly or view it on GitHub: https://github.com/dwmkerr/sharpgl/issues/59#issuecomment-71890755