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

List:       koffice-devel
Subject:    Re: Working on kword tables? (includes patch)
From:       David Faure <dfaure () klaralvdalens-datakonsult ! se>
Date:       2003-04-13 9:24:20
[Download RAW message or body]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Sunday 13 April 2003 07:01, Carl G Lewis wrote:
> 
> Hello, 
> 
> I am interested in doing some work on koffice, in particular trying to improve 
> tables in kword. 

Great.

> In the cvs log for kwtableframeset.cc, David says that he wants to move from 
> the m_cells QPtrList to the m_rowArray data structure. I could do this, if 
> no-one else is planning to do it immediately.

Excellent ;)

> I have read most of kwtableframeset.cc, and understand some of it, although 
> the important methods are quite difficult to fully understand, eg 
> drawBorders() and recalcRows(). drawBorders() seems to be involved in some of 
> the crashes, too.
>
> So my thoughts were that we could move to the new data structure, plus try and 
> do a bit of a cleanup (mainly introducing more (small) methods to make code 
> more comprehensible). As part of this I will also try to fix bugs as I can 
> figure out what they are.

Sounds good.

> Although it is probably better to fix bugs first then make changes, I am not 
> really sure if that is the easiest approach in this case :-P

Well, since this is about changing the underlying data structure, it is best
to make that change first, then debug. No point in debugging m_cells usage.

Make sure you understand how merged cells work, it affects most of the code.
*(m_rowArray[rowNum])[colNum] can return a Cell whose own row and column value 
are not equal to "rowNum" and "colNum", when we're looking at a merged cell
(that isn't the topleft one of the merged cells). In fact a 2x2 merged cell will
lead to 4 entries in the array, that point to the same Cell (you can remove the #if 0
code in getCell, it came from previous design solutions, but I believe this
one is the most efficient, since we can directly find out the topleft of the cell).

> Please let me know it is OK to start working on this.

Definitely!

> Also, here is a one-liner patch that fixes a crash. getCell() gets called from 
> drawBorders(), which sometimes passes in an invalid column, because it 
> subtracts 1 from the 'col' variable when it is zero. col is unsigned, so we 
> pass in 2^32 or so. 
Ouch.
Then I suggest that the code that subtracts 1 be fixed. Not only getCell() itself
(when it gets 2^32, something wrong has been done already).

> -    if ( row < m_rowArray.size() ) {
> +    if ( row < m_rowArray.size() && col < m_rowArray[row]->size() ) {

Looks good - applied.
Thanks!

- -- 
David Faure -- faure@kde.org, dfaure@klaralvdalens-datakonsult.se
Qt/KDE/KOffice developer
Klarälvdalens Datakonsult AB, Platform-independent software solutions
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.0.7 (GNU/Linux)

iD8DBQE+mSzG72KcVAmwbhARAmRZAKCdhYSExdi4Cplazt8TFI9opohVkQCeLmJW
c8yB7Eb/uq1lvhjs6EVrrs4=
=mBGa
-----END PGP SIGNATURE-----

_______________________________________________
koffice-devel mailing list
koffice-devel@mail.kde.org
http://mail.kde.org/mailman/listinfo/koffice-devel

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