D15428: [textlayout] Don't enter infinite loop when table are misfit
anthonyfieroni created this revision. anthonyfieroni added reviewers: Calligra: 3.0, danders, boemann. Herald added a project: Calligra: 3.0. Herald added a subscriber: Calligra-Devel-list. anthonyfieroni requested review of this revision. REVISION SUMMARY Also QTextLine can crash due to its validity, Qt implementation incorporate a pointer that isn't check exclusively but in isValid CCBUG: 381341 TEST PLAN Attached document is now open and pretty responsive due to its 243 pages :) REPOSITORY R8 Calligra REVISION DETAIL https://phabricator.kde.org/D15428 AFFECTED FILES libs/textlayout/KoTextLayoutNoteArea.cpp libs/textlayout/KoTextLayoutTableArea.cpp To: anthonyfieroni, #calligra:_3.0, danders, boemann Cc: Calligra-Devel-list, dcaliste, cochise, vandenoever
D15428: [textlayout] Don't enter infinite loop when table is misfit
anthonyfieroni retitled this revision from "[textlayout] Don't enter infinite loop when table are misfit" to "[textlayout] Don't enter infinite loop when table is misfit". REPOSITORY R8 Calligra REVISION DETAIL https://phabricator.kde.org/D15428 To: anthonyfieroni, #calligra:_3.0, danders, boemann Cc: Calligra-Devel-list, dcaliste, cochise, vandenoever
D15428: [textlayout] Don't enter infinite loop when table is misfit
anthonyfieroni edited the summary of this revision. REPOSITORY R8 Calligra REVISION DETAIL https://phabricator.kde.org/D15428 To: anthonyfieroni, #calligra:_3.0, danders, boemann Cc: Calligra-Devel-list, dcaliste, cochise, vandenoever
D15428: [textlayout] Don't enter infinite loop when table is misfit
anthonyfieroni updated this revision to Diff 41418. anthonyfieroni added a comment. Initialize labelYOffset even layout line is not valid REPOSITORY R8 Calligra CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15428?vs=41409&id=41418 REVISION DETAIL https://phabricator.kde.org/D15428 AFFECTED FILES libs/textlayout/KoTextLayoutNoteArea.cpp libs/textlayout/KoTextLayoutTableArea.cpp To: anthonyfieroni, #calligra:_3.0, danders, boemann Cc: Calligra-Devel-list, dcaliste, cochise, vandenoever
D15428: [textlayout] Don't enter infinite loop when table is misfit
boemann added inline comments.
INLINE COMMENTS
> KoTextLayoutNoteArea.cpp:143
> +
> +if (blockLayoutLine.isValid()) {
> +d->labelYOffset += blockLayoutLine.ascent();
I'm fine with this change
> KoTextLayoutTableArea.cpp:464
> +//if we couldn't fit the header rows
> +//try again don't reset cursor->row or we enter infinite loop
> +//cursor->row = 0;
nah this is too aggressive.
There are definitely cases where setting to 0 is the correct thing to do.
There might be some times we enter an infinite loop yes, but we need to catch
this is some other way
REPOSITORY
R8 Calligra
REVISION DETAIL
https://phabricator.kde.org/D15428
To: anthonyfieroni, #calligra:_3.0, danders, boemann
Cc: Calligra-Devel-list, dcaliste, cochise, vandenoever
D15428: [textlayout] Don't enter infinite loop when table is misfit
anthonyfieroni added inline comments. INLINE COMMENTS > boemann wrote in KoTextLayoutTableArea.cpp:464 > nah this is too aggressive. > There are definitely cases where setting to 0 is the correct thing to do. > There might be some times we enter an infinite loop yes, but we need to catch > this is some other way I should see how this would work, because if we reset it to 0 every time it takes same way to parse, as it can see cursor->row is force reset only here. REPOSITORY R8 Calligra REVISION DETAIL https://phabricator.kde.org/D15428 To: anthonyfieroni, #calligra:_3.0, danders, boemann Cc: Calligra-Devel-list, dcaliste, cochise, vandenoever
D15428: [textlayout] Don't enter infinite loop when table is misfit
boemann added a comment. The idea is that if we are at the beginning of a new page we don't get in here (virginpage is true) so we only reset if we are somewhere down on a page meaning we wil have more space to try on next time around But we should reset if the only thing we fitted was the table header as a table headerrow should never be the only thing on a page One reason why this might fail is if virginpage becomes false when we add the table headerrow (if we can fix this and that doesn't have other ill effect then I would prefer such a solution). So find out where we set virginpage to false, and make sure it doesn't go from true to false when adding the header row I hope this makes sense REPOSITORY R8 Calligra REVISION DETAIL https://phabricator.kde.org/D15428 To: anthonyfieroni, #calligra:_3.0, danders, boemann Cc: Calligra-Devel-list, dcaliste, cochise, vandenoever
