Re: Todays 1.6svn crash in various ways when opening existing documents

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Todays 1.6svn crash in various ways when opening existing documents

Abdelrazak Younes
Martin Vermeer wrote:

> On Fri, Nov 02, 2007 at 12:45:49PM +0100, Helge Hafting wrote:
>>  Start lyx, open a "recent document" - assert.
>>
>>  Case one (A11.lyx, beamer presentation)
>
> ...
>  
>>  Program received signal SIGSEGV, Segmentation fault.
>>  [Switching to Thread 0xa62949b0 (LWP 10363)]
>>  0x0832ec16 in lyx::InsetCollapsable::dimensionCollapsed (this=0x8c5a5e8)
>>     at ../../lyx-devel/src/insets/InsetCollapsable.cpp:178
>>  178                     layout_.labelstring, dim.wid, dim.asc, dim.des);
>>  (gdb) bt
>>  #0  0x0832ec16 in lyx::InsetCollapsable::dimensionCollapsed (this=0x8c5a5e8)
>>     at ../../lyx-devel/src/insets/InsetCollapsable.cpp:178
>>  #1  0x083301ef in lyx::InsetCollapsable::metrics (this=0x8c5a5e8,
>>     mi=@0xafab90c0, dim=@0xafab91dc)
>>     at ../../lyx-devel/src/insets/InsetCollapsable.cpp:218
>>  #2  0x0834eca8 in lyx::InsetFootlike::metrics (this=0x8c5a5e8,
>>  mi=@0xafab90c0,
>
> This is similar to the earlier reported bug for ERT.
>
> From memory (I am travelling) the fix is to replace the call
> in the constructor to InsetCollapse(bp) or ...(bp, status)
> by ...(bp, collapse). In ERT, Footlike, perhaps more.
>
> There is something fishy here that needs fixing at a more
> fundamental level, but please try this first as I cannot do
> much from here now.

OK, I've done some cleanup an the crashes are gone but the inset layouts
are broken!

Martin, Richard, do you have an idea here? I don't know if we shall we
fix Inset::getLayout() or if we shall assume that each inset hard-code
its own layout.

Help please,
Abdel.


Author: younes
Date: Fri Nov  2 18:47:51 2007
New Revision: 21382

URL: http://www.lyx.org/trac/changeset/21382
Log:
* InsetCollapsable:
- InsetCollapsable(): Move labelfont initialisation to
InsetCollapsable::setLayout().
- read(): reset the inset layout.

All other insets: get rid of redundant setLayout() calls.

This commit fixes the multiple crashes in trunk but the color used for
text and background are completely wrong...

Modified:
     lyx-devel/trunk/src/insets/InsetBox.cpp
     lyx-devel/trunk/src/insets/InsetBranch.cpp
     lyx-devel/trunk/src/insets/InsetCollapsable.cpp
     lyx-devel/trunk/src/insets/InsetERT.cpp
     lyx-devel/trunk/src/insets/InsetFoot.cpp
     lyx-devel/trunk/src/insets/InsetIndex.cpp
     lyx-devel/trunk/src/insets/InsetListings.cpp
     lyx-devel/trunk/src/insets/InsetMarginal.cpp
     lyx-devel/trunk/src/insets/InsetNote.cpp

Modified: lyx-devel/trunk/src/insets/InsetBox.cpp
URL:
http://www.lyx.org/trac/file/lyx-devel/trunk/src/insets/InsetBox.cpp?rev=21382
==============================================================================
--- lyx-devel/trunk/src/insets/InsetBox.cpp (original)
+++ lyx-devel/trunk/src/insets/InsetBox.cpp Fri Nov  2 18:47:51 2007
@@ -94,7 +94,6 @@
  InsetBox::InsetBox(BufferParams const & bp, string const & label)
  : InsetCollapsable(bp), params_(label)
  {
- setLayout(bp);
  init();
  }

@@ -135,7 +134,6 @@
  {
  params_.read(lex);
  InsetCollapsable::read(buf, lex);
- setLayout(buf.params());
  setButtonLabel();
  }


