search

alexrainman / CarouselView

CarouselView control for Xamarin Forms
MIT License
436 stars 176 forks source link

extra DataTemplate instantiations on iOS #158

Closed fschwiet closed 7 years ago

fschwiet commented 7 years ago

I created a branch with tracing to highlight the issue: https://github.com/fschwiet/CarouselView/tree/view-double-constructed

Repro steps:

  1. Open the demo app
  2. Click "Navigate".
  3. Count the number of times "!!!!!! PhotoUrl2 constructor called." shows up in the application output.

I can understand if this doesn't seem like a big deal, really its just a slight performance cost in most scenarios. In my scenario though I am using OxyPlot in the first DataTemplate and OxyPlot won't let you bind its PlotModel to more than one view, so I get an exception. There is a work-around to make it work in OxyPlot, but its complicated and I'd rather not. OxyPlot has had this restriction for a long time so I doubt they're going to change their behavior now (https://github.com/oxyplot/oxyplot-xamarin/issues/60).

alexrainman commented 7 years ago

There's no way to avoid this. In iOS, SizeChanged is called multiple times. Sometimes one time, sometimes two, and that's something i cannot control.

alexrainman commented 7 years ago

Also, internally UIPageViewController creates the next page for smooth swiping and that's what you maybe seeing.

alexrainman commented 7 years ago

Found a way to prevent multiple calls. Next release.

fschwiet commented 7 years ago

Thank you Alex.

Also, internally UIPageViewController creates the next page for smooth swiping and that's what you maybe seeing.

This sounds ok, as long as the instantiations for each DataTemplate don't overlap.

I did hit the crash in my project on Android, indicating one of the DataTemplates is being created twice. I haven't been able to repro it with the test project though. So I may still need to use the oxyplot workaround.

It makes me wonder if it'd be better (I don't think its possible now) to just pass in an array of Views instead of DataTemplates (in this case the number of views is quite small). I might give that a try, it seems like I could just make a change to where ever the current code checks for a DataTemplate.