Re: Second report from Ersin

Hi Ersin,

Sorry about the rather garbled email I sent yesterday. I'm glad you
could decipher it.

There are a number of different listener classes used in abiword. The
two main sorts attach to either the views (FV_View.cpp and
AV_View..cpp, these are view listeners) or the document
pd_Document.cpp, these are document listeners. There are also
listeners that react to changes in preferences, preference listeners.

The listeners are opaque classes designed to transport information
away from a model class.

The pd_Document class listeners are used to export to different
document formats, build layout views of document and for real-time
collaboration via abicollab.

The FV_View listeners are used to update state information about the
view on the current document on or near the caret. So for example the
abiword changes whether the bold button is pressed in when the caret
in in a region of bold text, or sets the font family and size based on
the current caret position using one or more view listeners.

The FV_View listeners are potentially called after every method in
FV_View is completed. Of course one the issues is to make sure sure
these are not called unnecessarily. AbiWord has infrastructure to do
this. I suspect that the current view listeners are called way too
often but on the other hand it is disastrous if a call to a view
listener is not made when it should be.

One of the features of listeners is that any number can be attached to
a view. In fact I'm currently designing a view listener to provide
accessibility information to an external accessibility infrastructure.
Like braille readers, text to speech etc.

Now your idea to offload some tasks from redrawUpdate to view
listeners is really good. I encourage you to keep thinking on those
lines!. There is already a listener that deals with scrolls:

src/af/xap/xp/xap_Scrollbar_ViewListener.cpp:

I don't know whether you can place your code within the existing
scroll listeners or whether it makes more sense to add a new listener
class.

One potential issue is to make sure your listener class is called
before abiword draws the screen (so that the fixes to the document are
made before we try to draw it) and also to ensure that that listener
is not called unnecessarily. So for example if you place a call to
your listener inside the FV_View::draw() method, make sure it is not
called again during the regular FV_View::updateListeners() call after
every operation. Or if it is make sure it just returns so that calling
it is not expensive.

cheers!

Martin

On Tue, Jun 15, 2010 at 7:12 AM, Ersin Akinci <ersin.akinci@gmail.com> wrote:
> Hi Martin,
>
> Thanks for the feedback!  I'm hoping to make some small commits either
> tonight or tomorrow, still doing a lot of experimenting.
>
> I started thinking about the listener classes.  What do they do, exactly,
> and how are they implemented?  It seems like adding some kind of code to one
> or more of the listeners would make sense, if indeed they "listen" for
> changes made to the document/window/etc.  I.e., when the window scrolls, the
> scroll listener updates which pages are on screen, and likewise for other
> events.
>
> Best,
> Ersin
>
> On Sun, Jun 13, 2010 at 9:51 AM, Martin Sevior <msevior@gmail.com> wrote:
>>
>> HI Ersin,
>>
>> Thanks very much for this report. It's great to hear about all your
>> progress. Feel free to make small commits as needed to your branch. It
>> helps to see what you're working and is is fare as well.
>>
>> With regards to re-using Joaquin's Red-back data structure, it is not
>> clear you win much over a vector of pointers to the page classes. The
>> vector allows extremely rapid searches (O(1) and O(Log(N) )and inserts
>> and deletes of pages are rather rare. I suspect it is not worth the
>> extra complexity to implement the red-black structure over the current
>> vector.
>>
>> Cheers!
>>
>> Martin
>>
>> On Sun, Jun 13, 2010 at 6:51 AM, Ersin Akinci <ersin.akinci@gmail.com>
>> wrote:
>> >
>> > Cheers everyone,
>> >
>> > I hope everyone's well!  I'm writing to apprise you of my progress on
>> > my GSoC project over the past two weeks.
>> >
>> > I've been working on optimizing the rendering algorithms in the
>> > FV_View classes, which have been considerably slowing down text input
>> > in very large documents (currently working with a 3000+ page text file
>> > of Casanova's autobiography).  As I mentioned in my previous report,
>> > FV_View::getPageScreenOffsets seemed to be the likely culprit, as it
>> > was iterating through each page starting from the first and ending
>> > with the current page in order to calculate a page's offsets on the
>> > screen, which in turn was continuously being called by
>> > FL_DocLayout::_redrawUpdate in order to determine which pages were on
>> > screen and had to be redrawn.  It turns out that this work was
>> > basically duplicated in FV_View::_draw, which does the actual work of
>> > drawing onto the screen, and I fixed it so that getPageScreenOffsets
>> > didn't need to be called to determine what pages are on screen.
>> > instead, the algorithm in _draw that does the same work sets a boolean
>> > variable (m_bOnScreen) for each page, and when the time comes to call
>> > getPageScreenOffsets for the purposes of figuring out what's on the
>> > page, AbiWord just checks the variable.  (NB, getPageScreenOffsets is
>> > still inefficient and still being called from time to time.)
>> >
>> > This has helped tremendously, but on my top of the line rig it still
>> > takes about 1-2 seconds to press spacebar at the end of a 3000 page
>> > plain text file, which is unacceptable.  The main problem now is that
>> > the algorithm in _draw essentially uses the same inefficient logic in
>> > getPageScreenOffsets: iterate through each page starting with the
>> > first and ending with the last until you get to the last, and
>> > determine which need to be drawn.  All to press spacebar!  To be more
>> > precise, it takes the first page of the current document section
>> > layout, but in text files since there's only one section that means
>> > going through all 3000+ pages in my test document.
>> >
>> > What I've been doing now is to rewrite _draw and various bits of the
>> > rendering engine so that any time AbiWord does something that would
>> > change the pages displayed on the screen, the called function updates
>> > a data structure that contains all the pages that should be on the
>> > screen, and then _draw simply renders those.  I haven't gotten around
>> > to implementing the structure, but I think that I'll end up using
>> > Joaquin's wonderful new red black tree, thus creating a second
>> > "pages-on-screen piece table".  Right now, I'm in the middle of
>> > identifying exactly where to put in calls to a new function that will
>> > determine that a particular page is on screen and in which functions
>> > to do so.  It will be complicated and messy, but I haven't thought of
>> > a better way to do it.  My strategy has been to focus on identifying
>> > the obvious functions first (e.g., scrolling, zooming, print preview,
>> > autoscroll when a new page is created), after which I will insert test
>> > code, then real code, and finally then remove the inefficient loop
>> > from _draw and test for corner cases where I still need to put in a
>> > call or two.
>> >
>> > I'm not thrilled about the solution I've found, but I can't think of
>> > anything better.  At the very least, I think it will be efficient, if
>> > not particularly elegant.  I'm open to suggestions and comments!
>> >
>> > Best,
>> > Ersin
>> >
>> > --
>> > Ersin Y. Akinci -- ersinakinci.com
>> >
>> > What Digital Revolution? -- www.whatdigitalrevolution.com
>> > Thinking critically about digital worlds.
>> >
>
>
>
> --
> Ersin Y. Akinci -- ersinakinci.com
>
> What Digital Revolution? -- www.whatdigitalrevolution.com
> Thinking critically about digital worlds.
>
Received on Tue Jun 15 01:40:27 2010