Modified: lyx-devel/trunk/src/insets/InsetBranch.cpp
URL:
http://www.lyx.org/trac/file/lyx-devel/trunk/src/insets/InsetBranch.cpp?rev=21382
==============================================================================
--- lyx-devel/trunk/src/insets/InsetBranch.cpp (original)
+++ lyx-devel/trunk/src/insets/InsetBranch.cpp Fri Nov  2 18:47:51 2007
@@ -45,7 +45,6 @@
  InsetBranchParams const & params)
  : InsetCollapsable(bp), params_(params)
  {
- setLayout(bp);
  init();
  }

@@ -86,8 +85,6 @@
  {
  params_.read(lex);
  InsetCollapsable::read(buf, lex);
- setLayout(buf.params());
- setButtonLabel();
  }


@@ -137,7 +134,6 @@
  InsetBranchMailer::string2params(to_utf8(cmd.argument()), params);
  params_.branch = params.branch;
  setLayout(cur.buffer().params());
- setButtonLabel();
  break;
  }


Modified: lyx-devel/trunk/src/insets/InsetCollapsable.cpp
URL:
http://www.lyx.org/trac/file/lyx-devel/trunk/src/insets/InsetCollapsable.cpp?rev=21382
==============================================================================
--- lyx-devel/trunk/src/insets/InsetCollapsable.cpp (original)
+++ lyx-devel/trunk/src/insets/InsetCollapsable.cpp Fri Nov  2 18:47:51 2007
@@ -82,14 +82,7 @@
  setDrawFrame(true);
  setFrameColor(Color_collapsableframe);
  setButtonLabel();
- // Fallback for lacking inset layout item
- layout_.bgcolor = Color_background;
-
- // FIXME: it seems some insets don't properly initialise that!
- layout_.labelfont = sane_font;
- layout_.labelfont.decSize();
- layout_.labelfont.decSize();
- layout_.labelfont.setColor(Color_collapsable);
+ setLayout(bp);
  }


@@ -110,7 +103,25 @@

  void  InsetCollapsable::setLayout(BufferParams const & bp)
  {
+ // Fallback for lacking inset layout item
+ layout_.bgcolor = Color_background;
+
+ // FIXME: it seems the default background is red!
  layout_ = getLayout(bp);
+
+
+ // FIXME: it seems the provided font is partly realized... so we
+ // re-initialize the label font in any case.
+ /*
+ if (layout_.labelfont != inherit_font)
+ return;
+ */
+
+ // FIXME: it seems some insets don't properly initialise that...
+ layout_.labelfont = sane_font;
+ layout_.labelfont.decSize();
+ layout_.labelfont.decSize();
+ layout_.labelfont.setColor(Color_collapsable);
  }


@@ -165,6 +176,7 @@
  status_ = isOpen() ? Open : Collapsed;

  setButtonLabel();
+ setLayout(buf.params());

  // Force default font, if so requested
  // This avoids paragraphs in buffer language that would have a

Modified: lyx-devel/trunk/src/insets/InsetERT.cpp
URL:
http://www.lyx.org/trac/file/lyx-devel/trunk/src/insets/InsetERT.cpp?rev=21382
==============================================================================
--- lyx-devel/trunk/src/insets/InsetERT.cpp (original)
+++ lyx-devel/trunk/src/insets/InsetERT.cpp Fri Nov  2 18:47:51 2007
@@ -61,7 +61,6 @@
  InsetERT::InsetERT(BufferParams const & bp, CollapseStatus status)
  : InsetCollapsable(bp, status)
  {
- setLayout(bp);
  init();
  }


Modified: lyx-devel/trunk/src/insets/InsetFoot.cpp
URL:
http://www.lyx.org/trac/file/lyx-devel/trunk/src/insets/InsetFoot.cpp?rev=21382
==============================================================================
--- lyx-devel/trunk/src/insets/InsetFoot.cpp (original)
+++ lyx-devel/trunk/src/insets/InsetFoot.cpp Fri Nov  2 18:47:51 2007
@@ -36,9 +36,7 @@

  InsetFoot::InsetFoot(BufferParams const & bp)
  : InsetFootlike(bp)
-{
- setLayout(bp);
-}
+{}


  InsetFoot::InsetFoot(InsetFoot const & in)

