Bug 106594

Created attachment 139661 [details]
Glitch while placing new staff on the map

Created attachment 139662 [details]
Game output to STDOUT and STDERR

I was able to check for this behaviour on a system with an integrated Intel GPU (HD Graphics 530 (Skylake GT2); PCIID: 0x1912) on Mesa 18.0.3 and it didn't show these glitches.

The OpenGL error line ("==OPENGL==> [location 'Bitmap::ConvertToTexture Before Texture Creation'] error code 0x502 (invalid operation)") is shown with the Intel GPU as well.

Still an issue with the following stack (fully updated Debian testing as a base):
GPU: Hawaii PRO [Radeon R9 290] (ChipID = 0x67b1)
Mesa: Git:master/79fe00efb4
libdrm: 2.4.92-1
LLVM: SVN:trunk/r333339 (7.0 devel)
X.Org: 2:1.19.6-1
Linux: 4.16.11
Firmware (firmware-amd-graphics): 20170823-1
libclc: Git:master/a2118d58fc
DDX (xserver-xorg-video-amdgpu): 18.0.1-1

Forcing the OpenGL level back to 3.0 (the same OpenGL level the Intel GPU from comment #3 supported) doesn't help either.

However, I did find, that when I downgrade to Debian's Mesa package 18.0.4-1 (built against LLVM 6.0 (package version 1:6.0-3+b1)), I cannot reproduce this bug any longer. Therefore the bug must have been introduced in either Mesa between 18.0.4 and the current Git HEAD or LLVM between the 6.0 release and the current SVN HEAD.

I just tested Debian's 18.1.0-1 package (also built against LLVM 6.0 (package version 1:6.0-3+b1)) and that version of Mesa doesn't produce the glitch either, thus narrowing the regression space to:
- Mesa: between 18.1 and current HEAD of Git
- LLVM: between 6.0 and current HEAD of SVN.

Further testing with a version of Debian's 18.1.0-1 package built against LLVM 7 (SVN r333339) didn't exhibit the bug either, therefore the bug must have been introduced between Mesa 18.1.0 and the current Git HEAD of Mesa. I'll see, if I can do a bisection.

The bisection result is:
> 9c7be67968aaba224d518dee86dff736a4b599c6 is the first bad commit
> commit 9c7be67968aaba224d518dee86dff736a4b599c6
> Author: Mathias Fröhlich <mathias.froehlich@web.de>
> Date:   Sun May 13 09:18:57 2018 +0200
>
>     mesa: Remove FLUSH_VERTICES from VAO state changes.
>    
>     Pending draw calls on immediate mode or display list calls do
>     not depend on changes of the VAO state. So, remove calls to
>     FLUSH_VERTICES and flag _NEW_ARRAY as appropriate.
>    
>     Reviewed-by: Brian Paul <brianp@vmware.com>
>     Signed-off-by: Mathias Fröhlich <Mathias.Froehlich@web.de>
>
> :040000 040000 ad95067168b41b30d17d7ff05ecd47be4ca150e4 97ab8bde466f83da431193b045a664e540595d80 M      src

Reverting that commit generates several conflicts. I'd probably have to revert the whole series?

Since this touches core Mesa, the bug shouldn't be constrained to radeonsi, I'll adjust the component accordingly.

Created attachment 139872 [details]
git bisect log

Created attachment 139883 [details] [review]
Make sure that immediate mode draws have landed before array draws are executed.

For some reason the trace did not replay locally. But I could look into the command sequence and I think I have spotted a use pattern that is broken currently and the problem most likely got exposed by the bisected patch. The real problem probably happend with an earlier change from my bigger series.

Can you check if the attached change helps for your problem please?

best
Mathias

(In reply to Mathias Fröhlich from comment #8)
> For some reason the trace did not replay locally.

If you saw a lot of black, you might have been hit by what I described in comment #0: for me the replay is a large window of black (several times the required size for my screen) with the game screen stuck in the lower left corner (can be seen, if you let apitrace generate the screenshots).
Otherwise the trace is working for me.

> But I could look into the command sequence and I think I have spotted a use
> pattern that is broken currently and the problem most likely got exposed by
> the bisected patch. The real problem probably happend with an earlier change
> from my bigger series.
> 
> Can you check if the attached change helps for your problem please?

I can confirm, that attachment 139883 [details] [review] RESOLVES the bug for me. You can have my
  Tested-by: Kai Wasserbäch <kai@dev.carbon-project.org>
for the patch from attachment 139883 [details] [review].

I used the following graphics stack (fully updated Debian testing as a base) for testing:
GPU: Hawaii PRO [Radeon R9 290] (ChipID = 0x67b1)
Mesa: Git:master/b9fb2c266a + attachment 139883 [details] [review]
libdrm: 2.4.92-1
LLVM: SVN:trunk/r333339 (7.0 devel)
X.Org: 2:1.19.6-1
Linux: 4.16.13
Firmware (firmware-amd-graphics): 20170823-1
libclc: Git:master/a2118d58fc
DDX (xserver-xorg-video-amdgpu): 18.0.1-1

To ensure no old shader from cache interfered, I deleted ~/.cache/mesa_shader_cache before running the game (did that during bisection as well).

Let me know, if you need anything else.

Fixed by the following commit on master:
> commit 1ac4439d6278e6c5f9da5499bbc50362f0c6759b
> Author: Mathias Fröhlich <mathias.froehlich@web.de>
> Date:   Fri Jun 1 19:10:08 2018 +0200
> 
>     mesa: Make sure that imm draws are flushed before other draws execute.
>     
>     The recent patch
>     
>         mesa: Remove FLUSH_VERTICES from VAO state changes.
>     
>         Pending draw calls on immediate mode or display list calls do
>         not depend on changes of the VAO state. So, remove calls to
>         FLUSH_VERTICES and flag _NEW_ARRAY as appropriate.
>     
>     uncovered a problem that non immediate mode draw calls do only
>     flush outstanding immediate mode draws if FLUSH_UPDATE_CURRENT
>     is set in ctx->Driver.NeedFlush.
>     In that case, due to the sequence of _mesa_set_draw_vao commands
>     we could end up with the VAO from the FLUSH_VERTICES call set
>     into gl_context::Array._DrawVAO when the array draw is executed.
>     So the change pulls FLUSH_CURRENT out of _mesa_validate_* calls
>     into the array draw calls being validated.
>     The change introduces a new macro FLUSH_FOR_DRAW beside FLUSH_VERTICES
>     and FLUSH_CURRENT that flushes on changed current attributes as well
>     as on outstanding immediate mode draw calls. Use FLUSH_FOR_DRAW
>     in the non immediate mode draw code paths.
>     
>     Reviewed-by: Marek Olšák <marek.olsak@amd.com>
>     Tested-by: Kai Wasserbäch <kai@dev.carbon-project.org>
>     Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106594
>     Signed-off-by: Mathias Fröhlich <Mathias.Froehlich@web.de>