Bug 1021803 - glamor (modeset/Xephyr) Wrong lineends in XDrawLine()
Summary: glamor (modeset/Xephyr) Wrong lineends in XDrawLine()
Status: RESOLVED FIXED
Alias: None
Product: openSUSE Distribution
Classification: openSUSE
Component: X.Org (show other bugs)
Version: Leap 42.2
Hardware: x86-64 openSUSE 42.2
: P2 - High : Normal (vote)
Target Milestone: ---
Assignee: Max Staudt
QA Contact: E-mail List
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-01-25 08:00 UTC by Martin Schreiber
Modified: 2017-06-26 14:40 UTC (History)
3 users (show)

See Also:
Found By: ---
Services Priority:
Business Priority:
Blocker: ---
Marketing QA Status: ---
IT Deployment: ---


Attachments
Testprogram (1.18 KB, text/plain)
2017-01-25 08:00 UTC, Martin Schreiber
Details
wrong.png (6.13 KB, image/png)
2017-01-25 08:01 UTC, Martin Schreiber
Details
ok.png (4.52 KB, image/png)
2017-01-25 08:02 UTC, Martin Schreiber
Details
Xorg log (34.26 KB, text/plain)
2017-01-25 08:02 UTC, Martin Schreiber
Details
xephyr1.png (5.35 KB, image/png)
2017-01-25 14:10 UTC, Martin Schreiber
Details
xephyr2.png (6.13 KB, image/png)
2017-01-25 14:10 UTC, Martin Schreiber
Details
OpenGL implementation of upper part of the test program (2.29 KB, text/plain)
2017-02-06 22:08 UTC, Max Staudt
Details
XSetLineAttributes(disp,gc,1,LineSolid,CapButt,JoinMiter); (6.12 KB, image/png)
2017-02-17 13:41 UTC, Martin Schreiber
Details
XSetLineAttributes(disp,gc,1,LineSolid,CapProjecting,JoinMiter); (6.12 KB, image/png)
2017-02-17 13:44 UTC, Martin Schreiber
Details
Test program with line width 1 (1.24 KB, text/plain)
2017-02-17 14:42 UTC, Max Staudt
Details
Line drawing program. (14.71 KB, image/png)
2017-03-04 11:35 UTC, Martin Schreiber
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Schreiber 2017-01-25 08:00:36 UTC
Created attachment 711516 [details]
Testprogram

While testing MSEide+MSEgui on Leap 42.2 on a Acer Veriton M4640G I found that XDrawLine() has wrong line ends, see attached test-program and screenshot (wrong.png).
32 bit openSUSE 13.2 on the same machine shows the correct result (ok.png).
Xorg log is attached.
Comment 1 Martin Schreiber 2017-01-25 08:01:49 UTC
Created attachment 711517 [details]
wrong.png
Comment 2 Martin Schreiber 2017-01-25 08:02:19 UTC
Created attachment 711518 [details]
ok.png
Comment 3 Martin Schreiber 2017-01-25 08:02:48 UTC
Created attachment 711519 [details]
Xorg log
Comment 4 Stefan Dirsch 2017-01-25 10:11:01 UTC
Hmm. Just to rule out an Intel Skylake driver related issue (glamor/DRI driver). Is this reproducable in Xephyr (non accelerated Xserver)?

  sudo zypper in xorg-x11-server-extra
  Xepyhr :99 &
  DISPLAY=:99 <test_program>
Comment 5 Martin Schreiber 2017-01-25 10:23:28 UTC
In Xephyr the display of the test program is OK.
Comment 6 Stefan Dirsch 2017-01-25 13:12:53 UTC
Hmm. Is there a difference when forcing Xephyr to start with glamor enabled?

  Xepyhr -verbosity 9 -glamor :99 &
  DISPLAY=:99 <test_program>

If this reproduces the issue, please try again via

  LIBGL_ALWAYS_SOFTWARE=1 Xepyhr -verbosity 9 -glamor :99 &
  DISPLAY=:99 <test_program>
