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

Added nullptr checks for mRenderManager in OSVRCustomPresent, OSVRCustomPresentD3D11, and OSVRCustomPresentOpenGL. #143

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

JeroMiya
Copy link
Contributor

See: #140

…tomPresentD3D11, and OSVRCustomPresentOpenGL.
check(rc == OSVR_RETURN_SUCCESS);
if (rc != OSVR_RETURN_SUCCESS)
{
UE_LOG(FOSVRCustomPresentLog, Warning,
Copy link
Contributor

Choose a reason for hiding this comment

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

This and the other messages will appear each time a frame tries to render, right? That's a lot of messages.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could probably limit it, but if they're getting it every frame something more serious is wrong instead of just a transient error.

Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense.

if (!mRenderManager)
{
UE_LOG(FOSVRCustomPresentLog, Warning,
TEXT("FOpenGLCustomPresent::FinishRendering() - mRenderManager is null. Should be non-null at this point."));
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe make this message say that rendering won't work and then remove the later messages but keep the checks?

@russell-taylor
Copy link
Contributor

Looks like it will work. Added comments about how to potentially reduce log-file messages.

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