Re: tdf#50916 : Calc : Dynamic column container

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 <dennisfrancis.in@gmail.com>
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 <
> dennisfrancis.in@gmail.com> 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 <libreoffice@kohei.us>
> > wrote:
> >
> > > > On October 14, 2015 at 11:18 AM Wols Lists <antlists@youngman.org.uk>
> > > 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
> > > LibreOffice@lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/libreoffice