-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Added support for Compatibility profile in Wgl and Glx code #15598
Added support for Compatibility profile in Wgl and Glx code #15598
Conversation
You can test this PR using the following package version. |
|
@cla-avalonia agree |
1 similar comment
@cla-avalonia agree |
src/Avalonia.OpenGL/GlVersion.cs
Outdated
@@ -11,12 +11,15 @@ public record struct GlVersion | |||
public GlProfileType Type { get; } | |||
public int Major { get; } | |||
public int Minor { get; } | |||
public bool EnableCompatibilityProfile { get; } // Only makes sense if Type is OpenGL and Version is >= 3.2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that GlVersion
struct is used not only for specifying the desired capabilities, but to report the properties of an existing context. So the property should be named IsCompatibilityProfile
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I renamed the property. And, right, it is used in AvaloniaNativeGlPlatformGraphics
, so I altered code here as well so that IsCompatibilityProfile
is correctly reported.
@@ -126,15 +126,20 @@ private static void DebugCallback(int source, int type, int id, int severity, in | |||
IntPtr context; | |||
using (shareContext?.Lock()) | |||
{ | |||
var profileMask = WGL_CONTEXT_CORE_PROFILE_BIT_ARB; | |||
if (version.EnableCompatibilityProfile && | |||
version.Major * 10 + version.Minor >= 32) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's not a proper way to compare versions. Consider using System.Version struct if you don't want to implement the logic manually.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah... yes, I copied/pasted this from some C code; seemed clever, but indeed, it only works because there was never a version 2.12 of OpenGL...
src/Avalonia.X11/Glx/GlxDisplay.cs
Outdated
if (profile.Type == GlProfileType.OpenGLES) | ||
if (profile.Type == GlProfileType.OpenGL && | ||
profile.EnableCompatibilityProfile && | ||
profile.Major * 10 + profile.Minor >= 32) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's not a proper way to compare versions. Consider using System.Version struct if you don't want to implement the logic manually.
You can test this PR using the following package version. |
* Added support for Compatibility profile in Wgl and Glx code * Fixes after code review --------- Co-authored-by: Olivier DALET <[email protected]>
What does the pull request do?
This adds an optional
EnableCompatibilityProfile
member toGlVersion
and the code supporting it in theCreateContext
methods ofGlxDisplay
andWglDisplay
. This way, we can now choose to enable the compatibility profile when passing specific (X11 and Win32) platform options along with the OpenGL versions.What is the updated/expected behavior with this PR?
Now, when customizing the OpenGL backend (Glx and Wgl only), one can choose to enable the compatibility profile
How was the solution implemented (if it's not obvious)?
I simply pass the relevant attributes to the function that creates the OpenGL context
Checklist
Breaking changes
None: the new member/parameter is optional and its default value is consistent with the previous beghavior
Fixed issues
this partially fixes #11977
Remarks
Although I didn't add any unit tests, I made sure the compatibility profile was indeed activated by tweaking the ControlCatalog demo (changes not included here). Tested this both in Windows and Linux (WSL)
I also tried to look whether things should be done in other platforms, but it seems all other platforms only support OpenGL ES; hence not relevant. I'm not too sure with respect to MacOS, but anyway I have no way to test on a Ma machine.
I branched this off
master
because I didn't know better, but I wonder if this could go to11.0.x
or11.1.x
: the code I modifed should have been prettty similar at the time and thus, it should be easy to have it work in older versions