https://bugs.documentfoundation.org/show_bug.cgi?id=128204
--- Comment #13 from Kohei Yoshida <[email protected]> --- There is a lot to unpack here, so this will be a bit lengthy. First off, as Kevin correctly explained, that use-optimal-row-height flag corresponds to the flag to re-calculate optimal height when a cell value changes during editing, and it is not a flag to indicate whether the row heights should be re-calculated on load. Doing so would (as we now know) cause a noticeable performance degradation during file load for everyone, which is not a good look. If the standard is not clear about when this flag should trigger recalculation in my opinion that point should be further clarified in the standard. As for improving performance on row height re-calculation, to me it’s a lost cause since that process is already known to be very expensive involving getting font metric information as well as other attributes of the text for every character involved. Caching certain font metric information was attempted in the past, which may improve performance in certain situations, but it’s unclear how much that would help with the row height re-calculation. Still, any attempt to speed it up would not come anywhere close to not running it. As for a potential fix, the logic Kevin suggested is a reasonable approach, though I’m not sure whether we should check for the row height being 0. That’s a corner case that would not happen when Calc is the generator since setting the height to 0 would set the hidden flag while leaving the original height unchanged. I would just leave it as the generator’s responsibility to never set the height to zero, or leave out the value in case the desired value is not clear. But it’s just my opinion. I think either approach is fine. Now, here is the bad news. The current ODF import filter code is notoriously hard to work with since it was built on (IMO) the wrong architectural basis of basing it on (mostly) UNO API. UNO API is designed for run-time automation with change notifications firing everywhere. Making it built on UNO API unfortunately resulted in significant performance issue not to mention making it very very difficult to follow, understand, and make significant design changes to the code since UNO promotes the idea of decoupling all the moving parts. My hope at the time was to slowly switch from populating the content via UNO API to doing the same directly with ScDocument via its import-time specialized accessor ScDocumentImport. You see some trace of my earlier attempt in this part of Calc’s code. If someone is up for it, my suggestion would be to try to populate the content via ScDocumentImport instead of using UNO API. ScDocumentImport has direct access to ScDocument’s private parts, and is designed to populate the document content without unnecessary change notifications etc. I did make quite some inroads toward using ScDocumentImport to speed up loading for other, non-UNO based import filters, but unfortunately I only made small progress with the native ODF import filter. It may be a good idea for someone to pick up the torch to continue further. Alternatively, it may be actually simpler to introduce an internal configuration option to toggle row height re-calculation on load (defaults to off, of course) to satisfy the use case in tdf#62268. Fixing the import filter code would be the ideal approach, but I’m not sure if anyone would want to even touch that code… I wouldn’t, at least not willingly. ;-) Only those with enough bandwidth could tackle that code, and I don’t have much bandwidth these days unfortunately. As a final aside, my frustration with working with this code also motivated me to re-architect the ODF import filter in orcus, which can be turned on in Calc with some effort (right now it’s disabled). But that filter is only 10 to 15% complete, so using that would be a long shot. Maybe someday it will become somewhat feature complete, but who knows. -- You are receiving this mail because: You are the assignee for the bug.
