Bug 99165 - Rendering of mitered curve changes with zoom level or at scroll boundaries
Summary: Rendering of mitered curve changes with zoom level or at scroll boundaries
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Impress (show other bugs)
Version:
(earliest affected)
Inherited From OOo
Hardware: x86 (IA32) Windows (All)
: medium normal
Assignee: Armin Le Grand (allotropia)
URL:
Whiteboard: target:5.2.0 target:5.3.0
Keywords:
Depends on:
Blocks:
 
Reported: 2016-04-08 14:47 UTC by Regina Henschel
Modified: 2016-09-29 08:50 UTC (History)
2 users (show)

See Also:
Crash report or crash signature:


Attachments
fat Bezier curve for testing (11.42 KB, application/vnd.sun.xml.impress)
2016-04-08 14:47 UTC, Regina Henschel
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Regina Henschel 2016-04-08 14:47:02 UTC
Created attachment 124188 [details]
fat Bezier curve for testing

Set LO to not use OpenGL, enable anti-aliasing.
Open attached file and look at it with several zoom levels. Starting add 140% and increasing in 1% steps should trigger the error.

Seen behavior: The rendering of the corner in x=5 y=4 varies with the zoom level.

Expected behavior: It is always drawn as line join "Bevel".

You can see the expected rendering, when you disable anti-aliasing.

I use Windows 7 here. It would be nice, if someone can test on other platforms.
Comment 1 Jens Mildner 2016-04-09 10:43:08 UTC
Confirmed on LO 5.0.5.2 on Windows XP SP3.

Disabling anti-aliasing solves this strange behaviour.
Comment 2 Armin Le Grand (allotropia) 2016-04-11 11:01:10 UTC
Checked what happens on WIn: Indeed the fat line is painted by gdiplus, in WinSalGraphicsImpl::drawPolyLine. The correct miter type is sued leading to
                const Gdiplus::REAL aMiterLimit(15.0);
                Gdiplus::DllExports::GdipSetPenMiterLimit(pTestPen, aMiterLimit);
                Gdiplus::DllExports::GdipSetPenLineJoin(pTestPen, Gdiplus::LineJoinMiter);

what should work. Also tried to use LineJoinMiterClipped which seems to be the better default to get the standard-fallback to bevel. Did not help, still happens.
BTW: Disabling AA falls back to decompositon, so this works.
Comment 3 Armin Le Grand (allotropia) 2016-04-11 13:13:10 UTC
Tried to use 'AddBeziers' (and AddLines) instead of 'AddBezier' in impAddB2DPolygonToGDIPlusGraphicsPathReal because I guessed that when adding single beziers where the end point of the current is always the start point of the next may indeed add double points to the Gdiplus::GraphicsPath, but the result is the same.
Comment 4 Armin Le Grand (allotropia) 2016-04-11 13:16:19 UTC
Just saw that it does not just depend on zoom - it also depends on paint position. Using the right (vertical) scrollbar shows that sometimes some miter geometry is painted, sometimes not. Very strange...
Comment 5 Armin Le Grand (allotropia) 2016-04-11 13:32:59 UTC
Tried to not set MiterLimit at all and use Win default (10.0f), same result. It can not have to do with zoom when taking the Gdiplus docs serious: "The miter limit is the maximum allowed ratio of miter length to stroke width.". Something relative (ratio) is scale-invariant per definition.
Comment 6 Armin Le Grand (allotropia) 2016-04-12 08:19:19 UTC
Adapted title to reflect that this also happens when verticaly zooming - the Miter edge representation gets painted in stripes (sometimes yes - sometimes no, mixed with scrolling in)
Comment 7 Armin Le Grand (allotropia) 2016-04-12 11:14:27 UTC
Found the reason. The example curves do not have control vectors in the common point which represents the 'tip'. This leads to bezier segments that have either start point and 1st control point equal (none set) or 2nd control point and last point equal (none set).
These are mathematically valid bezier definitions (and get painted), but MS Gdiplus fails in handling these to extract the needed vectors for generating the Miter (and probably bevel) geometry. The tangent calculation(s) (derivations) in point 0.0 and 1.0 get different (actually simplified). Our own geometry stack in basegfx knows and handles that (see B2DCubicBezier::getTangent), thus our internal geometry creation and decompositions work well.
MS cannot handle that, but there is no alternative call to add such bezier curves to a GraphicPath. Luckily, it is mathematically not hard to create replacement vectors/points, unluckily this has to be checked and calculated (and only for a detectable number of cases, too).
Checking how to do this best...
Comment 8 Armin Le Grand (allotropia) 2016-04-12 12:22:24 UTC
Have a solution for Win Gdiplus. Potentially, other Graphic sub-systems will have the same problem, need to check that...
Comment 9 Armin Le Grand (allotropia) 2016-04-12 12:46:58 UTC
OpenGL creates an endless Miter geometry for the example case, needs an new bug. Added Bug 99244 for this.
Comment 10 Armin Le Grand (allotropia) 2016-04-12 12:49:50 UTC
Loos like Cairo has the same/similar problems, at least when scrolling (tried in a local build), so also needs correction. There are three places with cairo_curve_towhich need correction.
Comment 11 Armin Le Grand (allotropia) 2016-04-12 13:40:15 UTC
Seems less critical in cairo, nonetheless correction will be safer. Looking to adapt places...
Comment 12 Armin Le Grand (allotropia) 2016-04-13 10:28:36 UTC
Done, fix on gerrit
Comment 13 Commit Notification 2016-04-13 10:30:46 UTC
Armin Le Grand committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=9cda847a0bec307a909b927e0928cdbb0b00fc81

tdf#99165 always provide control points for beziers

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 14 Commit Notification 2016-04-13 11:52:45 UTC
Armin Le Grand committed a patch related to this issue.
It has been pushed to "master":

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

tdf#99165 initialize nLastX, nLastY

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 15 Commit Notification 2016-07-07 20:35:47 UTC
Armin Le Grand committed a patch related to this issue.
It has been pushed to "master":

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

tdf#99165 avoid passing empty control points for beziers

It will be available in 5.3.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 16 Xisco Faulí 2016-09-26 10:10:12 UTC
Hello Armin,
Is this bug fixed?
If so, could you please close it as RESOLVED FIXED?
Comment 17 Armin Le Grand (allotropia) 2016-09-29 08:50:03 UTC
Checked, works.