feat: Enable to build SkyBoxAsset using OpenGL #2504
Conversation
Jklawreszuk
changed the title
| @@ -658,23 +658,23 @@ protected unsafe void InitializePlatformDevice(GraphicsProfile[] graphicsProfile | |||
| #endif | |||
|
|
|||
| #if STRIDE_UI_SDL | |||
| gameWindow = (Stride.Graphics.SDL.Window)windowHandle.NativeWindow; | |||
| gameWindow = windowHandle?.NativeWindow as SDL.Window ?? new SDL.Window(""); | |||
There was a problem hiding this comment.
What's this new window for ?
There was a problem hiding this comment.
Dump window is created to initialize the SDLContext which is then used for the OpenGL api as a context (see 699 line).
The author assumed when casting that the windowhandle object was not null but it wasn't, so I handled it
There was a problem hiding this comment.
For opengl on windows this would load in sdl when using a winforms window then ?
Also, shouldn't it be initialized/set in a more appropriate spot then ? I.E.: ensure window is init first and has a native window handle, then this scope is called and retrieves the window without creating one ?
There was a problem hiding this comment.
When it comes to Windows, from what I've noticed, Stride Asset Compiler (always) uses DirectX on Windows to compile assets and I haven't changed that. My change only applies to Linux (or MacOs if someone has decided to support it). So short answer is no
There was a problem hiding this comment.
As for the second question, this window object is used before the game starts so it will always be null. Also, I am not sure it even creates "real" window because Silk has command for this : SDL.CreateWindow()
There was a problem hiding this comment.
Apparently no one predicted it would be possible to run on Linux in the future 😅
There was a problem hiding this comment.
Anyways, If you have any suggestions feel free to give any ideas
There was a problem hiding this comment.
Not sure why the OGL stuff requires a window when the D3d one doesn't, but that's best left to a proper refactor I guess.
Can you split the ternary into a specific line with a comment mentioning that this is a workaround specifically for the asset compiler ?
There was a problem hiding this comment.
@Eideren Sure, Done!
There was a problem hiding this comment.
Correct me if I'm wrong, but I think OpenGL has always needed a window to establish its context (the surface where to draw). Direct3D also needed it up until D3D9, but if I remember it correctly, it was D3D10 or 11 that started to allow setting up a swapchain without an associated HWND.
As Xenko supported both D3D9 and OpenGL, that may be the reason for requiring a window. Now that D3D9 is deprecated and D3D11 no longer needs it, only OpenGL is left with that requirement (as Vulkan can also render without a window).
sources/engine/Stride.Rendering/Rendering/ComputeEffect/GGXPrefiltering/Hammersley.sdsl
Outdated
Show resolved
Hide resolved
Jklawreszuk
force-pushed
the
opengl-patch
branch
from
8b448ac
to
9cac654
Compare
Jklawreszuk
force-pushed
the
opengl-patch
branch
from
9cac654
to
409ab79
Compare
|
Thanks ! |
PR Details
This PR implements / fixes compiling Skybox Asset for OpenGL. I also made few related refactoring changes
Example of a game using Skybox : FirstPersonShooter
Related Issue
#2496
Types of changes
Checklist