Modified: lyx-devel/trunk/src/insets/InsetIndex.cpp
URL:
http://www.lyx.org/trac/file/lyx-devel/trunk/src/insets/InsetIndex.cpp?rev=21382
==============================================================================
--- lyx-devel/trunk/src/insets/InsetIndex.cpp (original)
+++ lyx-devel/trunk/src/insets/InsetIndex.cpp Fri Nov  2 18:47:51 2007
@@ -30,9 +30,7 @@

  InsetIndex::InsetIndex(BufferParams const & bp)
  : InsetCollapsable(bp)
-{
- setLayout(bp);
-}
+{}


  InsetIndex::InsetIndex(InsetIndex const & in)

Modified: lyx-devel/trunk/src/insets/InsetListings.cpp
URL:
http://www.lyx.org/trac/file/lyx-devel/trunk/src/insets/InsetListings.cpp?rev=21382
==============================================================================
--- lyx-devel/trunk/src/insets/InsetListings.cpp (original)
+++ lyx-devel/trunk/src/insets/InsetListings.cpp Fri Nov  2 18:47:51 2007
@@ -59,7 +59,6 @@
  InsetListings::InsetListings(BufferParams const & bp,
InsetListingsParams const & par)
  : InsetCollapsable(bp, par.status())
  {
- setLayout(bp);
  init();
  }


Modified: lyx-devel/trunk/src/insets/InsetMarginal.cpp
URL:
http://www.lyx.org/trac/file/lyx-devel/trunk/src/insets/InsetMarginal.cpp?rev=21382
==============================================================================
--- lyx-devel/trunk/src/insets/InsetMarginal.cpp (original)
+++ lyx-devel/trunk/src/insets/InsetMarginal.cpp Fri Nov  2 18:47:51 2007
@@ -25,9 +25,7 @@

  InsetMarginal::InsetMarginal(BufferParams const & bp)
  : InsetFootlike(bp)
-{
- setLayout(bp);
-}
+{}


  InsetMarginal::InsetMarginal(InsetMarginal const & in)

Modified: lyx-devel/trunk/src/insets/InsetNote.cpp
URL:
http://www.lyx.org/trac/file/lyx-devel/trunk/src/insets/InsetNote.cpp?rev=21382
==============================================================================
--- lyx-devel/trunk/src/insets/InsetNote.cpp (original)
+++ lyx-devel/trunk/src/insets/InsetNote.cpp Fri Nov  2 18:47:51 2007
@@ -116,7 +116,6 @@
  : InsetCollapsable(bp)
  {
  params_.type = notetranslator().find(label);
- setLayout(bp);
  setButtonLabel();
  }

@@ -175,8 +174,6 @@
  {
  params_.read(lex);
  InsetCollapsable::read(buf, lex);
- setLayout(buf.params());
- setButtonLabel();
  }


@@ -202,7 +199,6 @@
  InsetNoteMailer::string2params(to_utf8(cmd.argument()), params_);
  // get a bp from cur:
  setLayout(cur.buffer().params());
- setButtonLabel();
  break;

  case LFUN_INSET_DIALOG_UPDATE:



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Todays 1.6svn crash in various ways when opening existing documents

Abdelrazak Younes
Abdelrazak Younes wrote:

> OK, I've done some cleanup an the crashes are gone but the inset layouts
> are broken!
>
> Martin, Richard, do you have an idea here? I don't know if we shall we
> fix Inset::getLayout() or if we shall assume that each inset hard-code
> its own layout.

OK, I see that the font definitions comes from 'lib/stdinsets.inc'. For
example we have this for footnotes:

InsetLayout Foot
        LabelString           foot
        LatexType             command
        LatexName             footnote
        Font
          Color               foreground
          Size                Small
          Family              Roman
          Shape               Up
          Series              Medium
          Misc                No_Emph
          Misc                No_Noun
          Misc                No_Bar
        EndFont
        LabelFont
          Color               Green
          Size                Small
        EndFont
        MultiPar              true
End

It seems that this is not properly parsed then. I am going to remove the
work around in the code so that we can fix the parsing instead.

Abdel.

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Todays 1.6svn crash in various ways when opening existing documents

Martin Vermeer-2
In reply to this post by Abdelrazak Younes
On Fri, Nov 02, 2007 at 06:51:54PM +0100, Abdelrazak Younes wrote:

