NUKE glconfig_t, use WindowConfig instead

[VReaperV]

Cgame-side pr: Unvanquished/Unvanquished#3405

Fixes #517

Move the stuff actually used by cgame (window and display sizes and display aspect) to WindowConfig. Merge the rest with glconfig2_t into GLConfig.

I've noticed this issue earlier when adding things to glconfig2 and when working on daemon-vulkan: RE_BeginRegistration() uses glconfig_t* glConfigOut, glconfig2_t glConfig2Out as its parameters, even though it only needs the window and display sizes from the first one and nothing else.

[slipher]

Other information about the renderer setup is potentially interesting to the cgame besides the window information. Stuff in there might be used to disable incompatible effects or fall back on alternative ones. For example there is the buildable range intersection feature which doesn't work if there is a float color buffer. Or the cgame might refrain from sending add light IPCs if dynamic lights are disabled.

[VReaperV]

Other information about the renderer setup is potentially interesting to the cgame besides the window information. Stuff in there might be used to disable incompatible effects or fall back on alternative ones. For example there is the buildable range intersection feature which doesn't work if there is a float color buffer. Or the cgame might refrain from sending add light IPCs if dynamic lights are disabled.

I think it's better to put those in a different struct that would just contain the information on rendering feature availability. Such struct could also contain information on whether a given feature can be enabled and what is required for it (so e. g. the normal mapping option would be greyed out and would show that normal mapping is required if the latter is disabled).

[VReaperV]

Also, none of the features mentioned are in the glconfig_t struct anyway.

[VReaperV]

In fact, we can have an std::vector of the supported rendering features in both cgame and engine. Each element would be something like:

struct EngineFeature {
  uint32 version;
  bool8_t enabled;
  bool8_t required;
  bool8_t missingHWRequirements;
  std::vector<uint32> missingFeatureRequirements;
  std::string name;
};

and there would be an enum corresponding to each one (to keep strings translatable in the graphics settings).

Then, if cgame receives a vector with more features than it's aware of, it would simply ignore the extra ones and perhaps print their names into the log. If the engine sends a vector with less features, the extra ones would be implicitly disabled, perhaps with some log print about those features requiring a newer engine version.

All of that would allow adding support for optional rendering features that require adding extra codepaths in cgame. For example, GPU particles could be an option that, if detected to be enabled in the engine, would be used by an up-to-date cgame, or otherwise just ignored.

[VReaperV]

Or just keep it as a fixed-size array. That would be simpler, but would require a major engine release to add anything new to it.

[slipher]

Having a vector instead of the fixed-size struct sounds like a good idea. I guess missingFeatureRequirements would mostly be GL extensions? Maybe it would be easier to use strings instead of an enum as it seems tedious to make an enum for all the GL extensions. Also the implementation details of a feature may change, so it would be good to be able to express missing features that weren't planned for in the ABI.

[VReaperV]

I guess missingFeatureRequirements would mostly be GL extensions?

I was thinking more so just referencing the other EngineFeatures, but you're right it would be useful to make it able to reference strings as well. The only issue would be translating them I guess? Hence the enum thing.

[slipher]

Allowing strings would be nice since if you update things without a new major release, the cgame can fall back on displaying the string verbatim if there is a feature it doesn't know about. A feature requirement item could be a pair of an enum { GL_EXTENSION, CVAR, OTHER } specifying which type of thing is missing, and a string saying a specific one. Then the cgame could have a mapping of cvar names to translatable strings describing the feature name.

[VReaperV]

Allowing strings would be nice since if you update things without a new major release, the cgame can fall back on displaying the string verbatim if there is a feature it doesn't know about. A feature requirement item could be a pair of an enum { GL_EXTENSION, CVAR, OTHER } specifying which type of thing is missing, and a string saying a specific one. Then the cgame could have a mapping of cvar names to translatable strings describing the feature name.

That sounds good.

[slipher]

The general idea seems fine, but I'm afraid that with the way the PR is structured now, the for-0.56 branch will be constantly broken, since each time a line of code referencing glConfig or glConfig2 is edited on master, it will fail to merge or fail to compile once merged. So it would be better to arrange the changes so that we do the large-scale renaming on master now. For example you could remove glconfig_t from the renderer and only build it for IPC, while adding and using WindowConfig in the renderer. Or you could rename glconfig_t to WindowConfig right now in the engine and cgame, and just remove the extra fields on 0.56.

[VReaperV]

The general idea seems fine, but I'm afraid that with the way the PR is structured now, the for-0.56 branch will be constantly broken, since each time a line of code referencing glConfig or glConfig2 is edited on master, it will fail to merge or fail to compile once merged. So it would be better to arrange the changes so that we do the large-scale renaming on master now. For example you could remove glconfig_t from the renderer and only build it for IPC, while adding and using WindowConfig in the renderer. Or you could rename glconfig_t to WindowConfig right now in the engine and cgame, and just remove the extra fields on 0.56.

Good point. I've moved most of the changes to #1764, so this pr will only need to move WindowConfig to tr_types and use it in the IPC code.

[slipher]

The part for the master branch was merged so this now needs a rebase.

[VReaperV]

Rebased.

[VReaperV]

CI failure is unrelated:

/home/vsts/work/1/s/src/common/Command.cpp:588:20: error: unused variable 'False' [-Werror,-Wunused-variable]
588 | volatile bool False = false;

[slipher]

LGTM