Bugs i found recently

Tue Nov 5 08:17:46 UTC 2013

On Tue, Nov 5, 2013 at 10:28 AM, Dirk Hohndel <dirk at hohndel.org> wrote:

> On Tue, 2013-11-05 at 09:33 -0500, Tomaz Canabrava wrote:
>
> >
> >         > Some easy and straight forward ones:
> >         > * When pressing "ok" after edited a dive, all the open trips
> >         in
> >         >   the dive list gets folded. This is quite annoying, because
> >         it moves
> >         >   the scroll to.
> >
> >
> >         This seems to be caused by us calling DiveListView::reload()
> >         from
> >         MainWindow::refreshDisplay() - doing so loses the 'expanded'
> >         state of
> >         trips.
> >
> >         When playing around with this I found that we also always fell
> >         back to
> >         sorting by column 0 in descending order. I was able to fix
> >         that one, but
> >         got lost trying to figure out how to remember the expanded
> >         state for the
> >         trips.
> >
> >         We have a bit for that in our data structures, but when trying
> >         to
> >         connect to the expanded() signal of the TreeView in order to
> >         keep that
> >         bit in sync with what's displayed... I still have a lot to
> >         learn about
> >         Qt programming, I guess :-/
> >
> Tomaz - can you help with this one, too? We rebuild the divelist because
> editing may have changed things around - I'm actually not 100% sure
> that's even necessary, but in the past that has been the way we went
> because there were too many odd corner cases.
>
> So we have two directions we could go here...
> a) figure out how to maintain the 'expanded' state of trips across the
> rebuild
> b) figure out if we can avoid recreating the model and just make sure
> that it updates correctly
>
> >         > * Merge selected dives only merges two of tree selected. Now
> >         when i
> >         >   tested with 5 selected dives it merged the first tree of
> >         them.
> >
> >
> >         Urgs. This was a subtle bug. When looping over the dive list
> >         with
> >         for_each_dive we increment 'i' in every step - but if two
> >         dives get
> >         merged, then the indices of all following dives change (we
> >         have one
> >         fewer dive in the list after all).
> >
> >         Playing with that while testing showed that there still are
> >         bugs hidden
> >         in the dive merging code if you merge dives that shouldn't be
> >         merged.
> >         E.g. when the dives are not close together (I think we should
> >         just
> >         refuse to merge those), but there appears to also be an issue
> >         with
> >         getting the tank pressure in the samples correct. If the
> >         second dive is
> >         on a fresh tank (which basically means that most likely you
> >         shouldn't
> >         have merged these two dives, right?) then that isn't correctly
> >         reflected
> >         in the merged dive.
> >
> >         > * Unable to remove pressure information from a cylinder. The
> >         old value
> >         >   comes back.
> >         > * When rescaling the window and the different pains, the
> >         >   information-overlay can end up off-screen.
> >
> >
> >         Those two sound very much like something that Tomaz should
> >         look into :-)
> >
> >
> >
> >
> > Fixed. :)
>
> Thank you. Where can I pull the fix? :-)
> (not that I'm likely to do that right now - it's 12:30am and I just
> arrived in Palau after 24h of travel)
>
> >
> >         > And a couple of harder:
> >         > * multiple times i noted that the ui just went bad, and
> >         refused to
> >         >   switch between dives, or kept showing the divetrip info
> >         instead of the
> >         >   dive pain. This happened after working with the ui for 10+
> >         minutes.
> >
> >
> >         That's scary.
> >
> >         > * I noted that either via selecting multiple dives or some
> >         bug i
> >         >   accidentally edited the wrong dives, and wrote over other
> >         dives. This
> >         >   might be due to ui messing up the selections, or due to
> >         other bugs.
> >         >   A problem here is that the ui doesn't give you any
> >         feedback when you
> >         >   are actually editing multiple dives.
> >
> >
> >         Even more scary. That's the type of bugs I really want to get
> >         rid of
> >         before a release. I know this sucks, but can you try and
> >         create a
> >         sequence of steps that reproduces this? Ideally with the test
> >         dives so
> >         that the developers can reproduce it? Or you can do this with
> >         your dive
> >         file if you are welling to share that with us.
> >
> >         I stressed Subsurface with my medium sized XML file for a
> >         while (I have
> >         about 260 dives in it, but the file is large and complex as
> >         many of
> >         these dives have multiple dive computers) and haven't managed
> >         to get
> >         things out of sync in a way that corrupted data.
> >
> >         But I agree with you that we need visual feedback when editing
> >         multiple
> >         dives. So I implemented at least that.
> >
> >
> >
> > One thing that can help finding those bugs:
> >
> > Since we are working with Qt model system, we *must* not select or
> > deselect dives manually, but we need to ask the model to desselect it.
> > ( this is also true on the Gtk Version, not qt related, but model view
> > related. )
> >
> >
> > When one part of the code 'select_dive' and that doesn't update the
> > Model, the current selection for the user is what's painted on the
> > dive list, not what's correctly selected via code, so the user edits
> > something thinking that he's editing the *visual selection* whereas
> > he's really editing the newly dive created by add dive. :)
>
>
> > I'v found a couple of places that a 'select_dive' or 'unselect_dive'
> > is being used outside of the model control, and this is bad and can
> > cause this kind of wrong behavior.
> >
> >
>
> Is there a helper function we should call to make sure things stay in
> sync? That's usually the easiest way to ensure we have a consistent view
> everywhere.
> >
> >         > And one missing feature that we had in the gtk version:
> >         > * Ability to add gaschange and bookmark events to a dive.
> >
> >
> >         I vaguely remember having started to work at this at some
> >         point... but
> >         clearly nothing ever came of it. Something for the todo list.
> >
>
> I'm still trying to figure out what happened to the code that I wrote to
> implement this...
>
> /D
>

Dirk and All,

This two weeks before the beta, the only thing that I'm doing at nights is
Subsurface, so yes.. I'll try to help on every bug that I can put my hands
on.
:)

Tomaz
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.hohndel.org/pipermail/subsurface/attachments/20131105/d448e209/attachment-0001.html>