>  Martin Vermeer wrote:
> > On Fri, Nov 02, 2007 at 12:45:49PM +0100, Helge Hafting wrote:
> >>  Start lyx, open a "recent document" - assert.
> >>
> >>  Case one (A11.lyx, beamer presentation)
> > ...  
> >>  Program received signal SIGSEGV, Segmentation fault.
> >>  [Switching to Thread 0xa62949b0 (LWP 10363)]
> >>  0x0832ec16 in lyx::InsetCollapsable::dimensionCollapsed (this=0x8c5a5e8)
> >>     at ../../lyx-devel/src/insets/InsetCollapsable.cpp:178
> >>  178                     layout_.labelstring, dim.wid, dim.asc, dim.des);
> >>  (gdb) bt
> >>  #0  0x0832ec16 in lyx::InsetCollapsable::dimensionCollapsed
> >> (this=0x8c5a5e8)
> >>     at ../../lyx-devel/src/insets/InsetCollapsable.cpp:178
> >>  #1  0x083301ef in lyx::InsetCollapsable::metrics (this=0x8c5a5e8,
> >>     mi=@0xafab90c0, dim=@0xafab91dc)
> >>     at ../../lyx-devel/src/insets/InsetCollapsable.cpp:218
> >>  #2  0x0834eca8 in lyx::InsetFootlike::metrics (this=0x8c5a5e8,  
> >> mi=@0xafab90c0,
> > This is similar to the earlier reported bug for ERT.
> > From memory (I am travelling) the fix is to replace the call
> > in the constructor to InsetCollapse(bp) or ...(bp, status)
> > by ...(bp, collapse). In ERT, Footlike, perhaps more.
> > There is something fishy here that needs fixing at a more
> > fundamental level, but please try this first as I cannot do
> > much from here now.
>
>  OK, I've done some cleanup an the crashes are gone but the inset layouts are
>  broken!
>
>  Martin, Richard, do you have an idea here? I don't know if we shall we fix
>  Inset::getLayout() or if we shall assume that each inset hard-code its own
>  layout.
>
>  Help please,
>  Abdel.

Abdel,

this is precisely the way _not_ to do it. The calls to
setLayout() in the various insets are designed (by Jean-Marc
during the Bromarv meeting) to get precisely the right
insetlayout based on the inset's name().

I fought with this fruitlessly until Jean-Marc showed me. You
reverted to my old non-working code :-(

Also, you shouldn't modify layout_ from within the inset.
(can this be enforced by const?).

Can you not simply fully realize the labelfont (or a  local
copy) before using it in dimensionCollapsed()? A minimal fix.

- Martin

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Todays 1.6svn crash in various ways when opening existing documents

Abdelrazak Younes
Martin Vermeer wrote:

> On Fri, Nov 02, 2007 at 06:51:54PM +0100, Abdelrazak Younes wrote:
>>  Martin Vermeer wrote:
>>> On Fri, Nov 02, 2007 at 12:45:49PM +0100, Helge Hafting wrote:
>>>>  Start lyx, open a "recent document" - assert.
>>>>
>>>>  Case one (A11.lyx, beamer presentation)
>>> ...  
>>>>  Program received signal SIGSEGV, Segmentation fault.
>>>>  [Switching to Thread 0xa62949b0 (LWP 10363)]
>>>>  0x0832ec16 in lyx::InsetCollapsable::dimensionCollapsed (this=0x8c5a5e8)
>>>>     at ../../lyx-devel/src/insets/InsetCollapsable.cpp:178
>>>>  178                     layout_.labelstring, dim.wid, dim.asc, dim.des);
>>>>  (gdb) bt
>>>>  #0  0x0832ec16 in lyx::InsetCollapsable::dimensionCollapsed
>>>> (this=0x8c5a5e8)
>>>>     at ../../lyx-devel/src/insets/InsetCollapsable.cpp:178
>>>>  #1  0x083301ef in lyx::InsetCollapsable::metrics (this=0x8c5a5e8,
>>>>     mi=@0xafab90c0, dim=@0xafab91dc)
>>>>     at ../../lyx-devel/src/insets/InsetCollapsable.cpp:218
>>>>  #2  0x0834eca8 in lyx::InsetFootlike::metrics (this=0x8c5a5e8,  
>>>> mi=@0xafab90c0,
>>> This is similar to the earlier reported bug for ERT.
>>> From memory (I am travelling) the fix is to replace the call
>>> in the constructor to InsetCollapse(bp) or ...(bp, status)
>>> by ...(bp, collapse). In ERT, Footlike, perhaps more.
>>> There is something fishy here that needs fixing at a more
>>> fundamental level, but please try this first as I cannot do
>>> much from here now.
>>  OK, I've done some cleanup an the crashes are gone but the inset layouts are
>>  broken!
>>
>>  Martin, Richard, do you have an idea here? I don't know if we shall we fix
>>  Inset::getLayout() or if we shall assume that each inset hard-code its own
>>  layout.
>>
>>  Help please,
>>  Abdel.
>
> Abdel,
>
> this is precisely the way _not_ to do it. The calls to
> setLayout() in the various insets are designed (by Jean-Marc
> during the Bromarv meeting) to get precisely the right
> insetlayout based on the inset's name().

