Skip to content

Conversation

@FireController1847
Copy link
Member

@FireController1847 FireController1847 commented Nov 9, 2025

📑 Summary

Beginning work on adding the Crystal Sdl2 windowing implementation and a very basic OpenGL renderer implementation.

I am aware of the lack of an Sdl2 to OpenGL connector. This will be a future PR/commit.

⚒️ Changes

Caution

Accurately describing the changes made in this PR will greatly improve the speed at which we can review your PR. Semantic Versioning can be helpful in determining the scope of your change, and will help in determining the appropriate version number, merge location, and timing of the integration of your change.

➕ Additions

Added the following projects to the repository:

  • Modules/Crystal/CatalystUI.Modules.Crystal.Sdl2
  • Modules/Crystal/CatalystUI.Modules.Crystal.OpenGL
  • Modules/Crystal/CatalystUI.Modules.Crystal.Glfw3OpenGLSurfaceConnector

Additionally, changes from about four months ago in the original CatalystUI development project have now been migrated into the latest version, including a refactored implementation of SdlDisplay, SdlWindow, and OpenGLRenderer (to merge the previous OpenGLThread and OpenGLRenderer implementations together).

✖️ Modifications

Primary changes from previous commits in this iteration of Catalyst include:

  • Moving the EdidHelper from a Glfw3 specific implementation to the Crystal Core.
  • Resolutions to recursive issues in thread exception handling.
  • Additions and modifications to various Crystal Core interfaces.

All changes are noted in the individual commits.

➖ Removals

💬 Review

Important

Accurately listing key items for review will help us review your PR more quickly. Please include large changes, breaking changes, and any other changes that may require special attention.

Reviewers will check off items in the list as they are reviewed, in addition to the normal GitHub review process. They may request changes if any items are not adequately addressed or add additional items as necessary.

  • Ensure consistency with the logic of GlfwWindow with SdlWindow.
  • Ensure SdlWindow contains implementations MATCHING the available implementations of GlfwWindow.
    • Ensure supported window events match.
    • Ensure supported window functions match.
    • Ensure supported window properties match.
  • In-depth is not required on the OpenGL module, but ensuring a basic outline of functionality is preferred.
  • Ensure all classes follow the CatalystUI model and design.
  • Basic spelling and grammar check across the board.

CatalystUI Framework for .NET Core
Copyright (c) 2025 CatalystUI LLC. All rights reserved.

* Added an implementation of IDisplay using Sdl2.
* Created a new Catalyst.Modules.Crystal.Sdl2 project.
* Added an implementation of IWindow using Sdl2.
@FireController1847 FireController1847 self-assigned this Nov 9, 2025
Copilot AI review requested due to automatic review settings November 9, 2025 05:57
@FireController1847 FireController1847 added ACT: In Progress A pull request which is currently in-progress. CAT: Feature Request A category of issues which are feature requests. P3: Medium Priority A bug or feature request which can wait some time to be resolved. labels Nov 9, 2025
@FireController1847 FireController1847 marked this pull request as draft November 9, 2025 05:58
@FireController1847 FireController1847 changed the base branch from main to beta November 9, 2025 05:59

This comment was marked as outdated.

FireController1847 and others added 5 commits November 8, 2025 23:02
…dow/SdlWindow.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: FireController#1847 <usfirepilot123@gmail.com>
* Resolved exceptions from the DelegateQueue causing a recursive loop when attempting to unwind the stack.
* Resolved exceptions from the worker thread in CatalystApp from being consumed.
* Created a new Catalyst.Modules.Crystal.OpenGL project.
* Resolved naming inconsistencies in the builder extensions for Glfw3.
* Added an implementation of OpenGLMacNativeConnector.
* Added an implementation of the OpenGL INativeApi wrapper.
* Added a basic impl for IRenderer for OpenGLRenderer
* Minor changes to IRenderer (adding the dispatcher, depth clarification for Clear, and nullability on SetTarget)
* Added RendererOptions
* Added RenderLayerException
* Added CreateRenderer to IRendererLayer
* Fixed naming in the native connector
In theory, the basics for a renderer should now be complete.
@FireController1847 FireController1847 changed the title Add Sdl2 windowing Add Sdl2 windowing & OpenGL rendering (basic) Nov 11, 2025
@FireController1847 FireController1847 added ACT: Review A pull request which is currently under review. and removed ACT: In Progress A pull request which is currently in-progress. labels Nov 11, 2025
@FireController1847 FireController1847 marked this pull request as ready for review November 11, 2025 04:34
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 38 out of 38 changed files in this pull request and generated 63 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

* Added a "quickstart" BasicWindow example project
* Resolved get/init/set issues with WindowOptions & RendererOptions (no need for them to be read-only).
Copy link
Member

@ItzNxthaniel ItzNxthaniel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall LGTM, I would like to discuss my two comments before explicit approval.

* Ensured threads properly wait if there's no delegates instead of spin-looping.
* Resolved excessive fetching of TargetFramerate in OpenGLRenderer (moved to use underlying value)
* Resolved publish profiles not exporting the correct architectures (Arm64 vs x64)
* Resolved changes by Copilot
* Resolved chnages by @ThisIsBrady
* Resolved changes by @ItzNxthaniel
* This was put inside the lock causing it to wait forever because delegates could never be enqueued :(
* Was not setting the disposal flag early enough so resources were still being used after they were disposed of.
* Removed disposal exception for _queue.Execute() due to the rate at which it is fired.
Copy link
Member

@ItzNxthaniel ItzNxthaniel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@FireController1847 FireController1847 merged commit 29fa353 into beta Nov 12, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ACT: Review A pull request which is currently under review. CAT: Feature Request A category of issues which are feature requests. P3: Medium Priority A bug or feature request which can wait some time to be resolved.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants