Skip to content

Conversation

@ThePuzzlemaker
Copy link
Contributor

@ThePuzzlemaker ThePuzzlemaker commented Feb 19, 2025

Preface

I am absolutely willing to split this PR into many as necessary to ease review once I am done with it, and I am of the understanding that this PR will probably not land as-is once complete. I am not a C++ programmer by trade or by hobby (I mostly do Rust code) so if there's anything you see that's a bit, well, icky, just let me know and I am willing to fix it.

This PR builds on #550, and on @TheGondos's imgui_dlg branch which extends on #550 further. If/when #550 and associated dialog changes are merged, I will rebase this PR onto that branch.

Rationale

Orbiter does not work well on Linux as of the moment, only partially working with WINE and dxvk. It requires a lot of changes to undo 25+ years of Windows-only development, but this can be changed. Using SDL3 (which is the recommended version of SDL by its developers), many of the hard things of cross-platform support, including module loading, window initialization, input support, etc., become easy API calls to a relatively lightweight translation layer. SDL3, and in extension SDL as a whole, have been battle-tested in the game development industry (for example, SDL2 is used in many Valve titles).

Methodology

This PR is (and will be) huge.

Breaking changes are made in some cases, and I believe this is justified since the x64 release will break binary compatibility anyway, so we might as well merge some additional minor breaking changes at the same time to reduce stress on module/vessel maintainers. [Update 2025-02-22: I've realized I've probably made too many breaking changes right now, so I think many of these will go into a different "Cross-platform" PR.]

D3D9Client will not be deprecated. I plan to work on my own Vulkan client after I'm done with this, but I will probably never reach the level of @jarmonik's amazing work on D3D9Client. Instead, D3D9Client will be changed as part of this or another PR to use cross-platform APIs for initialization and interaction with the Orbiter core, but ultimately will still use D3D9 and Windows-specific APIs under the hood, and just won't be able to be built on non-Windows targets.

Breaking Changes

Build

CMAKE_CXX_STANDARD_REQUIRED is now ON -- C++17 conformance is required by the building compiler. MSVC has supported almost all features of C++17 (except one, std::hash<std::filesystem::path>, which is not currently used in this codebase) since VS2017 version 15.7 (compiler version 19.14.26428.0). Thus, you don't need to have a very up-to-date compiler to still build Orbiter.

Modules must now link with SDL3. If building with Orbiter's CMake system, this is as simple as adding SDL3::SDL3 to the target_link_libraries invocation. I tried making it so this was not necessary but it was non-trivial and would've led to other issues down the line.

SDKs

  • GraphicsAPI.h: Render windows are now SDL wrappers. I have yet to add "proper" docs but these classes are fairly self-explanatory.
  • ModuleAPI.h: The signature of clbkProcessMouse has changed to take SDL mouse events rather than WinAPI event types. The conversion is fairly straightforward.

Appendix

I want to thank all the Orbiter developers for their excellent work making this simulator. I hope that this PR can help continue to make Orbiter better and even more accessible to people. I understand that this PR, once finished, will likely be a large burden on reviewers, and I will do whatever I can to ease such burden. And again, I'm not particularly steadfast on any code here. If you ask me to change something, I will almost certainly be willing to change it.

Up to date as of 2025-02-26, changes up to 3e1c67c

@jarmonik
Copy link
Contributor

Sounds good to me. However, I would not merge MFD and MFD2 classes because there are plenty of MFD's using the old windows GDI interface. As far as I can tell, there are no problems continuing to support it on windows. Linux could simply deal with the problem by not loading modules those are using GDI.

Also the GDI is extensively used by large projects like NASSP in other aspects beyond MFD's, so, might be good idea the talk to them before discontinuing GDI support, I try to listen developers and their wishes,

If the GDI support is continued on windows what harm does it do on linux ?

@ThePuzzlemaker
Copy link
Contributor Author

ThePuzzlemaker commented Feb 21, 2025

If the GDI support is continued on windows what harm does it do on linux ?

Ultimately, I guess it's not that much harm -- it's just a lot of #ifdefs could be required which would clutter things up, but I understand your reasoning.

