RE: [XaraXtreme-dev] Abort on arrow keypress

["Phil Martin" <Phil@xxxxxxxx>]

Yes, with your fix, CurrentSpread is only changed by derived transform
classes when they are handling dragging so a non-drag transform will
never change it (or StartSpread) and RecordSelectionTwice will always be
FALSE.

But it would be slightly better if that were more explicit and I'll look
into that.

Phil 

> -----Original Message-----
> From: owner-dev@xxxxxxxxxxxxxxxx 
> [mailto:owner-dev@xxxxxxxxxxxxxxxx] On Behalf Of Gerry Iles
> Sent: 28 June 2006 17:06
> To: dev@xxxxxxxxxxxxxx
> Subject: RE: [XaraXtreme-dev] Abort on arrow keypress
> 
> Oh yeah... I was looking at the Xtreme source when I thought 
> that it shouldn't ever get past the BOOL check and noticed 
> that line in DoWithParam wasn't present in LX but didn't 
> check what was different elsewhere...
> 
> Are you saying that it should be working now as it is?
> 
> Gerry
> 
> -----Original Message-----
> From: owner-dev@xxxxxxxxxxxxxxxx 
> [mailto:owner-dev@xxxxxxxxxxxxxxxx] On Behalf Of Phil Martin
> Sent: 28 June 2006 17:01
> To: dev@xxxxxxxxxxxxxx
> Subject: RE: [XaraXtreme-dev] Abort on arrow keypress
> 
> Alex,
> 
> That flag is ancient history. I replaced it with a cleaner 
> virtual function. The transformation subclasses now say 
> whether they allow movement of nodes between spreads by 
> overriding CanChangeSpread().
> 
> During dragging that function determines whether StartSpread 
> and CurrentSpread differ and it's the comparison of 
> StartSpread and CurrentSpread that is the primary factor in 
> RecordSelectionTwice, the call to CanChangeSpread() is just a 
> belt-and-braces safety check.
> 
> Phil 
> 
> > -----Original Message-----
> > From: owner-dev@xxxxxxxxxxxxxxxx
> > [mailto:owner-dev@xxxxxxxxxxxxxxxx] On Behalf Of Gerry Iles
> > Sent: 28 June 2006 16:32
> > To: dev@xxxxxxxxxxxxxx
> > Subject: RE: [XaraXtreme-dev] Abort on arrow keypress
> > 
> > Looking at the code some more the following is missing from
> > DoWithParam:
> > 
> > // It is not possible to move between spreads in an immediate 
> > operation CanChangeToNewSpread = FALSE;
> > 
> > This is why RecordSelectionTwice is ending up as non-FALSE in 
> > CompleteTransformation...
> > 
> > I'll let you patch that one if it's your bug... :)
> > 
> > Gerry
> > 
> > -----Original Message-----
> > From: owner-dev@xxxxxxxxxxxxxxxx
> > [mailto:owner-dev@xxxxxxxxxxxxxxxx] On Behalf Of Phil Martin
> > Sent: 28 June 2006 16:26
> > To: dev@xxxxxxxxxxxxxx
> > Subject: RE: [XaraXtreme-dev] Abort on arrow keypress
> > 
> > Thanks Gerry, I was was just thinking about looking into 
> that problem 
> > myself (it's a P1 on my list).
> > 
> > The use of StartSpread and CurrentSpread in 
> CompleteTransformation is 
> > a recent change I made to handle dragging across spread 
> boundaries but 
> > obviously I upset non-drag transforms without realising it.
> > 
> > The TransOperation cosntructor has worked for years without 
> > initialising it's other variables so although it really 
> should do so, 
> > I don't think we'll find any new problems because of it.
> > 
> > Phil
> > 
> > > -----Original Message-----
> > > From: owner-dev@xxxxxxxxxxxxxxxx
> > > [mailto:owner-dev@xxxxxxxxxxxxxxxx] On Behalf Of Gerry Iles
> > > Sent: 28 June 2006 16:19
> > > To: dev@xxxxxxxxxxxxxx
> > > Subject: RE: [XaraXtreme-dev] Abort on arrow keypress
> > > 
> > > Well, logically, I don't see any reason for the code to call 
> > > Spread::FindActiveLayer when handling cursor keys as it 
> doesn't do 
> > > anything to the active layer (the active layer being the 
> layer that 
> > > newly created objects get put on).
> > > 
> > > Looking at the code in CompleteTransformation, it should only be 
> > > calling FindActiveLayer if the operation is moving the
> > selection from
> > > one spread to another.  This shouldn't happen in the text
> > cursor case
> > > (until we have flowing text stories that can span multiple
> > spreads :)
> > > and I wouldn't have thought the nudge operations would
> > allow it either
> > > (though presumably dragging with the mouse would if you
> > actually had
> > > more than one spread).
> > > 
> > > Looking at the constructor for TransOperation it doesn't
> > actually set
> > > all of its member variables sensibly.  I've just committed a fix
> > > (r1382) that sets the StartSpread and CurrentSpread to NULL
> > so that in
> > > release builds (when memory isn't auto-nulled on
> > allocation) the test
> > > in CompleteTransformation works correctly.
> > > 
> > > I haven't actually done a release build to test it but 
> this should 
> > > sort out this specific problem.  There may be other
> > problems caused by
> > > the other member variables not being initialised properly 
> and these 
> > > should really be sorted out too.
> > > 
> > > Gerry
> > > 
> > > -----Original Message-----
> > > From: owner-dev@xxxxxxxxxxxxxxxx
> > > [mailto:owner-dev@xxxxxxxxxxxxxxxx] On Behalf Of Martin Wuerthner
> > > Sent: 28 June 2006 15:30
> > > To: dev@xxxxxxxxxxxxxx
> > > Subject: [XaraXtreme-dev] Abort on arrow keypress
> > > 
> > > When I press an arrow key (either to nudge an object or 
> to move the 
> > > cursor in a text object) XaraLX aborts with a SIGSEGV. 
> This always 
> > > happens in release builds, but not in debug builds. The
> > abort happens
> > > in Spread::FindActiveLayer at
> > > node.h:1137 but also at other places inside this routine. 
> > It appears
> > > that the this pointer of the FindActiveLayer method call is
> > invalid. 
> > > The caller is TransOperation::CompleteTransformation.
> > > 
> > > Interestingly, in a debug build, Spread::FindActiveLayer is
> > not even
> > > called when handling an arrow key, neither when nudging an
> > object nor
> > > when moving the cursor.
> > > 
> > > So, something is seriously wrong with the program flow
> > earlier on, but
> > > only in release builds.
> > > 
> > > This is with the current svn version (1381), build on SUSE
> > 10.0 using
> > > gcc 4.02.
> > > 
> > > Martin
> > > 
> > 
>