Bug 106810

Summary: ProgramBinary does not switch program correctly when using transform feedback
Product: Mesa Reporter: xinghua <xinghua.cao>
Component: Mesa coreAssignee: mesa-dev
Status: RESOLVED FIXED QA Contact: mesa-dev
Severity: normal    
Priority: medium CC: jljusten, yang.gu, yunchao.he
Version: git   
Hardware: Other   
OS: Linux (All)   
Whiteboard:
i915 platform: i915 features:

Description xinghua 2018-06-04 11:00:14 UTC
Steps:
1. Download chrome and install it on your Ubuntu, https://www.google.com/chrome/?platform=linux&extra=devchannel.
2. Open the link, https://www.khronos.org/registry/webgl/sdk/tests/deqp/functional/gles3/negativeshaderapi.html?webglVersion=2&quiet=0
3. some cases fail, if not, please continuously press the "refresh" button.(Because this issue happens when using ProgramBinary, not LinkProgram) 

Note:
This case only tests that "A INVALID_OPERATION error is generated by LinkProgram if program is being used by one or more transform feedback objects".
List main code of a case as below, with simple vertex shader and pixel shader, and one output "gl_Position" of vertex shader, which used for transform feedback.

var program = new gluShaderProgram.ShaderProgram(gl, gluShaderProgram.makeVtxFragSources(vertexShaderSource, fragmentShaderSource));
var buf;
var tfID;
var tfVarying = ['gl_Position'];
tfID = gl.createTransformFeedback();
buf = gl.createBuffer();
gl.useProgram(program.getProgram());
gl.transformFeedbackVaryings(program.getProgram(), tfVarying, gl.INTERLEAVED_ATTRIBS);
gl.linkProgram(program.getProgram());
gl.bindTransformFeedback(gl.TRANSFORM_FEEDBACK, tfID);
gl.bindBuffer(gl.TRANSFORM_FEEDBACK_BUFFER, buf);
gl.bufferData(gl.TRANSFORM_FEEDBACK_BUFFER, 32, gl.DYNAMIC_DRAW);
gl.bindBufferBase(gl.TRANSFORM_FEEDBACK_BUFFER, 0, buf);
gl.beginTransformFeedback(gl.TRIANGLES);
this.expectError(gl.NO_ERROR);            

gl.linkProgram(program.getProgram());
this.expectError(gl.INVALID_OPERATION);

ShaderProgram construct function also calls linkProgram to generate "program " object. Above code totally calls linkProgram three times.
The first time runs this case, could pass.
In order to improve performance, chromium had cached program binary after calling this first two linkProgram calls, so would cache two program binary objects (programBinary1 and ProgramBinary2), programBinary1 without transform feedback, programBinary2 with transform feedback. The second time run this case, the code maybe as below,

gl.programBinary(programBinary1...);
var buf;
var tfID;
var tfVarying = ['gl_Position'];
tfID = gl.createTransformFeedback();
buf = gl.createBuffer();
gl.useProgram(program.getProgram());
gl.transformFeedbackVaryings(program.getProgram(), tfVarying, gl.INTERLEAVED_ATTRIBS);
gl.programBinary(programBinary2...);
gl.bindTransformFeedback(gl.TRANSFORM_FEEDBACK, tfID);
gl.bindBuffer(gl.TRANSFORM_FEEDBACK_BUFFER, buf);
gl.bufferData(gl.TRANSFORM_FEEDBACK_BUFFER, 32, gl.DYNAMIC_DRAW);
gl.bindBufferBase(gl.TRANSFORM_FEEDBACK_BUFFER, 0, buf);
gl.beginTransformFeedback(gl.TRIANGLES);
this.expectError(gl.NO_ERROR);       //!!!!!!! Error happens here, glBeginTransformFeedback(no varyings to record)
gl.linkProgram(program.getProgram());
this.expectError(gl.INVALID_OPERATION);

When calling gl.programBinary(programBinary2...), Mesa did not implicitly call useProgram to swich "gl_program" from programBinary1 to programBinary2, make "gl_program" object of programBinary2 as current program, even these two "gl_program" object shared the same "gl_shader_program".
Please help check whether this investigation is correct. Thank you.
Comment 1 Jordan Justen 2018-06-04 17:30:23 UTC
What do you mean "implicitly call useProgram"? I think ProgramBinary
should overwrite the currently bound program. (Whatever UseProgram last
set it to.)

I'm not saying that I see anything specifically wrong with the test
cases.

I tried loading this page on chrome, and I did see some failures.

I also tried loading it on Firefox, and I did not see any errors.
(I tried to refresh a few times too.)
Comment 2 xinghua 2018-06-05 01:07:52 UTC
(In reply to Jordan Justen from comment #1)
> What do you mean "implicitly call useProgram"? I think ProgramBinary
> should overwrite the currently bound program. (Whatever UseProgram last
> set it to.)
> 
> I'm not saying that I see anything specifically wrong with the test
> cases.
> 
> I tried loading this page on chrome, and I did see some failures.
> 
> I also tried loading it on Firefox, and I did not see any errors.
> (I tried to refresh a few times too.)

Thank you for your reply.

you could not reproduce this issue on Firefox. May the Firefox not cache program binary, it re-compile and re-link program, mesa could call link_program to switch "gl_program" object correctly?
Comment 3 xinghua 2018-06-05 01:13:15 UTC
(In reply to Jordan Justen from comment #1)
> What do you mean "implicitly call useProgram"? I think ProgramBinary
> should overwrite the currently bound program. (Whatever UseProgram last
> set it to.)
> 
> I'm not saying that I see anything specifically wrong with the test
> cases.
> 
> I tried loading this page on chrome, and I did see some failures.
> 
> I also tried loading it on Firefox, and I did not see any errors.
> (I tried to refresh a few times too.)

"implicitly call useProgram" does not mean to call this function directly, only need to switch "gl_program" correctly as useProgram, reference some code as useProgram.
Comment 4 Timothy Arceri 2018-06-06 00:03:07 UTC
I believe this is the relevant spec quote from this bug:

From Section 7.3. (PROGRAM OBJECTS) of the OpenGL 4.5 spec:

"If LinkProgram or ProgramBinary successfully re-links a program object that is active for any shader stage, then the newly generated executable code will
be installed as part of the current rendering state for all shader stages where the program is active. Additionally, the newly generated executable code is made part of the state of any program pipeline for all stages where the program is attached."
Comment 5 Jordan Justen 2018-06-11 18:07:52 UTC
Should be fixed by:

commit e266b320590ebbeadf7c98b0b493d89886534ccb
Author: Jordan Justen
Date:   Wed Jun 6 01:57:15 2018 -0700

    mesa/program_binary: add implicit UseProgram after successful ProgramBinary

A related piglit test was added in:

commit f1dc46ddf8c1f033ae24d322deff663889b2267d
Author: Jordan Justen
Date:   Fri Jun 8 22:27:57 2018 -0700

    ARB_get_program_binary: Test that restoring active program takes effect
Comment 6 Tapani Pälli 2018-10-24 05:12:23 UTC
*** Bug 106881 has been marked as a duplicate of this bug. ***

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.