Year I noticed that... calling virtual methods in ctors is dangerous and
should be avoided.

>
> I fought with this fruitlessly until Jean-Marc showed me. You
> reverted to my old non-working code :-(

Calm down, I found a better solution (appended below, I guess you cannot
read lyx-cvs?).

>
> Also, you shouldn't modify layout_ from within the inset.
> (can this be enforced by const?).

We can do that yes.

>
> Can you not simply fully realize the labelfont (or a  local
> copy) before using it in dimensionCollapsed()? A minimal fix.

I fixed it by realizing it at setLayout() time. It works fine now but
some insets don't have entry in stdinsets.inc.

Abdel.


Author: younes
Date: Fri Nov  2 22:27:41 2007
New Revision: 21392

URL: http://www.lyx.org/trac/changeset/21392
Log:
Further cleanup of collapsable insets. The layouts are now properly read
and applied.


Modified:
     lyx-devel/trunk/src/Text3.cpp
     lyx-devel/trunk/src/insets/Inset.h
     lyx-devel/trunk/src/insets/InsetBox.cpp
     lyx-devel/trunk/src/insets/InsetCollapsable.cpp
     lyx-devel/trunk/src/insets/InsetCollapsable.h

Modified: lyx-devel/trunk/src/Text3.cpp
URL: http://www.lyx.org/trac/file/lyx-devel/trunk/src/Text3.cpp?rev=21392
==============================================================================
--- lyx-devel/trunk/src/Text3.cpp (original)
+++ lyx-devel/trunk/src/Text3.cpp Fri Nov  2 22:27:41 2007
@@ -189,6 +189,9 @@
  Inset * inset = createInset(&cur.bv(), cmd);
  if (!inset)
  return false;
+
+ if (InsetCollapsable * ci = inset->asInsetCollapsable())
+ ci->setLayout(cur.bv().buffer().params());

  cur.recordUndo();
  if (cmd.action == LFUN_INDEX_INSERT) {

Modified: lyx-devel/trunk/src/insets/Inset.h
URL:
http://www.lyx.org/trac/file/lyx-devel/trunk/src/insets/Inset.h?rev=21392
==============================================================================
--- lyx-devel/trunk/src/insets/Inset.h (original)
+++ lyx-devel/trunk/src/insets/Inset.h Fri Nov  2 22:27:41 2007
@@ -35,6 +35,7 @@
  class FuncRequest;
  class FuncStatus;
  class InsetIterator;
+class InsetCollapsable;
  class InsetLayout;
  class InsetList;
  class InsetMath;
@@ -88,6 +89,10 @@
  virtual InsetText * asTextInset() { return 0; }
  /// is this inset based on the TextInset class?
  virtual InsetText const * asTextInset() const { return 0; }
+ /// is this inset based on the InsetCollapsable class?
+ virtual InsetCollapsable * asInsetCollapsable() { return 0; }
+ /// is this inset based on the InsetCollapsable class?
+ virtual InsetCollapsable const * asInsetCollapsable() const { return 0; }
 
  /// the real dispatcher
  void dispatch(Cursor & cur, FuncRequest & cmd);

Modified: lyx-devel/trunk/src/insets/InsetBox.cpp
URL:
http://www.lyx.org/trac/file/lyx-devel/trunk/src/insets/InsetBox.cpp?rev=21392
==============================================================================
--- lyx-devel/trunk/src/insets/InsetBox.cpp (original)
+++ lyx-devel/trunk/src/insets/InsetBox.cpp Fri Nov  2 22:27:41 2007
@@ -199,7 +199,6 @@
  //lyxerr << "InsetBox::dispatch MODIFY" << endl;
  InsetBoxMailer::string2params(to_utf8(cmd.argument()), params_);
  setLayout(cur.buffer().params());
- setButtonLabel();
  break;
  }


Modified: lyx-devel/trunk/src/insets/InsetCollapsable.cpp
URL:
http://www.lyx.org/trac/file/lyx-devel/trunk/src/insets/InsetCollapsable.cpp?rev=21392
==============================================================================
--- lyx-devel/trunk/src/insets/InsetCollapsable.cpp (original)
+++ lyx-devel/trunk/src/insets/InsetCollapsable.cpp Fri Nov  2 22:27:41 2007
@@ -81,8 +81,6 @@
  setAutoBreakRows(true);
  setDrawFrame(true);
  setFrameColor(Color_collapsableframe);
- setButtonLabel();
- setLayout(bp);
  }


