Fix: unbreak compilation in OpenBSD #1186
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## next #1186 +/- ##
==========================================
- Coverage 36.62% 36.60% -0.02%
==========================================
Files 50 50
Lines 11555 11555
==========================================
- Hits 4232 4230 -2
- Misses 7323 7325 +2
|
|
thanks for the pull request, we definitely need to figure this out. have you tried retrieving the also cc @omar-polo as he fixed picom building on openbsd too in the pull request linked above. |
|
Yes, I have seen Omar's work regarding this problem, in fact, you can follow what we have uploaded to the OpenBSD list here. The workaround that Omar has uploaded is a bit more "ugly" and not directly applicable to upstream to maintain, since it changes some things in the code base, making redirects to use glEGLGetQueryObjectui64v in several parts of picom's upstream code. The solution I propose is less invasive in that case, it maintains the original code, makes use of preprocessors that only apply to OpenBSD and that have no impact on the rest of the supported OS (upstream build-tests for Linux and FreeBSD go without errors ). All this taking advantage of the libepoxy library that comes with Xenocara, which does provide access to glGetQueryObjectui64v. I just recompiled picom with all these changes, I don't use the ports and it works without a problem. |
|
Yeah, my attempt was very ugly. I thought it was more or less the same issue, but I couldn't declare a global function pointer like I did in #930 since the prototype would be slightly different. So, my quick workaround was to use a different name. It works, I could ensure that the glx backend is working fine on OpenBSD-CURRENT on amdgpu, but it's so ugly. I don't know the pros and cons of using libepoxy, I'm afraid. |
Same here @omar-polo but this fix is a better option than my previous proposal (changing the GLuint64 of the last_render timestamps, to be able to use glGetQueryObjectuiv). In any case, libepoxy is part of Xenocara and as indicated in their project, the support for managing Mesa OpenGL functions is complete. |
|
thanks for the PR. I will review it later, but first, can you squash the style fixes into your first commit? |
Do you know the reason of this?
hmm, is it because it's old? or is there some other reason this symbol is not available? is it possible to use |
|
oh, this gets a bit tricky. let's see what @yshui thinks.
there is the #1169 that will replace the current real-time priority setting logic. however, it's incomplete and still uses the |
|
@yshui rebase done |
|
@yshui in OpenBSD sched.h show this It seems to not be implemented in the same way as in other systems.
Mesa in OpenBSD is I'm using AMDGPU for more info/context. |
|
@absolutelynothelix I know #1169 but I have one question over this implementation This requires org.freedesktop.RealtimeKit1 native support on dbus implementation for OpenBSD? Because if so, I haven't seen anything like rtkit on OpenBSD |
|
|
The old code work very well 😊 |
|
@yukiteruamano the new code fallbacks to can we use |
src/picom.c
Outdated
| return; | ||
| } | ||
| log_info("Set real-time scheduling priority to %d", priority); | ||
| #else // OpenBSD real-time scheduling support |
There was a problem hiding this comment.
you are duplicating more code than is necessary. also I think you need to add dependency('threads')
There was a problem hiding this comment.
You are right, I fix this and dedup code too
|
and please remember to put the gl change and sched change into separate commits, thanks. |
|
let me setup an openbsd build machine for CI so this kind of breakage doesn't happen again. |
|
@yshui the dependency is only for OpenBSD, and libepoxy is integrated on Xenocara for default. The use of this new code don't require install new packages in others OS. I don't test |
I see. anyway, let's give |
|
@yshui Testing with
Testing Searching symbol name
Yeah, I understand this, in fact, it was what led me to see the port, because I saw that it was 11.1 but OpenBSD was still using 10.2. And shortly after I saw @omar-polo work (he did a different workaround) and I had already put in a different fix (quite ugly the truth) but this PR shows me a better solution (although it depends on libepoxy in OpenBSD). |
can you post the changes you made? |
|
@yshui changes marked with + |
|
@yshui OK, I made a forward: Using this code (based in get_proc used in egl.c): Or this: I made the code compile. But, using my picom config, the progam break in core dump: I suspect this is because glGetQueryObjectui64v is not found as a symbol within libgl for Mesa on OpenBSD. The other possible solution (without use libepoxy in OpenBSD) is this change: In one occasion, I compiled and used picom with this change (aprox: 2 days using this changes), in my case without failures. But I'm afraid it may cause problems with other configurations (i.e: high vsync, vrr, multiple monitors) due to the lower resolution in the timestamp (from GLuint64 to GLuint) |
|
@yukiteruamano, the |
|
@absolutelynothelix same problem, compile but core dump in exec using my actual config picom-build.mp4 |
|
@yshui with Work, compile and go with my actual config. @absolutelynothelix, you're right with the "" missing in the code. EDIT: I have done more tests. I deleted the build folder, rebuilt the project with Meson (removing the libepoxy dependency) and compiled again. It does not work. This code only works if during configuration with Meson I put the libepoxy dependency, otherwise it doesn't compile. |
|
Upload video with the test. First of all, I use the same code The first build in the video is done by reconfiguring the project with Meson, so that it does not use the libepoxy dependency in OpenBSD. I have removed the libexpoy headers as well (in Second build, I reconfigure the project with Meson, but this time with the libepoxy dependency. I do not put the libexpoy headers, I leave them as is the original picom code, and I only add the code indicated above. The project compiles and runs correctly, an ldd shows that libepoxy is part of the libraries used by the newly built picom. It is clear that the version of Mesa in Xenocara depends on libepoxy to be able to access certain OpenGL functions, and `` is not the only one that is under the same issue. picom-build-test-v2.mp4 |
|
@yukiteruamano can you post just text logs of the error? should be easier for you, and for me. |
|
Finally, I managed to compile everything with the new code and get it to work. I needed to remove another modification I had made to the code that I had forgotten. Thanks for your help and feedback :) |
The following changes allow us to build v11, v11.1 and the next branch of picom within OpenBSD.
What happens is that in OpenBSD the glGetQueryObjectui64v symbol is not available in the version of Mesa that is integrated in Xenocara and because of this, the compilation of the new versions (v11 +) fails. For this reason, OpenBSD ports still maintain v10.2.
These changes in PR allow us to compile v11+ using libepoxy (new dependency for the OpenBSD build) so we can use glGetQueryObjectui64v (epoxy_glGetQueryObjectui64v) and compile picom with minimal changes (only preprocessors in the headers, the rest of the code remains the same) without affect the code and its functionality in the rest of the OS.
I also add a preprocessor to condition code for handling real-time scheduling, since in OpenBSD sched_getparam and sched_setparam fail, so we stick with the old code using pthread_getschedparam and pthread_setschedparam.
Changes tested and compiled in OpenBSD --current (picom is my daily default compositor).