Hi All I have compiled a writeup @ https://niocs.github.io/LOBook/misc/16kcols.html to help new contributors who want to work on this bug tdf#50916. I will try to keep it updated on new developments as more patches are added.
Thanks, Dennis On Mon, Jun 20, 2016 at 3:40 PM, Dennis Francis <[email protected]> wrote: > Adding Markus and Eike to make sure they did not miss my last post in this > thread. > > Thanks in advance for your feedback. > > Thanks, > Dennis > > > On Fri, Jun 10, 2016 at 3:08 PM, Dennis Francis < > [email protected]> wrote: > >> Hi All >> >> It has been quite a while since I worked on this bug. Lately I have been >> thinking about "a better way to handle row formattings". >> In the current setting, if someone formats whole row, it is required that >> we have all columns from 0 to MAXCOL allocated, which is bad when MAXCOL is >> a big number. >> >> The formatting attributes of a column are stored in ScColumn::pAttrArray >> ( type is ScAttrArray* ) >> >> The methods of ScAttrArray are mostly to set and get specific attributes >> in addition to >> SetPattern(), SetPatternArea(), GetPattern() and GetPatternRange() which >> sets/gets whole set of formatting attributes for a row/row segment. >> >> One of my ideas to solve the row formatting issue was to create and >> maintain a ScAttrArray object in ScTable (say aRowAttrArray) to hold only >> the row formattings. >> In the ScTable *set* methods related to formatting we could check if the >> request is for the column range 0 to MAXCOL (full row operation) and >> store the specified row formatting in aRowAttrArray in addition to >> letting the existing columns to receive the row formatting. >> >> *For example* : >> >> void ScTable::ApplyStyleArea( SCCOL nStartCol, SCROW nStartRow, SCCOL >> nEndCol, SCROW nEndRow, const ScStyleSheet& rStyle ) >> { >> if (ValidColRow(nStartCol, nStartRow) && ValidColRow(nEndCol, >> nEndRow)) >> { >> PutInOrder(nStartCol, nEndCol); >> PutInOrder(nStartRow, nEndRow); >> for (SCCOL i = nStartCol; i <= nEndCol; i++) >> aCol[i].ApplyStyleArea(nStartRow, nEndRow, rStyle); >> } >> } >> >> can be modified to : >> >> void ScTable::ApplyStyleArea( SCCOL nStartCol, SCROW nStartRow, SCCOL >> nEndCol, SCROW nEndRow, const ScStyleSheet& rStyle ) >> { >> if (ValidColRow(nStartCol, nStartRow) && ValidColRow(nEndCol, >> nEndRow)) >> { >> PutInOrder(nStartCol, nEndCol); >> PutInOrder(nStartRow, nEndRow); >> >> if( nStartCol == 0 && nEndCol == MAXCOL ) >> { >> aRowAttrArray.ApplyStyleArea(nStartRow, nEndRow, >> const_cast<ScStyleSheet*>(&rStyle)); >> SCCOL nLastCol = aCol.size() - 1; >> for (SCCOL i = 0; i <= nLastCol; i++) >> aCol[i].ApplyStyleArea(nStartRow, nEndRow, rStyle); >> } >> else >> { >> if ( aCol.size() <= nEndCol ) >> aCol.CreateCol( nEndCol, nTab ); // This method has to be >> added again as the commit for loplugin:unusedmethods removed it. >> for (SCCOL i = nStartCol; i <= nEndCol; i++) >> aCol[i].ApplyStyleArea(nStartRow, nEndRow, rStyle); >> } >> } >> } >> >> Now this aRowAttrArray can be used to instantiate pAttrArray of column to >> be created later on as it represents the attributes of all columns that are >> yet to be created. >> >> Next we need to modify the Get methods of ScTable related to formatting >> in a way that it will respond with >> correct formatting on the not-yet-created columns. >> >> *For example* : >> >> const ScPatternAttr* ScTable::GetPattern( SCCOL nCol, SCROW nRow ) const >> { >> if (ValidColRow(nCol,nRow)) >> return aCol[nCol].GetPattern( nRow ); >> else >> { >> OSL_FAIL("wrong column or row"); >> return pDocument->GetDefPattern(); // for safety >> } >> } >> >> needs to be modified to : >> >> const ScPatternAttr* ScTable::GetPattern( SCCOL nCol, SCROW nRow ) const >> { >> if (!ValidColRow(nCol,nRow)) >> { >> OSL_FAIL("wrong column or row"); >> return pDocument->GetDefPattern(); // for safety >> } >> if ( nCol < aCol.size() ) >> return aCol[nCol].GetPattern( nRow ); >> return aRowAttrArray.GetPattern( nRow ); >> } >> >> >> >> >> While the above idea might work for a new document getting edited; but I >> am not sure what the situation is when a non trivial document is >> loaded/saved. During file loading if row attributes get applied column by >> column separately, it will defeat the idea presented in the sample code >> above. >> *For example*, a row stylesheet get applied by the import code by calling >> either : >> >> 1) for ( nCol = 0; nCol <= MAXCOL; ++nCol) >> ScTable::ApplyStyle(nCol, nRow, rStyle) >> or >> >> 2) ScTable::ApplyStyleArea( 0, nRow, MAXCOL, nRow, rStyle ) >> >> In case 2) we can avoid creating all columns by special handling, but not >> in case 1) >> >> In oox and excel import filters, it looks like attributes are applied >> column by column (more like case 1) >> See http://opengrok.libreoffice.org/xref/core/sc/source/ >> filter/oox/sheetdatabuffer.cxx#496 and >> http://opengrok.libreoffice.org/xref/core/sc/source/ >> filter/excel/xistyle.cxx#2030 >> where it calls ScDocumentImport::setAttrEntries( SCTAB nTab, SCCOL nCol, >> Attrs& rAttrs ), which in-turn sets the Attr entries by directly >> manipulating pAttrArray of each column. >> In oox and excel formats, I am not sure if there is a way to store row >> attributes. >> >> For ods doc, I could not find direct evidence on how row attributes are >> applied, but I found out that ods has a way to store row attributes by >> examining the content.xml of an ods file with a row formatting, so >> the import code would definitely have a chance to call the Set*Area() or >> Set*Range() or ApplyStyleArea() directly rather than setting attributes for >> each column in a loop for the row attributes. >> >> >> It would be great if someone could comment on the approach and offer some >> advice on my import code doubts presented above. >> >> >> >> Thanks a lot, >> Dennis >> >> On Wed, Jan 20, 2016 at 1:34 PM, Dennis Francis < >> [email protected]> wrote: >> >>> Hi All >>> >>> I have submitted a prelim patch at https://gerrit.libreoffice.org/21620 >>> following Kohei's suggestion of >>> solving this bug incrementally. >>> >>> Thanks, >>> Dennis >>> >>> On Wed, Oct 14, 2015 at 10:22 PM, Kohei Yoshida <[email protected]> >>> wrote: >>> >>>> >>>> >>>> > On October 14, 2015 at 11:18 AM Wols Lists <[email protected]> >>>> wrote: >>>> > >>>> > >>>> > On 14/10/15 02:08, Kohei Yoshida wrote: >>>> > > Also, this will be an on-going process. This is not going to be >>>> like >>>> > > "if you do A, B and C it's okay to increase the column size and no >>>> > > problems will occur". Rather, we'll probably encounter lots of >>>> > > performance issues that we'll have to spend some time fixing after >>>> the >>>> > > column size is increased. >>>> > >>>> > Just throwing an idea into the mix... >>>> > >>>> > Howsabout a function that, on being given the cell co-ordinates, >>>> returns >>>> > a pointer to the cell. Force everything through that. >>>> >>>> Its internal representation is no longer cell-based. So, there would be >>>> no >>>> "pointer to cell" to return. >>>> _______________________________________________ >>>> LibreOffice mailing list >>>> [email protected] >>>> http://lists.freedesktop.org/mailman/listinfo/libreoffice >>>> >>> >>> >> >
_______________________________________________ LibreOffice mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/libreoffice
