Bug 83573 - [swrast] piglit fs-op-not-bool-using-if regression
Summary: [swrast] piglit fs-op-not-bool-using-if regression
Status: RESOLVED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Other (show other bugs)
Version: git
Hardware: x86-64 (AMD64) Linux (All)
: medium normal
Assignee: Kenneth Graunke
QA Contact:
URL:
Whiteboard:
Keywords: bisected, regression
Depends on:
Blocks:
 
Reported: 2014-09-06 21:35 UTC by Vinson Lee
Modified: 2016-07-09 00:10 UTC (History)
4 users (show)

See Also:
i915 platform:
i915 features:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Vinson Lee 2014-09-06 21:35:00 UTC
mesa: dc0bd799cabdd974b05bd217304944392169fb50 (master 10.4.0-devel)

$ ./bin/shader_runner generated_tests/spec/glsl-1.10/execution/built-in-functions/fs-op-not-bool-using-if.shader_test -auto
Probe color at (1,0)
  Expected: 0.000000 0.000000 1.000000 1.000000
  Observed: 1.000000 1.000000 0.000000 1.000000
PIGLIT: {"result": "fail" }

6df0fd8fe9ccf5c927797897277343f068420a45 is the first bad commit
commit 6df0fd8fe9ccf5c927797897277343f068420a45
Author: Matt Turner <mattst88@gmail.com>
Date:   Fri Aug 8 11:58:16 2014 -0700

    mesa: Upload boolean uniforms using UniformBooleanTrue.
    
    Reviewed-by: Anuj Phogat <anuj.phogat@gmail.com>

:040000 040000 15acd6eba095cfb2dcb699134349f6ccc5a79484 16675d631f977bec6d671bd8dc577b5372d93d71 M	src
bisect run success
Comment 1 Kenneth Graunke 2014-09-08 15:56:51 UTC
I thought Marek's patch (mesa: set UniformBooleanTrue = 1.0f by default) would fix this, but it doesn't.  Using integer 1 works.

Setting DEBUG_PROG = 1 in prog_execute.c and making ADD print values in hexadecimal, I see:

----false case----
ADD (3f800000 3f800000 3f800000 3f800000) = (80000000 80000000 80000000 80000000
) + (3f800000 3f800000 3f800000 3f800000)
ADD (1 1 1 1) = (-0 -0 -0 -0) + (1 1 1 1)

----true case----
ADD (ce7e0000 ce7e0000 ce7e0000 ce7e0000) = (ce7e0000 ce7e0000 ce7e0000 ce7e0000) + (3f800000 3f800000 3f800000 3f800000)
ADD (-1.06535e+09 -1.06535e+09 -1.06535e+09 -1.06535e+09) = (-1.06535e+09 -1.06535e+09 -1.06535e+09 -1.06535e+09) + (1 1 1 1)

So, it looks like we're seeing 1.0f = 3f800000 numerically converted to a float somewhere (rather than bitcast).  0x3f800000 = 1065353216 = 1.06535322E9.  The ADD negates op[0], giving -1.06535322E9 = 0xce7e0000.

I'm not sure where that's happening yet.
Comment 2 Ilia Mirkin 2014-09-08 16:01:25 UTC
On quick inspection that's happening in

_mesa_propagate_uniforms_to_driver_storage

The format gets set to uniform_bool_float which in turn goes through

((float *) dst)[c] = (float) *isrc;
Comment 3 Kenneth Graunke 2014-09-08 16:09:29 UTC
Right, I just found that too.  Testing a patch to fix it now.
Comment 4 Kenneth Graunke 2014-09-08 21:30:34 UTC
Patch on the mailing list:
http://mid.gmane.org/1410211719-31585-1-git-send-email-kenneth@whitecape.org
Comment 5 Tapani Pälli 2014-09-09 05:09:25 UTC
> I thought Marek's patch (mesa: set UniformBooleanTrue = 1.0f by default)
> would fix this, but it doesn't.  Using integer 1 works.

Which is the exact reason why my patch to fix this issue used 1 :) I did not bother to check why it works though, just checked that this was the way it was before.

http://lists.freedesktop.org/archives/mesa-dev/2014-August/066580.html
Comment 6 Kenneth Graunke 2014-09-09 20:18:36 UTC
Yep, you were right...

Fixed in master with:

commit e36bbff0e6d6d2baeaeab153b93e201674be2056
Author: Kenneth Graunke <kenneth@whitecape.org>
Date:   Mon Sep 8 14:28:39 2014 -0700

    ir_to_mesa: Stop converting uniform booleans.
    
    Excess conversions considered harmful.
    
    Recently Matt reworked the boolean uniform handling to use the value of
    UniformBooleanTrue, rather than integer 1, when uploading uniforms:
    
        mesa: Upload boolean uniforms using UniformBooleanTrue.
        glsl: Use UniformBooleanTrue value for uniform initializers.
    
    Marek then set the default to 1.0f for drivers without native integer
    support:
    
        mesa: set UniformBooleanTrue = 1.0f by default
    
    However, ir_to_mesa was assuming a value of integer 1, and arranging for
    it to be converted to 1.0f on upload.  Since Marek's commit, we were
    uploading 1.0f = 0x3f800000 which was being interpreted as the integer
    value 1065353216 and converted to float as 1.06535322E9, which broke
    assumptions in ir_to_mesa that "true" was exactly 1.0f.
    
    +13 Piglits on classic swrast (fs-bool-less-compare-true,
    {vs,fs}-op-not-bool-using-if, glsl-1.20/execution/uniform-initializer).
    
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=83573
    Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
    Reviewed-by: Matt Turner <mattst88@gmail.com>
Comment 7 François MASSON 2014-11-07 23:20:07 UTC
This affect r300 too.
Please consider commiting to 10.3.


bug/show.html.tmpl processed on Mar 25, 2017 at 07:46:52.
(provided by the Example extension).