Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanup of control code, enable MSAA, improvements to event handling #86

Merged
merged 23 commits into from
May 30, 2024

Conversation

NogginBops
Copy link
Member

@NogginBops NogginBops commented Jul 21, 2022

The goal of this PR is to remove some layers of complexity from the control by both removing some of the bad abstractions introduced as well as introduce diligent error checking which will aid immensely with further development of the control.

Fixes #97
Fixes #115
Fixes #126

Fixes #118
Fixes #64
Fixes #38
Fixes #70
Fixes #82

@NogginBops NogginBops added enhancement New feature or request 4.0 labels Jul 21, 2022
@NogginBops NogginBops added this to the 4.3.0 milestone Jul 21, 2022
@NogginBops NogginBops changed the title [WIP] Cleanup of control code Cleanup of control code Feb 15, 2023
Copy link
Member

@varon varon left a comment

Choose a reason for hiding this comment

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

These are generally solid changes. Feel free to ignore anything marked Nitpick based on your judgement.

Much of this comes down to personal style, but I do strongly feel that the DxGLFramebuffer helped clarity of the code by keeping the single responsibility principle.

This was clearly separated from the Renderer, which operates on the Framebuffer.

Generally I try to design around deeper object composition hierarchies rather than larger objects as it helps keep the level of discoverability very high.

If we do end up keeping the DxGLFramebuffer class, one improvement that would fit well in this theme would be to not execute the GL and DX logic in the constructor - I really didn't like that, and it should likely have been in a static factory method instead.

Thanks again for doing this - I hope the review comments are helpful.

src/Example/Example.csproj Outdated Show resolved Hide resolved
src/GLWpfControl/DXGLContext.cs Outdated Show resolved Hide resolved
src/GLWpfControl/DXGLContext.cs Outdated Show resolved Hide resolved
src/GLWpfControl/DXGLContext.cs Outdated Show resolved Hide resolved
src/GLWpfControl/GLWpfControl.cs Outdated Show resolved Hide resolved
src/GLWpfControl/GLWpfControlSettings.cs Outdated Show resolved Hide resolved
src/GLWpfControl/GLWpfControl.cs Outdated Show resolved Hide resolved
@NogginBops NogginBops mentioned this pull request May 30, 2024
@NogginBops NogginBops changed the title Cleanup of control code Cleanup of control code, enable MSAA, improvements to event handling May 30, 2024
@NogginBops
Copy link
Member Author

I think I'm done with this PR for 4.3.0. Will look at fixing #109 and #30 for 4.3.1 or 4.4.0.

@NogginBops NogginBops merged commit 7c2bb4b into opentk:master May 30, 2024
@NogginBops NogginBops deleted the cleanup branch May 30, 2024 17:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment