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,
    [EMAIL PROTECTED], [EMAIL PROTECTED])
    at ../../lyx-devel/src/insets/InsetCollapsable.cpp:218
#2 0x0834eca8 in lyx::InsetFootlike::metrics (this=0x8c5a5e8, [EMAIL PROTECTED],
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 via email to