Closed Bug 572295 Opened 11 years ago Closed 11 years ago

Add "X11Util" to be shared between X11 toolkits

Categories

(Core :: Graphics, enhancement)

All
Linux
enhancement
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: cjones, Assigned: cjones)

Details

Attachments

(3 files)

One persistent thorn in my side has been getting the default X Display.  I've seen code for this for gtk/qt copied around about 5 times.

I also ran into a case a few days ago when I wanted an "auto X pointer", on which XFree would be invoked when it went out of scope.  That's enough to merit "X11Util" to me!
Assignee: nobody → jones.chris.g
Attachment #451499 - Flags: review?(karlt)
Attachment #451499 - Flags: review?(joe)
Comment on attachment 451499 [details] [diff] [review]
Add X11Util.h for X11 toolkits

Looks sensible to me, thanks.

>+inline Display*
>+XDisplay()
>+{
>+  return DISPLAY();
>+}

When I added GetXDisplay(), I initially called it XDisplay() but roc pointed out that it looked like an Xlib function.
How about DefaultXDisplay()?

I would move the DISPLAY stuff to this function, to avoid polluting the preprocessor's namespace.

>+template<typename T>
>+struct NS_STACK_CLASS AutoXFree

NS_STACK_CLASS doesn't seem the right thing here.
I think we really want NS_IS_NOT_SAFE_TO_COPY, but I don't know whether we have that.
Attachment #451499 - Flags: review?(karlt) → review+
Comment on attachment 451499 [details] [diff] [review]
Add X11Util.h for X11 toolkits


>diff --git a/gfx/public/X11Util.h b/gfx/public/X11Util.h

>+#if defined(MOZ_WIDGET_GTK2)
>+#  include <gdk/gdkx.h>
>+#  define DISPLAY GDK_DISPLAY
>+#elif defined(MOZ_WIDGET_QT)
>+#  include <QX11Info>
>+#  define DISPLAY QX11Info::display
>+#else
>+#  error Unknown toolkit
>+#endif 

I'm not in love with this #define of DISPLAY. I'd much rather separate implementations of XDisplay(), tbh.
Attachment #451499 - Flags: review?(joe) → review+
Oright, meant to post the updated patch but got sidetracked.
http://hg.mozilla.org/mozilla-central/rev/28f111fe6ee9
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Qt build busted
http://tinderbox.mozilla.org/showlog.cgi?log=Mobile/1277169298.1277169408.27698.gz

I think there is some conflict with Qt defines... like CursorShape
Attachment #452939 - Flags: review?(jones.chris.g)
Comment on attachment 452939 [details] [diff] [review]
Build fix for Qt

If this problem pops up more, we should create a wrapper for QX11Info.h that fixes up this conflict automatically.  It's going to be ugly copying and pasting this macro around everywhere QX11Info.h might be included before X.h.
Attachment #452939 - Flags: review?(jones.chris.g) → review+
You need to log in before you can comment on or make changes to this bug.