Comment 7 Martin Schreiber 2017-01-25 14:10:08 UTC
Created attachment 711587 [details]
xephyr1.png
Comment 8 Martin Schreiber 2017-01-25 14:10:43 UTC
Created attachment 711589 [details]
xephyr2.png
Comment 9 Martin Schreiber 2017-01-25 14:11:43 UTC
"
mse@mseleap:~/proj/testcase/leap/x11lineend>  Xephyr -verbosity 9 -glamor :99 &
[1] 19983
mse@mseleap:~/proj/testcase/leap/x11lineend> (II) AIGLX: enabled GLX_MESA_copy_sub_buffer
(II) AIGLX: Loaded and initialized swrast
(II) GLX: Initialized DRISWRAST GL provider for screen 0
(II) XKB: Reusing cached keymap
(II) XKB: Reusing cached keymap
DISPLAY=:99 ./x11lineend
"
Lineendings are wrong, dashed line is solid, see xephyr1.png.

"
mse@mseleap:~/proj/testcase/leap/x11lineend> LIBGL_ALWAYS_SOFTWARE=1 Xephyr -verbosity 9 -glamor :99 &
[1] 20315
mse@mseleap:~/proj/testcase/leap/x11lineend> (II) AIGLX: enabled GLX_MESA_copy_sub_buffer
(II) AIGLX: Loaded and initialized swrast
(II) GLX: Initialized DRISWRAST GL provider for screen 0
(II) XKB: Reusing cached keymap
mse@mseleap:~/proj/testcase/leap/x11lineend> DISPLAY=:99 ./x11lineend
"
Vertical line OK, horizontal line wrong,  dashed line is solid, see xephyr2.png.
Comment 10 Stefan Dirsch 2017-01-25 14:51:09 UTC
Thanks! We can reproduce the issue here. With glamorified Xephyr, but also on Intel Skylake and Broadwell (glamorified modesetting DDX). We're going to look into this ASAP.
Comment 11 Stefan Dirsch 2017-01-25 14:54:51 UTC
As a workaround you may switch to intel DDX, but this driver has different issues. :-(
Comment 12 Max Staudt 2017-02-06 16:28:25 UTC
I've identified the xserver commits that cause the distortion in Xephyr:

dc9fa9080a1cb994b4e54a341d2245f442dac576
 -> Breaks the corners
 -> Makes the dashed line appear at all

d18f5801c9a632dd4d9f8b7912491b6623e943d5 (the next one)

 -> Fills the dashed line into a continuous one

What's interesting is that with LIBGL_ALWAYS_SOFTWARE=1 one of the corners is rendered correctly. Also, we get different behavior with Xephyr -glamor_gles2 (where available in newer versions) and various hardware renderers, so this may well be (partially?) an issue with the OpenGL backends.
Comment 13 Max Staudt 2017-02-06 22:08:50 UTC
Created attachment 713076 [details]
OpenGL implementation of upper part of the test program

This test program reimplements the upper half of the X11 program's drawing operations in straight OpenGL. It exhibits the same issues as the X11 program in Comment 0 when the latter is run through GLAMOR, starting with X server commit dc9fa9080a1cb994b4e54a341d2245f442dac576.

What's interesting is that LIBGL_ALWAYS_SOFTWARE=1 does not improve the output. No idea why that is.

On the other hand, all rightmost and bottommost pixels of the lines are missing. This is probably what keithp meant in the commit mentioned above: There is special handling for the last pixel. However, that last pixel is not always immediately to the right of the last one, so the workaround that's currently in GLAMOR is not a general solution.
Comment 17 Max Staudt 2017-02-07 15:24:11 UTC
Reported part 1 upstream: https://bugs.freedesktop.org/show_bug.cgi?id=99705
Comment 18 Max Staudt 2017-02-07 17:54:15 UTC
Reported part 2 upstream: https://bugs.freedesktop.org/show_bug.cgi?id=99708
Comment 19 Max Staudt 2017-02-17 12:56:43 UTC
Okay, so as far as I can see from the opstream discussion at

  https://lists.x.org/archives/xorg-devel/2017-February/052624.html

it's better to use 1-width lines instead of 0-width lines when you need exact rasterization. So given the X spec and the reactions, changing your drawing code seems to be the technically correct fix.

Should such bugs accumulate, then we will consider disabling the line acceleration code in GLAMOR. For now, it looks like 0 width lines are meant for draft use, not for beautiful UIs.


Does switching to 1 width lines on the toolkit side solve your issue?
Comment 20 Martin Schreiber 2017-02-17 13:41:22 UTC
Created attachment 714597 [details]
XSetLineAttributes(disp,gc,1,LineSolid,CapButt,JoinMiter);
Comment 21 Martin Schreiber 2017-02-17 13:44:21 UTC
Created attachment 714598 [details]
XSetLineAttributes(disp,gc,1,LineSolid,CapProjecting,JoinMiter);
Comment 22 Max Staudt 2017-02-17 13:54:53 UTC
From this point onwards, you're using Xorg's software drawing routines, which should be clearly specified and produce reproducible output, unline zero width lines. So you can realign your code to this.

FYI, here's the upstream discussion:

  https://lists.x.org/archives/xorg-devel/2017-February/thread.html#52624

Basically, it seems that using 0 width lines is a problem in the first place, and the only thing to debate is consistency between the software and hardware renderer in the 0 width case. For GUI toolkits, using 1-width lines is a better fit.

As for the dashed line, according to the discussion above, that seems like either a bug in in the Intel driver in Mesa, or in Mesa's software renderer. Please report this upstream at bugzilla.freedesktop.org - make sure to reference fdo#99708 as well as this bug.

Once a fix is known, we would be very grateful for feedback - we will then do our best to integrate it into openSUSE for you.
Comment 23 Martin Schreiber 2017-02-17 14:11:35 UTC
> Does switching to 1 width lines on the toolkit side solve your issue?

No, see attachments XSetLineAttributes.
From
https://tronche.com/gui/x/xlib/GC/manipulating.html#cap-style
"
The cap-style defines how the endpoints of a path are drawn:
CapNotLast 	This is equivalent to CapButt except that for a line-width of zero the final endpoint is not drawn.
CapButt 	The line is square at the endpoint (perpendicular to the slope of the line) with no projection beyond.
CapRound 	The line has a circular arc with the diameter equal to the line-width, centered on the endpoint. (This is equivalent to CapButt for line-width of zero).
CapProjecting 	The line is square at the end, but the path continues beyond the endpoint for a distance equal to half the line-width. (This is equivalent to CapButt for line-width of zero). 
"
so it looks to me line endpoints are well defined in Xlib. A line draw command which sometimes draws start-/endpoints and sometimes not is pretty useless IMHO. And MSEide+MSEgui is also a modern GUI-toolkit which does not "mostly pushing huge images". ;-)
Comment 24 Max Staudt 2017-02-17 14:24:59 UTC
(In reply to Martin Schreiber from comment #23)
> so it looks to me line endpoints are well defined in Xlib. A line draw
> command which sometimes draws start-/endpoints and sometimes not is pretty
> useless IMHO.

Yes, I fully agree. So if Xorg does not do the right thing, that's a bug with Xorg, not openSUSE ;)


> And MSEide+MSEgui is also a modern GUI-toolkit which does not
> "mostly pushing huge images". ;-)

