[Bug 45468] New: Phase Shift: tails on sustain notes not visible unless GLSL is disabled
https://bugs.winehq.org/show_bug.cgi?id=45468 Bug ID: 45468 Summary: Phase Shift: tails on sustain notes not visible unless GLSL is disabled Product: Wine Version: 3.12 Hardware: x86 OS: Linux Status: UNCONFIRMED Severity: minor Priority: P2 Component: directx-d3d Assignee: wine-bugs(a)winehq.org Reporter: q3aiml+wine(a)gmail.com Distribution: --- Created attachment 61811 --> https://bugs.winehq.org/attachment.cgi?id=61811 missing tail by default During a song the notes that should be held (sustain notes) should have tails rendered after the note indicating the hold. These tails are not visible by default under wine. There are no visible artifacts. The notes instead appear like normal, non-sustain notes. Steps to reproduce: * Install version 1.27 of Phase Shift (http://www.dwsk.co.uk/phase_shift_downloads.html, https://mega.co.nz/#!JkwSxayT!cKdLVz8GrRrla2jkDmOvlMWxtmsdbWvoboP_vmgLw3o) * Run 'winetricks vcrun2013' to fix missing msvcr120.dll._strerror_s on startup * Select Quickplay in main menu (use number 1 key to select) * Select song Positive Force, a song that comes bundled with the full 1.27 download, or any other song with sustain notes * Press 1 to join with keyboard * Select Guitar intrument and Hard difficulty, or any other song and difficulty combo with sustain notes * Press 1 to continue and 1 again when prompted to press green to begin * Observe that the first note of Positive Force should have a green tail (follow by grey if the note is missed), but instead appears as a normal, non-sustain note The tails render properly after setting UseGLSL to disabled and MaxVersionGL to 0x30001 (cannot disable GLSL with GL 3.2 or newer). I think this has something to do with how the generated ffp emulating shaders calculate the alpha channel and apply an alpha test. Here is what I am seeing leading to the pixels being discarded: # relevant uniforms specular_enable = [1, 1, 1, 0] alpha_test_ref = 0.0117647061124444 ffp_material.diffuse = [0, 0, 0, 0] # relevant bits of vertex shader ffp_varying_diffuse.w = ffp_material.diffuse.w; # relevant bits of fragment shader // ffp_varying_diffuse.w = 0 -> ret.w = 0 regardless of tex0.w ret.w = tex0.w * ffp_varying_diffuse.w; // specular_enable.w = 0 and ret.w = 0 -> ps_out[0].a = 0 ps_out[0] = ffp_varying_specular * specular_enable + ret; // ps_out[0].a = 0, alpha_test_ref = 0.01..., alpha test fails and discards pixel if (!(ps_out[0].a >= alpha_test_ref)) discard; -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45468 --- Comment #1 from Andy Clayton <q3aiml+wine(a)gmail.com> --- Created attachment 61812 --> https://bugs.winehq.org/attachment.cgi?id=61812 has tail when glsl is disabled -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45468 --- Comment #2 from Andy Clayton <q3aiml+wine(a)gmail.com> --- Created attachment 61813 --> https://bugs.winehq.org/attachment.cgi?id=61813 full vertex shader -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45468 --- Comment #3 from Andy Clayton <q3aiml+wine(a)gmail.com> --- Created attachment 61814 --> https://bugs.winehq.org/attachment.cgi?id=61814 full fragment shader -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45468 --- Comment #4 from Henri Verbeet <hverbeet(a)gmail.com> --- Possibly, although I don't immediately see a reason to think it would be incorrect to discard the fragment in that case. Since you seem to have done some debugging on this already, does it help if you simply don't generate the alpha-test code? (That's in shader_glsl_generate_alpha_test().) What is the value of WINED3D_RS_ALPHAREF set by the application? The value of alpha_test_ref would suggest "3". Does it also help to set MaxVersionGL without disabling GLSL? When not using core contexts, we use the fixed function alpha test, instead of doing it in the shader. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45468 --- Comment #5 from Andy Clayton <q3aiml+wine(a)gmail.com> --- Henri: thanks so much for reading and helping. You are absolutely right about the discard being valid. I messed up. Even without the discard nothing is rendered since ret.w always equals 0 and blending is enabled with glBlendFunc(GL_SRC_ALPHA, GL_ONE_MINUS_SRC_ALPHA). It then makes sense that removing the alpha-test shader code does not help. Setting MaxVersionGL while leaving GLSL enabled does remove the alpha-test code but also does not help. You are correct that the value of WINED3D_RS_ALPHAREF is 3. It does help to change the ret.w assign from `ret.w = tex0.w * ffp_varying_diffuse.w;` to `ret.w = tex0.w;`. I am just picking gl and d3d up again so I have to do some more reading and experiments to have a guess at what is going on with ffp_varying_diffuse's alpha. Or if you have any more insights to share that would also be great. I have been meaning to attach logs but need to cut them down to a reasonable size without losing anything important. Thanks again for looking and sorry for my confusion. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45468 --- Comment #6 from Henri Verbeet <hverbeet(a)gmail.com> --- (In reply to Andy Clayton from comment #5)
I am just picking gl and d3d up again so I have to do some more reading and experiments to have a guess at what is going on with ffp_varying_diffuse's alpha. Or if you have any more insights to share that would also be great. I have been meaning to attach logs but need to cut them down to a reasonable size without losing anything important.
With GLSL disabled, you should be getting ARB_fragment_program shaders for FFP (fragment) emulation. Those should be equivalent to the GLSL code, but it may make sense to explicitly verify that. We do use fixed-function vertex processing with GLSL disabled. The code for setting that up is select_vertex_implementation(), select_fragment_implementation() and select_shader_backend(). It should be possible to use fixed-function vertex processing together with the GLSL shader backend by returning "&ffp_vertex_pipe" in select_vertex_implementation(). It may be worth giving that a try, but note that that combination hasn't been used for several years now; it may not work at all anymore. ffp_material.diffuse ultimately comes from the current material, set by wined3d_device_set_material(). It's probably also a good idea to check the ambient and emissive light and material values, particularly the alpha values. Alpha is usually set to 0.0f for those, but I'm not entirely confident we handle non-zero alpha values correctly there. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45468 --- Comment #7 from Andy Clayton <q3aiml+wine(a)gmail.com> --- (In reply to Henri Verbeet from comment #6)
ffp_material.diffuse ultimately comes from the current material, set by wined3d_device_set_material()
Do you think somehow WINED3D_RS_DIFFUSEMATERIALSOURCE = WINED3D_MCS_COLOR1 could be getting ignored? In the logs I see DIFFUSEMATERIALSOURCE set to COLOR1: 0009:trace:d3d:wined3d_device_set_render_state device 0xd0fae0, state WINED3D_RS_DIFFUSEMATERIALSOURCE (0x91), value 0x1. And with fixed-function vertex processing wine issues glColorMaterial(GL_FRONT_AND_BACK, GL_DIFFUSE) prior to the relevant glDrawArrays(). Based on shader_glsl_ffp_mcs() it seems like if WINED3D_MCS_COLOR1 is applied then the GLSL vertex shader would be ffp_varying_diffuse.xyz = ffp_material.ambient.xyz * ambient + ffp_attrib_diffuse.xyz * diffuse + ffp_material.emissive.xyz; ffp_varying_diffuse.w = ffp_attrib_diffuse.w; rather than the WINED3D_MCS_MATERIAL/ffp_material.diffuse variant being used. Changing the shader to use ffp_attrib_diffuse does result in the tails appearing. With GLSL there is a glVertexAttribPointer() call for location 5, which the shader assigns to ffp_attrib_diffuse, so there are unused values being passed. I did a little looking at the condition in wined3d_ffp_get_vs_settings(), but I do see a trace from wined3d_stream_info_from_declaration() which seems to say that the use_map flag FFP_DIFFUSE = 5 is getting flipped: 0009:trace:d3d:wined3d_stream_info_from_declaration Load fixed function array 5 [usage WINED3D_DECL_USAGE_COLOR, usage_idx 0, input_slot 0, offset 24, stride 36, format WINED3DFMT_B8G8R8A8_UNORM, class WINED3D_INPUT_PER_VERTEX_DATA, step_rate 0]. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45468 --- Comment #8 from Henri Verbeet <hverbeet(a)gmail.com> --- (In reply to Andy Clayton from comment #7)
Do you think somehow WINED3D_RS_DIFFUSEMATERIALSOURCE = WINED3D_MCS_COLOR1 could be getting ignored? In the logs I see DIFFUSEMATERIALSOURCE set to COLOR1:
That sounds plausible, but the question would be why.
Based on shader_glsl_ffp_mcs() it seems like if WINED3D_MCS_COLOR1 is applied then the GLSL vertex shader would be
ffp_varying_diffuse.xyz = ffp_material.ambient.xyz * ambient + ffp_attrib_diffuse.xyz * diffuse + ffp_material.emissive.xyz; ffp_varying_diffuse.w = ffp_attrib_diffuse.w;
rather than the WINED3D_MCS_MATERIAL/ffp_material.diffuse variant being used.
Right.
passed. I did a little looking at the condition in wined3d_ffp_get_vs_settings(), but I do see a trace from wined3d_stream_info_from_declaration() which seems to say that the use_map flag FFP_DIFFUSE = 5 is getting flipped:
What does wined3d_ffp_get_vs_settings() end up setting "settings->diffuse_source" to? Is that value used correctly by shader_glsl_generate_ffp_vertex_shader()? -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45468 --- Comment #9 from Andy Clayton <q3aiml+wine(a)gmail.com> --- It (In reply to Henri Verbeet from comment #8)
What does wined3d_ffp_get_vs_settings() end up setting "settings->diffuse_source" to? Is that value used correctly by shader_glsl_generate_ffp_vertex_shader()?
It looks like it does not get that far. set_glsl_shader_program() reuses "ctx_data->glsl_program->vs" because the vertex shader hasn't been invalidated in "shader_update_mask". And the prior draw happened to have a vertex declaration without color/diffuse data, so the shader that carries over is based on the wined3d_ffp_get_vs_settings() of a use_map without WINED3D_FFP_DIFFUSE. Throwing "context->shader_update_mask |= 1u << WINED3D_SHADER_TYPE_VERTEX;" in glsl_vertex_pipe_vdecl() is enough for the tails to display. Would storing and comparing "use_map" similar to "swizzle_map" make any sense? -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45468 --- Comment #10 from Henri Verbeet <hverbeet(a)gmail.com> --- (In reply to Andy Clayton from comment #9)
It looks like it does not get that far. set_glsl_shader_program() reuses "ctx_data->glsl_program->vs" because the vertex shader hasn't been invalidated in "shader_update_mask". And the prior draw happened to have a vertex declaration without color/diffuse data, so the shader that carries over is based on the wined3d_ffp_get_vs_settings() of a use_map without WINED3D_FFP_DIFFUSE. Ah, that makes sense.
Throwing "context->shader_update_mask |= 1u << WINED3D_SHADER_TYPE_VERTEX;" in glsl_vertex_pipe_vdecl() is enough for the tails to display. Would storing and comparing "use_map" similar to "swizzle_map" make any sense? I think so.
-- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45468 --- Comment #11 from Henri Verbeet <hverbeet(a)gmail.com> --- Created attachment 61921 --> https://bugs.winehq.org/attachment.cgi?id=61921 patch Does the attached patch help? -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45468 --- Comment #12 from Andy Clayton <q3aiml+wine(a)gmail.com> --- (In reply to Henri Verbeet from comment #11)
Does the attached patch help?
It does! Thanks so much for your help with this. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45468 Henri Verbeet <hverbeet(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Fixed by SHA1| |5e3866cbdbbb63980eed056784d | |afaf58d6adff5 Status|UNCONFIRMED |RESOLVED Resolution|--- |FIXED --- Comment #13 from Henri Verbeet <hverbeet(a)gmail.com> --- Should be fixed by commit 5e3866cbdbbb63980eed056784dafaf58d6adff5. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=45468 Alexandre Julliard <julliard(a)winehq.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED --- Comment #14 from Alexandre Julliard <julliard(a)winehq.org> --- Closing bugs fixed in 3.15. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
participants (1)
-
wine-bugs＠winehq.org