VirtualBox

Ticket #16052 (closed defect: fixed)

Opened 4 years ago

Last modified 3 years ago

patches for Xorg-1.19 -> fixed in major releases greater than 5.1

Reported by: sergiomb Owned by:
Component: 3D support Version: VirtualBox 5.1.6
Keywords: Cc:
Guest type: X11 Host type: Linux

Description

Hello, Fedora 25 have RC1 for xorg 1.19 in updates-testing (don't ask me why) we have 2 problems when building in downstream way ,

[PATCH] dix: Remove redundant ChangeWindowProperty (  https://lists.x.org/archives/xorg-devel/2015-November/048030.html )

and also WakeupHandlerProcPtr was removed  https://lists.x.org/archives/xorg-devel/2015-November/047844.html

Attachments

for_uptream.patch Download (1.2 KB) - added by sergiomb 4 years ago.
VirtualBox-4.3.10-xserver_guest.patch Download (2.7 KB) - added by sergiomb 4 years ago.
VirtualBox-5.0.18-xserver_guest.patch Download (4.7 KB) - added by sergiomb 4 years ago.
new version

Change History

Changed 4 years ago by sergiomb

comment:1 Changed 4 years ago by michael

Thank you for the patch. I am curious though why you are still building the user-space X.Org driver rather than the kernel driver for X.Org Server 1.19.

Changed 4 years ago by sergiomb

comment:2 Changed 4 years ago by sergiomb

I (and all other downstreams btw) build Vbox with VBOX_USE_SYSTEM_XORG_HEADERS=1 , we patch also build Additions to use SYSTEM XORG HEADERS, I hope I am replying to the question. Now thinking on you are asking, I think we don't need vboxvideo_drv_system.so anymore , so maybe it it better make build skip this part, instead fixing the build ? isn't it ?

Thanks.

Last edited 4 years ago by sergiomb (previous) (diff)

comment:3 Changed 4 years ago by sergiomb

Michael,

Like you wrote if I don't build vboxvideo_drv_system.so, I don't need patch ./src/VBox/Additions/x11/vboxvideo

In my previous comment since we use SYSTEM XORG HEADERS, maybe this is a good opportunity to also add VBOX_USE_SYSTEM_XORG_HEADERS in ./src/VBox/Additions/common/crOpenGL/Makefile.kmk and ./src/VBox/Additions/common/VBoxGuestLib/Makefile.kmk ... I'll send to you patches that I'm thinking on, after sleep !

comment:4 Changed 4 years ago by michael

Presumably all that is needed is to conditionally not define VBOX_MESA_INCS in Config.kmk. Just tested and it seems to work fine. In fact we only need this ourselves to build on old systems which do not provide sufficiently up-to-date GL libraries. I presume that a Make variable to disable building the X.Org drivers altogether would make you life easier too.

comment:5 follow-up: ↓ 6 Changed 4 years ago by michael

No, wrong after all - there have been some subtle but relevant changes in GL function parameters and things.

comment:6 in reply to: ↑ 5 Changed 4 years ago by sergiomb

Replying to michael:

No, wrong after all - there have been some subtle but relevant changes in GL function parameters and things.

why you say that ? , I'm testing Fedora 24 in a vm-guest (Xorg 1.18) and works well , even Gnome in xorg-wayland ! Before build Vbox, we remove all bundle X11 sources with

rm -r src/VBox/Additions/x11/x11include/
rm -r src/VBox/Additions/x11/x11stubs/

Maybe you need also use Xorg system on ./src/VBox/Additions/common/crOpenGL/Makefile.kmk like VirtualBox-4.3.10-xserver_guest.patch in attach does ...

Thanks.

comment:7 Changed 4 years ago by michael

Why do you not remove the VBOX_MESA_INCS bit in the GL code includes? Not that it is likely to be very relevant if you remove the files anyway.

comment:8 Changed 4 years ago by sergiomb

yes, I also should remove VBOX_MESA_INCS from Config.kmk, like you said is not relevant, note taken :)

Changed 4 years ago by sergiomb

new version

comment:9 Changed 4 years ago by sergiomb

Hi Michael,

I follow your advise and in ./Config.kmk I do not define VBOX_MESA_INCS (using VBOX_USE_SYSTEM_XORG_HEADERS variable), so IMHO may be included in VBox official source code.

 Config.kmk                                          |    2 ++
 src/VBox/Additions/common/VBoxGuestLib/Makefile.kmk |    2 ++
 src/VBox/Additions/common/crOpenGL/Makefile.kmk     |   20 +++++++++++++++++++-
 src/VBox/Additions/x11/vboxvideo/Makefile.kmk       |    2 +-

src/VBox/Additions/x11/vboxvideo/Makefile.kmk maybe should not be included, is the way of fix vbox build against new Xorg 1.19 .

Analysing build.log, I just found one "-I" with "x11include", and it is x11include/mesa-11.0.7 (1), happens when is building obj/VBoxEGL/egl.o . ./src/VBox/Additions/common/crOpenGL/Makefile.kmk we still define VBoxEGL_INCS = $(VBOX_PATH_X11_ROOT)/mesa-11.0.7 ... , but for now we have a big improve.

Thank you.

(1)  https://copr.fedorainfracloud.org/coprs/sergiomb/vboxfor23/build/464163/

 https://copr-be.cloud.fedoraproject.org/results/sergiomb/vboxfor23/fedora-rawhide-x86_64/00464163-VirtualBox/

comment:10 follow-up: ↓ 12 Changed 4 years ago by michael

Just done these two commits. Will they help you?

Two notes: you don't need to disable building VBoxGuestR3LibXFree86 and VBoxGuestR3LibXOrg: make will recognise that these are dependencies of things you are not building (X server drivers) and skip them automatically. And in your patch you disabled VBOX_NO_NATIVEGL for system headers builds. Are you sure you checked that this is the right thing to do?

bugref:3810: X11 Guest Additions maintenance: Add two new defines - VBOX_USE_SYSTEM_GL_HEADERS and VBOX_NO_LEGACY_XORG_X11 - for the benefit of people doing distribution builds. They make us use system OpenGL headers for building our libGL and prevent us building X.Org drivers and stub libraries for linking on old distributions. These changes are to be tested by people who need them, not by upstream developers.

Index: Config.kmk
===================================================================
--- Config.kmk	(revision 111282)
+++ Config.kmk	(revision 111283)
@@ -2378,11 +2378,11 @@
  endif
 
  if1of ($(KBUILD_TARGET), freebsd linux solaris)
- # VBOX_PATH_MESA_SOURCE = $(PATH_ROOT)/src/libs/mesa-7.2
-  VBOX_PATH_MESA_SOURCE = $(VBOX_PATH_X11_ROOT)/mesa-7.2
-  VBOX_GL_INCS = \
+  ifndef VBOX_USE_SYSTEM_GL_HEADERS
+   VBOX_GL_INCS = \
         $(VBOX_PATH_X11_ROOT)/glproto-1.4.10 \
         $(VBOX_PATH_X11_ROOT)/mesa-7.2
+  endif
  endif
 
  VBOX_DARWIN_OPENGL_INST     = obj/VBoxOGL/GL/
Index: src/VBox/Additions/x11/Makefile.kmk
===================================================================
--- src/VBox/Additions/x11/Makefile.kmk	(revision 111282)
+++ src/VBox/Additions/x11/Makefile.kmk	(revision 111283)
@@ -20,13 +20,19 @@
 # Include sub-makefiles.
 if1of ($(KBUILD_TARGET), freebsd linux netbsd openbsd solaris)
  include $(PATH_SUB_CURRENT)/VBoxClient/Makefile.kmk
- include $(PATH_SUB_CURRENT)/vboxvideo/Makefile.kmk
- ifneq ($(KBUILD_TARGET), solaris)
-  include $(PATH_SUB_CURRENT)/vboxmouse/Makefile.kmk
+ ifndef VBOX_NO_LEGACY_XORG_X11
+  include $(PATH_SUB_CURRENT)/vboxvideo/Makefile.kmk
+  ifneq ($(KBUILD_TARGET), solaris)
+   include $(PATH_SUB_CURRENT)/vboxmouse/Makefile.kmk
+  endif
+  # This should logically only be controlled by VBOX_NO_LEGACY_XORG_X11,
+  # as it is not used for drivers at all, but rather to build X11 clients
+  # on systems missing needed libraries.
+  ## @todo fix at some later point when it will not break people's workflows.
+  ifndef VBOX_USE_SYSTEM_XORG_HEADERS
+   include $(PATH_SUB_CURRENT)/x11stubs/Makefile.kmk
+  endif
  endif
- ifndef VBOX_USE_SYSTEM_XORG_HEADERS
-  include $(PATH_SUB_CURRENT)/x11stubs/Makefile.kmk
- endif
 endif
 
 include $(FILE_KBUILD_SUB_FOOTER)

bugref:3810: X11 Guest Additions maintenance: check VBOX_USE_SYSTEM_GL_HEADERS when building the OpenGL libraries.

Index: src/VBox/Additions/common/crOpenGL/Makefile.kmk
===================================================================
--- src/VBox/Additions/common/crOpenGL/Makefile.kmk	(revision 111283)
+++ src/VBox/Additions/common/crOpenGL/Makefile.kmk	(revision 111284)
@@ -67,7 +67,10 @@
 VBoxOGL_TEMPLATE       = VBOXCROGLR3GUESTDLL
 VBoxOGL_INCS           = .
 if1of ($(KBUILD_TARGET), linux solaris freebsd)
- VBoxOGL_INCS     += \
+ ifdef VBOX_USE_SYSTEM_GL_HEADERS
+  VBoxOGL_INCS += x11
+ else
+  VBoxOGL_INCS     += \
 	$(VBOX_PATH_X11_ROOT)/libXdamage-1.1 \
 	$(VBOX_PATH_X11_ROOT)/libXcomposite-0.4.0 \
 	$(VBOX_PATH_X11_ROOT)/libXext-1.3.1 \
@@ -78,8 +81,8 @@
 	$(VBOX_PATH_X11_ROOT)/libx11-1.1.5-other \
 	$(VBOX_PATH_X11_ROOT)/xextproto-7.1.1 \
 	$(VBOX_PATH_X11_ROOT)/xproto-7.0.18 \
-	$(VBOX_GL_INCS) \
-	$(PATH_ROOT)/src/VBox/Additions/x11/x11include/libdrm-2.4.13
+	$(VBOX_GL_INCS)
+ endif
  VBoxOGL_DEFS     += VBOX_NO_NATIVEGL
 endif
 
@@ -213,11 +216,15 @@
 	$(PATH_STAGE_LIB)/additions/VBoxCrHgsmi$(VBOX_SUFF_LIB)
 
 if1of ($(KBUILD_TARGET), linux solaris freebsd)
- VBoxOGL_LIBS += \
+ ifdef VBOX_USE_SYSTEM_GL_HEADERS
+  VBoxOGL_LIBS += Xcomposite Xdamage Xfixes Xext
+ else
+  VBoxOGL_LIBS += \
  	$(PATH_STAGE_LIB)/libXcomposite.so \
  	$(PATH_STAGE_LIB)/libXdamage.so \
  	$(PATH_STAGE_LIB)/libXfixes.so \
  	$(PATH_STAGE_LIB)/libXext.so
+ endif
  ifdef VBoxOGL_FAKEDRI
   ifeq ($(KBUILD_TARGET), freebsd)
     VBoxOGL_LIBS += \

comment:11 follow-up: ↓ 13 Changed 4 years ago by michael

Actually a third question. Is the "+ VBoxOGL_INCS += x11" really necessary? Though I take it you did check that.

comment:12 in reply to: ↑ 10 Changed 4 years ago by sergiomb

Replying to michael:

Just done these two commits. Will they help you?

Two notes: you don't need to disable building VBoxGuestR3LibXFree86 and VBoxGuestR3LibXOrg: make will recognise that these are dependencies of things you are not building (X server drivers) and skip them automatically. And in your patch you disabled VBOX_NO_NATIVEGL for system headers builds. Are you sure you checked that this is the right thing to do?

bugref:3810: X11 Guest Additions maintenance: Add two new defines - VBOX_USE_SYSTEM_GL_HEADERS and VBOX_NO_LEGACY_XORG_X11 - for the benefit of people doing distribution builds. They make us use system OpenGL headers for building our libGL and prevent us building X.Org drivers and stub libraries for linking on old distributions. These changes are to be tested by people who need them, not by upstream developers.

IIUC , you are saying that I will be able to add VBOX_USE_SYSTEM_GL_HEADERS=1 and VBOX_NO_LEGACY_XORG_X11=1 in my localconfig.kmk ? should be a awesome.

Today I also added VBOX_USE_SYSTEM_XORG_HEADERS around VBoxEGL_INCS (the last one which use x11includes sources) .

 https://pkgs.rpmfusion.org/cgit/free/VirtualBox.git/tree/VirtualBox-5.0.18-xserver_guest.patch#n126

comment:13 in reply to: ↑ 11 Changed 4 years ago by sergiomb

Replying to michael:

Actually a third question. Is the "+ VBoxOGL_INCS += x11" really necessary? Though I take it you did check that.

I used :

+ VBoxOGL_INCS     += \
+	/usr/include/x11 \
+	/usr/include/xorg \
+	/usr/include/pixman-1 \
+ 	$(VBOX_MESA_INCS) \
+	/usr/include/drm \
+	/usr/include/libdrm

is just safer, but I can test not include it even x11

pkg-config pixman-1 --cflags
-I/usr/include/pixman-1

pkg-config libdrm --cflags
-I/usr/include/libdrm

pkg-config xorg-server --cflags
-fvisibility=hidden -I/usr/include/xorg -I/usr/include/pixman-1 -I/usr/include/libdrm -I/usr/include/X11/dri

/usr/include/X11/ (with capital X) belongs to xorg-x11-proto-devel

/usr/include/drm/ belongs to kernel-headers, this one maybe just need when we build kernel modules , but is in other build (kmod builds) .

comment:14 follow-up: ↓ 15 Changed 4 years ago by michael

Yes, I was surprised about the lower case x11 too, but since you are testing this not me I left it. If you can confirm that the last change works for you I will also make VBoxEGL_INCS dependent on VBOX_USE_SYSTEM_GL_HEADERS. This will be next major release only, I currently do not plan to back-port it to 5.1. I dare say you can carry patches that long...

comment:15 in reply to: ↑ 14 Changed 4 years ago by sergiomb

Replying to michael:

Yes, I was surprised about the lower case x11 too,

x11 with lower case is a mistake, probably made by me, should be X11 .

but since you are testing this not me I left it. If you can confirm that the last change works for you I will also make VBoxEGL_INCS dependent on VBOX_USE_SYSTEM_GL_HEADERS. This will be next major release only, I currently do not plan to back-port it to 5.1. I dare say you can carry patches that long...

I'd love have the patches to begin test it myself .

comment:16 Changed 4 years ago by sergiomb

I update my patch (1) , you are correct, I can, not set VBOX_MESA_INCS. I Found another x11 with lower case , maybe is what you questioned and can be removed (line 118) (I even tested and compiles vboxvideo_drv_system.so )

(1)  https://pkgs.rpmfusion.org/cgit/free/VirtualBox.git/tree/VirtualBox-5.0.18-xserver_guest.patch?id=92f4bb5773a971eaa37a9348eb630b31786a6d29

 https://pkgs.rpmfusion.org/cgit/free/VirtualBox.git/plain/VirtualBox-5.0.18-xserver_guest.patch?id=92f4bb5773a971eaa37a9348eb630b31786a6d29

Last edited 4 years ago by sergiomb (previous) (diff)

comment:17 Changed 4 years ago by michael

--- src/VBox/Additions/common/crOpenGL/Makefile.kmk	(revision 111284)
+++ src/VBox/Additions/common/crOpenGL/Makefile.kmk	(working copy)
@@ -67,9 +67,7 @@
 VBoxOGL_TEMPLATE       = VBOXCROGLR3GUESTDLL
 VBoxOGL_INCS           = .
 if1of ($(KBUILD_TARGET), linux solaris freebsd)
- ifdef VBOX_USE_SYSTEM_GL_HEADERS
-  VBoxOGL_INCS += x11
- else
+ ifndef VBOX_USE_SYSTEM_GL_HEADERS
   VBoxOGL_INCS     += \
 	$(VBOX_PATH_X11_ROOT)/libXdamage-1.1 \
 	$(VBOX_PATH_X11_ROOT)/libXcomposite-0.4.0 \
@@ -716,7 +714,9 @@
 
 VBoxEGL_TEMPLATE       = VBOXCROGLR3GUESTDLL
 VBoxEGL_SOURCES        = egl.c
-VBoxEGL_INCS           = $(VBOX_PATH_X11_ROOT)/mesa-11.0.7
+ifndef VBOX_USE_SYSTEM_GL_HEADERS
+ VBoxEGL_INCS           = $(VBOX_PATH_X11_ROOT)/mesa-11.0.7
+endif
 VBoxEGL_LIBS           = $(VBOX_LIB_OGL) $(VBOX_LIB_IPRT_GUEST_R3_SHARED)
 VBoxEGL_SONAME.linux   = libEGL.so.1

comment:18 Changed 4 years ago by michael

  • Summary changed from patches for Xorg-1.19 to patches for Xorg-1.19 -> fixed in major releases greater than 5.1

I assume I can close this.

comment:19 Changed 4 years ago by sergiomb

yes

comment:20 Changed 3 years ago by frank

  • Status changed from new to closed
  • Resolution set to fixed
Note: See TracTickets for help on using tickets.

www.oracle.com
ContactPrivacy policyTerms of Use