Bug Hunting Session
Bug 99315 - Rendering of a double border line changed
Summary: Rendering of a double border line changed
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: graphics stack (show other bugs)
Version:
(earliest affected)
5.2.0.0.alpha0+
Hardware: All All
: medium normal
Assignee: Miklos Vajna
URL:
Whiteboard: target:5.2.0 target:5.1.4
Keywords: regression
Depends on:
Blocks:
 
Reported: 2016-04-15 07:46 UTC by Miklos Vajna
Modified: 2016-10-25 19:08 UTC (History)
1 user (show)

See Also:
Crash report or crash signature:


Attachments
Reproducer document (7.40 KB, application/vnd.oasis.opendocument.spreadsheet)
2016-04-15 07:46 UTC, Miklos Vajna
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Miklos Vajna 2016-04-15 07:46:07 UTC
See the attached test document. It has a double border. Given that both the inner and the outer border are 0.03cm, the rendering result on the screen should be (at 130%, assuming 96 DPI) ideally around 1.47 pixels, so realistically 1 pixel wide, ignoring anti-aliasing.

Expected result:

Two separate lines, each 1px wide.

Actual result:

A single line, 5px wide.
Comment 1 Miklos Vajna 2016-04-15 07:46:24 UTC
Created attachment 124354 [details]
Reproducer document
Comment 2 Miklos Vajna 2016-04-15 07:59:47 UTC
Given that we have a long history of introducing regressions with fixing border rendering, I'm trying to collect what I found in the bugzilla and git history that's related to this problem:

1) OOo and LO 3.5 used to render two separate 1px-wide lines. That matches the ideal result and if the rendering changes, I consider that a regression.

2) It seems OOo didn't use drawinglayer for border rendering, so parallel debugging OOo and LO master doesn't make sense.

3) The last time the rendering was correct was before Kohei's 2c62596cf264ef10749d8bfdb2bb2ebef2d98fbc for bug 75260, but that commit is needed, as in general we do not want the double-thin behavior for ODF double borders. It was just an accident that for the bugdoc it resulted in correct rendering behavior.

One detail that looks problematic in that commit is the +1.0 in makeSolidLinePrimitive(), as adding a hardcoded number can't be correct: it will have a different result when the input is 15 twips or 1 pixel (the result will be 16 twips or 2 pixels, which are different widths on the screen).

Avoiding that +1 gives us two lines instead of one: a 1px one and a 2px one (width).

4) Another commit is Moggi's 2c91cb08d65cd35fa8ef6eaca3677aa82fb58cbe for bug 33634, which changes ints to doubles in svx::frame::Style. With that, it's possible that e.g. with a 22.480221 offset the 1.47 width becomes 22.480221 -> 23.950220999999999, which gets rounded to 22 -> 24, and the result is a line of 2px width.

I guess the way to fix that is to detect the situation in vclpixelprocessor2d, and do pixel correction of the border polygon there.

Note that the document has a double border type, both Excel import and the UI can only create double-thin border type, so the problem is specific to legacy ODS files.
Comment 3 Commit Notification 2016-04-15 14:48:18 UTC
Miklos Vajna committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=1ee570a4e625719f8bf270d372926c0d829ae6f0

Related: tdf#99315 BorderLinePrimitive2D: fix solid line primitive width

It will be available in 5.2.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 4 Miklos Vajna 2016-04-15 15:32:39 UTC
The above fixes 3) but not 4), so not closing yet.
Comment 5 Commit Notification 2016-04-18 19:08:46 UTC
Miklos Vajna committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=422f10c5d7ebe6f4b778636c9c1eb6dbdf708a27

tdf#99315 VclPixelProcessor2D: fix double border line width

It will be available in 5.2.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 6 Commit Notification 2016-04-20 12:11:39 UTC
Miklos Vajna committed a patch related to this issue.
It has been pushed to "libreoffice-5-1":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=a5928a57a53df84cacd2975e227f9e03d50456a6&h=libreoffice-5-1

tdf#99315 VclPixelProcessor2D: fix double border line width

It will be available in 5.1.4.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.