[prev in list] [next in list] [prev in thread] [next in thread] 

List:       lyx-devel
Subject:    Re: LyX crash after font assert
From:       Richard Heck <rgheck () comcast ! net>
Date:       2012-06-09 12:56:16
Message-ID: 4FD347F0.1030604 () comcast ! net
[Download RAW message or body]

On 06/09/2012 08:35 AM, Tommaso Cucinotta wrote:
> On 08/06/12 04:15, Richard Heck wrote:
>> On 6/7/12 6:15 PM, Tommaso Cucinotta wrote:
>>> Do u mind explaining what does this code do ?
>>>
>>> +void BufferView::makeDocumentClass()
>>> +{
>>> +       DocumentClassConstPtr olddc = 
>>> buffer_.params().documentClassPtr();
>>> +       buffer_.params().makeDocumentClass();
>>> +       updateDocumentClass(olddc);
>>> +}
>>>
>> Here's how to think of it. Suppose you add a new module to your 
>> document. Then we need to do two things: (i) Load the module's code; 
>> (ii) Adapt the Buffer to the presence of the new module. This code 
>> does those two steps, whenever you do anything that requires them. 
>> The BufferParams::makeDocumentClass() routine takes the base 
>> TextClass (layout file) and loads modules and local layout into it, 
>> creating a new DocumentClass. But we then have to make sure that the 
>> Buffer itself uses this DocumentClass and, e.g., that each paragraph 
>> references *its* layouts as opposed to the ones in the now discarded 
>> DocumentClass. That's what updateDocumentClass() does.
>
> Ok, so it seems to me that, at the level of the document model, 
> params().makeDocumentClass() and BufferView::updateDocumentClass() 
> (or, perhaps, cap::switchBetweenClasses()) should happen atomically 
> together, i.e., makeDocumentClass() might call 
> cap::switchBetweenClasses(), or maybe better have a 
> Buffer::makeDocumentClass() that calls both. Later, if the view 
> happens to have further objects referencing the old class (e.g., 
> cursor ?), then it would be its own responsibility to update those 
> further pointers.
>
> For example, I noticed that readHeader() calls makeDocumentClass() but 
> apparently no further update is done on paragraphs. Now, 
> BufferView.cpp calls readHeader() and afterwards updateDocumentClass() 
> so it seems ok. However, GuiApplication.cpp, in SAVE_AS_DEFAULT, calls 
> readHeader() but not updateDocumentClass() afterwards.
> Also, what about other calls to params().makeDocumentClass() ?
>
> $ find src/ -name '*.cpp' -exec grep -Hn makeDocumentClass {} \;
> src/Buffer.cpp:848:    params().makeDocumentClass();
> src/BufferParams.cpp:353:    makeDocumentClass();
> src/BufferParams.cpp:2023:void BufferParams::makeDocumentClass()
> src/BufferView.cpp:944:void BufferView::makeDocumentClass()
> src/BufferView.cpp:947:    buffer_.params().makeDocumentClass();
> src/BufferView.cpp:1263:        makeDocumentClass();
> src/BufferView.cpp:1279:        makeDocumentClass();
> src/BufferView.cpp:1308:        makeDocumentClass();
> src/BufferView.cpp:1332:        makeDocumentClass();
> src/frontends/qt4/FindAndReplace.cpp:506:    dest_bv.makeDocumentClass();
> src/frontends/qt4/GuiDocument.cpp:2056:    bp_.makeDocumentClass();
> src/frontends/qt4/GuiDocument.cpp:2220:    bp_.makeDocumentClass();
> src/frontends/qt4/GuiDocument.cpp:2337:        
> param_copy.makeDocumentClass();
>
> The calls in GuiDocument.cpp seem to never be followed by an 
> updateDocumentClass(), but perhaps these calls operate on temporary 
> BufferParams and no document's paragraphs updates are needed.
>
I looked through these, but should probably do so again. The ones in 
GuiDocument aren't a problem, since the BufferParams objects there 
aren't actually associated with a document; they're representing the 
BufferParams of a document, but they're just a copy. So we don't need to 
change any paragraphs. The one in Buffer itself is just part of reading 
the file, and the actual DocumentClass gets set later, if necessary.

I agree this is sub-standard, and everything ought to be done at once. 
But especially in GuiDocument, this isn't possible.

Richard

[prev in list] [next in list] [prev in thread] [next in thread]