search

minetest / irrlicht

Minetest's fork of Irrlicht
Other
115 stars 87 forks source link

Undefine SDL2 DirectFB video driver #278

Closed okias closed 10 months ago

okias commented 10 months ago
  1. we don't need it
  2. it's dropped in SDL 3
  3. it breaks compilation on Alpine and postmarketOS with SDL2 enabled (and the bug never going to get fixed on SDL2 side)
sfan5 commented 10 months ago

This seems like a strange place to apply that fix.

As far as I understand SDL defines its features at compile time and saves them in SDL_config.h. This configuration is controlled by the SDL package maintainer (Alpine and/or postmarketOS in this case). So if it is known that DirectFB is broken why not disable it in that place?

okias commented 10 months ago

It's a bit weird place, thou:

I got inspired by the second link. This removed the support for the Irrlicht, while keeping software really using DirectFB in Alpine having SDL2 support available.

In general it could be patched in Alpine or DirectFB disabled in SDL2 https://gitlab.alpinelinux.org/alpine/aports/-/merge_requests/59109

sfan5 commented 10 months ago

Personally I would prefer if the the distribution would patch this when needed but also this single line change doesn't hurt and has no downsides. I'll ask someone else to decide.

SmallJoker commented 10 months ago

My two cents: this line is OK for me but there should be a comment explaining why this is needed and when it can be removed in the future (e.g. forcing SDL3 dependency).