Bug 109601 - [Regression] RuneLite GPU rendering broken on 18.3.x
Summary: [Regression] RuneLite GPU rendering broken on 18.3.x
Status: RESOLVED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Drivers/DRI/i965 (show other bugs)
Version: 18.3
Hardware: x86-64 (AMD64) Linux (All)
: medium normal
Assignee: Intel 3D Bugs Mailing List
QA Contact: Intel 3D Bugs Mailing List
URL:
Whiteboard:
Keywords: bisected, regression
Depends on:
Blocks: mesa-19.0
  Show dependency treegraph
 
Reported: 2019-02-09 23:10 UTC by Sultan Alsawaf
Modified: 2019-02-15 05:03 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:


Attachments
Tarball containing glxinfo and api traces (168.93 MB, application/x-xz)
2019-02-09 23:10 UTC, Sultan Alsawaf
Details
RuneLite GPU rendering broken (149.67 KB, image/png)
2019-02-09 23:12 UTC, Sultan Alsawaf
Details
RuneLite GPU rendering working (1.10 MB, image/png)
2019-02-09 23:12 UTC, Sultan Alsawaf
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Sultan Alsawaf 2019-02-09 23:10:03 UTC
Created attachment 143351 [details]
Tarball containing glxinfo and api traces

With the upgrade from Mesa 18.2.x to 18.3.x, GPU rendering with the Old School RuneScape RuneLite client is broken with Intel graphics. This issue occurs 100% of the time with Mesa 18.3.3 on both my laptop with an i7-8750H (for which this bug report is for), and my laptop with an N3160. Attached is a tarball containing:
-a glxinfo dump
-an api trace with RuneLite GPU rendering working correctly on Mesa 18.2.6
-an api trace with RuneLite GPU rendering not working with Mesa 18.3.3
Comment 1 Sultan Alsawaf 2019-02-09 23:12:22 UTC
Created attachment 143352 [details]
RuneLite GPU rendering broken
Comment 2 Sultan Alsawaf 2019-02-09 23:12:56 UTC
Created attachment 143353 [details]
RuneLite GPU rendering working

I've also attached screenshots that demonstrate the issue.
Comment 3 Lionel Landwerlin 2019-02-10 03:07:55 UTC
Thanks a lot for the apitrace files.
Bisected to :

commit 4dd5263663585b55119e49c8dfade015c86aff1a (refs/bisect/bad)
Author: Jason Ekstrand <jason.ekstrand@intel.com>
Date:   Tue Aug 14 14:37:39 2018 -0500

    nir/algebraic: Add more extract_[iu](8|16) optimizations
    
    This adds the "(a << N) >> M" family of mask or sign-extensions.  Not a
    huge win right now but this pattern will soon be generated by NIR format
    lowering code.
    
    Shader-db results on Kaby Lake:
    
        total instructions in shared programs: 15166918 -> 15166916 (<.01%)
        instructions in affected programs: 36 -> 34 (-5.56%)
        helped: 2
        HURT: 0
    
    Reviewed-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
    Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Comment 4 Lionel Landwerlin 2019-02-10 03:35:14 UTC
The particular nir replacement that seems to cause issues is :

(('ishr', 'a@32', 16), ('extract_i16', a, 1), '!options->lower_extract_word')

I can't see anything wrong with it, so maybe there is an issue in our backend.
Comment 5 Lionel Landwerlin 2019-02-10 15:38:34 UTC
Jason: Any idea?
Comment 6 Lionel Landwerlin 2019-02-10 16:29:05 UTC
Some more digging shows that it seems to be a geometry shader has the issue.
Disabling the replacement only on that shader fixes the issue (this replacement is also used in a compute shader).
Here is the code of the geometry shader :

/*
 * Copyright (c) 2018, Adam <Adam@sigterm.info>
 * All rights reserved.
 *
 * Redistribution and use in source and binary forms, with or without
 * modification, are permitted provided that the following conditions are met:
 *
 * 1. Redistributions of source code must retain the above copyright notice, this
 *    list of conditions and the following disclaimer.
 * 2. Redistributions in binary form must reproduce the above copyright notice,
 *    this list of conditions and the following disclaimer in the documentation
 *    and/or other materials provided with the distribution.
 *
 * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
 * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
 * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
 * DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR
 * ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
 * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
 * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
 * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
 * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
 * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 */

#version 330

#define PI 3.1415926535897932384626433832795f
#define UNIT PI / 1024.0f

layout(triangles) in;
layout(triangle_strip, max_vertices = 3) out;

layout(std140) uniform uniforms {
  int cameraYaw;
  int cameraPitch;
  int centerX;
  int centerY;
  int zoom;
  int cameraX;
  int cameraY;
  int cameraZ;
  ivec2 sinCosTable[2048];
};

uniform mat4 projectionMatrix;

in ivec3 vPosition[];
in vec4 vColor[];
in float vHsl[];
in vec4 vUv[];
in float vFogAmount[];

out vec4 Color;
out float fHsl;
out vec4 fUv;
out float fogAmount;

