Bug 99708

Created attachment 129399 [details]
Reference output

This is what the output should look like when rendered correctly.

In this case, I have used Xephyr *without* GLAMOR.

Created attachment 129400 [details]
Broken output on Xephyr

This is what the output looks like when GLAMOR is used through Xephyr -glamor.

Created attachment 129401 [details]
Broken output on modesetting/Intel on Broadwell

This is what the output looks like when GLAMOR is used through the modesetting DDX on Intel.

Created attachment 129402 [details]
Broken output on modesetting/Intel on Skylake

Another kind of wrong output can be seen in the openSUSE bug report, copied here for documentation purposes. This is on the original reporter's Skylake machine, whereas the previous output was on my Broadwell box. All tests are on Leap 42.2.

*** Bug 99849 has been marked as a duplicate of this bug. ***

Created attachment 130001 [details]
Broken output on Xephyr with .w replaced by .r


As discussed in fdo#99849, this seems to be a problem with the glamor code:

https://bugs.freedesktop.org/show_bug.cgi?id=99849#c6

I've replaced the .w in on_off_fs_exec[] by .r and the line still doesn't look right in Xephyr - see attachment.

Given that in fdo#99705, we also had variations in rendering when compared to the fallback code, maybe it makes sense to disable the glamor code path for now, so as to not confuse users why things look differently on one system or another?

(In reply to Max Staudt from comment #6)
> Created attachment 130001 [details]
> Broken output on Xephyr with .w replaced by .r
> 
> 
> As discussed in fdo#99849, this seems to be a problem with the glamor code:
> 
> https://bugs.freedesktop.org/show_bug.cgi?id=99849#c6
> 
> I've replaced the .w in on_off_fs_exec[] by .r and the line still doesn't
> look right in Xephyr - see attachment.
Oh I missed that. Looks like the calculations are slightly off somewhere (apparently a segment should cover two pixels but only covers one).

> 
> Given that in fdo#99705, we also had variations in rendering when compared
> to the fallback code, maybe it makes sense to disable the glamor code path
> for now, so as to not confuse users why things look differently on one
> system or another?
I don't think it would be really too difficult to fix, but I don't have time to look at glamor.

Actually my analysis was incorrect.
glamor does in fact play around with the texture swizzles, hence .w should be correct.
However, the setup looks busted, as it actually uses the wrong texture in the end.
Here:

/* Set the dash pattern as texture 1 */

    glActiveTexture(GL_TEXTURE1);
    glBindTexture(GL_TEXTURE_2D, dash_priv->base.fbo->tex);
    glUniform1i(prog->dash_uniform, 1);
    glUniform1f(prog->dash_length_uniform, dash_pixmap->drawable.width);

That's not what this does, since dash_priv seems to be the pixmap for the actual drawable, not the dash pattern (I'm not sure, maybe some dash_priv assignment earlier was bogus). Hence this will actually use the main drawable itself as the texture (also known as rendering feedback loop - not good...). The results of that can be anything. (If dash_priv actually were the dash pixmap, I'm not quite convinced the texture swizzles would be set up alright.)

Not sure what a correct fix would be, as it is at this point the dash pattern is luckily already set up as texture 0 (and something set up the correct texture swizzles for it too), so setting that uniform to 0 instead works (in a apitrace, I can't be bothered to recompile this).
Setting up textures and fbos is somewhere buried deeper inside and someone with a bit of knowledge of glamor sure should be able to figure it out...

As a side note, this also relies on GL_REPEAT texture wrap mode without setting it explicitly. I have no idea if that's safe in general to assume it hasn't been set to something else...

Roland, thanks a bunch for investigating this!

Unfortunately, my GL is only very rudimentary, so I hope one of the GLAMOR people can have a look...

(In reply to Max Staudt from comment #9)
> Roland, thanks a bunch for investigating this!
> 
> Unfortunately, my GL is only very rudimentary, so I hope one of the GLAMOR
> people can have a look...

FWIW it looks like Eric Anholt fixed this just recently:
https://cgit.freedesktop.org/xorg/xserver/commit/?id=fe0b297420fc1de8a7fab28457d0864b3182e967
Maybe you can give it a try.

Thanks for the pointer, Roland.

This commit indeed fixes the dashed lines, so let's close the bug.

Thanks also to Eric for fixing this.