Fix discrete GPU preference symbols being exported from the wrong place

[binary1248]

Moved NvOptimusEnablement and AmdPowerXpressRequestHighPerformance out of sfml-main and into a macro the user can place in their own translation unit when they need it.

The current implementation suffers from a bunch of problems:

• It doesn't work when using the console subsystem, see NvOptimusEnablement not exported for /SUBSYSTEM:CONSOLE #1192
• It can't work when linking all of SFML dynamically (this implies not linking sfml-main obviously)
• When linking sfml-main for portability, the user can't opt out of preferring the discrete GPU when they know they won't need the extra performance
• This "feature" was never documented, only people following the commit history or who browse through the code will know this exists
• Exporting symbols via __declspec(dllexport) from a static library is just weird in itself anyway

With this change, the user will have full control over when they want this preference to be made, and the symbols will always be exported from the right place: the final executable. And there is the added bonus that this will actually show up in the documentation as well (might still be hard to find though).

[LaurentGomila]

I'm not sure if the top of Window.hpp is the right place to define this stuff. Maybe it would deserve its own header?

this will actually show up in the documentation as well (might still be hard to find though)

I don't know if it works with macros too, but you can use \relates Class to attach it to the documentation page of Class.

[MarioLiebisch]

Haven't tried it myself, but this seems to be the intended approach.

[binary1248]

@LaurentGomila @MarioLiebisch I moved the definition into its own file and added a paragraph with a reference to it in Window.hpp. \relates doesn't seem to work on macros.

[eXpl0it3r]

Shouldn't header be included in the top-level Window.hpp as well, so you don't specifically have to include the header and define the macro?

[binary1248]

I was thinking of providing this as a "convenience header" much like what <SFML/OpenGL.hpp> is now. But... if it makes more sense to automatically include this with <SFML/Window.hpp> then I can fix that too. What do the others think?

[eXpl0it3r]

I mean, it doesn't really do any harm in directly including it and potentially reduces one extra step when explaining how to use it.

[binary1248]

@eXpl0it3r Done.

[LaurentGomila]

Because I found nothing else to say: there's one empty line that should be removed here 😁

[binary1248]

@LaurentGomila Fixed.

[Ceylo]

Shouldn't it be part of current Window module? ie in SFML/Window/GpuPreference.hpp
You include it automatically from <SFML/Window.hpp> and it changes the OpenGL setup in some way, which is currently the Window module responsibility with Context settings & such. Both make me think it should belong to the Window module.

[binary1248]

Theoretically you can use this macro even in a console application in which you only make use of sfml-network even though it wouldn't make much sense. I wouldn't go so far as to say it changes any behaviour we should handle differently. This also expresses a preference, not a demand, we can't and shouldn't rely on it having any effect. When choosing a spot to place it, I figured this has a similar purpose as <SFML/OpenGL.hpp>, being more of a convenience than a necessity so I might as well place it in the same location.

[LaurentGomila]

I didn't notice that before, and yes I admit that it looks confusing. Either make it part of the window module and include it in SFML/Window.hpp, or leave it as a standalone functionality and don't include it in any module by default.

Which solution to choose, I'm not sure 😬

[binary1248]

Well... until now, the user could always assume that using something from an include file would imply having to link against that module as well. This works out for <SFML/OpenGL.hpp> as well since it is not actually part of any module and only a convenience header. The same can be said about <SFML/GpuPreference.hpp>. If the user is so inclined, they could even include both of these headers for mere convenience and use something else for their window management needs.

[LaurentGomila]

Ok, so let's keep it separate from the Window module.

[eXpl0it3r]

I didn't realize that it wasn't "part" of a module when I suggested to add it to Window.hpp. Should have taken a closer look. Since it really is unrelated to SFML window or graphics, I think it's okay to just have it similar to the OpenGL header.

[eXpl0it3r]

Cna you revert it again, @binary1248? 😄

[binary1248]

What do you mean with revert it again?

[MarioLiebisch]

Maybe it's just me, but I'd consider it useful to rename this from GpuPreference to GpuSelection as it feels more intuitive. In addition, I'd also provide a second set of macros to prefer the on-board GPU even though they wouldn't do anything in your everyday app. It would still provide confirmation "yes, indeed use the on-board one!" though.

[eXpl0it3r]

Remove it from the general Window.hpp as to keep it separate from the Window module.

[binary1248]

@MarioLiebisch Selection is misleading, because we don't select anything. We express a preference/hint. It is still up to the driver to pick a GPU for us. If the user chooses to do so, they can still override which GPU will be used. The opposite is also true, there is no way to express a preference for the iGPU. The hints only have an effect when they are exported and their values set to 1. Setting their values to 0 would be the same as not exporting them at all meaning "no preference". In that case it is up to the driver to guess which GPU to use.

[MarioLiebisch]

Yeah, true.

How reliable would it be to try detect on-board cards and spit out a warning on context creation for debug builds in case the macro isn't used (e.g. due to not knowing about it)?

[binary1248]

Not reliable at all? An iGPU in a hybrid graphics setup could be the only available GPU in another system. OpenGL also doesn't provide any reliable way of enumerating adapters like D3D or Vulkan does, so that is also sort of out of the question.

[binary1248]

@eXpl0it3r Done.