It wasn't my intention to downplay MSEide+MSEgui, rather I was making the point that toolkits should as yours deserve to be rendered correctly!

In any case, unlike GTK/Qt, you're targeting X11 instead of pushing huge images, which is a completely valid thing to do. And you expect the server to behave correctly (validly so), so if it doesn't, then that's a valid Xorg bug in my opinion. That's what I was getting at.
Comment 25 Max Staudt 2017-02-17 14:42:12 UTC
Created attachment 714610 [details]
Test program with line width 1

Okay, something strange happened. I think I reproduced it once, and now I can no longer reproduce the bug with line width 1. Can you please double check?

I have attached the program I tested this with, it's your initial code plus your new line to set the line width.
Comment 26 Martin Schreiber 2017-02-17 14:42:57 UTC
Is it correct that in order to get the old behavior with CapButt/0-linewidth on newer X11 implementations one can set linewidth to 1 and lineend to CapProjecting on solid lines and for dashed lines one has to set linewidth to 1 and lineend to CapButt?
Comment 27 Max Staudt 2017-02-17 14:43:29 UTC
Ah, I got confused with the two different cap styles. Let me have another look!
Comment 28 Martin Schreiber 2017-02-17 14:45:38 UTC
> Okay, something strange happened. I think I reproduced it once, and now I can
> no longer reproduce the bug with line width 1.

