Bug 100690 - [Regression, bisected] TotalWar: Warhammer corrupted graphics
Summary: [Regression, bisected] TotalWar: Warhammer corrupted graphics
Status: RESOLVED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Mesa core (show other bugs)
Version: git
Hardware: Other All
: medium normal
Assignee: Kenneth Graunke
QA Contact: mesa-dev
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-04-15 14:41 UTC by Gregor Münch
Modified: 2017-05-11 08:25 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
notice the red squares, which shouldnt be there (625.33 KB, image/jpeg)
2017-04-15 14:41 UTC, Gregor Münch
Details
Proposed patch (git am) (1.74 KB, patch)
2017-04-25 01:16 UTC, Kenneth Graunke
Details | Splinter Review
Patch to drop format-based completeness checks (1.74 KB, patch)
2017-04-25 05:51 UTC, Kenneth Graunke
Details | Splinter Review
Minor glitches (618.88 KB, image/jpeg)
2017-04-29 20:37 UTC, talonz
Details

Description Gregor Münch 2017-04-15 14:41:39 UTC
Created attachment 130856 [details]
notice the red squares, which shouldnt be there

Worldmap in TotalWar shows some fancy colors.
Radeon HD 7970, LLVM 5.0.0svn_r300398

c5bf7cb52942cb7df9f5d73746ffbf3c102d12cc is the first bad commit
commit c5bf7cb52942cb7df9f5d73746ffbf3c102d12cc
Author: Kenneth Graunke <kenneth@whitecape.org>
Date:   Thu Feb 23 15:04:52 2017 -0800

    mesa: Require mipmap completeness for glCopyImageSubData(), sometimes.
    
    This patch makes glCopyImageSubData require mipmap completeness when the
    texture object's built-in sampler object has a mipmapping MinFilter.
    
    Fixes (on i965):
    dEQP-GLES31.functional.debug.negative_coverage.*.buffer.copy_image_sub_data
    
    Reviewed-by: Roland Scheidegger <sroland@vmware.com>

git bisect log
git bisect start
# bad: [667da4eaed37de143681711344aba19373bee1d0] winsys/amdgpu: sparse buffer creation / destruction / commitment
git bisect bad 667da4eaed37de143681711344aba19373bee1d0
# good: [7ee91af30074a4381e4353122319e3b4b3fe7cbd] r600g: check NULL return from r600_aligned_buffer_create
git bisect good 7ee91af30074a4381e4353122319e3b4b3fe7cbd
# good: [e113dfabad5c60ce3082c65abe3b2e5689bdf31b] intel: Add INTEL_CFLAGS to aubinator CFLAGS.
git bisect good e113dfabad5c60ce3082c65abe3b2e5689bdf31b
# good: [03a67fbbf7847cbdcc26b3bd86ee43e09a55cce9] radv: fix order of the guardband register emission.
git bisect good 03a67fbbf7847cbdcc26b3bd86ee43e09a55cce9
# good: [3dfe61ed6ec6773c2373ec7a139b7dfe794f60c8] gallium: decrease the size of pipe_box - 24 -> 16 bytes
git bisect good 3dfe61ed6ec6773c2373ec7a139b7dfe794f60c8
# bad: [c6f69eea6ac549fc2ffa46944de4dd82c9b53329] anv/pipeline: Properly handle unset gl_Layer and gl_ViewportIndex
git bisect bad c6f69eea6ac549fc2ffa46944de4dd82c9b53329
# good: [18b12bf53351e1a902dc1f2e527a94ec8d8f3eff] targets: export radeon winsys_create functions to silence LLVM warning
git bisect good 18b12bf53351e1a902dc1f2e527a94ec8d8f3eff
# good: [567d77885e8579486354843e7280428dc96d4bd9] intel: genxml: add RING_BUFFER_CTL registers
git bisect good 567d77885e8579486354843e7280428dc96d4bd9
# good: [1fde054b8f8435706d567d0584c44f9fc686a97c] intel/isl: Refactor and clerify gen8 alignment calculations
git bisect good 1fde054b8f8435706d567d0584c44f9fc686a97c
# bad: [c5bf7cb52942cb7df9f5d73746ffbf3c102d12cc] mesa: Require mipmap completeness for glCopyImageSubData(), sometimes.
git bisect bad c5bf7cb52942cb7df9f5d73746ffbf3c102d12cc
# good: [c161a104629ad49dc6a4f7d1fe82e87fc08121fe] libgl-xlib: Link with libunwind.
git bisect good c161a104629ad49dc6a4f7d1fe82e87fc08121fe
# first bad commit: [c5bf7cb52942cb7df9f5d73746ffbf3c102d12cc] mesa: Require mipmap completeness for glCopyImageSubData(), sometimes.

