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

Fixed getting actual bounds and refresh rate of the output (monitor/display) from GraphicsOutput.CurrentDisplayMode when using Direct3D #2494

Merged
merged 4 commits into from
Nov 10, 2024

Conversation

Feralnex
Copy link
Contributor

@Feralnex Feralnex commented Oct 20, 2024

PR Details

Fix for the bug #2492.

References:

Related Issue

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My change requires a change to the documentation.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • I have built and run the editor to try this change out.

…isplay) from GraphicsOutput.CurrentDisplayMode when using Direct3D
@Feralnex Feralnex changed the title Fixed getting actual bounds and refresh rate of the output (monitor/d… Fixed getting actual bounds and refresh rate of the output (monitor/display) from GraphicsOutput.CurrentDisplayMode when using Direct3D Oct 20, 2024
@Feralnex
Copy link
Contributor Author

@Feralnex please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.

@dotnet-policy-service agree [company="{your company}"]

Options:

  • (default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.
@dotnet-policy-service agree
  • (when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.
@dotnet-policy-service agree company="Microsoft"

Contributor License Agreement

@dotnet-policy-service agree

Copy link
Collaborator

@Eideren Eideren left a comment

Choose a reason for hiding this comment

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

Looks good, thanks !

}
catch (Exception) { }
Copy link
Collaborator

Choose a reason for hiding this comment

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

In which case would it throw ? Might be best to still log it, perhaps through a global logger for this class;

private static readonly Logger Log = GlobalLogger.GetLogger(typeof(GraphicsOutput).FullName);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a good idea, to be honest I don't know exactly in which cases it might throw, I followed the steps taken from FindClosestMatchingDisplayMode in the same file.

@Eideren
Copy link
Collaborator

Eideren commented Oct 26, 2024

Actually, you might want to change TryFindMatchingDisplayMode and/or the caller to reflect how you changed the method;

private void InitializeCurrentDisplayMode()
{
currentDisplayMode = TryFindMatchingDisplayMode(Format.R8G8B8A8_UNorm)
?? TryFindMatchingDisplayMode(Format.B8G8R8A8_UNorm);
}

So, TryFindMatchingDisplayMode should return null if the format doesn't match for that logic to work

@Eideren Eideren merged commit 767e143 into stride3d:master Nov 10, 2024
6 checks passed
@Eideren
Copy link
Collaborator

Eideren commented Nov 10, 2024

Thanks !

@Feralnex
Copy link
Contributor Author

You're welcome!

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

Successfully merging this pull request may close these issues.

2 participants