@@ -108,15 +106,11 @@
  layout_.bgcolor = Color_background;

  layout_ = getLayout(bp);
- if (layout_.labelfont != inherit_font)
- return;

  // FIXME: put this in the InsetLayout parsing?
- // Fallback for lacking inset layout labelfont.
- layout_.labelfont = sane_font;
- layout_.labelfont.decSize();
- layout_.labelfont.decSize();
- layout_.labelfont.setColor(Color_collapsable);
+ layout_.labelfont.realize(sane_font);
+
+ setButtonLabel();
  }


@@ -170,7 +164,6 @@
  if (!token_found)
  status_ = isOpen() ? Open : Collapsed;

- setButtonLabel();
  setLayout(buf.params());

  // Force default font, if so requested

Modified: lyx-devel/trunk/src/insets/InsetCollapsable.h
URL:
http://www.lyx.org/trac/file/lyx-devel/trunk/src/insets/InsetCollapsable.h?rev=21392
==============================================================================
--- lyx-devel/trunk/src/insets/InsetCollapsable.h (original)
+++ lyx-devel/trunk/src/insets/InsetCollapsable.h Fri Nov  2 22:27:41 2007
@@ -41,7 +41,9 @@
  InsetCollapsable(BufferParams const &, CollapseStatus status =
Inset::Open);
  ///
  InsetCollapsable(InsetCollapsable const & rhs);
- ///
+
+ InsetCollapsable * asInsetCollapsable() { return this; }
+ InsetCollapsable const * asInsetCollapsable() const { return this; }
  docstring name() const { return from_ascii("Collapsable"); }
  ///
  void setLayout(BufferParams const &);

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Todays 1.6svn crash in various ways when opening existing documents

Martin Vermeer-2
On Sat, Nov 03, 2007 at 12:00:42AM +0100, Abdelrazak Younes wrote:

...

> >>  OK, I've done some cleanup an the crashes are gone but the inset layouts
> >> are  broken!
> >>
> >>  Martin, Richard, do you have an idea here? I don't know if we shall we
> >> fix  Inset::getLayout() or if we shall assume that each inset hard-code
> >> its own  layout.
> >>
> >>  Help please,
> >>  Abdel.
> > Abdel,
> > this is precisely the way _not_ to do it. The calls to setLayout() in the
> > various insets are designed (by Jean-Marc during the Bromarv meeting) to
> > get precisely the right insetlayout based on the inset's name().
>
>  Year I noticed that... calling virtual methods in ctors is dangerous and
>  should be avoided.

OK...

> > I fought with this fruitlessly until Jean-Marc showed me. You
> > reverted to my old non-working code :-(
>
>  Calm down, I found a better solution (appended below, I guess you cannot
>  read lyx-cvs?).

Yes, but I didn't notice until later. Looks like you got it
working again (?) but the thingy in Text3.cpp doesn't look
pretty.

> > Also, you shouldn't modify layout_ from within the inset. (can this be
> > enforced by const?).
>
>  We can do that yes.

Would be a good thing.
 
> > Can you not simply fully realize the labelfont (or a  local
> > copy) before using it in dimensionCollapsed()? A minimal fix.
>
>  I fixed it by realizing it at setLayout() time. It works fine now but some
>  insets don't have entry in stdinsets.inc.

