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

Use libepoxy #1189

Merged
merged 4 commits into from
Feb 11, 2024
Merged

Use libepoxy #1189

merged 4 commits into from
Feb 11, 2024

Conversation

yshui
Copy link
Owner

@yshui yshui commented Feb 10, 2024

No description provided.

Copy link

codecov bot commented Feb 10, 2024

Codecov Report

Attention: 12 lines in your changes are missing coverage. Please review.

Comparison is base (dff77aa) 36.60% compared to head (642a43a) 36.21%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             next    #1189      +/-   ##
==========================================
- Coverage   36.60%   36.21%   -0.39%     
==========================================
  Files          50       50              
  Lines       11558    11469      -89     
==========================================
- Hits         4231     4154      -77     
+ Misses       7327     7315      -12     
Files Coverage Δ
src/backend/gl/gl_common.c 26.23% <100.00%> (ø)
src/backend/gl/gl_common.h 19.44% <ø> (-10.79%) ⬇️
src/backend/gl/glx.c 32.01% <ø> (-6.50%) ⬇️
src/opengl.h 60.46% <ø> (ø)
src/vblank.c 0.00% <ø> (ø)
src/opengl.c 0.00% <0.00%> (ø)
src/log.c 67.06% <16.66%> (+1.94%) ⬆️
src/backend/gl/egl.c 17.24% <33.33%> (-7.12%) ⬇️

... and 2 files with indirect coverage changes

Copy link
Collaborator

@absolutelynothelix absolutelynothelix 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, builds and works for me. a few things may need to be addressed before merging.

CHANGELOG.md Outdated Show resolved Hide resolved
flake.nix Outdated Show resolved Hide resolved
#define check_ext(name) eglext.has_##name = egl_has_extension(dpy, #name)
#define check_ext(name) \
eglext.has_##name = epoxy_has_egl_extension(dpy, #name); \
log_info("Extension %s - %s", #name, eglext.has_##name ? "present" : "absent");
Copy link
Collaborator

Choose a reason for hiding this comment

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

honestly, i don't like the format of this message but i can't think of anything really better right now.

#define check_ext(name) glxext.has_##name = glx_has_extension(dpy, screen, #name)
#define check_ext(name) \
glxext.has_##name = epoxy_has_glx_extension(dpy, screen, #name); \
log_info("Extension " #name " - %s", glxext.has_##name ? "present" : "absent")
Copy link
Collaborator

Choose a reason for hiding this comment

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

same as above.

Copy link
Collaborator

Choose a reason for hiding this comment

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

also, this line is a bit different compared to the one above. let's make them the same at least (don't miss the semicolon at the end!).

Copy link
Owner Author

Choose a reason for hiding this comment

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

no i need to remove the semicolon on the other line.

src/meson.build Outdated Show resolved Hide resolved
src/meson.build Outdated Show resolved Hide resolved
src/meson.build Outdated Show resolved Hide resolved
@yshui yshui force-pushed the libepoxy branch 2 times, most recently from 9329112 to d8e3130 Compare February 10, 2024 17:06
@yukiteruamano
Copy link
Contributor

@yshui @absolutelynothelix I'm going to try the changes in OpenBSD, there shouldn't be any problems with using libepoxy, but it's better to clear up any doubts

Add libepoxy dependency to CI manifest and Nix.

For Nix, we need to set shellHook to workaround a NixOS limitation, see:

NixOS/nixpkgs#287763

Signed-off-by: Yuxuan Shui <yshuiv7@gmail.com>
There is actually no specification what symbols are exported from a
libGL implementation. The (extremely outdated) OpenGL ABI specification
says only GL 1.2 functions are guaranteed. Don't know how relevant that
is now, but different libGL implementations do export different set of
symbols. On Linux we are most likely to be linked with libglvnd, which
has everything we need. But on other platforms this is not necessarily
the case, for example on OpenBSD we are missing glGetQueryObjectui64v.

Use libepoxy so we can outsource this problem and never worry about it
ever again. Plus it also saves us from calling GetProcAddress ourselves.

Changes other than trivial build fixes I have to make:

1. Can't use eglCreatePlatformWindowSurface/eglGetPlatformDisplay.
   libepoxy checks for EGL 1.5 when resolving these functions. But
   without a current context, libepoxy assumes we only have EGL 1.4.
   This creates a chicken and egg problem - we need a display to call
   eglGetPlatformDisplay. We have to use the *EXT version instead.

Signed-off-by: Yuxuan Shui <yshuiv7@gmail.com>
So we don't need maintain our own version.

Signed-off-by: Yuxuan Shui <yshuiv7@gmail.com>
Signed-off-by: Yuxuan Shui <yshuiv7@gmail.com>
@yshui yshui merged commit 0e1628e into next Feb 11, 2024
18 checks passed
@yshui yshui deleted the libepoxy branch February 11, 2024 16:27
@absolutelynothelix
Copy link
Collaborator

@yukiteruamano, we now have an openbsd ci so picom should at least always build there. sorry for the inconvenience!

@yukiteruamano
Copy link
Contributor

Yeah, I see that and I test the last changes, compile without issues

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

3 participants