Merge pull request #102 from fyne-io/feature/positioning2

Move window positioning logic out of X11

internal/ui/screens_embed.go

@@ -3,6 +3,7 @@ package ui
 import "fyne.io/fynedesk"
 
 type embeddedScreensProvider struct {
+	active  *fynedesk.Screen
 	screens []*fynedesk.Screen
 }
 
@@ -18,8 +19,12 @@ func (esp embeddedScreensProvider) Screens() []*fynedesk.Screen {
 	return esp.screens
 }
 
+func (esp embeddedScreensProvider) SetActive(s *fynedesk.Screen) {
+	esp.active = s
+}
+
 func (esp embeddedScreensProvider) Active() *fynedesk.Screen {
-	return esp.Screens()[0]
+	return esp.active
 }
 
 func (esp embeddedScreensProvider) Primary() *fynedesk.Screen {
@@ -36,6 +41,6 @@ func (esp embeddedScreensProvider) ScreenForGeometry(x int, y int, width int, he
 
 // NewEmbeddedScreensProvider returns a screen provider for use in embedded desktop mode
 func newEmbeddedScreensProvider() fynedesk.ScreenList {
-	return &embeddedScreensProvider{[]*fynedesk.Screen{{Name: "(Embedded)", X: 0, Y: 0,
-		Width: 1280, Height: 1024, Scale: 1.0}}}
+	screen := &fynedesk.Screen{Name: "(Embedded)", X: 0, Y: 0, Width: 1280, Height: 1024, Scale: 1.0}
+	return &embeddedScreensProvider{active: screen, screens: []*fynedesk.Screen{screen}}
 }

internal/x11/border.go

@@ -5,27 +5,23 @@ package x11
 import (
 	"fyne.io/fynedesk"
 	wmTheme "fyne.io/fynedesk/theme"
+	"fyne.io/fynedesk/wm"
 )
 
 // BorderWidth is the number of pixels required for a border
 func BorderWidth(win XWin) uint16 {
 	if !win.Properties().Decorated() {
 		return 0
 	}
-	return ScaleToPixels(wmTheme.BorderWidth, fynedesk.Instance().Screens().ScreenForWindow(win))
+	return uint16(wm.ScaleToPixels(wmTheme.BorderWidth, fynedesk.Instance().Screens().ScreenForWindow(win)))
 }
 
 // ButtonWidth is the number of pixels required for a border button
 func ButtonWidth(win XWin) uint16 {
-	return ScaleToPixels(wmTheme.ButtonWidth, fynedesk.Instance().Screens().ScreenForWindow(win))
+	return uint16(wm.ScaleToPixels(wmTheme.ButtonWidth, fynedesk.Instance().Screens().ScreenForWindow(win)))
 }
 
 // TitleHeight is the number of pixels required for a title bar
 func TitleHeight(win XWin) uint16 {
-	return ScaleToPixels(wmTheme.TitleHeight, fynedesk.Instance().Screens().ScreenForWindow(win))
-}
-
-// ScaleToPixels calculates the pixels required to show a specified Fyne dimension on the given screen
-func ScaleToPixels(i int, screen *fynedesk.Screen) uint16 {
-	return uint16(float32(i) * screen.CanvasScale())
+	return uint16(wm.ScaleToPixels(wmTheme.TitleHeight, fynedesk.Instance().Screens().ScreenForWindow(win)))
 }

internal/x11/property.go

@@ -43,6 +43,7 @@ var (
 		"_NET_FRAME_EXTENTS",
 		"_NET_MOVERESIZE_WINDOW",
 		"_NET_NUMBER_OF_DESKTOPS",
+		"_NET_WM_FULL_PLACEMENT",
 		"_NET_WM_FULLSCREEN_MONITORS",
 		"_NET_WM_MOVERESIZE",
 		"_NET_WM_NAME",

internal/x11/win/client.go

@@ -3,16 +3,17 @@
 package win
 
 import (
+	"fyne.io/fyne"
+
 	"github.com/BurntSushi/xgb/xproto"
 	"github.com/BurntSushi/xgbutil/ewmh"
 	"github.com/BurntSushi/xgbutil/icccm"
 	"github.com/BurntSushi/xgbutil/xevent"
 	"github.com/BurntSushi/xgbutil/xprop"
 
-	"fyne.io/fyne"
-
 	"fyne.io/fynedesk"
 	"fyne.io/fynedesk/internal/x11"
+	"fyne.io/fynedesk/wm"
 )
 
 type client struct {
@@ -55,6 +56,7 @@ func NewClient(win xproto.Window, wm x11.XWM) x11.XWin {
 		c.iconic = true
 		xproto.UnmapWindow(wm.Conn(), win)
 	} else {
+		c.positionNewWindow()
 		c.newFrame()
 	}
 
@@ -323,6 +325,44 @@ func (c *client) newFrame() {
 	c.frame = newFrame(c)
 }
 
+func (c *client) positionIsValid(x, y int) bool {
+	for _, screen := range fynedesk.Instance().Screens().Screens() {
+		if screen.X <= x && screen.X+screen.Width > x &&
+			screen.Y <= y && screen.Y+screen.Height > y {
+			return true
+		}
+	}
+
+	return false
+}
+
+func (c *client) positionNewWindow() {
+	attrs, err := xproto.GetGeometry(c.wm.Conn(), xproto.Drawable(c.win)).Reply()
+	if err != nil {
+		fyne.LogError("Get Geometry Error", err)
+		return
+	}
+
+	requestPosition := false
+	hints, err := icccm.WmNormalHintsGet(c.wm.X(), c.win)
+	if err == nil {
+		if hints.Flags&icccm.SizeHintPPosition != 0 || hints.Flags&icccm.SizeHintUSPosition != 0 {
+			requestPosition = true
+		}
+	}
+
+	x, y, w, h := int(attrs.X), int(attrs.Y), uint(attrs.Width), uint(attrs.Height)
+	if !requestPosition || !c.positionIsValid(x, y) {
+		decorated := !windowBorderless(c.wm.X(), c.win)
+		x, y, w, h = wm.PositionForNewWindow(int(attrs.X), int(attrs.Y), uint(attrs.Width), uint(attrs.Height),
+			decorated, fynedesk.Instance().Screens())
+	}
+
+	xproto.ConfigureWindowChecked(c.wm.Conn(), c.win, xproto.ConfigWindowX|xproto.ConfigWindowY|
+		xproto.ConfigWindowWidth|xproto.ConfigWindowHeight, []uint32{uint32(x), uint32(y),
+		uint32(w), uint32(h)}).Check()
+}
+
 func (c *client) stateMessage(state int) {
 	stateChangeAtom, err := xprop.Atm(c.wm.X(), "WM_CHANGE_STATE")
 	if err != nil {

internal/x11/win/frame.go

@@ -56,8 +56,8 @@ func newFrame(c *client) *frame {
 	decorated := c.Properties().Decorated()
 	maximized := c.Maximized()
 	screen := fynedesk.Instance().Screens().ScreenForGeometry(int(x), int(y), int(w), int(h))
-	borderWidth := x11.ScaleToPixels(wmTheme.BorderWidth, screen)
-	titleHeight := x11.ScaleToPixels(wmTheme.TitleHeight, screen)
+	borderWidth := uint16(wm.ScaleToPixels(wmTheme.BorderWidth, screen))
+	titleHeight := uint16(wm.ScaleToPixels(wmTheme.TitleHeight, screen))
 	if full || maximized {
 		activeHead := fynedesk.Instance().Screens().ScreenForGeometry(int(attrs.X), int(attrs.Y), int(attrs.Width), int(attrs.Height))
 		x = int16(activeHead.X)
@@ -668,6 +668,7 @@ func (f *frame) updateGeometry(x, y int16, w, h uint16, force bool) {
 	newScreen := fynedesk.Instance().Screens().ScreenForWindow(f.client)
 	if newScreen != currentScreen {
 		f.updateScale()
+		fynedesk.Instance().Screens().SetActive(newScreen)
 	}
 }
 

internal/x11/wm/desk.go

@@ -424,6 +424,17 @@ func screenNameFromRootTitle(title string) string {
 	return title[len(ui.RootWindowName):]
 }
 
+func (x *x11WM) setActiveScreenFromWindow(win fynedesk.Window) {
+	if win == nil || fynedesk.Instance() == nil {
+		return
+	}
+
+	windowScreen := fynedesk.Instance().Screens().ScreenForWindow(win)
+	if windowScreen != nil {
+		fynedesk.Instance().Screens().SetActive(windowScreen)
+	}
+}
+
 func (x *x11WM) setInitialWindowAttributes(win xproto.Window) {
 	err := xproto.ChangeWindowAttributesChecked(x.x.Conn(), win, xproto.CwCursor,
 		[]uint32{uint32(x11.DefaultCursor)}).Check()

internal/x11/wm/events.go

@@ -105,6 +105,7 @@ func (x *x11WM) handleClientMessage(ev xproto.ClientMessageEvent) {
 		}
 	case "_NET_ACTIVE_WINDOW":
 		x.handleActiveWin(ev)
+		x.setActiveScreenFromWindow(c)
 	case "_NET_WM_FULLSCREEN_MONITORS":
 		// TODO WHEN WE SUPPORT MULTI-MONITORS - THIS TELLS WHICH/HOW MANY MONITORS
 		// TO FULLSCREEN ACROSS

internal/x11/wm/screens.go

@@ -39,6 +39,10 @@ func NewX11ScreensProvider(mgr fynedesk.WindowManager) fynedesk.ScreenList {
 	return screensProvider
 }
 
+func (xsp *x11ScreensProvider) SetActive(s *fynedesk.Screen) {
+	xsp.active = s
+}
+
 func (xsp *x11ScreensProvider) Active() *fynedesk.Screen {
 	return xsp.active
 }

screen.go

@@ -13,6 +13,7 @@ type ScreenList interface {
 	RefreshScreens()                                   // RefreshScreens asks the ScreenList implementation to reload it's data
 	AddChangeListener(func())                          // Add a change listener to be notified if the screens change
 	Screens() []*Screen                                // Screens returns a Screen type slice of each available physical screen
+	SetActive(*Screen)                                 // Set the specified screen to be considered active
 	Active() *Screen                                   // Active returns the screen index of the currently active screen
 	Primary() *Screen                                  // Primary returns the screen index of the primary screen
 	ScreenForWindow(Window) *Screen                    // Return the screen that a window is located on

test/desktop.go

@@ -17,7 +17,8 @@ type Desktop struct {
 
 // NewDesktop returns a new in-memory desktop instance
 func NewDesktop() *Desktop {
-	screens := &testScreensProvider{screens: []*fynedesk.Screen{{Name: "Screen0", X: 0, Y: 0, Width: 2000, Height: 1000, Scale: 1.0}}}
+	screen := &fynedesk.Screen{Name: "Screen0", X: 0, Y: 0, Width: 2000, Height: 1000, Scale: 1.0}
+	screens := &testScreensProvider{screens: []*fynedesk.Screen{screen}, active: screen, primary: screen}
 	return &Desktop{settings: &Settings{}, icons: &testAppProvider{}, screens: screens}
 }
 

test/screens.go

@@ -10,37 +10,44 @@ type testScreensProvider struct {
 
 // NewScreensProvider returns a simple screen manager for the specified screens
 func NewScreensProvider(screens ...*fynedesk.Screen) fynedesk.ScreenList {
-	return &testScreensProvider{screens: screens}
+	if screens == nil {
+		screens = []*fynedesk.Screen{{Name: "Screen0", X: 0, Y: 0, Width: 2000, Height: 1000, Scale: 1.0}}
+	}
+	return &testScreensProvider{screens: screens, active: screens[0], primary: screens[0]}
 }
 
-func (tsp testScreensProvider) RefreshScreens() {
+func (tsp *testScreensProvider) RefreshScreens() {
 	return
 }
 
-func (tsp testScreensProvider) AddChangeListener(func()) {
+func (tsp *testScreensProvider) AddChangeListener(func()) {
 	// no-op
 }
 
-func (tsp testScreensProvider) Screens() []*fynedesk.Screen {
+func (tsp *testScreensProvider) Screens() []*fynedesk.Screen {
 	return tsp.screens
 }
 
-func (tsp testScreensProvider) Active() *fynedesk.Screen {
-	return tsp.screens[0]
+func (tsp *testScreensProvider) SetActive(s *fynedesk.Screen) {
+	tsp.active = s
 }
 
-func (tsp testScreensProvider) Primary() *fynedesk.Screen {
-	return tsp.screens[0]
+func (tsp *testScreensProvider) Active() *fynedesk.Screen {
+	return tsp.active
 }
 
-func (tsp testScreensProvider) Scale() float32 {
+func (tsp *testScreensProvider) Primary() *fynedesk.Screen {
+	return tsp.primary
+}
+
+func (tsp *testScreensProvider) Scale() float32 {
 	return 1.0
 }
 
-func (tsp testScreensProvider) ScreenForWindow(win fynedesk.Window) *fynedesk.Screen {
+func (tsp *testScreensProvider) ScreenForWindow(win fynedesk.Window) *fynedesk.Screen {
 	return tsp.Screens()[0]
 }
 
-func (tsp testScreensProvider) ScreenForGeometry(x int, y int, width int, height int) *fynedesk.Screen {
+func (tsp *testScreensProvider) ScreenForGeometry(x int, y int, width int, height int) *fynedesk.Screen {
 	return tsp.Screens()[0]
 }

wm/borderutil.go

@@ -0,0 +1,8 @@
+package wm
+
+import "fyne.io/fynedesk"
+
+// ScaleToPixels calculates the pixels required to show a specified Fyne dimension on the given screen
+func ScaleToPixels(i int, screen *fynedesk.Screen) int {
+	return int(float32(i) * screen.CanvasScale())
+}

wm/position.go

@@ -0,0 +1,21 @@
+package wm
+
+import (
+	"fyne.io/fynedesk"
+	"fyne.io/fynedesk/theme"
+)
+
+// PositionForNewWindow returns the suggested position for a new window of the given geometry.
+// The screen list hints at available space, but normally list.Active() is the best.
+func PositionForNewWindow(x, y int, w, h uint, decorated bool,
+	screens fynedesk.ScreenList) (int, int, uint, uint) {
+
+	target := screens.Active()
+	offX, offY := 0, 0
+	if decorated {
+		offX = ScaleToPixels(theme.BorderWidth, target)
+		offY = ScaleToPixels(theme.TitleHeight, target)
+	}
+
+	return target.X + offX, target.Y + offY, w, h
+}

wm/position_test.go

@@ -0,0 +1,27 @@
+package wm
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	"fyne.io/fynedesk"
+	"fyne.io/fynedesk/test"
+	"fyne.io/fynedesk/theme"
+)
+
+func TestPositionForNewWindow_TopLeft(t *testing.T) {
+	screen := &fynedesk.Screen{X: 50, Y: 0, Width: 500, Height: 500, Scale: 1}
+	x, y, _, _ := PositionForNewWindow(0, 0, 100, 100, true, test.NewScreensProvider(screen))
+
+	assert.Equal(t, 50+theme.BorderWidth, x)
+	assert.Equal(t, theme.TitleHeight, y)
+}
+
+func TestPositionForNewWindow_TopLeftBorderless(t *testing.T) {
+	screen := &fynedesk.Screen{X: 50, Y: 0, Width: 500, Height: 500, Scale: 1}
+	x, y, _, _ := PositionForNewWindow(0, 0, 100, 100, false, test.NewScreensProvider(screen))
+
+	assert.Equal(t, 50, x)
+	assert.Equal(t, 0, y)
+}