Fix/line breaking - #4377
Conversation
|
|
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Ok, here is the current pitfall:
So what are the options?
Additionally I have to mention that even if we use Noto in the COLR1 version for chromium and gecko, there will be slightly differences because different versions of chromium support different font features and e.g. our skia-canvas does not support black and white variation codes but the chrome browser does (#3671) What to do? |
yeah im not sure. Cosnidering its 3 weeks after 5.0.0, and 17% of users have upgraded, our aim should be to match 4.3. But changing anything now does worry me about user frustration, but leaving it to 5.1 to fix things that broke in 5.0 will be just as frustrating. So I dont know what is best, other than asap if theyre going to be in 5.0
Did I break this when optimising the parsing so that it wouldnt try and chunk a 10_000 character long string into 1000s or lines, to then only draw 3 of them? Also, if top and middle align show the first lines, should bottom align show the last or first lines?
Probably is (cherry pick once merged). Thats how I've been applying things, and this area should still match almost identically.
This is appealing to me. If we can have chrome and the backend working well and the same for 5.0 then I think that is good enough for today, getting other browsers to work can be the next patch or 5.1
Yeah a lot of the tests I have mostly left to claude. The ones I have read looked fine, but I havent read all of it (way too time consuming and tedious) and figured that its better than not having any tests. |
Ok, I'll go ahead with providing a current but size-adjusted font that works for all but Safari. We'll leave this broken for Safari and see what happens.
I did fix this in the first round, but now have decided to stay with the behaviour of 4.3. It seems that it is like that for a long time and maybe the change was intentional. At least I found it a little bit irritating because our default alignment is center/center that the shown part of the text was changing and it is not easy to understand what part is shown. So the font and test updates will be the last part of this fix. |
|
After thinking about this now for a day and some experimentation, I have a slightly different pitch: We now have a Monospaced font (also Noto) and maybe want to support more fonts in the future. Actually it would be best to not tweak the fonts, but tweak the rendering in the canvas at a font based level. Unfortunately this is not easy to do and also questionable if we should do it for a tool like Companion where typography is not the top priority. Anyhow different fonts should match when used in the same line. So my current go is to adjust the Arimo font to match all of the other used Noto fonts. That way we always can add and update Noto without tweaking and Arimo should stay quite fixed. To compensate for the wider lines of Noto, I'll introduce a factor that reduces the resulting metrics to the current metrics of Arimo (roughly 0.85). We'll call that our new line height 1.0. I think that fits nicely in the future strategy, fixes some existing font problems and doesn't break too much. |
…same height, fix line break for empty lines at start or end, fix unit tests
|
Always when you think, you're done... Actually finished with my list with the now new known issue of problematic emoji rendering in preview renderer in WebKit browsers. Another little thing I noticed but am willing to ignore for now: The preview now shows a tiny bit of shift when changing the alignment of 100% text. The canvas in core does not. I assume some rounding errors. Not to ignore: The text shrinking doesn't work smooth when the maximum text size is close to 100% but not exactly 100%. |
…ased fontsize by relative lineheight
|
Getting close, now a bug seems to have (re-)appeared that the browser canvas works different than the node canvas. |
There seem to be various issues with the line breaking and shrinking and text handling that have accumulated over many versions or are fresh.
Related issues: #3671, #4305, #4343, #4359, #4372,
I'm trying to get this straight asap.
Some of the bugs seem to be there for quite a while so the question is if this is now still a fix or a change. I guess some presets and styles had been created with the last breaking behavior. I don't know if it is better to say: "now the old stuff works again as expected and you in-between guys are out of luck" or "the existing bug is now the new normal and the old stuff has been broken so long that it doesn't matter anymore".
Overall it should affect only some rather seldom cases.
Line breaking will now again first shrink and only break inside a word if there is no shrinking option any more. This will affect a lot of dynamic text in a positive way but should not affect presets or styles that have been carefully tweeked. (actually that was bugging me for quite a while and I'm so happy to fix this)
Second difference is the alignment when the test overflows the space. Since quite a while it seems that in case of overflow always the beginning of the text was fitted into the available space and then aligned. This now changes it back to the original concept that also uses the bottom lines or the center lines for bottom or center alignment. I think both variations are not 100% great, the last behavior was maybe 50% nice and the original behavior 60% nice. Given that this shouldn't be a feature that people use on purpose, I think it is acceptable to go back to the original behavior and maybe even improve this a little in the future (not part of the fix).
Line height measurement is rather exact and line height of 100% still gives no visual shift when alignment changes
unicode chars are now again preserved when breaking a line
some unicode chars are not rendered
style preview and button rendering are using different emoji fonts
line height is still fixed to 1.0 and not 1.1 like originally. I think this change is quite old too and we should keep it for now and make this a configurable property eventually
a lot of tests are failing now because my cheapo free AI wasn't able to fix them in acceptable time. Also some tests are looking useless to me, had they been checked in deph?
Unfortunately I have based that branch on main. I don't know if it is easy to merge into 5.0 stable