Exception thrown on resize

[ElectronicLocal]

I installed beta 3, made the necessary code adjustments needed to keep up with Ninja.

One of my particularly graphics intensive indicators now throws an exception any time I click on Maximize/Restore (and a few other basic chart movement actions).

One code change is that I now included RemoveDrawObject, whereas, I had it commented out for beta 2 due to random crashes (#8257).


I tried commenting it out but the crash still occurred, so I can't say if it is involved.

I have attached a trace file with a number of the exceptions in it.


Gary

[NinjaTrader_ChelseaB]

Hello ElectronicLocal,

Are you able to reproduce the issue without any indicators applied to the chart?

If not, may I test your indicator to see if I can reproduce?

[ElectronicLocal]

Subject: Offending code attached.

I was able to narrow down the code that is causing the exception.
It is a very simple indicator that simply draws a line in OnRender.
I had a 4 range tick chart on the NQ.
The indicator appears to work OK if it is simply loaded.
When chart loses focus by clicking on some other window and then user
clicks back on chart: It is OK.
However, when user clicks anywhere in the top title/menu bar such as DataSeries button, or minimize/maximize buttons or even open space on the bar,
Then I get a dialog box warning me of an unhandled exception.

And note that the exception does NOT occur in my code when executing OnRender.
It appears to occur because of something the code does wrong in dealing with the SharpDX drawing classes.
When the RenderTarget.DrawLine line is commented out, no exception occurs.

I have attached 3 files:
- the trace file showing the exception occurring in: NinjaTrader.Gui.Chart.Chart.OnIsResizingChanged
- The error dialog
- The sample indicator that causes the exception.

Please guide me in the proper usage of the SharpDX.

Thanks,
Gary

[GaryAlbers]

Exception in OnIsResizingChanged

This may have slipped through the cracks.
The code attached in post #3 demonstrates the exception.

Hoping to hear something on this.

Gary

[NinjaTrader_Brett]

Thanks for bringing back up.

I took a look at the code and the problem is as follows: There are multiple render targets with DirectX and a DXBrush resource can only be used on a single render target.When you resize the window or look at the preview image in the bottom a new render target is created which causes the exception.

To resolve the issue there are two ways to fix. One way is a bandaid and the other way is the better way to resolve which we introduced in Beta 3 and will have documentation up next week on how to use.

Option 1) Change OnBarUpdate to set a flag to that will be used to toggle the color. Then in the OnRender method create the brush based on the flag and render it with that Brush. This insures that each time OnRender gets called no matter what RenderTarget it is called on that the brush is created on the right RenderTarget context. However it is not the best programming practice to constantly create new brushes, its fine for some use cases but if you are doing a lot of brush changing its better to cache your brushes and reuse them for better performance.

Option 2) Use the new OnRenderTargetChanged() overridable method. This method gets called each time a RenderTarget is created and destroyed. It is here that you can create and dispose your render resources. Documentation coming end of this week on next on how to use. I'll update this thread when its up.

[GaryAlbers]

OnRenderTargetChanged called too often?

Brett,

I added the OnRenderTargetChanged method to my sample indicator (see attached).
It did stop the crash - Yeah!

However, I was surprised at how many times the RenderTarget is changing.
It appeared to change every click on the chart plus the other events you mentioned.
That will be a lot of disposing and recreating of those ShaprDX resources.

Is this expected?

Thanks,
Gary

[GaryAlbers]

DX resource lifecycle

Brett,

Are there any other DX resources that need to be disposed and then recreated each time OnRenderTargetChanged is called, such as:
- SharpDX.Direct2D1.Brush - yes
- SharpDX.Vector2 - ?
- SharpDX.RectangleF - ?
- SharpDX.DirectWrite.TextFormat - ?
- SharpDX.DirectWrite.TextLayout - ?

In general, we really need to know the required life cycle of each DX resource
- Should we cache them?
- Should we create and dispose per OnRender?

Thanks,
Gary

[NinjaTrader_Brett]

Expected since the other render target is used for hit testing which occurs when you click the mouse.

You should always try to to cache to save on performance but sometimes you just can't due to some dynamic rendering use case in which case you instantiate and dispose per OnRender call.

[-=Edge=-]

I added the OnRenderTargetChanged method to my sample indicator (see attached). It did stop the crash - Yeah!

Now that Brett has mentioned the OnRenderTargetChanged(), I'm trying to get my head around when/why/where this would be more appropriate and/or efficient to use versus OnRender.. Hopefully the upcoming documentation will help make more sense of that..

I'm sure your example was a greatly simplified version, and I probably don't fully understand your ultimate goal here, but is there a specific reason you need to assign your changing colors to a SharpDX Brush, and not just use a System.Windows.Media Brush for that? Then in onrender, assign a dx brush to use that Media Brush?

