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

Fix: unbreak compilation on OpenBSD #1188

Merged
merged 1 commit into from
Feb 10, 2024
Merged

Fix: unbreak compilation on OpenBSD #1188

merged 1 commit into from
Feb 10, 2024

Conversation

yukiteruamano
Copy link
Contributor

@yshui As you had asked me, the final changes each in their own commit, I have already compiled the binary and it works without problems. Take a look.

picom-unbreak-openbsd.log

Copy link

codecov bot commented Feb 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (0f22b70) 36.62% compared to head (023103c) 36.62%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             next    #1188   +/-   ##
=======================================
  Coverage   36.62%   36.62%           
=======================================
  Files          50       50           
  Lines       11555    11555           
=======================================
  Hits         4232     4232           
  Misses       7323     7323           
Files Coverage Δ
src/picom.c 62.21% <ø> (ø)

Copy link
Owner

@yshui yshui left a comment

Choose a reason for hiding this comment

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

Thanks! Looks good, but I have a question.

@@ -1213,6 +1213,14 @@ bool gl_last_render_time(backend_t *base, struct timespec *ts) {
return false;
}

/// Obtain proc address for glGetQueryObjectui64v in OpenBSD
/// because the symbol function not appear directly in libgl for Mesa in this OS
#ifdef __OpenBSD__
Copy link
Owner

Choose a reason for hiding this comment

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

Do we need this ifdef?

Copy link
Owner

Choose a reason for hiding this comment

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

And we should save this pointer somewhere, instead of calling GetProcAddress every frame.

@yshui
Copy link
Owner

yshui commented Feb 9, 2024

And you second commit's message is copied twice

/// because the symbol function not appear directly in libgl for Mesa in this OS
#ifdef __OpenBSD__
static PFNGLGETQUERYOBJECTUI64VPROC glGetQueryObjectui64v = NULL;
glGetQueryObjectui64v = (PFNGLGETQUERYOBJECTUI64VPROC) \
Copy link
Owner

Choose a reason for hiding this comment

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

What's this \ for?

Comment on lines 1218 to 1240
// We need obtain proc pointer for glGetQueryObjectui64v function.
// In OpenBSD this function no appear publicly, glXGetProcAddress resolve the issue.
if(ptrglGetQueryObjectui64v == NULL) {
ptrglGetQueryObjectui64v = (PFNGLGETQUERYOBJECTUI64VPROC)
glXGetProcAddress("glGetQueryObjectui64v");

GLuint64 time;
ptrglGetQueryObjectui64v(gd->frame_timing[gd->current_frame_timing ^ 1],
GL_QUERY_RESULT, &time);
ts->tv_sec = (long)(time / 1000000000);
ts->tv_nsec = (long)(time % 1000000000);
gl_check_err();
}
else {
GLuint64 time;
ptrglGetQueryObjectui64v(gd->frame_timing[gd->current_frame_timing ^ 1],
GL_QUERY_RESULT, &time);
ts->tv_sec = (long)(time / 1000000000);
ts->tv_nsec = (long)(time % 1000000000);
gl_check_err();
}

return true;
Copy link
Owner

Choose a reason for hiding this comment

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

there is no need to copy-and-paste code here.

a good place for GetProcAddress is gl_init.

@yshui
Copy link
Owner

yshui commented Feb 10, 2024

@yukiteruamano there are some extra considerations i just found out about the opengl problem. we might go a different route, so let's merge the sched fix first

@yshui yshui force-pushed the next branch 2 times, most recently from 77aeedb to 902b5d5 Compare February 10, 2024 10:15
OpenBSD don't have support for sched_getparam(), sched_setparam(), or
sched_setscheduler() functions (yet). In this case, we need use
pthead-equivalents for real-time sched for picom. Theses changes add
this support.

Authored-by: Jose Maldonado aka Yukiteru <josemald89@gmail.com>
Signed-off-by: Yuxuan Shui <yshuiv7@gmail.com>
@yshui
Copy link
Owner

yshui commented Feb 10, 2024

@yukiteruamano thanks for the PR, and for being patient with the feedbacks. 👍

@yshui yshui merged commit 726b8d0 into yshui:next Feb 10, 2024
16 checks passed
@yukiteruamano
Copy link
Contributor Author

@yshui Not problem, thanks for you help, my knowledge in C/git is limited, but with this changes I have been able to unbreak the build and use picom v11+ without problems with amdgpu on OpenBSD, and if the changes arrive and are maintained upstream they will generate fewer problems to compile and maintain the project on OpenBSD.

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.

None yet

2 participants