I would hope that the Sketchpad system eventually becomes fully fleshed out so we can deprecate GDI usage and make everything fully cross-platform. If you have any questions about APIs, I'd suggest looking at DearImGui's DrawList which doesn't seem to have too bad an API; it might be useful as a reference to flesh out the Sketchpad API so that it supports whatever devs need.

But yes, completely removing GDI without first properly deprecating and superseding it is not a good idea, so I'll work on those changes (including reverting the MFD/MFD2 merge in 0109d67) once I'm finished getting D3D9Client working again (currently it's having some odd issues, and I'm not exactly sure where they're coming from, so I'll have to really scrutinize my code)

@ThePuzzlemaker
Copy link
Contributor Author

I think once I get everything working again I'll probably split the Launchpad into a different PR.

@ThePuzzlemaker
Copy link
Contributor Author

D3D9Client is not entirely working yet... see this reply on the forum

@jarmonik
Copy link
Contributor

I browsed through the modifications I noticed a lot of changes that doesn't seem to be relevant to SDL such as scenarios, documentation, *.jpg's and *.ico's. Are those slipped there by accident ? It's pretty hard to make sense out of it. Also, there are plenty of white space changes. I always check my code before commit and revert white space changes and I hope you could too. Changes in white space just make spotting relevant changes a little more difficult, increasing work load.

@ThePuzzlemaker
Copy link
Contributor Author

ThePuzzlemaker commented Feb 23, 2025

The scenario and documentation changes are relevant, but more as part of the Launchpad changes--that will be split into another PR.

As for the texture stuff, that's cause I moved the location of the Launchpad bitmaps to be in the Textures folder as this is easier than finding a (eventually cross-platform) way to store and load resources within a binary. (Not until #embed in C++23 at least, I guess). Again, also part of the Launchpad PR and will likely be changed anyway.

And the whitespace changes are mostly incidental, cause my editor refuses to save files with trailing whitespace. I'll make sure any irrelevant whitespace changes get removed.

@ThePuzzlemaker
Copy link
Contributor Author

ThePuzzlemaker commented Feb 23, 2025

I've squashed the previous some-odd commits because it made it easier to revert unnecessary changes which will be split into a future separate launchpad PR.

There are still a few things that need to be done (i.e., porting the whole keyboard and joystick input system), but this is now a much more minimal PR.

And again, I apologize if there are whitespace changes still. I tried to remove as many as possible but my editor isn't conducive to it (even when I disabled the "remove trailing whitespace" option)

@ThePuzzlemaker
Copy link
Contributor Author

ThePuzzlemaker commented Feb 23, 2025

It appears my editor has decided to continue removing trailing whitespace. I think this is because of the .editorconfig at the root of the repo. I'll get to removing the extra whitespace changes soon.

It might be worth making a PR using something like clang-format which will automatically format code to a specific (quite customizable) style. But that's somewhat irrelevant to this PR, just a thought.

[WIP] Beginnings of SDL3 port

[WIP] Start refactor of launchpad

Start Scn tab & change font

Move Src/Bitmaps to Textures/OrbiterCore

Scn desc & better ImGui ctx

Also started porting some old HTMLHelp/HYPERDESC scenarios to the new Markdown system

Finished porting scn's

Update scn. dev docs

Disable imgui ini and logfile

ConsumeEvent quit outparam & launchpad config

Fix markdown h2s

Unload images on scn reselect

Checklists/Quickstart: h2s

Save scenario dialog

Also, some fixes to ScnPath that 1. makes it thread-safe and 2. makes it not crash half the time with long strings

Detour: modernize paths & some assoc. strings

Remove some now irrelevant comments

[WIP] Testing the waters

Nothing builds yet. This commit may be reverted but it removes (most) of the windows-specific includes

Revert "[WIP] Testing the waters"

This reverts commit fde97e1.

Safety wrappers

[WIP] MFDAPI: De-Windowsify

Can't guarantee this works right now, hence WIP

UIUtil: Make ImCtx more generic

More work

Sorry, I don't really know what to call this commit. And it breaks things. And it's too many unrelated things.

