Bug 81500 - wrong color in flightgear for the c172p if "Atmospheric light scattering" is used
Summary: wrong color in flightgear for the c172p if "Atmospheric light scattering" is ...
Status: RESOLVED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Mesa core (show other bugs)
Version: 10.2
Hardware: Other All
: medium normal
Assignee: Marek Olšák
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-07-18 15:26 UTC by Barto
Modified: 2019-05-07 04:55 UTC (History)
3 users (show)

See Also:
i915 platform:
i915 features:


Attachments
wrong color for the c172p plane, it's green instead of grey (546.45 KB, image/png)
2014-07-18 15:26 UTC, Barto
Details
wrong color for the c172p plane, it's green instead of grey (546.45 KB, image/png)
2014-07-18 15:27 UTC, Barto
Details
glxinfo for ati radeon hd4650 pcie (58.61 KB, text/plain)
2014-07-18 15:30 UTC, Barto
Details
bisect log, 12 commits who can be the guilty (5.27 KB, text/plain)
2014-07-18 22:27 UTC, Barto
Details
the final bisect log, commit d030a3404ca0fedf365cb0fd41eaad7abc8ff132 is the first bad commit (2.26 KB, text/plain)
2014-07-19 00:33 UTC, Barto
Details
reverse the commit d030a3404ca0fedf365cb0fd41eaad7abc8ff132 (3.34 KB, patch)
2014-07-19 01:50 UTC, Barto
Details | Splinter Review
2 outputs of MESA_GLSL=dump (102.88 KB, application/x-gzip)
2014-07-21 20:51 UTC, Barto
Details
Work-around build break during bisect of 10.2 branch (899 bytes, patch)
2014-07-30 22:48 UTC, Ian Romanick
Details | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Barto 2014-07-18 15:26:28 UTC
Created attachment 103045 [details]
wrong color for the c172p plane, it's green instead of grey

I use mesa 10.2.3 with archlinux 64 bits,

I have an ati radeon HD4650 Pcie graphic card ( with the open source driver radeon ), 4 Gb ram,

I notice a bug if I use flightgear 3.0.0 and mesa 10.2.3 :

- if I select the "cessna c172p" aircraft and If I use the "Atmospheric light scattering" option ( in render options from flightgear ) the color of the c172p is wrong, it's green instead of grey,

you can see this with the screenshot,

I can do an apitrace if you want it,

