HomeWildfire Games

Refactor HWDetect and rendering options setup.

Description

Refactor HWDetect and rendering options setup.

Remove duplication when setting graphic options by reading the configDB directly.
Properly protect the ModIO config keys.

Approved By: linkmauve

Refs #5538

Differential Revision: https://code.wildfiregames.com/D1931

Event Timeline

Nescio added a subscriber: Nescio.Jun 7 2020, 9:07 PM

When launching 0 A.D. or opening Atlas, I now get the following error:

ERROR:
JavaScript error: hwdetect/hwdetect.js line 369
TypeError: Engine.ConfigDB_CreateValue is not a function
global.RunHardwareDetection@hwdetect/hwdetect.js:369:1
Silier added a subscriber: Silier.Jun 7 2020, 9:10 PM

if windows, you need to rebuild, there have not been autobuild yet

Nescio added a comment.Jun 7 2020, 9:39 PM

if windows, you need to rebuild, there have not been autobuild yet

Fedora 32 here, and I did rebuild 0 A.D., and cleared the cache. Maybe I need to run clean-workspaces.sh, though, I'll try again.

wraitii added a comment.EditedJun 7 2020, 9:43 PM

Er, indeed, there's an issue. I guess it doesn't trigger for most people, including me, because none of those are actually being called by hwdetect.

Feel free to raise a concern, I'll see if it's easy to fix else I'll revert.

Nescio raised a concern with this commit.Jun 7 2020, 9:56 PM

So I did a clean-workspaces.sh and a make clean before rebuilding everything from scratch, same difference, the error is till there.

This commit now has outstanding concerns.Jun 7 2020, 9:56 PM
Nescio removed an auditor: Nescio.Jun 7 2020, 10:21 PM

The error is solved by D2799/rP23748 (and rP23749).

This commit no longer requires audit.Jun 7 2020, 10:21 PM

Was it tested for platforms without GL_EXT_texture_compression_s3tc?

wraitii added a subscriber: fabio.Jun 8 2020, 7:24 PM

Was it tested for platforms without GL_EXT_texture_compression_s3tc?

I followed @fabio 's advice on this, assuming him to be the highest authority on the question.

vladislavbelov added inline comments.Jun 8 2020, 7:25 PM
/ps/trunk/source/ps/VideoMode.h
1

Forgot year.

Was it tested for platforms without GL_EXT_texture_compression_s3tc?

I followed @fabio 's advice on this, assuming him to be the highest authority on the question.

But does it work for users without that extension?

By statistics the number of such users mostly equal to the number of users with GL1. So if the commit breaks such users, then why we didn't have a discussion about that?

Stan added a subscriber: Stan.Jun 8 2020, 7:41 PM

You can't really play the game without s3tc anyway as some textures we bundle are DDS and there are a lot of those for terrains and units.

By statistics the number of such users mostly equal to the number of users with GL1.

Is that a correct statement given https://trac.wildfiregames.com/ticket/4803 ?

In rP23747#42762, @Stan wrote:

You can't really play the game without s3tc anyway as some textures we bundle are DDS and there are a lot of those for terrains and units.

Can I? We have software ST3C reader in text_dds.cpp. I assume we can but slower.

Is that a correct statement given https://trac.wildfiregames.com/ticket/4803 ?

I think so, but it's only on paper. In real life we still have users with legacy software.

I think so, but it's only on paper. In real life we still have users with legacy software.

My understanding of the ticket was that these users could fix their legacy software by installing newer drivers.

I think so, but it's only on paper. In real life we still have users with legacy software.

My understanding of the ticket was that these users could fix their legacy software by installing newer drivers.

In theory - yes, but isn't that software also limited by hardware?

In theory - yes, but isn't that software also limited by hardware?

@fabio Didn't seem to think so. The fact that this was related to a patent is encouraging in that respect.

I'm OK with reverting the S3TC change, but I think we should make sure it's not actually dead weight -> I would wait for user reports of actual issues to act on this.

Nescio removed a subscriber: Nescio.Jun 8 2020, 8:21 PM
Imarok added a subscriber: Imarok.Jun 12 2020, 11:28 PM
../../../source/ps/GameSetup/HWDetect.cpp:138:13: warning: ‘bool IsOverridden(const char*)’ defined but not used [-Wunused-function]
 static bool IsOverridden(const char* setting)
fabio added a comment.EditedJul 4 2020, 9:40 PM

Hi, here are my comments! (Sorry for the delay :) )

Was it tested for platforms without GL_EXT_texture_compression_s3tc?

force_s3tc_enable was a mesa (Linux driver) trick to force S3TC on old drivers, that worked only on games with already compressed textures (like 0ad). That mesa trick was not able to compress textures (games which provided uncompressed textures that have to be compressed by the driver). So it properly worked for 0ad (which pre-compress the textures with nvidia-texture-tools) but it won't on all games.

I think by now we shouldn't worry anymore about systems missing S3TC support. AFAIK it was only missing on mesa driver before 17.3 released in December 2017. But even before that most Linux distros were shipping the additional libtxc-dxtn0 library enabling support for S3TC textures. For example the now becoming End of Life Debian 8 already had it as a mesa dependency. I don't think such old systems can still build 0ad (unless backporting other dependencies, updating gcc and so on) and eventually they could also backport libtxc-dxtn0 anyway.

Given that, I think removing support for force_s3tc_enable is a nice and welcomed cleanup.

The other point to remove altogether "S3TC disabling" (nos3tc option) would be a bit different. Last time I tried it with software rendering it was very slow (so hwdetect.js disabled it on software renderers). That would be the only somewhat useful case (but mostly for debugging, not for playing for real, unless you have a system with a very fast CPU but no GPU where a software renderers could give enough fps). Since you already removed it I wouldn't worry bringing it back.

Also, unless I am missing something (I checked the source on the web), support for disabling s3tc was completely removed from the engine, but hwdetect.js still has some traces of it (including the just discussed disabling it for software renderers) that should also be removed.

(Also the fixed renderpath I wonder it could also be removed in the future...)