It'll probably be squashed anyway eventually so it probably won't matter (famous last words)

Enforce C++ standard

TabModule: oops forgot the buttons

TabModule: oops forgot locked items

Automatically convert many files to UTF-8

About tab work and fix module loading

Oops, more printf issues

Properly center about tab

(Mostly) finish about tab

Remove old about tab code comments

Refactor away some complicating templates

Options tab

This also messes around with some other stuff I found while doing the options tab (e.g., Markdown link color, the theme, cross-platform paths, etc.)

Revert "[WIP] MFDAPI: De-Windowsify"

This reverts commit 0109d67

Remove some overreaching changes

Per feedback. D3D9Client still does not work, but I'm working on it :P

Revert oapi::Font::{Width, Height}

Probably could be better in a different PR

sdl::UnmanagedWindow: not a stopgap for now

SDLWrappers.cpp: fix build on release

UI options: font size

Start getting things working again

Some small fixes

No point in switching to float

SliderScalar exists; making a "Reset" variation isn't the *best* way to do this but, hey, it works

Sadly we can't do templates across the DLL boundary :(

Probably helps to test changes first

Fix a few things

Reintroduces one breaking change but like, it only happens in D3D9Client and is a <5 line change solution so it's not a *huge* deal
These are necessary for the Launchpad, but I might go about it differently in that PR. They're not needed here though.
I'm honestly not sure if this behaviour is implemented elsewhere, or what the exact purpose of this originally was, since this was only called in the render window.
@ThePuzzlemaker
Copy link
Contributor Author

ThePuzzlemaker commented Feb 26, 2025

Mouse input is currently broken (at least for me) -- for some reason only every other right click is detected. I found a forum post about this on the SDL forums but it's from 2002... I highly doubt a bug lasted that long, so I'm pretty sure I'm doing something wrong. I've asked the people of the SDL Discord so we'll see I guess. If anyone can reproduce please let me know! This issue has been fixed; it had to do with SDL_SetWindowRelativeMouseMode. My guess is that SDL's auto-mouse-capture is a bit weird with relative mode and so does weird things.

As for keyboard input, it is working satisfactorily. I'm not sure if this method is the best way of doing things (using a linked list to buffer events since SDL doesn't buffer them like DirectInput does), but it's certainly the easiest and least invasive way.

@ThePuzzlemaker ThePuzzlemaker marked this pull request as ready for review March 1, 2025 02:56
@ThePuzzlemaker ThePuzzlemaker changed the title [WIP, DO NOT MERGE YET] SDL3 port for cross-platform support SDL3 port for cross-platform support Mar 1, 2025
@ThePuzzlemaker
Copy link
Contributor Author

ThePuzzlemaker commented Mar 1, 2025

This PR is now ready for review, but will not be ready for merge until #550 (et seq.) are merged

This will probably need some shakeout testing to ensure it doesn't silently break certain things, but major functionality which was touched by this code is appearing to work well for me.

There is one unresolved FIXME, in Src/Orbiter/Orbiter.cpp:2004, which I'm open for suggestions on how to resolve; ultimately it's hard to simply force-destroy a std::shared_ptr<T> without crashing. I think a saner option might be to run CloseSession but I'm not familiar enough with Orbiter core code to know if that would cause issues if KillVessels failed.

I apologize if there are still any whitespace/encoding changes that accidentally got kept. I've tried to fix these but Git and my editor(s) are not being very nice about it.

Here's a link to the GitHub review tab with only my changes selected, since it's a bit annoying to get to.

@DragonSWDev
Copy link

DragonSWDev commented Mar 4, 2025

As for the D3D9Client how about using DXVK that can be built as native Linux library and used for porting Direct3D code to run on Linux? I think Valve uses it on some of their older Source 1 based games. Probably some changes will be needed but I guess it should be easier than writing new client from scratch.

@ThePuzzlemaker
Copy link
Contributor Author

@DragonSWDev I've had some major visual glitches with DXVK that don't happen on Windows.

@DragonSWDev
Copy link

@ThePuzzlemaker Maybe things will be different if Direct3D 9 code will be compiled natively to Linux with DXVK native?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants