Feedback Subsurface-mobile 2.2.3 (4.9.3.258) for iOS

[Dirk Hohndel]

On Mon, Nov 04, 2019 at 01:16:55PM +0100, Berthold Stoeger wrote:
> On Montag, 4. November 2019 12:47:48 CET Dirk Hohndel wrote:
> > The old version accessed data through pointers that weren't guaranteed to be
> > valid which caused random crashes. We now access the data from the model
> > which simply causes more overhead. I'm still looking for ways to make this
> > faster, but realistically it needs a redesign
> 
> I'm not sure that I agree. For every field, the old version went to the model 
> and got a DiveObjectHelper from which it extracted the display data. What has 
> changed is that now the string is extracted by the model itself and passed to 
> QML. There should be the same number of model-lookups, just the strings being 
> extracted at different levels.

We had several versions of this and on iOS we were previously on a much
older release (I haven't updated the code there in quite a while). So I
think this was still the version that just cached a DiveObjectHelper array
and used that to access data.

Anyway, this whole mess is quite frustrating to me.
 
> I see no fundamental reason that the new version has to be slower and we 
> should try to figure out what is happening.

I wish I knew. Profiling doesn't seem to be all that insightful. But I'm
happy to hear what others come up with. But regardless - the
implementation of dive trip folding is just braindead. We always have a
model that includes ALL of your dives. And we set the height of all the
dives taht aren't shown to 0 (and the dive invisible). So if you have 20
trips on your screen with on average 15 dives, we actually have 320
elements on the screen, just almost all of them invisible.

> Concerning redesign: Is there a reason that a ListView was chosen over a 
> TreeView (which seems like the more logical choice to me)?

The initial implementation had only a flat dive list, no trips. And by the
time I got around to implement trips we had tried to move to QtQuick
Controls2, and for reasons that are beyond me, that one doesn't probide a
TreeView (yet? - there's no sign of it in Qt 5.14, either).

So my thought was to create a new DiveList implementation as a ListView
based on a model that contains only the dive trips - and that allowed
insertion and removal of the dives for exactly one expanded trip into that
model. I started coding this quite a while ago and never really got
anywhere. I think at this point this would have to be a complete restart.

/D