If I use an old version of mesa ( like 10.0.3 ) the bug is gone
Comment 1 Barto 2014-07-18 15:27:59 UTC
Created attachment 103046 [details]
wrong color for the c172p plane, it's green instead of grey
Comment 2 Barto 2014-07-18 15:30:53 UTC
Created attachment 103047 [details]
glxinfo for ati radeon hd4650 pcie
Comment 3 Kai 2014-07-18 15:33:29 UTC
(In reply to comment #0)
> I can do an apitrace if you want it,

I'd guess doing a git bisect would be more helpful, since this sounds like a regression.
Comment 4 Barto 2014-07-18 15:56:26 UTC
I did an apitrace, I will post the link for download the file,

I will do a bisect also,

after tests I notice that it's probably a bug with the code related to the r600 driver in mesa,

because if I run flightgear ( and the trace generated by apitrace ) without hardware acceleration ( "export LIBGL_ALWAYS_SOFTWARE=1" ) the bug is gone,

so it's seems that the r600 driver in mesa 10.2.3 has a problem, maybe a regression
Comment 5 Barto 2014-07-18 16:23:21 UTC
here is the apitrace :

http://demo.ovh.eu/download/365b2f85df30b16ec7539d7b9622542a/fgfs.tar.bz2

at start you will see a green aircraft if you graphic card is affected by the bug ( r600 driver for example ), 

then after fews seconds in the apitrace I uncheck the "atmospheric light scattering" option in flightgear, the aircraft will immediatly become "grey" after this modification,

if your graphic card is not affected by the bug ( or if you use an old version of mesa, like 10.0.3 ) the cessna c172p will be always in grey in the apitrace ( the good color for this plane ) even if "atmospheric light scattering" option is checked in flightgear options,

now I will do a bisect in order to find the guilty
Comment 6 Barto 2014-07-18 20:34:56 UTC
I have started a bisect,

I use these settings :

git bisect start
git bisect bad a06c9791d1b7fcedfb56ecbdc601d42fab196916 
git bisect good 5f41cae633af3603ab369c139bfe2de6bbcc6369

but when git wants me to test this commit :

Bisecting: 19 revisions left to test after this (roughly 4 steps)
[0939d3d0974a579fa65b76ebc6074d61e11f03b0] sso: Add display list support for ARB_separate_shader_objects new functions

I get an error during the compilation, it's impossible to build the commit "0939d3d0974a579fa65b76ebc6074d61e11f03b0" :


drivers/common/meta.c: In function '_mesa_meta_begin':
drivers/common/meta.c:1202:1: error: invalid storage class for function 'invert_z'
 invert_z(GLfloat normZ)

I don't know how to fix this error, is it normal that commit ""0939d3d0974a579fa65b76ebc6074d61e11f03b0" fails to compile ?

http://cgit.freedesktop.org/mesa/mesa/commit/?id=0939d3d0974a579fa65b76ebc6074d61e11f03b0
Comment 7 Ilia Mirkin 2014-07-18 20:39:04 UTC
(In reply to comment #6)
> I get an error during the compilation, it's impossible to build the commit
> "0939d3d0974a579fa65b76ebc6074d61e11f03b0" :
> 
> 
> drivers/common/meta.c: In function '_mesa_meta_begin':
> drivers/common/meta.c:1202:1: error: invalid storage class for function
> 'invert_z'
>  invert_z(GLfloat normZ)
> 
> I don't know how to fix this error, is it normal that commit
> ""0939d3d0974a579fa65b76ebc6074d61e11f03b0" fails to compile ?

git bisect skip

Note that bugs are added and fixed all the time too, so if you're unsure whether a commit is good or bad (e.g. the whole screen is black, or the plane is missing, or whatever), just skip it and it'll build something else. Huge regressions/build failures tend not to last too long in the tree, so it shouldn't be too bad.
Comment 8 Barto 2014-07-18 22:04:21 UTC
I have just realized that the last git version of mesa solves the bug :

Mesa 10.3.0-devel (git-f14d217)

but I will continue the bisect process in order to find the commit who triggers the bug in mesa 10.2.x,

it's difficult because there are a lot of commits who fail to compile :

drivers/common/meta.c: In function '_mesa_meta_begin':
drivers/common/meta.c:1202:1: error: invalid storage class for function 'invert_z'
 invert_z(GLfloat normZ)

$ git bisect skip
Bisecting: 5 revisions left to test after this (roughly 3 steps)
[4a868a984d6ae73eb38a69b045b004663cdac20c] mesa/sso: Refactor new function _mesa_bind_pipeline


here is the bissect log :

git bisect start
# bad: [a06c9791d1b7fcedfb56ecbdc601d42fab196916] docs: Add missing release notes for ARB_separate_shader_objects
git bisect bad a06c9791d1b7fcedfb56ecbdc601d42fab196916
# good: [5f41cae633af3603ab369c139bfe2de6bbcc6369] docs: Add release notes for the 10.1.6 release.
git bisect good 5f41cae633af3603ab369c139bfe2de6bbcc6369
# good: [81144c049bc7c12e4edcdf28f91c3c024c6e8b2b] meta: Silence several 'unused parameter' warnings
git bisect good 81144c049bc7c12e4edcdf28f91c3c024c6e8b2b
# good: [2d2e60bdeeee19798c9828b543ffb1aca3270325] gallium/docs: fix PIPE_CAP_ENDIANNESS delimiter, remove trailing spaces
git bisect good 2d2e60bdeeee19798c9828b543ffb1aca3270325
# good: [741f5d58e649cbc35c0d8661616f4e718b4718f0] radeon: Drop the remaining driver usage of _ReallyEnabled.
git bisect good 741f5d58e649cbc35c0d8661616f4e718b4718f0
# good: [2a255704564c9ed7cd1a081f7cc3828ee698aa2a] mesa: Implement glBindImageTextures
git bisect good 2a255704564c9ed7cd1a081f7cc3828ee698aa2a
# bad: [dc675919d30df407269a5e5b3d682e7801db3f1e] mesa: add missing null checks in _tnl_register_fastpath()
git bisect bad dc675919d30df407269a5e5b3d682e7801db3f1e
# skip: [0939d3d0974a579fa65b76ebc6074d61e11f03b0] sso: Add display list support for ARB_separate_shader_objects new functions
git bisect skip 0939d3d0974a579fa65b76ebc6074d61e11f03b0
# skip: [5699220cd5719be6fbafdefd75025a817bcb200a] glsl: Exit when the shader IR contains an interface block instance
git bisect skip 5699220cd5719be6fbafdefd75025a817bcb200a
# bad: [e608449d3e7dc86b90acfb31d9c948c57cf0e920] mesa/sso: Enable GL_ARB_separate_shader_objects by default
git bisect bad e608449d3e7dc86b90acfb31d9c948c57cf0e920
# skip: [8f5852bd2b91df7b259e5aeafb6a62a4268ca4c4] linker: Refactor code that builds hash tables of varyings during linking
git bisect skip 8f5852bd2b91df7b259e5aeafb6a62a4268ca4c4
# good: [e05cebafd8ff127ead71fadc20f2e2c8c719481a] clover: Add a stub implementation of clCreateImage() v3
git bisect good e05cebafd8ff127ead71fadc20f2e2c8c719481a
# skip: [c557eb77225433fa9415a94fc9db3ce36374df64] linker: Allow geometry shader without vertex shader for separable programs
git bisect skip c557eb77225433fa9415a94fc9db3ce36374df64
# skip: [ba7195d126ce20bf74a27725224662aaca4d90ef] glsl/tests: Add first simple tests of populate_consumer_input_sets
git bisect skip ba7195d126ce20bf74a27725224662aaca4d90ef
# good: [5998fd536a1bb1d13218c995aa69723c6767cf04] linker: Make lower_packed_varyings work with explicit locations
git bisect good 5998fd536a1bb1d13218c995aa69723c6767cf04
# skip: [7d73c3e99ec14031e3834096f7e8e257338b64d4] linker: Allow consumer stage or producer stage to be NULL
git bisect skip 7d73c3e99ec14031e3834096f7e8e257338b64d4
# skip: [7ff937e5793dc8709c916e043b41142033c8e69e] linker: Modify cross_validate_outputs_to_inputs to match using explicit locations
git bisect skip 7ff937e5793dc8709c916e043b41142033c8e69e
# skip: [fe37cb0ac67071759a88ea767027368399e1fdb6] linker: Refactor code that gets an input matching an output
git bisect skip fe37cb0ac67071759a88ea767027368399e1fdb6
# skip: [d030a3404ca0fedf365cb0fd41eaad7abc8ff132] linker: Sort shader I/O variables into a canonical order
git bisect skip d030a3404ca0fedf365cb0fd41eaad7abc8ff132
# skip: [1ff5a2b1ba2148b772f5e5c86d64c3cb18e1ce97] linker: Assign varying locations for separable programs
git bisect skip 1ff5a2b1ba2148b772f5e5c86d64c3cb18e1ce97
# skip: [ca21cffebd063354291d561eadc2ded8795a5333] meta: Fix saving the program pipeline state
git bisect skip ca21cffebd063354291d561eadc2ded8795a5333
Comment 9 Barto 2014-07-18 22:24:40 UTC
I finished the bisect process,

because of the multiple commands "git bisect skip"  the result of the bisect process is complicated,

in fact there are 5 commits who can be the guilty :

# only skipped commits left to test
# possible first bad commit: [e608449d3e7dc86b90acfb31d9c948c57cf0e920] mesa/sso: Enable GL_ARB_separate_shader_objects by default

# possible first bad commit: [0939d3d0974a579fa65b76ebc6074d61e11f03b0] sso: Add display list support for ARB_separate_shader_objects new functions

# possible first bad commit: [7ff937e5793dc8709c916e043b41142033c8e69e] linker: Modify cross_validate_outputs_to_inputs to match using explicit locations

# possible first bad commit: [d030a3404ca0fedf365cb0fd41eaad7abc8ff132] linker: Sort shader I/O variables into a canonical order

# possible first bad commit: [c557eb77225433fa9415a94fc9db3ce36374df64] linker: Allow geometry shader without vertex shader for separable programs

# possible first bad commit: [1ff5a2b1ba2148b772f5e5c86d64c3cb18e1ce97] linker: Assign varying locations for separable programs

# possible first bad commit: [7d73c3e99ec14031e3834096f7e8e257338b64d4] linker: Allow consumer stage or producer stage to be NULL

# possible first bad commit: [fe37cb0ac67071759a88ea767027368399e1fdb6] linker: Refactor code that gets an input matching an output

# possible first bad commit: [5699220cd5719be6fbafdefd75025a817bcb200a] glsl: Exit when the shader IR contains an interface block instance

# possible first bad commit: [ba7195d126ce20bf74a27725224662aaca4d90ef] glsl/tests: Add first simple tests of populate_consumer_input_sets

# possible first bad commit: [8f5852bd2b91df7b259e5aeafb6a62a4268ca4c4] linker: Refactor code that builds hash tables of varyings during linking

# possible first bad commit: [ca21cffebd063354291d561eadc2ded8795a5333] meta: Fix saving the program pipeline state

in order to get the true guilty it would be great if I can solve the compilation error :

drivers/common/meta.c: In function '_mesa_meta_begin':
drivers/common/meta.c:1202:1: error: invalid storage class for function 'invert_z'
 invert_z(GLfloat normZ)

I need a patch for this error in drivers/common/meta.c, in order to find which is guilty among the 5 commits,

full bisect log will come in attachment
Comment 10 Barto 2014-07-18 22:27:26 UTC
Created attachment 103066 [details]
bisect log, 12 commits who can be the guilty

the bisect log,

in fact there 12 commits who can be the guilty

The first bad commit could be any of:
7d73c3e99ec14031e3834096f7e8e257338b64d4
1ff5a2b1ba2148b772f5e5c86d64c3cb18e1ce97
fe37cb0ac67071759a88ea767027368399e1fdb6
5699220cd5719be6fbafdefd75025a817bcb200a
c557eb77225433fa9415a94fc9db3ce36374df64
ba7195d126ce20bf74a27725224662aaca4d90ef
d030a3404ca0fedf365cb0fd41eaad7abc8ff132
7ff937e5793dc8709c916e043b41142033c8e69e
8f5852bd2b91df7b259e5aeafb6a62a4268ca4c4
0939d3d0974a579fa65b76ebc6074d61e11f03b0
ca21cffebd063354291d561eadc2ded8795a5333
e608449d3e7dc86b90acfb31d9c948c57cf0e920
Comment 11 Barto 2014-07-19 00:29:04 UTC
I manage to find the commit who triggers the bug :

it's the commit d030a3404ca0fedf365cb0fd41eaad7abc8ff132 linker: Sort shader I/O variables into a canonical order
    
    v2: Rebase on removal of ir_variable::user_location.

http://cgit.freedesktop.org/mesa/mesa/commit/?id=d030a3404ca0fedf365cb0fd41eaad7abc8ff132

I use a workaround in order to solve the compilation error :


drivers/common/meta.c: In function '_mesa_meta_begin':
drivers/common/meta.c:1202:1: error: invalid storage class for function 'invert_z'
 invert_z(GLfloat normZ)

in order to do a full bisect I replaced some lines in meta.c for the 12 commits who have this compilation error ( line 579 to 584 ) :

if (ctx->Extensions.ARB_separate_shader_objects) {
         if (ctx->Pipeline.Current) {
            _mesa_reference_pipeline_object(ctx, &save->Pipeline,
                                            ctx->Pipeline.Current);
            _mesa_BindProgramPipeline(0);
      }

with these lines :

      if (ctx->Pipeline.Current) {
         _mesa_reference_pipeline_object(ctx, &save->Pipeline,
                                         ctx->Pipeline.Current);
         _mesa_BindProgramPipeline(0);
      }

now if a 10.2.4 version of mesa is scheduled it would be great to revert this commit d030a3404ca0fedf365cb0fd41eaad7abc8ff132 in order to solve this bug
Comment 12 Barto 2014-07-19 00:33:28 UTC
Created attachment 103068 [details]
the final bisect log, commit d030a3404ca0fedf365cb0fd41eaad7abc8ff132 is the first bad commit

the final bisect log,

commit d030a3404ca0fedf365cb0fd41eaad7abc8ff132 is the first bad commit
Comment 13 Barto 2014-07-19 01:50:11 UTC
Created attachment 103069 [details] [review]
reverse the commit d030a3404ca0fedf365cb0fd41eaad7abc8ff132

this patch has been tested with the recent mesa 10.2.4, it fixes the bug,

it's a simple patch who reverts the changes made by commit d030a3404ca0fedf365cb0fd41eaad7abc8ff132, 

not sure if it's a safe way
Comment 14 Barto 2014-07-20 15:50:09 UTC
Do you know if a mesa 10.2.5 will contain my patch in order to solve this bug ?

mesa 10.2.4 has been released without this patch,

and I notice that mesa 10.3.x doesn't have the bug ( because the file /src/glsl/link_varyings.cpp doesn't have the changes made by the faulty commit d030a3404ca0fedf365cb0fd41eaad7abc8ff132 )
Comment 15 Ilia Mirkin 2014-07-20 16:01:23 UTC
(In reply to comment #14)
> Do you know if a mesa 10.2.5 will contain my patch in order to solve this
> bug ?

Unlikely. Stable releases tend to contain cherry-picks of patches from master, not patches attached to random bugs. (Exceptions can of course be made.)

> and I notice that mesa 10.3.x doesn't have the bug ( because the file
> /src/glsl/link_varyings.cpp doesn't have the changes made by the faulty
> commit d030a3404ca0fedf365cb0fd41eaad7abc8ff132 )

Perhaps it got reverted/somehow fixed? If you can figure out what fixes this in master, it can be cherry-picked back to the 10.2 branch for the next stable release. Or if the fixes are deemed too invasive, perhaps the revert will indeed be appropriate.
Comment 16 Barto 2014-07-20 16:18:58 UTC
(In reply to comment #15)
> Unlikely. Stable releases tend to contain cherry-picks of patches from
> master, not patches attached to random bugs. (Exceptions can of course be
> made.)

for me with my ati radeon HD4650 ( who uses r600 driver ) it's not a random bug, the bug is always reproducible with flightgear 3.0.0,

the use case is simple : a C172P airplane and the use of the option "Atmospheric light scattering", with my graphic card ati HD4650 ( and maybe other models ) something goes wrong in the mesa code,

but I am not an expert in opengl programming, 

perhaps we must contact the author of the faulty commit ( Ian Romanick ) :

http://cgit.freedesktop.org/mesa/mesa/commit/?id=d030a3404ca0fedf365cb0fd41eaad7abc8ff132

he may know the solution with the apitrace, a missing update of a variable related to the R600 driver for example ?


> 
> Perhaps it got reverted/somehow fixed? If you can figure out what fixes this
> in master, it can be cherry-picked back to the 10.2 branch for the next
> stable release. Or if the fixes are deemed too invasive, perhaps the revert
> will indeed be appropriate.

I think also that someone who works in 10.2 branch has reverted this commit because he found some bugs or strange results in test softwares like piglit ?
Comment 17 Barto 2014-07-20 16:20:46 UTC
(In reply to comment #16)

> I think also that someone who works in 10.2 branch has reverted this commit
> because he found some bugs or strange results in test softwares like piglit ?

sorry, I meant "someone who works in 10.3 branch" of course
Comment 18 Ilia Mirkin 2014-07-20 16:26:26 UTC
(In reply to comment #16)
> (In reply to comment #15)
> > Unlikely. Stable releases tend to contain cherry-picks of patches from
> > master, not patches attached to random bugs. (Exceptions can of course be
> > made.)
> 
> for me with my ati radeon HD4650 ( who uses r600 driver ) it's not a random
> bug, the bug is always reproducible with flightgear 3.0.0,

I meant "bugzilla bugs". This is just not how development is done in mesa (or most open-source projects).

> > Perhaps it got reverted/somehow fixed? If you can figure out what fixes this
> > in master, it can be cherry-picked back to the 10.2 branch for the next
> > stable release. Or if the fixes are deemed too invasive, perhaps the revert
> > will indeed be appropriate.
> 
> I think also that someone who works in 10.2 branch has reverted this commit
> because he found some bugs or strange results in test softwares like piglit ?

Not sure what you're saying... no one "works in 10.2 branch". Development is done on master, and fixes get cherry-picked back to the stable branches. Of course sometimes fixes are made "accidentally", or someone forgets that the fix would apply to a previous release, or whatever.

In this case, sounds like the fix is already in master. So it's a question of identifying which commit it is, and cherry-picking it back to the 10.2 branch, or figuring out a different approach if the fix is deemed too invasive.
Comment 19 Barto 2014-07-20 17:31:30 UTC
(In reply to comment #14)
> and I notice that mesa 10.3.x doesn't have the bug ( because the file
> /src/glsl/link_varyings.cpp doesn't have the changes made by the faulty
> commit d030a3404ca0fedf365cb0fd41eaad7abc8ff132 )

I made a mistake here, in fact the file /src/glsl/link_varyings.cpp in 10.3 branch has the changes made by the commit d030a3404ca0fedf365cb0fd41eaad7abc8ff132,

but the difference is that some commits in 10.3 branchs has solved the bug, because the 10.3 version of /src/glsl/link_varyings.cpp is not identical to the 10.2.4 version,

by retracing the changes between 10.2.4 and 10.3.x for this file I think I can identify the commit who has solved the bug
Comment 20 Ilia Mirkin 2014-07-20 17:40:11 UTC
(In reply to comment #19)
> (In reply to comment #14)
> > and I notice that mesa 10.3.x doesn't have the bug ( because the file
> > /src/glsl/link_varyings.cpp doesn't have the changes made by the faulty
> > commit d030a3404ca0fedf365cb0fd41eaad7abc8ff132 )
> 
> I made a mistake here, in fact the file /src/glsl/link_varyings.cpp in 10.3
> branch has the changes made by the commit
> d030a3404ca0fedf365cb0fd41eaad7abc8ff132,
> 
> but the difference is that some commits in 10.3 branchs has solved the bug,
> because the 10.3 version of /src/glsl/link_varyings.cpp is not identical to
> the 10.2.4 version,
> 
> by retracing the changes between 10.2.4 and 10.3.x for this file I think I
> can identify the commit who has solved the bug

Just a word of caution -- the fix might have been to a different part of the code, and not to that particular file. You can use bisect again to figure out the change that fixed it, just reverse the meanings of 'good' and 'bad'. A "good" commit would be one that has the issue, and a "bad" commit would be one where the issue is resolved. Then the first "bad" commit will point you at the commit that resolves the problem.
Comment 21 Barto 2014-07-20 18:54:32 UTC
(In reply to comment #20)
> 
> Just a word of caution -- the fix might have been to a different part of the
> code, and not to that particular file. You can use bisect again to figure
> out the change that fixed it, just reverse the meanings of 'good' and 'bad'.
> A "good" commit would be one that has the issue, and a "bad" commit would be
> one where the issue is resolved. Then the first "bad" commit will point you
> at the commit that resolves the problem.

I did this type of bisect,

and I managed to find the commit in 10.3.x who solves the bug : it's the commit 	1d9f74eda75da05b4d5c7df5fc1e6f5ab8d88322 :

http://cgit.freedesktop.org/mesa/mesa/commit/?id=1d9f74eda75da05b4d5c7df5fc1e6f5ab8d88322&utm_source=anzwix

the good news is that if I apply the patch of this commit with 10.2.4 source code then the bug is solved,

Do you think is it safe to push this commit in 10.2 branch of mesa ?

for me it seems ok but I didn't do the piglit tests
Comment 22 Ilia Mirkin 2014-07-20 19:11:22 UTC
(In reply to comment #21)
> (In reply to comment #20)
> > 
> > Just a word of caution -- the fix might have been to a different part of the
> > code, and not to that particular file. You can use bisect again to figure
> > out the change that fixed it, just reverse the meanings of 'good' and 'bad'.
> > A "good" commit would be one that has the issue, and a "bad" commit would be
> > one where the issue is resolved. Then the first "bad" commit will point you
> > at the commit that resolves the problem.
> 
> I did this type of bisect,
> 
> and I managed to find the commit in 10.3.x who solves the bug : it's the
> commit 	1d9f74eda75da05b4d5c7df5fc1e6f5ab8d88322 :
> 
> http://cgit.freedesktop.org/mesa/mesa/commit/
> ?id=1d9f74eda75da05b4d5c7df5fc1e6f5ab8d88322&utm_source=anzwix
> 
> the good news is that if I apply the patch of this commit with 10.2.4 source
> code then the bug is solved,
> 
> Do you think is it safe to push this commit in 10.2 branch of mesa ?
> 
> for me it seems ok but I didn't do the piglit tests

From what I can see, this commit has nothing to do with any issues introduced by the originally bad commit... this will require some investigation.

Summary:

apitrace: http://demo.ovh.eu/download/365b2f85df30b16ec7539d7b9622542a/fgfs.tar.bz2

first bad commit: d030a3404ca0fedf365cb0fd41eaad7abc8ff132
commit that "fixes" it on master: 1d9f74eda75da05b4d5c7df5fc1e6f5ab8d88322
Comment 23 Barto 2014-07-20 19:29:55 UTC
(In reply to comment #22)

> From what I can see, this commit has nothing to do with any issues
> introduced by the originally bad commit... this will require some
> investigation.
> 
> Summary:
> 
> apitrace:
> http://demo.ovh.eu/download/365b2f85df30b16ec7539d7b9622542a/fgfs.tar.bz2
> 
> first bad commit: d030a3404ca0fedf365cb0fd41eaad7abc8ff132
> commit that "fixes" it on master: 1d9f74eda75da05b4d5c7df5fc1e6f5ab8d88322

yes it's strange, 

it seems that this commit can cancel the bugs made by the bad commit d030a3404ca0fedf365cb0fd41eaad7abc8ff132, I don't know how,

I could use gdb with breakpoints but my knowledge in opengl programming is too weak, 

anyway this commit 1d9f74eda75da05b4d5c7df5fc1e6f5ab8d88322 solves the problem, but I will understand if you think it's too dangerous to use this commit in 10.2 branch of mesa
Comment 24 Barto 2014-07-21 13:26:04 UTC
(In reply to comment #22)

> From what I can see, this commit has nothing to do with any issues
> introduced by the originally bad commit... this will require some
> investigation.
> 

Could it be a gcc issue ?

I use gcc 4.9.0 20140604 (prerelease) with archlinux 64 bits,

maybe some lines in commit d030a3404ca0fedf365cb0fd41eaad7abc8ff132 trigger a weird bug in gcc during compilation,

and maybe commit 1d9f74eda75da05b4d5c7df5fc1e6f5ab8d88322 restores a correct behaviour from gcc ( this commit changes something in /src/glsl/Makefile.sources )


another explanation is that some functions in /src/glsl/link_varyings.cpp are sometimes disabled by changes made by commit 1d9f74eda75da05b4d5c7df5fc1e6f5ab8d88322, the faulty function may never been called, which prevent the bug ( cessna C172P in green if an ati radeon HD4650 is used and ""Atmospheric light scattering" option enabled in flightgear ),

I could use a debug version of mesa in order to see if some functions in /src/glsl/link_varyings.cpp are really called if commit 1d9f74eda75da05b4d5c7df5fc1e6f5ab8d88322 is used, I think there is probably a side effect elsewhere with this commit which cancels the bug ( and may create another bugs undiscovered for now )
Comment 25 Ian Romanick 2014-07-21 20:25:48 UTC
Can you provide output of MESA_GLSL=dump with vanilla 10.2.x and 10.2.x with d030a340 reverted?
Comment 26 Barto 2014-07-21 20:51:12 UTC
Created attachment 103227 [details]
2 outputs of MESA_GLSL=dump

Hi Ian,

here are in attachement the 2 outputs of MESA_GLSL=dump, 2 files in a tar.gz archive :

- output_mesa_glsl_10.2.4_vanilla ---> the output with a vanilla version of 10.2.4 mesa, I ran the apitrace file

- output_mesa_glsl_10.2.4_with_d030a340_reverted ---> the output with 10.2.4 mesa with d030a340 reverted, the same apitrace file was running
Comment 27 Benjamin Bellec 2014-07-23 11:41:45 UTC
@Barto,
When you launch Flightgear in a terminal, do you have any log when enabling the "Atmospheric light scattering" option.

Personally, I launch FlighGear with this command :
$ /usr/bin/fgfs --fg-root=/usr/share/flightgear --fg-scenery=/usr/share/flightgear/Scenery --airport=KPAO --aircraft=c172p-canvas --enable-random-objects --enable-horizon-effect --enable-enhanced-lighting --enable-ai-models --enable-ai-traffic --disable-real-weather-fetch --enable-clouds3d --geometry=1024x768

And when I enable the problematic option, I got this log from FlighGear :
FRAGMENT glCompileShader "/usr/share/flightgear/Shaders/urban-lightfield.frag" FAILED
FRAGMENT Shader "/usr/share/flightgear/Shaders/urban-lightfield.frag" infolog:
0:196(22): preprocessor error: syntax error, unexpected IDENTIFIER, expecting NEWLINE
0:193(1): preprocessor error: Unterminated #if


glLinkProgram "" FAILED
Program "" infolog:
error: linking with uncompiled shader


Due you have some king of message too ?

Note that I'm using FlightGear 2.10 (from Fedora 19 repo) and an Evergreen card with Mesa 10.3-devel.
Comment 28 Barto 2014-07-23 13:09:09 UTC
(In reply to comment #27)
> @Barto,
> when I enable the problematic option, I got this log from FlighGear :
> FRAGMENT glCompileShader
> "/usr/share/flightgear/Shaders/urban-lightfield.frag" FAILED
> FRAGMENT Shader "/usr/share/flightgear/Shaders/urban-lightfield.frag"
> infolog:
> 0:196(22): preprocessor error: syntax error, unexpected IDENTIFIER,
> expecting NEWLINE
> 0:193(1): preprocessor error: Unterminated #if
> 
> 
> glLinkProgram "" FAILED
> Program "" infolog:
> error: linking with uncompiled shader
> 
> 
> Due you have some king of message too ?
> 
> Note that I'm using FlightGear 2.10 (from Fedora 19 repo) and an Evergreen
> card with Mesa 10.3-devel.

I don't have this error message, but I use flightgear 3.0.0, not 2.10,

you must use flightgear 3.0.0 because a lot of bugs in flightgear have been fixed between the 2.10 version and 3.0.0 version,

an alternative is to run the apitrace I have provided in attachment ( just install the apitrace program, and run "apitrace replay fgfs.trace" ), tell me if you see a green C172p ( I use the "c172p" plane, not the "c172p-canvas" version )

my graphic card is an ati radeon HD4650 pcie ( RV730 chipset )

OpenGL renderer string: Gallium 0.4 on AMD RV730
OpenGL core profile version string: 3.3 (Core Profile) Mesa 10.2.4
OpenGL core profile shading language version string: 3.30

if you don't see a green C172p in the apitrace then the bug may occur only with some mesa driver ( r600 for example )
Comment 29 Barto 2014-07-26 17:09:01 UTC
the status of this bug report is still on "NEEDINFO", but all informations have been provided,

if developpers need other informations then I can provide it
Comment 30 Ian Romanick 2014-07-30 22:48:42 UTC
Created attachment 103704 [details] [review]
Work-around build break during bisect of 10.2 branch
Comment 31 Ian Romanick 2014-07-30 23:16:55 UTC
I looked at the two dumps.  Unsurprisingly, there are a bunch of differences.  In the places I examined closely, everything looked correct before and after.

I also ran the trace on an Intel Ivybridge system.  On current tip of the 10.2 branch (816d37e5c), I get a gray plane.  I also get the same results at d030a340 (the bisected guilty commit) and c557eb77 (the commit before).

One thing I noticed... on the IVB system there were a lot of error messages about "glFogf(pname = GL_FOG_DISTANCE_MODE_NV, param = GL_EYE_PLANE_ABSOLUTE_NV)", which means the application is using GL_NV_fog_distance.  This extension isn't supported on Intel drivers.  It's possible that the difference between your Radeon run and my Intel run is related to this extension.

What happens if you run with this environment variable set?

    MESA_EXTENSION_OVERRIDE=-GL_NV_fog_distance

I suspect there is some pre-existing (and still-existing) bug that d030a340 revealed and 1d9f74ed re-hides.
Comment 32 Ian Romanick 2014-07-30 23:21:20 UTC
(In reply to comment #31)
> What happens if you run with this environment variable set?
                         ^ FlightGear (not the existing trace)

> 
>     MESA_EXTENSION_OVERRIDE=-GL_NV_fog_distance

And... if it's still wrong, could you provide a new trace?
Comment 33 Barto 2014-07-30 23:48:23 UTC
(In reply to comment #31)

> 
> What happens if you run with this environment variable set?
> 
>     MESA_EXTENSION_OVERRIDE=-GL_NV_fog_distance
> 
> I suspect there is some pre-existing (and still-existing) bug that d030a340
> revealed and 1d9f74ed re-hides.

I still get a green airplane if I run the apitrace with "MESA_EXTENSION_OVERRIDE=-GL_NV_fog_distance" on a vanilla mesa 10.2.4,

I think the changes made by the bad commit ( d030a340 ) is not fully compatible with r600 driver, maybe if you test with a radeon graphic card who uses r600 driver you may find more easily the solution,

the bug is triggered in the apitrace when I use the "Atmospheric light scattering" option in flightgear, at start in the apitrace this option is already enabled, that's why the plane is green, then few seconds later you can see in the apitrace that I disabled this option, the plane becomes grey, 

if you are able to locate the opengl instructions in the apitrace related to this option "Atmospheric light scattering" then it may help in the resolution of this bug ?
Comment 34 Barto 2014-07-31 00:01:43 UTC
I run also the game flightgear with "MESA_EXTENSION_OVERRIDE=-GL_NV_fog_distance" and I still get a green C172p aircraft on a vanilla mesa 10.2.4,

I will provide a new apitrace in a few minutes
Comment 35 Barto 2014-07-31 00:14:47 UTC
(In reply to comment #32)
> (In reply to comment #31)
> > What happens if you run with this environment variable set?
>                          ^ FlightGear (not the existing trace)
> 
> > 
> >     MESA_EXTENSION_OVERRIDE=-GL_NV_fog_distance
> 
> And... if it's still wrong, could you provide a new trace?

here is the new trace :

http://demo.ovh.eu/download/22249cf920efa0a5dedee71782980a51/fgfs2.tar.bz2

the c172 aircraft is still in green with "MESA_EXTENSION_OVERRIDE=-GL_NV_fog_distance"
Comment 36 Ian Romanick 2014-07-31 16:19:23 UTC
The new trace still looks correct on my system with 10.2.4.  I work at Intel, and I don't have any Radeon hardware newer than an R200. :)

Perhaps Marek or one of the other Radeon developers can look into this a bit.  It may be that the bug is in the compiler / linker, but I'm still suspicious that the bisected change just uncovered a pre-existing bug.
Comment 37 Timothy Arceri 2019-05-07 04:55:29 UTC
Closing. The bug was fixed in master, the report remained open to try to get a fix for Mesa 10.2 which never happened.


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.