Page MenuHomeWildfire Games

Deny to run the game with Fixed rendering pipeline
ClosedPublic

Authored by vladislavbelov on Jul 25 2020, 5:07 PM.

Details

Reviewers
asterix
Angen
linkmauve
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Commits
rP24110: Refactors water shader to move refraction in a separate function.
Trac Tickets
#5791
Summary

Before removing the whole "fixed" pipeline we need to add an assertion. Because user have to know the game can't work.

Test Plan
  1. Apply the patch and compile the game
  2. Make sure that it works for ARB and GLSL pipelines (to trigger "fixed" check you might add override_renderpath = "fixed"; line to the end of the RunDetection function inside hwdetect.js)

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

vladislavbelov created this revision.Jul 25 2020, 5:07 PM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/2782/display/redirect

Stan added a subscriber: Stan.Jul 25 2020, 5:20 PM
Stan added inline comments.
source/ps/GameSetup/GameSetup.cpp
1010 ↗(On Diff #12922)

L" The game does not support pre-shader graphics cards."

1014 ↗(On Diff #12922)

Exit, maybe? I have a feeling this will display the ugly popup nobody understands...

vladislavbelov added inline comments.Jul 25 2020, 5:22 PM
source/ps/GameSetup/GameSetup.cpp
1014 ↗(On Diff #12922)

Maybe, I just follow other exits from this function.

Stan added inline comments.Jul 25 2020, 5:26 PM
source/ps/GameSetup/GameSetup.cpp
1006 ↗(On Diff #12922)

So this will be removed but the check will stay after the complete removal, correct? So users won't be greated by nothing?

1014 ↗(On Diff #12922)

Exception seems to not match the actuall error. Video Mode Failed would be weird, since the gui is displayed :D

I wholeheartedly agree with the intent here.
It might be better to exit from the GUI, and make the GUI message "more red" to inform the player better. Not sure, your call imo.

It might be better to exit from the GUI, and make the GUI message "more red" to inform the player better. Not sure, your call imo.

We can't do something in GUI, because we can't render our GUI without "fixed" pipeline for users who does support only "fixed" pipeline.

source/ps/GameSetup/GameSetup.cpp
1006 ↗(On Diff #12922)

No, that won't be removed.

1014 ↗(On Diff #12922)

No, GUI won't be displayed.

We can't do something in GUI, because we can't render our GUI without "fixed" pipeline for users who does support only "fixed" pipeline.

Ah, yes, such an obvious thing to miss :p
Welp, go ahead.

Stan added inline comments.Jul 25 2020, 6:45 PM
source/ps/GameSetup/GameSetup.cpp
1006 ↗(On Diff #12922)

So we will keep the fixed option forever even though there won't be any such option ?

asterix accepted this revision.Jul 25 2020, 6:47 PM
asterix added a subscriber: asterix.

I think this is nice change and since above people also agree, I agree too but maybe more noble would be to in warning write that they need drivers with some version of graphics library and other supported shader pipelines. I cannot comment on code though.

This revision is now accepted and ready to land.Jul 25 2020, 6:47 PM

I think this is nice change and since above people also agree, I agree too but maybe more noble would be to in warning write that they need drivers with some version of graphics library and other supported shader pipelines. I cannot comment on code though.

We had that warning for about 7 years. So I think current warning is enough.

source/ps/GameSetup/GameSetup.cpp
1006 ↗(On Diff #12922)

Only option, not the code beside that. Because we have hwdetect which might switch rendering to "fixed" for some bugged GPU drivers.

Alright then.

No objection if it gets committed.

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/2816/display/redirect

Stan added a comment.Jul 31 2020, 7:20 PM

New behavior seems better as the message is correct and the game actually exits anyway even if you press continue

Angen added a subscriber: Angen.Oct 4 2020, 12:41 PM

After pressing continue and application closing, I got this:

Assertion failed: "!m_Worker"
Location: UserReport.cpp:514 (CUserReporter::~CUserReporter)

Else works fine.

vladislavbelov edited the test plan for this revision. (Show Details)Oct 4 2020, 4:05 PM

Proper implementation of fatal error.

In D2908#132784, @Angen wrote:

After pressing continue and application closing, I got this:

Now you shouldn't be able to press "Continue" on any platform.

Vulcan added a comment.Oct 4 2020, 4:26 PM

Successful build - Chance fights ever on the side of the prudent.

builderr-debug-macos.txt
../../../source/simulation2/scripting/JSInterface_Simulation.cpp:155:4: warning: suggest braces around initialization of subobject [-Wmissing-braces]
                        CFixedVector2D(-halfSize.X, -halfSize.Y),
                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
../../../source/third_party/fmt/format.cpp:145:7: warning: '_POSIX_C_SOURCE' is not defined, evaluates to 0 [-Wundef]
#if ((_POSIX_C_SOURCE >= 200112L || _XOPEN_SOURCE >= 600) && !_GNU_SOURCE) || (defined(__ANDROID__) && __ANDROID__)
      ^
../../../source/third_party/fmt/format.cpp:145:37: warning: '_XOPEN_SOURCE' is not defined, evaluates to 0 [-Wundef]
#if ((_POSIX_C_SOURCE >= 200112L || _XOPEN_SOURCE >= 600) && !_GNU_SOURCE) || (defined(__ANDROID__) && __ANDROID__)
                                    ^
2 warnings generated.
In file included from ../../../source/graphics/tests/test_Camera.cpp:17:
/Users/wfg/Jenkins/workspace/macos-differential/source/graphics/tests/test_Camera.h:168:4: warning: suggest braces around initialization of subobject [-Wmissing-braces]
                        CVector3D(-101.0f, -101.0f, 101.0f),
                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
In file included from ../../../source/simulation2/tests/test_SerializeTemplates.cpp:17:
/Users/wfg/Jenkins/workspace/macos-differential/source/simulation2/tests/test_SerializeTemplates.h:39:4: warning: suggest braces around initialization of subobject [-Wmissing-braces]
                        3, 0, 1, 4, 1, 5
                        ^~~~~~~~~~~~~~~~
                        {               }
1 warning generated.
builderr-release-macos.txt
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libnetwork.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libtinygettext.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libtinygettext.a(tinygettext.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/liblobby.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libglooxwrapper.a(precompiled.o) has no symbols
../../../source/simulation2/scripting/JSInterface_Simulation.cpp:155:4: warning: suggest braces around initialization of subobject [-Wmissing-braces]
                        CFixedVector2D(-halfSize.X, -halfSize.Y),
                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libsimulation2.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libscriptinterface.a(precompiled.o) has no symbols
../../../source/third_party/fmt/format.cpp:145:7: warning: '_POSIX_C_SOURCE' is not defined, evaluates to 0 [-Wundef]
#if ((_POSIX_C_SOURCE >= 200112L || _XOPEN_SOURCE >= 600) && !_GNU_SOURCE) || (defined(__ANDROID__) && __ANDROID__)
      ^
../../../source/third_party/fmt/format.cpp:145:37: warning: '_XOPEN_SOURCE' is not defined, evaluates to 0 [-Wundef]
#if ((_POSIX_C_SOURCE >= 200112L || _XOPEN_SOURCE >= 600) && !_GNU_SOURCE) || (defined(__ANDROID__) && __ANDROID__)
                                    ^
2 warnings generated.
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libengine.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libgraphics.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libatlas.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libgui.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/liblowlevel.a(dbghelp.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/liblowlevel.a(file_stats.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/liblowlevel.a(vfs_path.o) has no symbols
In file included from ../../../source/graphics/tests/test_Camera.cpp:17:
/Users/wfg/Jenkins/workspace/macos-differential/source/graphics/tests/test_Camera.h:168:4: warning: suggest braces around initialization of subobject [-Wmissing-braces]
                        CVector3D(-101.0f, -101.0f, 101.0f),
                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
In file included from ../../../source/simulation2/tests/test_SerializeTemplates.cpp:17:
/Users/wfg/Jenkins/workspace/macos-differential/source/simulation2/tests/test_SerializeTemplates.h:39:4: warning: suggest braces around initialization of subobject [-Wmissing-braces]
                        3, 0, 1, 4, 1, 5
                        ^~~~~~~~~~~~~~~~
                        {               }
1 warning generated.

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/1627/display/redirect

Angen accepted this revision.Oct 6 2020, 6:19 PM

I tested it with renderpath = "fixed" in combination with preferglsl = "true" and preferglsl = "false".
In both cases, error window appeared and application closed successfully.
Only break and exit actions have been available.
Codewise implementation looks reasonable.

renderpath = "default" works ok.

I would like to mention, that trying to start atlas from command line on windows pyrogenesis.exe -editor does not show any error, as well as using Atlas.bat in command line, what can be a bit confusing why it is not starting at all, but one could argue it is obvious from trying to start pyrogenesis itself.

Hovewer, atlas cannot be started with "fixed" pipeline at all even in current state, so accept.

source/lib/debug.h
1 ↗(On Diff #13602)

^

Angen added a comment.Oct 6 2020, 6:33 PM

For continue removal completeness, one can still ignore error window on windows using x when application continues execution.

Angen requested changes to this revision.Oct 6 2020, 6:38 PM

Because what title says, and comment above, I have to request changes.

This revision now requires changes to proceed.Oct 6 2020, 6:38 PM

Interpret close button as exit in case we can't continue.

Vulcan added a comment.Oct 6 2020, 9:46 PM

Successful build - Chance fights ever on the side of the prudent.

builderr-debug-macos.txt
../../../source/simulation2/scripting/JSInterface_Simulation.cpp:155:4: warning: suggest braces around initialization of subobject [-Wmissing-braces]
                        CFixedVector2D(-halfSize.X, -halfSize.Y),
                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
../../../source/third_party/fmt/format.cpp:145:7: warning: '_POSIX_C_SOURCE' is not defined, evaluates to 0 [-Wundef]
#if ((_POSIX_C_SOURCE >= 200112L || _XOPEN_SOURCE >= 600) && !_GNU_SOURCE) || (defined(__ANDROID__) && __ANDROID__)
      ^
../../../source/third_party/fmt/format.cpp:145:37: warning: '_XOPEN_SOURCE' is not defined, evaluates to 0 [-Wundef]
#if ((_POSIX_C_SOURCE >= 200112L || _XOPEN_SOURCE >= 600) && !_GNU_SOURCE) || (defined(__ANDROID__) && __ANDROID__)
                                    ^
2 warnings generated.
In file included from ../../../source/graphics/tests/test_Camera.cpp:17:
/Users/wfg/Jenkins/workspace/macos-differential/source/graphics/tests/test_Camera.h:168:4: warning: suggest braces around initialization of subobject [-Wmissing-braces]
                        CVector3D(-101.0f, -101.0f, 101.0f),
                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
In file included from ../../../source/simulation2/tests/test_SerializeTemplates.cpp:17:
/Users/wfg/Jenkins/workspace/macos-differential/source/simulation2/tests/test_SerializeTemplates.h:39:4: warning: suggest braces around initialization of subobject [-Wmissing-braces]
                        3, 0, 1, 4, 1, 5
                        ^~~~~~~~~~~~~~~~
                        {               }
1 warning generated.
builderr-release-macos.txt
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libnetwork.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libtinygettext.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libtinygettext.a(tinygettext.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/liblobby.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libglooxwrapper.a(precompiled.o) has no symbols
../../../source/simulation2/scripting/JSInterface_Simulation.cpp:155:4: warning: suggest braces around initialization of subobject [-Wmissing-braces]
                        CFixedVector2D(-halfSize.X, -halfSize.Y),
                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libsimulation2.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libscriptinterface.a(precompiled.o) has no symbols
../../../source/third_party/fmt/format.cpp:145:7: warning: '_POSIX_C_SOURCE' is not defined, evaluates to 0 [-Wundef]
#if ((_POSIX_C_SOURCE >= 200112L || _XOPEN_SOURCE >= 600) && !_GNU_SOURCE) || (defined(__ANDROID__) && __ANDROID__)
      ^
../../../source/third_party/fmt/format.cpp:145:37: warning: '_XOPEN_SOURCE' is not defined, evaluates to 0 [-Wundef]
#if ((_POSIX_C_SOURCE >= 200112L || _XOPEN_SOURCE >= 600) && !_GNU_SOURCE) || (defined(__ANDROID__) && __ANDROID__)
                                    ^
2 warnings generated.
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libengine.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libgraphics.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libatlas.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libgui.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/liblowlevel.a(dbghelp.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/liblowlevel.a(file_stats.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/liblowlevel.a(vfs_path.o) has no symbols
In file included from ../../../source/graphics/tests/test_Camera.cpp:17:
/Users/wfg/Jenkins/workspace/macos-differential/source/graphics/tests/test_Camera.h:168:4: warning: suggest braces around initialization of subobject [-Wmissing-braces]
                        CVector3D(-101.0f, -101.0f, 101.0f),
                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
In file included from ../../../source/simulation2/tests/test_SerializeTemplates.cpp:17:
/Users/wfg/Jenkins/workspace/macos-differential/source/simulation2/tests/test_SerializeTemplates.h:39:4: warning: suggest braces around initialization of subobject [-Wmissing-braces]
                        3, 0, 1, 4, 1, 5
                        ^~~~~~~~~~~~~~~~
                        {               }
1 warning generated.

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/1629/display/redirect

Angen accepted this revision.Oct 7 2020, 6:40 PM
This revision is now accepted and ready to land.Oct 7 2020, 6:40 PM
linkmauve accepted this revision.Oct 23 2020, 10:14 PM
linkmauve added a subscriber: linkmauve.

I’ve tested that this patch indeed prevents the fixed renderer from spawning a window on Wayland.

Without it I got the main menu, followed by a dialog telling me about the issues with the fixed pipeline, with it I got this error message (twice) in the console:

GameSetup.cpp(1010): Your graphics card doesn't appear to be fully compatible with OpenGL shaders. The game does not support pre-shader graphics cards. You are advised to try installing newer drivers and/or upgrade your graphics card. For more information, please see http://www.wildfiregames.com/forum/index.php?showtopic=16734
Your graphics card doesn't appear to be fully compatible with OpenGL shaders. The game does not support pre-shader graphics cards. You are advised to try installing newer drivers and/or upgrade your graphics card. For more information, please see http://www.wildfiregames.com/forum/index.php?showtopic=16734
Location: GameSetup.cpp:1010 (InitGraphics)

Followed by a traceback, and then Error: Can't open display: and whether to break, debug or exit.

So I’d say everything works as expected!

Owners added a subscriber: Restricted Owners Package.Oct 25 2020, 10:01 PM