~eliasnaur/gio-patches

This thread contains a patchset. You're looking at the original emails, but you may wish to use the patch review UI. Review patch
4 3

[PATCH gio] app: support changing Window options at runtime

~pierrec
Details
Message ID
<161773233210.25873.4896016494156407275-0@git.sr.ht>
DKIM signature
missing
Download raw message
Patch: +88 -4
From: pierre <pierre.curto@gmail.com>

A Window can now be requested to change its options after
it has been started via its Option method.

All options are supported on macOS, Windows and X11.
On Wayland, only the Size and Title options can be changed
at runtime.

Signed-off-by: pierre <pierre.curto@gmail.com>
---
-

 app/internal/wm/os_macos.go   |  5 ++--
 app/internal/wm/os_wayland.go | 15 +++++++++++-
 app/internal/wm/os_windows.go | 15 ++++++++++++
 app/internal/wm/os_x11.go     | 46 +++++++++++++++++++++++++++++++++++
 app/window.go                 | 11 +++++++++
 5 files changed, 88 insertions(+), 4 deletions(-)

diff --git a/app/internal/wm/os_macos.go b/app/internal/wm/os_macos.go
index d088a924..b9266d83 100644
--- a/app/internal/wm/os_macos.go
+++ b/app/internal/wm/os_macos.go
@@ -172,11 +172,10 @@ func (w *window) Option(opts *Options) {
func (w *window) SetWindowMode(mode WindowMode) {
	switch mode {
	case w.mode:
		return
	case Fullscreen:
	case Windowed, Fullscreen:
		C.gio_toggleFullScreen(w.window)
		w.mode = mode
	}
	w.mode = mode
}

func (w *window) SetCursor(name pointer.CursorName) {
diff --git a/app/internal/wm/os_wayland.go b/app/internal/wm/os_wayland.go
index 1e8c59d9..03790959 100644
--- a/app/internal/wm/os_wayland.go
+++ b/app/internal/wm/os_wayland.go
@@ -181,6 +181,7 @@ type window struct {

	mu        sync.Mutex
	animating bool
	opts      *Options
	needAck   bool
	// The most recent configure serial waiting to be ack'ed.
	serial   C.uint32_t
@@ -357,7 +358,7 @@ func (d *wlDisplay) createNativeWindow(opts *Options) (*window, error) {
	C.xdg_surface_add_listener(w.wmSurf, &C.gio_xdg_surface_listener, unsafe.Pointer(w.surf))
	C.xdg_toplevel_add_listener(w.topLvl, &C.gio_xdg_toplevel_listener, unsafe.Pointer(w.surf))

	w.Option(opts)
	w.setOptions(opts)

	if d.decor != nil {
		// Request server side decorations.
@@ -910,6 +911,13 @@ func (w *window) WriteClipboard(s string) {
}

func (w *window) Option(opts *Options) {
	w.mu.Lock()
	w.opts = opts
	w.mu.Unlock()
	w.disp.wakeup()
}

func (w *window) setOptions(opts *Options) {
	_, _, cfg := w.config()
	if o := opts.Size; o != nil {
		w.width = cfg.Px(o.Width)
@@ -1143,8 +1151,10 @@ func (w *window) process() {
	w.mu.Lock()
	readClipboard := w.readClipboard
	writeClipboard := w.writeClipboard
	opts := w.opts
	w.readClipboard = false
	w.writeClipboard = nil
	w.opts = nil
	w.mu.Unlock()
	if readClipboard {
		r, err := w.disp.readClipboard()
@@ -1163,6 +1173,9 @@ func (w *window) process() {
	if writeClipboard != nil {
		w.disp.writeClipboard([]byte(*writeClipboard))
	}
	if opts != nil {
		w.setOptions(opts)
	}
	// pass false to skip unnecessary drawing.
	w.draw(false)
}
diff --git a/app/internal/wm/os_windows.go b/app/internal/wm/os_windows.go
index 611c7cb0..05d440ea 100644
--- a/app/internal/wm/os_windows.go
+++ b/app/internal/wm/os_windows.go
@@ -66,6 +66,7 @@ type window struct {
const (
	_WM_REDRAW = windows.WM_USER + iota
	_WM_CURSOR
	_WM_OPTION
)

type gpuAPI struct {
@@ -317,6 +318,8 @@ func windowProc(hwnd syscall.Handle, msg uint32, wParam, lParam uintptr) uintptr
			windows.SetCursor(w.cursor)
			return windows.TRUE
		}
	case _WM_OPTION:
		w.setOptions()
	}

	return windows.DefWindowProc(hwnd, msg, wParam, lParam)
@@ -520,6 +523,18 @@ func (w *window) readClipboard() error {
}

func (w *window) Option(opts *Options) {
	w.mu.Lock()
	w.opts = opts
	w.mu.Unlock()
	if err := windows.PostMessage(w.hwnd, _WM_OPTION, 0, 0); err != nil {
		panic(err)
	}
}

func (w *window) setOptions() {
	w.mu.Lock()
	opts := w.opts
	w.mu.Unlock()
	if o := opts.Size; o != nil {
		dpi := windows.GetSystemDPI()
		cfg := configForDPI(dpi)
diff --git a/app/internal/wm/os_x11.go b/app/internal/wm/os_x11.go
index b921d468..3c18241a 100644
--- a/app/internal/wm/os_x11.go
+++ b/app/internal/wm/os_x11.go
@@ -89,6 +89,7 @@ type x11Window struct {

	mu        sync.Mutex
	animating bool
	opts      *Options

	pointerBtns pointer.Buttons

@@ -98,6 +99,7 @@ type x11Window struct {
		content []byte
	}
	cursor pointer.CursorName
	mode   WindowMode
}

func (w *x11Window) SetAnimating(anim bool) {
@@ -124,6 +126,20 @@ func (w *x11Window) WriteClipboard(s string) {
}

func (w *x11Window) Option(opts *Options) {
	w.mu.Lock()
	w.opts = opts
	w.mu.Unlock()
	w.wakeup()
}

func (w *x11Window) setOptions() {
	w.mu.Lock()
	defer w.mu.Unlock()
	opts := w.opts
	if opts == nil {
		return
	}
	w.opts = nil
	dpy := w.x
	win := w.xw
	cfg := w.cfg
@@ -190,6 +206,8 @@ func (w *x11Window) SetCursor(name pointer.CursorName) {

func (w *x11Window) SetWindowMode(mode WindowMode) {
	switch mode {
	case w.mode:
		return
	case Windowed:
		C.XDeleteProperty(w.x, w.xw, w.atoms.wmStateFullscreen)
	case Fullscreen:
@@ -197,7 +215,34 @@ func (w *x11Window) SetWindowMode(mode WindowMode) {
			32, C.PropModeReplace,
			(*C.uchar)(unsafe.Pointer(&w.atoms.wmStateFullscreen)), 1,
		)
	default:
		return
	}
	w.mode = mode
	// "A Client wishing to change the state of a window MUST send
	//  a _NET_WM_STATE client message to the root window (see below)."
	var xev C.XEvent
	ev := (*C.XClientMessageEvent)(unsafe.Pointer(&xev))
	*ev = C.XClientMessageEvent{
		_type:        C.ClientMessage,
		display:      w.x,
		window:       w.xw,
		message_type: w.atoms.wmState,
		format:       32,
	}
	arr := (*[5]C.long)(unsafe.Pointer(&ev.data))
	arr[0] = 2 // _NET_WM_STATE_TOGGLE
	arr[1] = C.long(w.atoms.wmStateFullscreen)
	arr[2] = 0
	arr[3] = 1 // application
	arr[4] = 0
	C.XSendEvent(
		w.x,
		C.XDefaultRootWindow(w.x), // MUST be the root window
		C.False,
		C.SubstructureNotifyMask|C.SubstructureRedirectMask,
		&xev,
	)
}

func (w *x11Window) ShowTextInput(show bool) {}
@@ -287,6 +332,7 @@ loop:
				}
			}
		}
		w.setOptions()
		// Clear notifications.
		for {
			_, err := syscall.Read(w.notify.read, buf)
diff --git a/app/window.go b/app/window.go
index e8f691a9..20ee8b07 100644
--- a/app/window.go
+++ b/app/window.go
@@ -211,6 +211,17 @@ func (w *Window) Invalidate() {
	}
}

// Option applies the options to the window.
func (w *Window) Option(opts ...Option) {
	go w.driverDo(func() {
		o := new(wm.Options)
		for _, opt := range opts {
			opt(o)
		}
		w.driver.Option(o)
	})
}

// ReadClipboard initiates a read of the clipboard in the form
// of a clipboard.Event. Multiple reads may be coalesced
// to a single event.
-- 
2.30.2

[gio/patches] build success

builds.sr.ht
Details
Message ID
<CAGUODLQZHW6.3SLGM82TLKJGO@cirno>
In-Reply-To
<161773233210.25873.4896016494156407275-0@git.sr.ht> (view parent)
DKIM signature
missing
Download raw message
gio/patches: SUCCESS in 19m59s

[app: support changing Window options at runtime][0] from [~pierrec][1]

[0]: https://lists.sr.ht/~eliasnaur/gio-patches/patches/21851
[1]: mailto:pierre.curto@gmail.com

✓ #479091 SUCCESS gio/patches/apple.yml   https://builds.sr.ht/~eliasnaur/job/479091
✓ #479094 SUCCESS gio/patches/openbsd.yml https://builds.sr.ht/~eliasnaur/job/479094
✓ #479093 SUCCESS gio/patches/linux.yml   https://builds.sr.ht/~eliasnaur/job/479093
✓ #479092 SUCCESS gio/patches/freebsd.yml https://builds.sr.ht/~eliasnaur/job/479092
Details
Message ID
<CAH9SKM07UIW.10L3Z9K8T6BN0@testmac>
In-Reply-To
<161773233210.25873.4896016494156407275-0@git.sr.ht> (view parent)
DKIM signature
fail
Download raw message
DKIM signature: fail
Almost there.

On Tue Apr 6, 2021 at 18:46 CEST, ~pierrec wrote:
> diff --git a/app/internal/wm/os_x11.go b/app/internal/wm/os_x11.go
> index b921d468..3c18241a 100644
> --- a/app/internal/wm/os_x11.go
> +++ b/app/internal/wm/os_x11.go
> @@ -124,6 +126,20 @@ func (w *x11Window) WriteClipboard(s string) {
>  }
>  
>  func (w *x11Window) Option(opts *Options) {
> +	w.mu.Lock()
> +	w.opts = opts
> +	w.mu.Unlock()
> +	w.wakeup()
> +}
> +
> +func (w *x11Window) setOptions() {
> +	w.mu.Lock()
> +	defer w.mu.Unlock()

Release the lock as soon as possible (after reading opts), to reduce the
window of opportunity for deadlock.

> +	opts := w.opts
> +	if opts == nil {
> +		return
> +	}
> +	w.opts = nil
>  	dpy := w.x
>  	win := w.xw
>  	cfg := w.cfg

These local variables are no longer needed now that setOptions runs on
the window goroutine, right?

> @@ -190,6 +206,8 @@ func (w *x11Window) SetCursor(name pointer.CursorName) {
>  
>  func (w *x11Window) SetWindowMode(mode WindowMode) {
>  	switch mode {
> +	case w.mode:
> +		return
>  	case Windowed:
>  		C.XDeleteProperty(w.x, w.xw, w.atoms.wmStateFullscreen)
>  	case Fullscreen:
Details
Message ID
<CAG3idSe7gHs5zKE3sq5pKuu8--vN6uto+vthS6aKxOb7o5UkCA@mail.gmail.com>
In-Reply-To
<CAH9SKM07UIW.10L3Z9K8T6BN0@testmac> (view parent)
DKIM signature
pass
Download raw message
Le mer. 7 avr. 2021 à 08:16, Elias Naur <mail@eliasnaur.com> a écrit :
>
> Almost there.
>
> On Tue Apr 6, 2021 at 18:46 CEST, ~pierrec wrote:
> > diff --git a/app/internal/wm/os_x11.go b/app/internal/wm/os_x11.go
> > index b921d468..3c18241a 100644
> > --- a/app/internal/wm/os_x11.go
> > +++ b/app/internal/wm/os_x11.go
> > @@ -124,6 +126,20 @@ func (w *x11Window) WriteClipboard(s string) {
> >  }
> >
> >  func (w *x11Window) Option(opts *Options) {
> > +     w.mu.Lock()
> > +     w.opts = opts
> > +     w.mu.Unlock()
> > +     w.wakeup()
> > +}
> > +
> > +func (w *x11Window) setOptions() {
> > +     w.mu.Lock()
> > +     defer w.mu.Unlock()
>
> Release the lock as soon as possible (after reading opts), to reduce the
> window of opportunity for deadlock.
>

That doesn't sound too reassuring :)

> > +     opts := w.opts
> > +     if opts == nil {
> > +             return
> > +     }
> > +     w.opts = nil
> >       dpy := w.x
> >       win := w.xw
> >       cfg := w.cfg
>
> These local variables are no longer needed now that setOptions runs on
> the window goroutine, right?
>
> > @@ -190,6 +206,8 @@ func (w *x11Window) SetCursor(name pointer.CursorName) {
> >
> >  func (w *x11Window) SetWindowMode(mode WindowMode) {
> >       switch mode {
> > +     case w.mode:
> > +             return
> >       case Windowed:
> >               C.XDeleteProperty(w.x, w.xw, w.atoms.wmStateFullscreen)
> >       case Fullscreen:
Details
Message ID
<CAHAW5J6URE8.CX4PYNVRB3OD@testmac>
In-Reply-To
<CAG3idSe7gHs5zKE3sq5pKuu8--vN6uto+vthS6aKxOb7o5UkCA@mail.gmail.com> (view parent)
DKIM signature
pass
Download raw message
On Wed Apr 7, 2021 at 09:02 CEST, Pierre Curto wrote:
> Le mer. 7 avr. 2021 à 08:16, Elias Naur <mail@eliasnaur.com> a écrit :
> >
> > Almost there.
> >
> > On Tue Apr 6, 2021 at 18:46 CEST, ~pierrec wrote:
> > > diff --git a/app/internal/wm/os_x11.go b/app/internal/wm/os_x11.go
> > > index b921d468..3c18241a 100644
> > > --- a/app/internal/wm/os_x11.go
> > > +++ b/app/internal/wm/os_x11.go
> > > @@ -124,6 +126,20 @@ func (w *x11Window) WriteClipboard(s string) {
> > >  }
> > >
> > >  func (w *x11Window) Option(opts *Options) {
> > > +     w.mu.Lock()
> > > +     w.opts = opts
> > > +     w.mu.Unlock()
> > > +     w.wakeup()
> > > +}
> > > +
> > > +func (w *x11Window) setOptions() {
> > > +     w.mu.Lock()
> > > +     defer w.mu.Unlock()
> >
> > Release the lock as soon as possible (after reading opts), to reduce the
> > window of opportunity for deadlock.
> >
>
> That doesn't sound too reassuring :)
>

I don't know of any actual risk of deadlock. I just don't like holding
locks across function calls, even though in this case the calls are
low-level X11 functions.

Elias
Reply to thread Export thread (mbox)