telerik / UI-For-UWP

This repo contains the source code for Telerik UI for Universal Windows Platform (UWP), which includes 20+ UI controls for developers building UWP applications.
http://www.telerik.com/uwp/
Other
1.16k stars 234 forks source link

Broken Cartesian chart lines #141

Open VitalyKnyazev opened 7 years ago

VitalyKnyazev commented 7 years ago

After upgrade to v1.0.0.4 Cartesian chart lines are rendered with gaps on ARM devices:

1 2

APopatanasov commented 7 years ago

Hi @VitalyKnyazev ,

We were able to reproduce the described by you visualization issue of the LineSeries of the Chart. It is caused by the new rendering mechanism that is using the Composition API - more specifically by the "PrepareLineRenderVisual" method and the way the lines are rotated. This could be used as a start point for fixing this issue. If you are interested on fixing this issue you could send us a pull request with a suggested fix and we will be glad to review it.

As a workaround you could disable that rendering mechanism by creating a custom ContainerVisualFactory and simply prevent drawing the Lines with the Composition API: public class CustomContainerVisualFactory : ContainerVisualsFactory { protected override bool CanDrawContainerVisual(PresenterBase visualElement) { if (visualElement is LineSeries) { return false; } return base.CanDrawContainerVisual(visualElement); } } and in XAML: `

</telerikChart:RadCartesianChart.ContainerVisualsFactory>`

VitalyKnyazev commented 7 years ago

Wow! After looking at PrepareLineRenderVisual I have noticed numerous Count() calls throughout the code. Now I can safely add another observation to above issue: v1.0.0.4 was also much slower when rendering charts with thousands of points. Count() is a very expensive operation, especially when called twice from within "for" loop on a hot path.

APopatanasov commented 7 years ago

Thanks for your feedback. We really appreciate it.

We agree that the method could be improved and will be grateful for any help (if you have interest on working on this issue) via pull requests that could resolve the issue or simply to improve the current implementation. The mentioned method will be of great help and could be used as a start point for anybody interested on fixing the issue.

The Composition API is only used for rendering some of the Series and could easily be stopped by using the mentioned above approach if you are looking for a workaround. Again thanks for bringing this to our attention.

VitalyKnyazev commented 7 years ago

Hi Atanas, I think chart drawing issue is not so easy, at least I don't know how to fix it. For now I just created PR #146 to improve PrepareLineRenderVisual performance and improved overall LINQ usage. See also line 71 in LineRenderer, probably copy/paste error. Once this PR is approved I will continue reviewing code for other potential improvements.

APopatanasov commented 7 years ago

Hi @VitalyKnyazev ,

Thanks a lot for you collaboration. I will need some time to fully check the pull-request you have made - if there are some issues with it I will write you back in the PR. In the meantime could you please read the Contributor License Agreement and sign it - without signing the agreement I could not approve your request.

As for the drawing issue I agree that it is a little bit tricky. As currently the Composition API provides only SpriteVisual elements (which are basically rectangles) filling the observed gaps may require some precise calculations for the placement and rotations of the points and there isn't even a guarantee that all gaps could be filled. Your collaborations on fixing it will be highly appreciated.