OK... but I agree with your suspicion that this is better
done in insetlayout parsing.
 
>  Abdel.

- Martin

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Todays 1.6svn crash in various ways when opening existing documents

Abdelrazak Younes
Martin Vermeer wrote:
> On Sat, Nov 03, 2007 at 12:00:42AM +0100, Abdelrazak Younes wrote:
...
>>> I fought with this fruitlessly until Jean-Marc showed me. You
>>> reverted to my old non-working code :-(
>>  Calm down, I found a better solution (appended below, I guess you cannot
>>  read lyx-cvs?).
>
> Yes, but I didn't notice until later. Looks like you got it
> working again (?)

Yes.

> but the thingy in Text3.cpp doesn't look
> pretty.

Really? In any case, I don't think we need to set the layout. Why not
just accessing it through Inset::getLayout() when we need it? This will
avoid an unnecessary copy and get rid of the layout_ member altogether.

>
>>> Also, you shouldn't modify layout_ from within the inset. (can this be
>>> enforced by const?).
>>  We can do that yes.
>
> Would be a good thing.

Or see above.


>>> Can you not simply fully realize the labelfont (or a  local
>>> copy) before using it in dimensionCollapsed()? A minimal fix.
>>  I fixed it by realizing it at setLayout() time. It works fine now but some
>>  insets don't have entry in stdinsets.inc.
>
> OK... but I agree with your suspicion that this is better
> done in insetlayout parsing.

I'll do that.

Abdel.

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Todays 1.6svn crash in various ways when opening existing documents

Andre Poenitz-3
In reply to this post by Martin Vermeer-2
On Sat, Nov 03, 2007 at 01:12:24AM +0200, Martin Vermeer wrote:

> On Sat, Nov 03, 2007 at 12:00:42AM +0100, Abdelrazak Younes wrote:
>
> ...
>
> > >>  OK, I've done some cleanup an the crashes are gone but the inset layouts
> > >> are  broken!
> > >>
> > >>  Martin, Richard, do you have an idea here? I don't know if we shall we
> > >> fix  Inset::getLayout() or if we shall assume that each inset hard-code
> > >> its own  layout.
> > >>
> > >>  Help please,
> > >>  Abdel.
> > > Abdel,
> > > this is precisely the way _not_ to do it. The calls to setLayout() in the
> > > various insets are designed (by Jean-Marc during the Bromarv meeting) to
> > > get precisely the right insetlayout based on the inset's name().
> >
> >  Year I noticed that... calling virtual methods in ctors is dangerous and
> >  should be avoided.
>
> OK...

Well, it has a well-defined semantics (the incarnation of the currently
constructed part is called). It's not 'dangerous' but usually not too
helpful.

Andre'
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Todays 1.6svn crash in various ways when opening existing documents

Abdelrazak Younes
Andre Poenitz wrote:

> On Sat, Nov 03, 2007 at 01:12:24AM +0200, Martin Vermeer wrote:
>> On Sat, Nov 03, 2007 at 12:00:42AM +0100, Abdelrazak Younes wrote:
>>
>> ...
>>
>>>>>  OK, I've done some cleanup an the crashes are gone but the inset layouts
>>>>> are  broken!
>>>>>
>>>>>  Martin, Richard, do you have an idea here? I don't know if we shall we
>>>>> fix  Inset::getLayout() or if we shall assume that each inset hard-code
>>>>> its own  layout.
>>>>>
>>>>>  Help please,
>>>>>  Abdel.
>>>> Abdel,
>>>> this is precisely the way _not_ to do it. The calls to setLayout() in the
>>>> various insets are designed (by Jean-Marc during the Bromarv meeting) to
>>>> get precisely the right insetlayout based on the inset's name().
>>>  Year I noticed that... calling virtual methods in ctors is dangerous and
>>>  should be avoided.
>> OK...
>
> Well, it has a well-defined semantics (the incarnation of the currently
> constructed part is called).

I know but I sometimes forgot that in the past.

> It's not 'dangerous' but usually not too
> helpful.

Well it was not really dangerous but when your code relies on the
inheriting class calling a virtual method without enforcing it can
result in crashes.

Abdel.

Loading...