<div dir="ltr"><div>Adding Markus and Eike to make sure they did not miss my last post in this thread.</div><div><br></div><div>Thanks in advance for your feedback.</div><div><br></div><div>Thanks,</div><div>Dennis</div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Jun 10, 2016 at 3:08 PM, Dennis Francis <span dir="ltr"><<a href="mailto:dennisfrancis.in@gmail.com" target="_blank">dennisfrancis.in@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>Hi All</div><div><br></div><div>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".</div><div>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.</div><div><br></div><div>The formatting attributes of a column are stored in ScColumn::pAttrArray ( type is ScAttrArray* )</div><div><br></div><div>The methods of ScAttrArray are mostly to set and get specific attributes in addition to</div><div>SetPattern(), SetPatternArea(), GetPattern() and GetPatternRange() which sets/gets whole set of formatting attributes for a row/row segment.</div><div><br></div><div>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.</div><div>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</div><div>store the specified row formatting in aRowAttrArray in addition to letting the existing columns to receive the row formatting.</div><div><br></div><div>*For example* : </div><div><br></div><div>void ScTable::ApplyStyleArea( SCCOL nStartCol, SCROW nStartRow, SCCOL nEndCol, SCROW nEndRow, const ScStyleSheet& rStyle )</div><div>{</div><div> if (ValidColRow(nStartCol, nStartRow) && ValidColRow(nEndCol, nEndRow))</div><div> {</div><div> PutInOrder(nStartCol, nEndCol);</div><div> PutInOrder(nStartRow, nEndRow);</div><div> for (SCCOL i = nStartCol; i <= nEndCol; i++)</div><div> aCol[i].ApplyStyleArea(nStartRow, nEndRow, rStyle);</div><div> }</div><div>}</div><div><br></div><div>can be modified to :</div><div><br></div><div>void ScTable::ApplyStyleArea( SCCOL nStartCol, SCROW nStartRow, SCCOL nEndCol, SCROW nEndRow, const ScStyleSheet& rStyle )</div><div>{</div><div> if (ValidColRow(nStartCol, nStartRow) && ValidColRow(nEndCol, nEndRow))</div><div> {</div><div> PutInOrder(nStartCol, nEndCol);</div><div> PutInOrder(nStartRow, nEndRow);</div><div> </div><div> if( nStartCol == 0 && nEndCol == MAXCOL )</div><div> {</div><div> aRowAttrArray.ApplyStyleArea(nStartRow, nEndRow, const_cast<ScStyleSheet*>(&rStyle));</div><div> SCCOL nLastCol = aCol.size() - 1;</div><div> for (SCCOL i = 0; i <= nLastCol; i++)</div><div> aCol[i].ApplyStyleArea(nStartRow, nEndRow, rStyle); </div><div> }</div><div> else</div><div> {</div><div> if ( aCol.size() <= nEndCol )</div><div> aCol.CreateCol( nEndCol, nTab ); // This method has to be added again as the commit for loplugin:unusedmethods removed it.</div><div> for (SCCOL i = nStartCol; i <= nEndCol; i++)</div><div> aCol[i].ApplyStyleArea(nStartRow, nEndRow, rStyle);</div><div> }</div><div> }</div><div>}</div><div><br></div><div>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.</div><div><br></div><div>Next we need to modify the Get methods of ScTable related to formatting in a way that it will respond with</div><div>correct formatting on the not-yet-created columns.</div><div><br></div><div>*For example* :</div><div><br></div><div>const ScPatternAttr* ScTable::GetPattern( SCCOL nCol, SCROW nRow ) const</div><div>{</div><div> if (ValidColRow(nCol,nRow))</div><div> return aCol[nCol].GetPattern( nRow );</div><div> else</div><div> {</div><div> OSL_FAIL("wrong column or row");</div><div> return pDocument->GetDefPattern(); // for safety</div><div> }</div><div>}</div><div><br></div><div>needs to be modified to :</div><div><br></div><div>const ScPatternAttr* ScTable::GetPattern( SCCOL nCol, SCROW nRow ) const</div><div>{</div><div> if (!ValidColRow(nCol,nRow))</div><div> {</div><div> OSL_FAIL("wrong column or row");</div><div> return pDocument->GetDefPattern(); // for safety</div><div> }</div><div> if ( nCol < aCol.size() )</div><div> return aCol[nCol].GetPattern( nRow );</div><div> return aRowAttrArray.GetPattern( nRow );</div><div>}</div><div><br></div><div><br></div><div><br></div><div><br></div><div>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</div><div>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.</div><div>*For example*, a row stylesheet get applied by the import code by calling either :</div><div><br></div><div>1) for ( nCol = 0; nCol <= MAXCOL; ++nCol)</div><div> ScTable::ApplyStyle(nCol, nRow, rStyle)</div><div>or</div><div><br></div><div>2) ScTable::ApplyStyleArea( 0, nRow, MAXCOL, nRow, rStyle )</div><div><br></div><div>In case 2) we can avoid creating all columns by special handling, but not in case 1)</div><div><br></div><div>In oox and excel import filters, it looks like attributes are applied column by column (more like case 1)</div><div>See <a href="http://opengrok.libreoffice.org/xref/core/sc/source/filter/oox/sheetdatabuffer.cxx#496" target="_blank">http://opengrok.libreoffice.org/xref/core/sc/source/filter/oox/sheetdatabuffer.cxx#496</a> and</div><div><a href="http://opengrok.libreoffice.org/xref/core/sc/source/filter/excel/xistyle.cxx#2030" target="_blank">http://opengrok.libreoffice.org/xref/core/sc/source/filter/excel/xistyle.cxx#2030</a></div><div>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.</div><div>In oox and excel formats, I am not sure if there is a way to store row attributes.</div><div><br></div><div>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</div><div>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.</div><div><br></div><div><br></div><div>It would be great if someone could comment on the approach and offer some advice on my import code doubts presented above.</div><div><br></div><div><br></div><div><br></div><div>Thanks a lot,</div><div>Dennis</div></div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Jan 20, 2016 at 1:34 PM, Dennis Francis <span dir="ltr"><<a href="mailto:dennisfrancis.in@gmail.com" target="_blank">dennisfrancis.in@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><div><div><div>Hi All<br><br></div>I have submitted a prelim patch at <a href="https://gerrit.libreoffice.org/21620" target="_blank">https://gerrit.libreoffice.org/21620</a> following Kohei's suggestion of<br></div>solving this bug incrementally.<br><br></div>Thanks,<br></div>Dennis<br></div><div><div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Oct 14, 2015 at 10:22 PM, Kohei Yoshida <span dir="ltr"><<a href="mailto:libreoffice@kohei.us" target="_blank">libreoffice@kohei.us</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span><br>
<br>
> On October 14, 2015 at 11:18 AM Wols Lists <<a href="mailto:antlists@youngman.org.uk" target="_blank">antlists@youngman.org.uk</a>> wrote:<br>
><br>
><br>
> On 14/10/15 02:08, Kohei Yoshida wrote:<br>
> > Also, this will be an on-going process. This is not going to be like<br>
> > "if you do A, B and C it's okay to increase the column size and no<br>
> > problems will occur". Rather, we'll probably encounter lots of<br>
> > performance issues that we'll have to spend some time fixing after the<br>
> > column size is increased.<br>
><br>
> Just throwing an idea into the mix...<br>
><br>
> Howsabout a function that, on being given the cell co-ordinates, returns<br>
> a pointer to the cell. Force everything through that.<br>
<br>
</span>Its internal representation is no longer cell-based. So, there would be no<br>
"pointer to cell" to return.<br>
<div><div>_______________________________________________<br>
LibreOffice mailing list<br>
<a href="mailto:LibreOffice@lists.freedesktop.org" target="_blank">LibreOffice@lists.freedesktop.org</a><br>
<a href="http://lists.freedesktop.org/mailman/listinfo/libreoffice" rel="noreferrer" target="_blank">http://lists.freedesktop.org/mailman/listinfo/libreoffice</a><br>
</div></div></blockquote></div><br></div>
</div></div></blockquote></div><br></div>
</div></div></blockquote></div><br></div>