Verified bad commit with:
git revert c5bf7cb52942cb7df9f5d73746ffbf3c102d12cc
Comment 1 talonz 2017-04-25 00:57:06 UTC
I am also having this problem 
latest mesa-git & llvm-svn 
rx480
Comment 2 Kenneth Graunke 2017-04-25 01:14:34 UTC
Thank you for finding and reporting this!

The game appears to be calling glCopyImageSubData() on a RGBA_UINT32 texture, with only one miplevel, with min/mag filters set to GL_NEAREST_MIPMAP_LINEAR and GL_LINEAR.  Mipmap completeness isn't the problem - it's that we started paying attention to the min/mag filters and noticed that linear-filtering + integer formats = incomplete textures.  See GL 4.5 Section 8.17:

   "A texture is complete unless any of the following conditions hold true:

    * Any of:
       – The internal format of the texture is integer (see table 8.12).
       – The internal format is STENCIL_INDEX.
       – The internal format is DEPTH_STENCIL, and the value of
         DEPTH_STENCIL_TEXTURE_MODE for the texture is STENCIL_INDEX.

    and either the magnification filter is not NEAREST, or the minification filter
    is neither NEAREST nor NEAREST_MIPMAP_NEAREST."

Arguably, this could be considered an application bug.  However, the GL 4.5 conformance suite mandated that this very case should work until about a week before you reported this bug.  So, all the major drivers allowed this case.

I'm reopening the discussion at Khronos (internal bugzilla 16224) based on this information.  I strongly suspect that they'll say this should be allowed (at which point my patch is wrong), but I'm not certain exactly what cases will be allowed yet.

In the meantime, I'm happy to have my patch reverted in Mesa.
Comment 3 Kenneth Graunke 2017-04-25 01:16:07 UTC
Created attachment 131008 [details] [review]
Proposed patch (git am)

I suggested two options at Khronos: ignore min/mag filters (it makes no sense to consider them), or...more likely...ignore format-based completeness rules.

Here's a patch that implements the latter.  If you're able to test it, could you see if it fixes "Total War: WARHAMMER" rendering for you?  Thanks!
Comment 4 Kenneth Graunke 2017-04-25 05:51:51 UTC
Created attachment 131012 [details] [review]
Patch to drop format-based completeness checks

Whoops, sorry...failed to git commit --amend before uploading the patch, so that patch wouldn't compile.  Here's the one I actually tested.
Comment 5 Gregor Münch 2017-04-25 07:26:54 UTC
Thanks for the patch. I cant test it before Friday. Will let you know, in the meantime maybe someone else can test?
Im quite confident that Feral is willing to fix their games if its a bug against the spec.
Comment 6 Marc Di Luzio 2017-04-25 09:22:14 UTC
Cheers for the report Gregor.

Kenneth: if the verdict from Khronos suggests we're out of spec then we'll be happy to patch and fix. I'll keep an eye on this thread.
Comment 7 Kenneth Graunke 2017-04-26 18:31:00 UTC
Thanks :) I'm just concerned that since no drivers have been enforcing this rule, there might be a lot of apps that get hit by this issue.
Comment 8 talonz 2017-04-27 08:01:06 UTC
(In reply to Kenneth Graunke from comment #4)
> Created attachment 131012 [details] [review] [review]
> Patch to drop format-based completeness checks
> 
> Whoops, sorry...failed to git commit --amend before uploading the patch, so
> that patch wouldn't compile.  Here's the one I actually tested.

i will test this tonight, thanks for the patch
Comment 9 talonz 2017-04-29 08:34:52 UTC
Just tested the patch ... while not fixing the problem completely it has fixed the graphic glitches replacing the red and green squares with minor graphic glitches on the campaign map and on the battle map it has completely resolved my issue of large red blocks on the grass, i will post screenshots soon to better explain what i mean
Comment 10 Gregor Münch 2017-04-29 14:57:36 UTC
I just tested it too, for me the world map is fixed. But Im waiting for your screenshots to see what you see. ;)
Comment 11 talonz 2017-04-29 20:37:43 UTC
Created attachment 131153 [details]
Minor glitches

while no weird colouring you can still that there is a problem.  the game is perfectly playable now at least
Comment 12 Gregor Münch 2017-05-03 10:29:01 UTC
I saw something like that before. Sometimes the red squares appeared, sometimes it was like on your picture and sometimes it was just very dark when you zoomed out.
I played now different factions but after the patch I dont see anything wrong on the worldmap.
Could you try if reverting commit c5bf7cb52942cb7df9f5d73746ffbf3c102d12cc and see if the problem persists?
We should fix this bug before 17.1 final appears. Maybe we should simply revert the patch for the 17.1 branch.
Comment 13 Kenneth Graunke 2017-05-05 16:44:35 UTC
I sent a revert today.
Comment 14 Gregor Münch 2017-05-08 12:29:50 UTC
Thx!
Patch appeared in master and stable. So I think this is fixed.
Comment 15 talonz 2017-05-11 08:25:20 UTC
just updated mesa-git today and also have no problems, thank you for the patch and hard work.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.