Would this simplified example work for you without all those other steps?

[GaryAlbers]

Edge,

I believe your code is partially using option 1 of Brett's 2 suggested alternatives (post 4). It is reusing the Media.Brush but it is creating a new DXBrush in every call to OnRender.
If I understand this right, it would be better to cache those also.

Also, I believe your solution does not address the original exception I experienced,
which was caused by the RenderTarget changing and the previous render resources not being cleaned up. In my case, any time this indicator was on the chart and I clicked on the top tool bar, I would get an exception with error dialog.

Not sure what you meant by using OnRenderTargetChanged VERSUS OnRender.
I heard Brett to mean that I have to use them both.
My code now creates Media.Brushes one time. Then every time OnRenderTargetChanged is called, create a new DXBrush (per Media.Brush) and dispose of the old ones.
OnRender just uses the DXBrush now (does not create them).

I am still early in working all this out, but appear to be on the right track, 1 less crash

Gary

[-=Edge=-]

I believe your code is partially using option 1 of Brett's 2 suggested alternatives (post 4). It is reusing the Media.Brush but it is creating a new DXBrush in every call to OnRender. If I understand this right, it would be better to cache those also.

Both a new media and dx brush are creating just once in the variables, and then the media brush reused to conditionally assign the color in OnBarUpdate, and the dx brush reused (or reassigned) on each render.. I guess you could conditionally only reassign the dx brushes color if it had changed, but not sure that would help the performace a great deal.. Although I could be wrong.. Since the dx brush is only created once, you'll notice I only dispose of it once in the State.Terminated..

Also, I believe your solution does not address the original exception I experienced, which was caused by the RenderTarget changing and the previous render resources not being cleaned up. In my case, any time this indicator was on the chart and I clicked on the top tool bar, I would get an exception with error dialog.

Agreed, but I believe, and again could be wrong, you were only experiencing that error because of the way you were assigning the dx brush color with RenderTarget outside of OnRender..

Not sure what you meant by using OnRenderTargetChanged VERSUS OnRender. I heard Brett to mean that I have to use them both.

I don't, and have not so far, and guess if I'm doing it wrong and/or inefficiently, I could use some additional guidance on this topic..

[NinjaTrader_Brett]

Documentation is coming but basically this style coding will do all the magic for you. It disposes the brush on shutdown and creates the brush on start up for each render target.

private SharpDX.Direct2D1.Brush myBrush;

	public override void OnRenderTargetChanged()
		{
			if (myBrush != null)
				myBrush.Dispose();
			
			if (RenderTarget != null)
				myBrush = Brushes.Red.ToDxBrush(RenderTarget);
		}
		
protected override void OnRender(ChartControl chartControl, ChartScale chartScale)
		{
			RenderTarget.FillRectangle(new SharpDX.RectangleF(ChartPanel.X, ChartPanel.Y, ChartPanel.W, ChartPanel.H), myBrush);
		}

[GaryAlbers]

Edge,

Just to be clear, what I meant by creating a new DXBrush everytime in your OnRender code is this:

myBrushDX = myBrush.ToDxBrush(RenderTarget);

I believe this will create a new one each call.
I hope the upcoming documentation confirms or denies this belief.

I guess you could conditionally only reassign the dx brushes color if it had changed, but not sure that would help the performance a great deal.

It may not be a measurable performance hit but I am also wanting to be careful about garbage collection (windows handles, graphics objects, etc not getting cleaned up). I am not sure if garbage collection cleans them up completely or if they need proactive disposal. If they need disposal, but it's not done, then it could be a memory leak.

Agreed, but I believe, and again could be wrong, you were only experiencing that error because of the way you were assigning the dx brush color with RenderTarget outside of OnRender.

Creating DX resources in the method OnRenderTargetChanged means, by definition, we are creating them outside of OnRender. I understood the problem to be which instance of RenderTarget is used to create the resource, since Brett described the scenario where the instance of the RenderTarget changes.

Gary

[-=Edge=-]

Just to be clear, what I meant by creating a new DXBrush everytime in your OnRender code is this:

myBrushDX = myBrush.ToDxBrush(RenderTarget);

I believe this will create a new one each call.

hmmm.. I hope not, but wouldn't be the first (or last) time I was wrong..

Creating DX resources in the method OnRenderTargetChanged means, by definition, we are creating them outside of OnRender. I understood the problem to be which instance of RenderTarget is used to create the resource, since Brett described the scenario where the instance of the RenderTarget changes.

I was refereeing to the very first indicator code (post #3) that was getting the exception where the dx brush was being assigned in the OnBarUpdate..

[-=Edge=-]

My suggestion here, so that I don't totally confuse myself, is that NT revise the SampleCustomPlot indicator for efficiency, performance, and coding best practices..

This would definitely help clear up any confusion I might have on my part..