[Mesa-dev] [PATCH] meson: Add Haiku platform support v3

Fri Feb 16 22:35:31 UTC 2018

Quoting Emil Velikov (2018-02-16 14:07:26)
> On 16 February 2018 at 19:08, Alexander von Gluck IV
> <kallisti5 at unixzen.com> wrote:
> > February 16, 2018 11:49 AM, "Emil Velikov" <emil.l.velikov at gmail.com> wrote:
> >> Hi Alexander,
> >>
> >> Did you drop the ball on the autotools patches? I could re-spin them
> >> but have no way to test.
> >
> > I've been focused on meson since it's the future.
> > Feel free to re-spin them if you have the bandwidth.
> >
> >> There's a couple of comments inline, but nothing major.
> >>
> >> On 16 February 2018 at 00:32, Alexander von Gluck IV
> >> <kallisti5 at unixzen.com> wrote:
> >>
> >>> --- a/meson.build
> >>> +++ b/meson.build
> >>>
> >>> gl_priv_libs = []
> >>> -if dep_thread.found()
> >>> +if dep_thread.found() and host_machine.system() != 'haiku'
> >>> gl_priv_libs += ['-lpthread', '-pthread']
> >>> endif
> >>
> >> There's a bug report/fix in Meson for this one right?
> >> Please mention the ticket number of Meson version where the fix
> >> landed. Thus way one can drop this when we bump the Meson requirement.
> >
> > I did a bump of Meson on my Haiku build system and still saw the same
> > incorrect searching for -lpthread issues.  I think there are places where
> > -lpthread -pthread were specified without looking at dep_thread.found()
> > though which could be at play.  (see below)
> >
> Precisely those are existing bugs and should be separate, preparatory patch.
> 
> >>> if dep_m.found()
> >>> diff --git a/src/egl/meson.build b/src/egl/meson.build
> >>> index 6cd04567b0..09c28e9f83 100644
> >>> --- a/src/egl/meson.build
> >>> +++ b/src/egl/meson.build
> >>
> >> There are three unrelated things happening here:
> >>
> >>> @@ -21,9 +21,8 @@
> >>> c_args_for_egl = []
> >>> link_for_egl = []
> >>> deps_for_egl = []
> >>> -incs_for_egl = [
> >>> - inc_include, inc_src, inc_loader, inc_gbm, include_directories('main'),
> >>> -]
> >>> +incs_for_egl = [inc_include, inc_src, include_directories('main')]
> >>> +
> >>
> >> a) inc_gbm should be moved in the with_platform_drm hunk
> >
> > Can-do.  I wasn't 100% sure on when it was included so didn't want to disrupt
> > any non-Haiku build conditions.
> >
> It's an existing bug. Please flesh it out as separate commit.
> 
> >>> files_egl = files(
> >>> 'main/eglapi.c',
> >>> 'main/eglapi.h',
> >>> @@ -53,9 +52,6 @@ files_egl = files(
> >>> 'main/eglsync.h',
> >>> 'main/eglentrypoint.h',
> >>> 'main/egltypedefs.h',
> >>> - 'drivers/dri2/egl_dri2.c',
> >>> - 'drivers/dri2/egl_dri2.h',
> >>> - 'drivers/dri2/egl_dri2_fallbacks.h',
> >>> )
> >>>
> >>> linux_dmabuf_unstable_v1_protocol_c = custom_target(
> >>> @@ -100,6 +96,21 @@ g_egldispatchstubs_h = custom_target(
> >>> capture : true,
> >>> )
> >>>
> >>> +if with_dri2
> >>> + files_egl += files(
> >>> + 'drivers/dri2/egl_dri2.c',
> >>> + 'drivers/dri2/egl_dri2.h',
> >>> + 'drivers/dri2/egl_dri2_fallbacks.h',
> >>> + )
> >>> + incs_for_egl += [inc_loader, inc_gbm]
> >>> + c_args_for_egl += [
> >>> + '-DDEFAULT_DRIVER_DIR="@0@"'.format(dri_search_path),
> >>> + '-D_EGL_BUILT_IN_DRIVER_DRI2',
> >>> + ]
> >>> + link_for_egl += [libloader, libxmlconfig]
> >>> + deps_for_egl += dep_libdrm
> >>> +endif
> >>> +
> >>
> >> b) This should be within the with_platform_x11 hunk
> >
> > So with_platform_x11 always means with_dri2 regardless
> > of what with_dri2 is set to?
> >
> Errh, my bad. All of the whole
> with_platforms_{x11,wayland,...everything-but-haiku} should be within
> the with_dri2 block.
> 
> Alike the a) bit above, please keep that preparatory patch.
> 
> >>> if with_platform_x11
> >>> files_egl += files('drivers/dri2/platform_x11.c')
> >>> if with_dri3
> >>> @@ -133,6 +144,15 @@ if with_platform_android
> >>> deps_for_egl += dep_android
> >>> files_egl += files('drivers/dri2/platform_android.c')
> >>> endif
> >>> +if with_platform_haiku
> >>> + incs_for_egl += inc_haikugl
> >>> + c_args_for_egl += [
> >>> + '-D_EGL_BUILT_IN_DRIVER_HAIKU',
> >>> + ]
> >>> + files_egl += files('drivers/haiku/egl_haiku.cpp')
> >>> + link_for_egl += libgl
> >>> + deps_for_egl += cpp.find_library('be')
> >>> +endif
> >>
> >> c) this is the patch introducing haiku support
> >
> > These are all changes needed to properly build.
> >
> >>> --- /dev/null
> >>> +++ b/src/gallium/targets/haiku-softpipe/meson.build
> >>>
> >>> +libswpipe = shared_library(
> >>>
> >>> + dependencies : [
> >>> + driver_swrast, cpp.find_library('be'), cpp.find_library('translation'),
> >>> + cpp.find_library('network'), dep_unwind
> >>
> >> Some of these find_library calls are duplicated throughout. It would
> >> be more efficient and robust to have it done once.
> >> Say in the top level meson.build?
> >
> > That can be done.  Focused on functionality before optimizing in this first
> > commit.
> >
> 
> 
> >>> index 8aaf58a623..7a4bcd3329 100644
> >>> --- a/src/hgl/GLDispatcher.h
> >>> +++ b/src/hgl/GLDispatcher.h
> >>> @@ -15,7 +15,7 @@
> >>> #include <GL/gl.h>
> >>> #include <SupportDefs.h>
> >>>
> >>> -#include "glheader.h"
> >>> +#include "main/glheader.h"
> >>
> >> Please keep source changes separate from build system bits.
> >
> > Can-do. This was a requirement and matches the other dispatchers like glx.
> >
> >>> diff --git a/src/mapi/es1api/meson.build b/src/mapi/es1api/meson.build
> >>> index ea14654d2c..38a5747e9a 100644
> >>> --- a/src/mapi/es1api/meson.build
> >>> +++ b/src/mapi/es1api/meson.build
> >>>
> >>> --- a/src/mapi/es2api/meson.build
> >>> +++ b/src/mapi/es2api/meson.build
> >>> @@ -48,7 +48,7 @@ pkg.generate(
> >>> description : 'Mesa OpenGL ES 2.0 library',
> >>> version : meson.project_version(),
> >>> libraries : libgles2,
> >>> - libraries_private : '-lm -ldl -lpthread -pthread',
> >>> + libraries_private : gl_priv_libs,
> >>> )
> >>
> >> Unrelated fixes - make that a separate patch.
> >>
> >
> > This is actually needed to compile EGL under Haiku since
> > it is assuming -lpthread and -lm... so related.
> >
> These are preexisting bugs, I'm afraid. Please keep them a separate commit.
> 
> >>> diff --git a/src/meson.build b/src/meson.build
> >>> index 730b2ff6e4..4d5637f0aa 100644
> >>> --- a/src/meson.build
> >>> +++ b/src/meson.build
> >>>
> >>> +if with_glx != 'disabled'
> >>> + subdir('glx')
> >>> +endif
> >>
> >> Another unrelated fix?
> >
> > If you try and compile glx on Haiku you're gonna have a bad
> > day.  Setting glx to disabled should mean glx isn't built.
> >
> My meson isn't that bad - I can understand what the code does ;-)
> I'm saying that this is, yet another preexisting bug. And hence patch.
> 
> Off the top of my head, the above bugs don't existing in the autotools build.
> Which would explain why I suggested that one, as a stepping stone ;-)
> 
> >> Last but not least, please add a meson.build to the respective
> >> Makefile.ams' EXTRA_DIST.
> >
> > Can-do.  I wasn't making any changes to Makefile / Autotools
> > since the plan is to migrate away from it.
> >
> Pardon, I was too vague here. If the files are not in EXTRA_DIST
> they'll be missing from the tarball.
> 
> Thanks
> Emil

I have patches that address a few of these issues in my msvc branch, I'll dust
them off and send them to the list if you don't beat me to it.

Dylan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: signature
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180216/33574902/attachment.sig>