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

docs(client_core): 📝 Fix C API documentation #2564

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

Conversation

zmerp
Copy link
Member

@zmerp zmerp commented Dec 9, 2024

@ShootingKing-AM Let's merge this once you finished the PhoneVR integration rebase

@zmerp zmerp requested a review from ShootingKing-AM December 9, 2024 16:45
@@ -703,11 +704,13 @@ pub struct AlvrStreamConfig {
foveation_edge_ratio_y: f32,
}

/// Requires calling glMakeContext again after this function
#[no_mangle]
pub extern "C" fn alvr_initialize_opengl() {
GRAPHICS_CONTEXT.set(Some(Rc::new(GraphicsContext::new_gl())));
}
Copy link
Member

Choose a reason for hiding this comment

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

cant we call glMakeContext in here itself, since its falloff of wgpu implementation? Also client_core is in android native context also.

Copy link
Member Author

@zmerp zmerp Dec 10, 2024

Choose a reason for hiding this comment

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

Well, if you end up calling another alvr rendering function right after, the glMakeCurrent call becomes redundant. In alvr_client_openxr instead we call glMakeCurrent just before we need it instead of just after every alvr call.

Copy link
Member Author

@zmerp zmerp Dec 10, 2024

Choose a reason for hiding this comment

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

I don't want to be too nitpicky, since this C redering API is used only by PhoneVR. So if you request it i can make the glMakeCurrent calls in here. The only place where i can't is when destroying the GRAPHICS_CONTEXT, so you will need to either destroy your gl resources before that call, or call gmMakeCurrent yourself

@The-personified-devil
Copy link
Collaborator

Considering how clippy is complaining you better add the dead code and unused functions lint disable from the server c_api to this

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

Successfully merging this pull request may close these issues.

3 participants