/*
 * Copyright (c) 2018, Adam <Adam@sigterm.info>
 * All rights reserved.
 *
 * Redistribution and use in source and binary forms, with or without
 * modification, are permitted provided that the following conditions are met:
 *
 * 1. Redistributions of source code must retain the above copyright notice, this
 *    list of conditions and the following disclaimer.
 * 2. Redistributions in binary form must reproduce the above copyright notice,
 *    this list of conditions and the following disclaimer in the documentation
 *    and/or other materials provided with the distribution.
 *
 * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
 * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
 * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
 * DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR
 * ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
 * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
 * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
 * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
 * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
 * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 */

/*
 * Convert a vertex to screen space
 */
ivec3 toScreen(ivec3 vertex, int cameraYaw, int cameraPitch, int centerX, int centerY, int zoom) {
  int yawSin = int(65536.0f * sin(cameraYaw * UNIT));
  int yawCos = int(65536.0f * cos(cameraYaw * UNIT));

  int pitchSin = int(65536.0f * sin(cameraPitch * UNIT));
  int pitchCos = int(65536.0f * cos(cameraPitch * UNIT));

  int rotatedX = ((vertex.z * yawSin) + (vertex.x * yawCos)) >> 16;
  int rotatedZ = ((vertex.z * yawCos) - (vertex.x * yawSin)) >> 16;

  int var13 = ((vertex.y * pitchCos) - (rotatedZ * pitchSin)) >> 16;
  int var12 = ((vertex.y * pitchSin) + (rotatedZ * pitchCos)) >> 16;

  int x = rotatedX * zoom / var12 + centerX;
  int y = var13 * zoom / var12 + centerY;
  int z = -var12; // in OpenGL depth is negative

  return ivec3(x, y, z);
}

void main() {
  ivec3 cameraPos = ivec3(cameraX, cameraY, cameraZ);
  ivec3 screenA = toScreen(vPosition[0] - cameraPos, cameraYaw, cameraPitch, centerX, centerY, zoom);
  ivec3 screenB = toScreen(vPosition[1] - cameraPos, cameraYaw, cameraPitch, centerX, centerY, zoom);
  ivec3 screenC = toScreen(vPosition[2] - cameraPos, cameraYaw, cameraPitch, centerX, centerY, zoom);

  if (-screenA.z < 50 || -screenB.z < 50 || -screenC.z < 50) {
    // the client does not draw a triangle if any vertex distance is <50
    return;
  }

  vec4 tmp = vec4(screenA.xyz, 1.0);
  Color = vColor[0];
  fHsl = vHsl[0];
  fUv = vUv[0];
  fogAmount = vFogAmount[0];
  gl_Position  = projectionMatrix * tmp;
  EmitVertex();

  tmp = vec4(screenB.xyz, 1.0);
  Color = vColor[1];
  fHsl = vHsl[1];
  fUv = vUv[1];
  fogAmount = vFogAmount[1];
  gl_Position  = projectionMatrix * tmp;
  EmitVertex();

  tmp = vec4(screenC.xyz, 1.0);
  Color = vColor[2];
  fHsl = vHsl[2];
  fUv = vUv[2];
  fogAmount = vFogAmount[2];
  gl_Position  = projectionMatrix * tmp;
  EmitVertex();

  EndPrimitive();
}
Comment 7 Jason Ekstrand 2019-02-12 04:45:19 UTC
I've sent a patch to the ML which fixes this:

https://patchwork.freedesktop.org/patch/285517/

Unfortunately, it also seems to hang my GPU but that, I think, is unrelated.
Comment 8 Mark Janes 2019-02-12 18:38:01 UTC
should we have a piglit test for this case?
Comment 9 Lionel Landwerlin 2019-02-12 18:59:44 UTC
(In reply to Mark Janes from comment #8)
> should we have a piglit test for this case?

Yep!

I haven't had time to look at Jason's fix yet.
But I tried to build a piglit tests by dumping shifted values into an ssbo and the result made no sense at all.
Still need to debug.
Comment 10 Jason Ekstrand 2019-02-12 19:47:32 UTC
The problem is if you have something like this:

> int y = something;
> float x = -(y >> 16);

Then the negation gets lost.  Likely, we'd have the same problem with

> int y = something;
> float x = abs(y >> 16);

or either of those with an unsigned integer.
Comment 11 Lionel Landwerlin 2019-02-14 18:49:56 UTC
(In reply to Jason Ekstrand from comment #10)
> The problem is if you have something like this:
> 
> > int y = something;
> > float x = -(y >> 16);
> 
> Then the negation gets lost.  Likely, we'd have the same problem with
> 
> > int y = something;
> > float x = abs(y >> 16);
> 
> or either of those with an unsigned integer.

Thanks a bunch : https://patchwork.freedesktop.org/patch/286177/
Comment 12 Jason Ekstrand 2019-02-15 05:03:35 UTC
Fixed in master by the following commit:

commit 367b0ede4d9115aba772d6e46ec73642761f7ff6 (public/master)
Author: Jason Ekstrand <jason.ekstrand@intel.com>
Date:   Mon Feb 11 22:39:45 2019 -0600

    intel/fs: Bail in optimize_extract_to_float if we have modifiers
    
    This fixes a bug in runscape where we were optimizing x >> 16 to an
    extract and then negating and converting to float.  The NIR to fs pass
    was dropping the negate on the floor breaking a geometry shader and
    causing it to render nothing.
    
    Fixes: 1f862e923cb "i965/fs: Optimize float conversions of byte/word..."
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109601
    Tested-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
    Reviewed-by: Matt Turner <mattst88@gmail.com>


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.