Bug 102904

Summary: piglit and gl45 cts linker tests regressed
Product: Mesa Reporter: Mark Janes <mark.a.janes>
Component: Mesa coreAssignee: Nicolai Haehnle <prefect_>
Status: RESOLVED FIXED QA Contact: mesa-dev
Severity: normal    
Priority: medium    
Version: git   
Hardware: Other   
OS: All   
Whiteboard:
i915 platform: i915 features:

Description Mark Janes 2017-09-20 16:25:53 UTC
the following tests regressed:
piglit.spec.!opengl 3_0.bindfragdata-link-error
piglit.spec.arb_enhanced_layouts.linker.component-layout.fs-out-overlap3
piglit.spec.arb_enhanced_layouts.linker.component-layout.fs-out-overlap
piglit.spec.arb_enhanced_layouts.linker.component-layout.fs-out-overlap4
piglit.spec.arb_enhanced_layouts.linker.component-layout.fs-out-overlap-array
piglit.spec.arb_enhanced_layouts.linker.component-layout.fs-out-type-mismatch
piglit.spec.arb_enhanced_layouts.linker.component-layout.fs-out-type-mismatch-array
piglit.spec.arb_enhanced_layouts.linker.component-layout.fs-out-overlap2

gl45-cts.gtf33.gl3tests.explicit_attrib_location.explicit_attrib_location_overlapping_ranges

By the series ending in:
704ddbcdf693079d417d831abe073caad008ab01
Author:     Nicolai Hähnle <nicolai.haehnle@amd.com>
radeonsi: set MIP_POINT_PRECLAMP to 0

This fixes a bug with nearest ("point") mip selection when the fractional
part of max_lod is in (0.5,1). In this case, the spec mandates that
we still select the mip level ceil(max_lod) in the clamping case. However,
MIP_POINT_PRECLAMP will clamp before the mip selection, which is wrong.

Supposedly this setting was originally copied from the closed Vulkan
driver, but as far as I can tell, closed Vulkan was actually changed back
recently :)

Fixes dEQP-GLES3.functional.texture.mipmap.2d.max_lod.{nearest,linear}_nearest

Fixes: f7420ef5b464 ("radeonsi: enable some sampler fields to match the closed driver")
Reviewed-by: Marek Olšák <marek.olsak@amd.com>
Reviewed-by: Dave Airlie <airlied@redhat.com>

The problematic commit is likely to be
15cae12804e * glsl/linker: fix output variable overlap check


Sample output:
/tmp/build_root/m64/lib/piglit/bin/shader_runner /tmp/build_root/m64/lib/piglit/tests/spec/arb_enhanced_layouts/linker/component-layout/fs-out-overlap4.shader_test -auto -fbo
piglit: debug: Requested an OpenGL 3.2 Core Context, and received a matching 4.5 context

shader link error expected, but it was successful!
Comment 1 Mark Janes 2017-09-20 18:13:14 UTC
bisected to mesa 15cae12804ef288c7fb4cb9a38f7e32e6d8c4dc1
Comment 2 Kenneth Graunke 2017-09-20 18:34:13 UTC
In fs-out-overlap.shader_test, we have this:

   layout(location = 0) out vec2 a;

   // consumes Z/W components
   layout(location = 0, component = 1) out vec2 b;

Before the patch, we would do:

                  assigned[assigned_attr] = var;
                  assigned_attr++;

on the first variable.  But with the patch, we stop doing this, and fail to record the first variable - because there's no used location.  So, on the second variable, we look through the existing assigned attributes, and see nothing, and fail to raise the error.

I think we just need to move the code back.
Comment 3 Nicolai Hähnle 2017-09-20 19:59:19 UTC
Ah, my bad.

Moving the code back is incorrect though because it can overflow the array (that's the bug that the commit fixed). This should do it: https://patchwork.freedesktop.org/patch/178036/
Comment 4 Emil Velikov 2017-12-22 15:12:57 UTC
Patch was applied to master and propagated to the respective stable branches.

commit df8767a14e3eae4dcb8241b731b34e9379706795
Author: Nicolai Hähnle <nicolai.haehnle@amd.com>
Date:   Wed Sep 20 21:56:26 2017 +0200

    glsl/linker: properly fix output variable overlap check

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.