When it used percentage line height it *cuts* lower part of the text even at 100%, if percentage is lower it looks even worse. I'm not sure it's proper fix.
Details
Diff Detail
- Repository
- R8 Calligra
- Lint
Lint Skipped - Unit
Unit Tests Skipped
It's not a proper fix no - the height is correct - it's when drawing we have a problem.
The lines below will be drawn on top, and due to some other problem drawing lines will always clear the area first thus cutting away some of the text
Not sure if we can do anything about it
I vote to improve situation, it shouldn't have a reading/writing penalty/glitches in word processing program, no? I can try to correct drawing if it not fit the box, it will be more acceptable to you?
Yes you are more than welcome to try and fix the drawing - however the drawing is done like it is due to an issue with hoe qt renders compare to how it's supposed to look., so if we can fix both the original problem and the new without breaking anything else i'm all for fixing it
I investigate in drawing but i can't see how FormatRange is responsible for that. I think the problem is minimum height of the cell, if it changed in paintings it will allow to resize down, if we increase it, it will cannot.
Did you have a time to investigate? If you have suggest how to fix, i open document with Qt4 version it looks good.
No my linux installation has malfunctioned - could you please add some screenshots so i can see what the problem is more specifically?
In TEST PLAN has a screenshot 'before' you can see lower part of the text is cutoff.
https://phabricator.kde.org/file/data/dpjswbo33ottg5lrcpcf/PHID-FILE-lnwrz73xzlzj2ye3viou/Screenshot_20171228_211944.png
Hmm the line is inside a table, maybe that is what goes wrong - unfortunately the code is not easy to understand, but at least you can try and see why the value is different
It looks indeed like the descend of the line is not used when figuring out the row height - try doubling the font size and see if the cut is still everything below the baseline?
Ok please make the following test:
create 10 lines of text in LO see where the text ends change the lines to 101% and see it becomes just a little bit higher and not lower
This will tell us if LO works like your proposal - and if so then I will accept you patch :)
In Table -> Size -> Row Height... it has 'Fit to size' if it's checked you cannot cut text or make cell higher, but if it's not you can make it whatever you want that cutting or exceeded lines.
// Edit
It's not a percentage but a cm value to edit ?!
I have npt had time to try it out myself - and since you didn't make the test i asked for i have nothing new to tell
I try what you want (screenshot)
https://phabricator.kde.org/file/data/lpjuzqc2vi4a74potaml/PHID-FILE-53wfjewtgqjv4wm7tvgq/Screenshot_20180115_102720.png
As you can see if fit to size isn't checked text is *cutted* (selected one) but you can see it does not have percentage but cm.
This is not correct
I refer to https://www.w3.org/TR/xsl11/#line-height
which states that percentage is releative to fontheight - if not percentage is give a default of 120 percent is used
but you should not multiply 1.2 to the percentage
I say again that the fix should be found in the table code or rendering
Camilla Boemann skrev den 2018-05-28 20:20:
boemann added a comment.
View Revision [1]This is not correct
I refer to https://www.w3.org/TR/xsl11/#line-height
which states that percentage is releative to fontheight - if not
percentage is give a default of 120 percent is usedbut you should not multiply 1.2 to the percentage
Hmmm, it says:
"The computed value of the property is this percentage multiplied by the
element's computed font size"
My question: what is "element's computed font size"?
Could it be what is computed for "normal"?
A thought experiment:
If percent is 100, should we not get the same height as in "normal"?
I say again that the fix should be found in the table code or
rendering
You are probably right, but the the text isn't crystal clear to me...
I investigate couple of days, last year, in rendering and i think problem isn't there. So probably table, but i don't get suspicious code, so text layout can be.
Edit: And the problem persist only in percentage layout.
I have my linux partiotion working again, so I'l take a look this weekend - is there a bug number and a test document somewhere ?
Okay I basically believe you are right, but before I accept I ask you to make the corresponding changes in
textshape/dialogs/ParagraphIndentSpacing.cpp
Where the 120% should be 100% etc
Also having tested with Libreoffice I think the correct scale is 1.16
And finally the code you already wrote can be simplyfied by by not having an else but always multiplying
Also 5 lines below a similar factor for dropcaps should probably also be 1.16
Sorry it took me this long to look into it
in plugins/textshape/dialogs there is a file that handles line height (can't remember which exactly)
when converting between the 1x, 2x etc and a percentage it right now aplies the 1.2
but this is wrong: it should just convert 1x as 100%, 2x as 200%
The 1.2 (or actually 1..16) is just a behind the scenes detail handled in layout
Yeah :)
Seems a unit test needs adjustments, could you have a look?
https://build.kde.org/view/Calligra/job/Calligra%20calligra%20kf5-qt5%20FreeBSDQt5.10/lastCompletedBuild/testReport/(root)/TestSuite/libs_kotextlayout_TestBlockLayout/
given that we no multiply with1.16 the values should be: 12*1.16, 24*1.16, no
Don't know what the last is supposed to be but it should probably have the same treatment
It looks like - no
Actual (blockLayout->lineAt(1).y()): 120.875
Expected (0.0 + 18.0 + 100.0) : 118
and other it does not multiply by 1.16