Bug 103322 - Use floating point for glyph positioning in VCL
Summary: Use floating point for glyph positioning in VCL
Status: NEW
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: graphics stack (show other bugs)
Version:
(earliest affected)
Inherited From OOo
Hardware: All All
: medium enhancement
Assignee: Not Assigned
URL:
Whiteboard:
Keywords:
: 105936 106495 113665 (view as bug list)
Depends on:
Blocks: 44267 Font-Rendering 88991
  Show dependency treegraph
 
Reported: 2016-10-18 22:52 UTC by Khaled Hosny
Modified: 2018-08-16 11:35 UTC (History)
20 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Khaled Hosny 2016-10-18 22:52:47 UTC
Currently we use integers and do lots of calculations and recalculations resulting in rounding errors which mess up with text spacing specially at low resolution screens.

We should switch to using floats to store glyph positions instead, which would make the rounding errors less visible. It would also allow us to benefit from sub-pixel positioning support in the graphics libraries we use.
Comment 1 Buovjaga 2017-02-12 16:28:18 UTC
*** Bug 105936 has been marked as a duplicate of this bug. ***
Comment 2 DeepFlight5 2017-02-13 18:13:54 UTC
My Bug 105936 report was moved here. Probably the right place. But here I read "earliest affected version 5.2.2.2 release".

Not true on MacOSX: All LO versions up to 5.2.5 have no problems with glyphs positioning, all work excellent.

This problem begun only with LO 5.3, and there it is on MacOSX massively. I had to downgrade to LO 5.2.5. It's not possible to work with this broken font display.

(On Linux and Windows this might be different.)

The problem is HarfBuzz, which was only introduced in LO 5.3. I hope this gets fixed soon.
Comment 3 Khaled Hosny 2017-02-13 22:41:53 UTC
The underlying issue is as old as this code base, it started showing up on Mac because we no longer use Core Text (which uses floats for glyph positions) and share the same code on all platforms.
Comment 4 DeepFlight5 2017-02-14 19:58:51 UTC Comment hidden (no-value)
Comment 5 Khaled Hosny 2017-03-12 17:01:05 UTC
*** Bug 106495 has been marked as a duplicate of this bug. ***
Comment 6 DeepFlight5 2017-04-07 19:12:46 UTC
Sorry, my commentary from February had an error. It should read:

"I assume the "float" problem lies in the LibreOffice code and not in HarfBuzz, since HarfBuzz is used in Firefox and Chrome also, which have no problems with accurate glyph display. Will this "float" bug of LibreOffice be fixed anywhere soon?"
Comment 7 Khaled Hosny 2017-04-08 14:36:50 UTC
No one is currently working on this, feel free to work on it yourself or recruit someone to work on it.
Comment 8 Yousuf Philips (jay) (retired) 2017-10-02 17:55:00 UTC Comment hidden (no-value)
Comment 9 Khaled Hosny 2017-10-02 21:10:11 UTC Comment hidden (no-value)
Comment 10 Buovjaga 2017-12-28 19:00:31 UTC
*** Bug 113665 has been marked as a duplicate of this bug. ***
Comment 11 Mike Kaganski 2018-07-24 07:22:46 UTC
Do we have a (compact) API subset that needs to be changed from int to double to start the change?
Comment 12 Khaled Hosny 2018-07-24 09:47:12 UTC
(In reply to Mike Kaganski from comment #11)
> Do we have a (compact) API subset that needs to be changed from int to
> double to start the change?

I’d start with the “Text functions” section in include/vcl/outdev.hxx and go down and up from there.

We will need to switch from tools::Point, tools::Rectangle etc. to basegfx::B2DPoint, basegfx::B2DRectangle, not just int to float/double. May be using basegfx::B2IPoint first since they are still int-based, so the first round would focus on the difference in semantics and API between tools and basegfx, then next round would tackle the actual floating pint conversion.
Comment 13 Rachel Greenham 2018-07-31 10:35:22 UTC
For me it presents as a HiDPI bug, in that it's when desktop scaling is at 200%, 300%, text (any font) can be quite a reasonable editing size on screen but show this poor positioning of glyphs within each word, making it headache-inducing to look at, and actually a serious impediment to wanting to use the software. Of course it was there before, HiDPI just amplifies it, making it a problem at larger font sizes as displayed on screen.

As the worst problem is just the positioning of glyphs within words I looked for the code that does that. It seems to be mostly in vcl/source/gdi/CommonSalLayout.cxx, where the actual work is delegated to Harfbuzz, which appears to work in integers, but I think at a very high resolution (whatever a upem is). The values that then come out of that then have a scale applied at double, rounded, and I suspect subject to more scaling later, in particular with respect to the desktop scaling factor in HiDPI modes. And that's probably where it's going wrong.

I had a play with trying to fix things locally in here and in vcl/source/gdi/sallayout.cxx, changing all the glyph positioning code to use doubles (and B2DPoint etc), including changing GlyphItem (sallayout.hxx) to use those types for its positions, widths and offsets and rounding as necessary at the edges of that, but I did rather get lost in the weeds, specifically, where to find the edges of it, to minimise the disruption to the rest of the codebase.

I suspect a fix can't really be confined like that, and it's probably undesirable anyway. It looks like we'd probably want to make DeviceCoordinate a double (as controlled in config_host/config_vcl.h.in showing this has been at least partially prepared-for) and follow *everywhere* that leads, one such place being the text functions in outdev.hxx of course.

Then final rounding to integers for actual pixels on screen would probably want to happen in platform specific code. As absolutely late as possible, at the moment of output, anyway. Then if and when there's a platform that takes double coordinates that rounding simply doesn't have to happen there.

Regarding the suggestion of going via B2IPoint and friends as an intermediate step towards B2DPoint and friends, I note that the B2I types are carefully defined to use 32-bit ints (sal_Int32), whereas Point, Rectangle et al use long (which let's face it *is* 64-bit almost everywhere that matters now) as does much existing position-calculating code. So I'd worry that we'd be setting ourselves up for a fight to resolve issues relating to that loss of range when it's only needed as an intermediate step.

... All of which would be an absolutely gigantic changeset, no wonder no-one who knows the code wants to do it! The more I poked around in this the more I thought, "Oh God, no..." and "Can we *really* not localise this fix in sallayout?" :-) But no, ultimately changing *all* the device coordinate code to double is probably what has to happen one day.

Another thought I had, noticing that in OpenOffice.org Writer on a 4K screen of course has no HiDPI awareness to speak of, so its user interface is unusable, but zoom in on the text to an editing size and it looks fine - at 240% zoom you would expect that, of course. Pretty much the same applies in LibreOffice Writer if you set your desktop scaling to 100% on a 4K screen. So I wondered, what if instead you just *not* apply desktop scaling on the document view panes/frames whatever they're called here, and, um, *lie* on the user interface, ie: multiply the user selected zoom by the desktop scaling factor? :-)
Comment 14 Khaled Hosny 2018-08-04 12:18:00 UTC
Another alternative to floating point, is to keep using integers but use a scale, so instead of shaping at 10 pixels and drawing at the coordinates we get, we shape at 10 * SCALE and shape ant coordinates / SCALE. Which is what many other libraries do, and I vaguely remember that we have something similar by may be we are not using it thoroughly. Worth investigating.