Again our mails crossed. :-)
The difference is CapButt/CapProjecting.
Comment 29 Max Staudt 2017-02-17 14:52:00 UTC
(In reply to Martin Schreiber from comment #26)
> Is it correct that in order to get the old behavior with CapButt/0-linewidth
> on newer X11 implementations one can set linewidth to 1 and lineend to
> CapProjecting on solid lines and for dashed lines one has to set linewidth
> to 1 and lineend to CapButt?

Well, I wouldn't compare this to 0-width, since upstream said those are poorly specified. The question is, how are 1-width lines specified? And I expect Xorg's software renderer to do the right thing with widths of 1 or more. Basically, widths of 0 are a gimmick for fast, inaccurate output.
Comment 30 Max Staudt 2017-02-17 14:53:37 UTC
Hmm, there are actually totally different drawing routines for 0-width lines and 1+ width lines...

https://cgit.freedesktop.org/xorg/xserver/tree/mi/miwideline.c#n2498
Comment 31 Martin Schreiber 2017-02-17 15:03:23 UTC
From 1999 when I started MSEgui up to openSUSE 13.2 I never encountered another Xlib 0-line behavior with CapButt than painting the first and last line-points and to draw a 2/2 dashed line as 2/2. So we can rephrase the question as:

Is it correct that in order to get the first and last point of a 1 pixel width line one must set linewidth to 1 and lineends to CapProjecting on solid lines and for dashed lines one must set linewidth to 1 and lineend to CapButt?
Comment 32 Max Staudt 2017-02-17 15:15:26 UTC
To be fair, I don't have such intimate knowledge of X11 protocol/Xorg implementation details.
However, try a line width of 4. It seems like you want to use CapProjecting for the lines making up the squares, indeed.

As for the 0-width lines, GLAMOR wasn't part of the X server until very recently, and it seems to deliberately use imprecision in both the X11 and OpenGL specs to its advantage. Well, in the line ends case, that is. As for the dashing, that seems more like an OpenGL error.
So you probably never saw different behavior before simply because it was always the same code rendering your lines (I don't know what XAA drivers did, though).
Comment 33 Max Staudt 2017-02-17 15:27:03 UTC
I have reported the issue against Mesa as well:

https://bugs.freedesktop.org/show_bug.cgi?id=99849
Comment 34 Max Staudt 2017-02-17 15:31:28 UTC
Let's keep the bug open as a reminder of the upstream bugs.
Comment 35 Max Staudt 2017-03-03 17:30:31 UTC
Martin, would you be so kind as to try this X server for 42.2?

https://build.opensuse.org/package/show/home:mstaudt:1021803boo-glamor-xdrawline/xorg-x11-server.openSUSE_Leap_42.2_Update

There, I have disabled the GLAMOR acceleration that draws your 0-width lines in unexpected ways.

If you have any comments on performance, I would greatly appreciate them, too.
Comment 36 Martin Schreiber 2017-03-04 11:35:10 UTC
Created attachment 716339 [details]
Line drawing program.

Where can I get the RPM?
MSEgui has a workaround for the imprecise lineends where it changes 0-width lines to 1-width capprojecting for solid lines and 1-width capbutt for dashed lines. I made a MSEgui bechmark program, see attachment. The project is here:
https://gitlab.com/mseide-msegui/mseuniverse/tree/master/testcase/benchmark/linedrawing

Some results:

workaround dashed slanted linesegments/sec

openSUSE 13.2 32bit
0          0      0       3.9M
0          0      1       1.05M
0          1      0       2.26M
0          1      1       1.2M
1          0      0       980k
1          0      1       114k
1          1      0       64k
1          1      1       27.3k

Leap 42.2 Xorg 7.6_1.18.3-0-x86_64
0          0      0       7.68M
0          0      1       4.8M
0          1      0       5.8M no dashes visible!
0          1      1       4.09M no dashes visible!
1          0      0       184k
1          0      1       184k
1          1      0       9.1k
1          1      1       4.56k

Windows 7
0          0      0       3.55M
0          0      1       1.89M
0          1      0       750
0          1      1       184

Seeing the massive performance loss with 1-width lines I probably can not activate the workaround by default.
Comment 37 Max Staudt 2017-03-06 13:10:45 UTC
Whoops, I forgot to publish the build.

You can now install the new X server as follows:

  sudo zypper ar https://download.opensuse.org/repositories/home:/mstaudt:/1021803boo-glamor-xdrawline/openSUSE_Leap_42.2_Update/home:mstaudt:1021803boo-glamor-xdrawline.repo

  sudo zypper refresh

  sudo zypper dup


Once you're done testing:

  sudo zypper removerepo home_mstaudt_1021803boo-glamor-xdrawline
  sudo zypper refresh
  sudo zypper dup
Comment 38 Martin Schreiber 2017-03-07 14:23:44 UTC
Line-ends are OK.

Leap 42.2 Xorg 7.6_1.18.3-13.2-x86_64

workaround dashed slanted linesegments/sec
0          0      0       600k
0          0      1       335k
0          1      0       9k
0          1      1       4.5k
1          0      0       177k
1          0      1       177k
1          1      0       9.1k
1          1      1       4.5k
Comment 39 Max Staudt 2017-03-15 16:29:00 UTC
Martin, thanks for the benchmarks!

Unfortunately they show a regression from 13.2 to 42.2 - from a GUI writer point of view, is the speed still high enough? Do you think we shall we go ahead and disable GLAMOR acceleration for lines?
Comment 40 Martin Schreiber 2017-03-16 08:01:40 UTC
A difficult question.
Drawing of buttons, widget frames and the like probably is fast enough on modern hardware. For projects which use line drawing extensively (oscilloscope, line-charts...) the slowdown is drastic, 3.9M for openSUSE 13.2 to 600k for Leap 42.2 on the same machine.
Is there really no hope to improve hardware acceleration so it becomes usable for drawing lines including the endpoints?
Comment 41 Max Staudt 2017-03-17 13:39:44 UTC
Alright, we're in an unpleasant situation then.

I'll submit a patch to draw the 0-width dashed lines in software, and 0-width dashed lines in GLAMOR.

The rationale is that dashed lines should be much less common, and render completely wrong right now. On the other hand, solid lines are almost correct, and there was recently another post to the upstream email thread so there may be a proper fix.
Comment 42 Max Staudt 2017-03-17 14:17:07 UTC
Of course I meant that 0-width *solid* lines are still drawn through GLAMOR.

https://build.opensuse.org/request/show/480856
Comment 43 Max Staudt 2017-03-21 16:48:38 UTC
As hinted at by Roland in the upstream bug, Eric's upstream commit fe0b297420fc fixes the dashed lines.

I've replaced my software fallback hack with a backport of this patch, which should appear in the Leap repo once it passes QA.

As for the solid lines, they're probably less of an issue, and the upstream bug report is still open. Let's track this there and close the openSUSE bug for now.
Comment 44 Bernhard Wiedemann 2017-03-21 17:00:20 UTC
This is an autogenerated message for OBS integration:
This bug (1021803) was mentioned in
https://build.opensuse.org/request/show/481820 42.2 / xorg-x11-server
https://build.opensuse.org/request/show/481848 42.1 / xorg-x11-server
https://build.opensuse.org/request/show/481849 42.2 / xorg-x11-server
Comment 46 Swamp Workflow Management 2017-03-29 16:14:30 UTC
openSUSE-RU-2017:0856-1: An update that has two recommended fixes can now be installed.

Category: recommended (moderate)
Bug References: 1021803,1025985
CVE References: 
Sources used:
openSUSE Leap 42.2 (src):    xorg-x11-server-7.6_1.18.3-12.9.1
openSUSE Leap 42.1 (src):    xorg-x11-server-7.6_1.17.2-27.1
Comment 49 Swamp Workflow Management 2017-06-26 13:14:16 UTC
SUSE-SU-2017:1675-1: An update that solves one vulnerability and has 7 fixes is now available.

Category: security (moderate)
Bug References: 1019649,1021803,1025029,1025035,1025084,1025985,1032509,1039042
CVE References: CVE-2017-2624
Sources used:
SUSE Linux Enterprise Software Development Kit 12-SP2 (src):    xorg-x11-server-7.6_1.18.3-71.1
SUSE Linux Enterprise Server for Raspberry Pi 12-SP2 (src):    xorg-x11-server-7.6_1.18.3-71.1
SUSE Linux Enterprise Server 12-SP2 (src):    xorg-x11-server-7.6_1.18.3-71.1
SUSE Linux Enterprise Desktop 12-SP2 (src):    xorg-x11-server-7.6_1.18.3-71.1