Cleanup of hwdetect.js and its test.
Needs ReviewPublic
Actions

Authored by echotangoecho on Jan 23 2017, 9:06 PM.

Details

Reviewers

Trac Tickets

Summary

Remove the hardcoded lists of renderer names and instead try to do detection based on supported OpenGL version.
Remark: testing whether gl_major_version > 2 isn't very useful on OS X, as OS X doesn't allow one to use newer GL functions with the functions deprecated in GL 3.
This can't really be fixed until we drop all deprecated GL usage.

Test Plan

Test whether the expected options are set for the "problematic" hardware configurations using the attached test.js.

Diff Detail

Repository

rP 0 A.D. Public Repository

Branch

arcpatch-D86

Lint

No Linters Available

Unit

No Unit Test Coverage

Build Status

Buildable 607
Build 960: Vulcan Build	Jenkins
Build 959: arc lint + arc unit

Event Timeline

echotangoecho created this revision.Jan 23 2017, 9:06 PM

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running debug tests...
Running cxxtest tests (302 tests)..............................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/231/ for more details.

Harbormaster completed remote builds in B336: Diff 314.Jan 23 2017, 10:16 PM

I'm not sure I will review that very soon but I do need to have a look at hardware detection so I'm assigning myself, if you don't mind. If you want to have this committed soon you should ask someone to review it before I do :-)

Rebase, remove unused variable and fix some style issues in test.js

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (302 tests)..............................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (302 tests)..............................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/415/ for more details.

Harbormaster completed remote builds in B607: Diff 623.Feb 26 2017, 4:28 PM

fabio added a subscriber: fabio.Jun 3 2017, 2:17 PM

elexis mentioned this in rP19886: Update Mesa software renderer detection..Jul 8 2017, 6:30 PM

vladislavbelov added a reviewer: vladislavbelov.Nov 6 2017, 9:24 AM

fabio mentioned this in D1486: Graphics Improvements.May 16 2018, 11:16 AM

Some comments.

binaries/data/mods/mod/hwdetect/hwdetect.js
64	Why changing unix to linux here?
129	Do you really mean \|\| rather than && here? Also, is really needed disabling shadows on every Intel GPU? What about something like: if (os_linux && GL_RENDERER.match(/^Mesa DRI R[123]00 /) \|\| (GL_RENDERER.match(/Intel/) && gl_major_version < 3) \|\|
131	This conditions seems a bit difficult to understand. For example the last conditions disables shadow when OpenGL < 3 on every non Intel GPU? But previous conditions enable it on Intel with OpenGL 2? Is this what we want? We may simplify and just disable them on every OpenGL < 3 so to maximize performance?
215	Maybe leave GL_VENDOR and GL_EXTENSIONS commented if not actually used here? They may eventually be used in future.

I suppose, we should have some hardcoded lists (probably not these). Because there are many video cards and drivers that contain weird bugs, which nobody will fix. E.g. Chromium has a huge list of exceptions: https://cs.chromium.org/chromium/src/gpu/config/software_rendering_list.json

I'm letting Vlad handle this patch after the revamp of the feedback page ? I'm still subscribed here.

Itms removed a reviewer: Itms.Feb 24 2019, 5:34 PM

bb mentioned this in D1931: Refactor HWDetect and rendering options setup.Jun 1 2020, 6:47 PM

Poke @vladislavbelov