3d61f9d fix: resolve wrap-up review findings since v0.1.0 (#48) · neferwl

M internal/adapters/drm/overlay.go +3 -1


@@ -73,7 +73,9 @@ func overlayCandidate(s ports.Scene, surfaces map[ports.WindowID]ports.SurfaceCo
73	73		return ports.SceneWindow{}, ports.SurfaceContent{}, "window_above"
74	74		}
75	75		c := surfaces[w.ID]
76		-	if c.DMABuf != nil && !isYUVFormat(c.DMABuf.Format) && c.Opaque && len(c.Children) == 0 && c.Transform == 0 {
	76	+	// A viewport crop or PQ content is composed: the plane would show
	77	+	// the whole buffer, and raw PQ values on this SDR output.
	78	+	if c.DMABuf != nil && !isYUVFormat(c.DMABuf.Format) && c.Opaque && len(c.Children) == 0 && c.Transform == 0 && !cropped(c) && !c.Color.IsPQ2020() {
77	79		pick = w
78	80		continue
79	81		}

 			return ports.SceneWindow{}, ports.SurfaceContent{}, "window_above"
 		}
 		c := surfaces[w.ID]
-		if c.DMABuf != nil && !isYUVFormat(c.DMABuf.Format) && c.Opaque && len(c.Children) == 0 && c.Transform == 0 {
+		// A viewport crop or PQ content is composed: the plane would show
+		// the whole buffer, and raw PQ values on this SDR output.
+		if c.DMABuf != nil && !isYUVFormat(c.DMABuf.Format) && c.Opaque && len(c.Children) == 0 && c.Transform == 0 && !cropped(c) && !c.Color.IsPQ2020() {
 			pick = w
 			continue
 		}

M internal/adapters/drm/overlay_test.go +28 -0


@@ -78,6 +78,34 @@ func TestOverlayCandidate(t *testing.T) {
78	78		if _, _, reason := overlayCandidate(s, c); reason != "dim" {
79	79		t.Fatalf("dim reason %q", reason)
80	80		}
	81	+	// A viewport crop is composed: the plane would show the whole buffer.
	82	+	s, c = overlayScene()
	83	+	cropped := c[2]
	84	+	cropped.Source = [4]float32{10, 10, 50, 50}
	85	+	c[2] = cropped
	86	+	if _, _, reason := overlayCandidate(s, c); reason != "no_candidate" {
	87	+	t.Fatalf("cropped overlay reason %q", reason)
	88	+	}
	89	+	// A full-buffer source is no crop.
	90	+	cropped.Source = [4]float32{0, 0, 100, 100}
	91	+	c[2] = cropped
	92	+	if _, _, reason := overlayCandidate(s, c); reason != "" {
	93	+	t.Fatalf("full source reason %q", reason)
	94	+	}
	95	+	}
	96	+
	97	+	// The overlay is used only on SDR outputs: PQ content there needs the
	98	+	// renderer's tone mapping.
	99	+	func TestOverlaySkipsPQContent(t *testing.T) {
	100	+	o, _, _ := overlayOutput(t)
	101	+	o.overlay.formats = []ports.DMABufFormat{{Format: fourccXRGB}}
	102	+	s, c := overlayScene()
	103	+	pq := c[2]
	104	+	pq.Color = ports.SurfaceColor{TF: ports.ColorTFPQ, Primaries: ports.ColorPrimariesBT2020}
	105	+	c[2] = pq
	106	+	if ov, rest := o.overlayFrame(s, c); ov.fb != 0 \|\| len(rest.Windows) != 2 {
	107	+	t.Fatalf("PQ overlay %+v rest %+v", ov, rest.Windows)
	108	+	}
81	109		}
82	110
83	111		// The overlay is chosen and the composed scene leaves its window out;


@@ -78,6 +78,34 @@ func TestOverlayCandidate(t *testing.T) {
78	if _, _, reason := overlayCandidate(s, c); reason != "dim" {	78		if _, _, reason := overlayCandidate(s, c); reason != "dim" {
79	t.Fatalf("dim reason %q", reason)	79		t.Fatalf("dim reason %q", reason)
80	}	80		}
		81	+	// A viewport crop is composed: the plane would show the whole buffer.
		82	+	s, c = overlayScene()
		83	+	cropped := c[2]
		84	+	cropped.Source = [4]float32{10, 10, 50, 50}
		85	+	c[2] = cropped
		86	+	if _, _, reason := overlayCandidate(s, c); reason != "no_candidate" {
		87	+	t.Fatalf("cropped overlay reason %q", reason)
		88	+	}
		89	+	// A full-buffer source is no crop.
		90	+	cropped.Source = [4]float32{0, 0, 100, 100}
		91	+	c[2] = cropped
		92	+	if _, _, reason := overlayCandidate(s, c); reason != "" {
		93	+	t.Fatalf("full source reason %q", reason)
		94	+	}
		95	+	}
		96	+
		97	+	// The overlay is used only on SDR outputs: PQ content there needs the
		98	+	// renderer's tone mapping.
		99	+	func TestOverlaySkipsPQContent(t *testing.T) {
		100	+	o, _, _ := overlayOutput(t)
		101	+	o.overlay.formats = []ports.DMABufFormat{{Format: fourccXRGB}}
		102	+	s, c := overlayScene()
		103	+	pq := c[2]
		104	+	pq.Color = ports.SurfaceColor{TF: ports.ColorTFPQ, Primaries: ports.ColorPrimariesBT2020}
		105	+	c[2] = pq
		106	+	if ov, rest := o.overlayFrame(s, c); ov.fb != 0 \|\| len(rest.Windows) != 2 {
		107	+	t.Fatalf("PQ overlay %+v rest %+v", ov, rest.Windows)
		108	+	}
81	}	109		}
82		110
83	// The overlay is chosen and the composed scene leaves its window out;	111		// The overlay is chosen and the composed scene leaves its window out;

M internal/adapters/drm/scanout.go +6 -1


@@ -102,7 +102,7 @@ func scanoutCandidate(s ports.Scene, surfaces map[ports.WindowID]ports.SurfaceCo
102	102		return c, "buffer_transform"
103	103		case c.Width != w \|\| c.Height != h:
104	104		return c, "size_mismatch"
105		-	case c.Source[2] != 0 && c.Source != [4]float32{0, 0, float32(c.Width), float32(c.Height)}:
	105	+	case cropped(c):
106	106		// A viewport crop: the plane would show the whole buffer.
107	107		return c, "source_crop"
108	108		case c.LogicalW != full.Rect.W \|\| c.LogicalH != full.Rect.H:

@@ -113,6 +113,11 @@ func scanoutCandidate(s ports.Scene, surfaces map[ports.WindowID]ports.SurfaceCo
113	113		return c, ""
114	114		}
115	115
	116	+	// cropped reports whether a viewport source shows less than the buffer.
	117	+	func cropped(c ports.SurfaceContent) bool {
	118	+	return c.Source[2] != 0 && c.Source != [4]float32{0, 0, float32(c.Width), float32(c.Height)}
	119	+	}
	120	+
116	121		// clientSubsurface returns the content to scan out for a window whose
117	122		// root has no GPU buffer but one opaque subsurface over its geometry: Wine
118	123		// draws the frame in the root (wl_shm) and presents Vulkan in a subsurface

 		return c, "buffer_transform"
 	case c.Width != w || c.Height != h:
 		return c, "size_mismatch"
-	case c.Source[2] != 0 && c.Source != [4]float32{0, 0, float32(c.Width), float32(c.Height)}:
+	case cropped(c):
 		// A viewport crop: the plane would show the whole buffer.
 		return c, "source_crop"
 	case c.LogicalW != full.Rect.W || c.LogicalH != full.Rect.H:
 	return c, ""
 }
+// cropped reports whether a viewport source shows less than the buffer.
+func cropped(c ports.SurfaceContent) bool {
+	return c.Source[2] != 0 && c.Source != [4]float32{0, 0, float32(c.Width), float32(c.Height)}
+}
 // clientSubsurface returns the content to scan out for a window whose
 // root has no GPU buffer but one opaque subsurface over its geometry: Wine
 // draws the frame in the root (wl_shm) and presents Vulkan in a subsurface

M internal/adapters/wayland/colorrepresentation.go +10 -2


@@ -114,14 +114,22 @@ func (h representationSurface) SetChromaLocation(r colorrepresentation.WpColor
114	114		h.surf.pendingRepresentation.chroma = uint8(chroma)
115	115		}
116	116
117		-	// The metadata is checked against the committed buffer, including when a
118		-	// client changes representation without attaching a new buffer.
	117	+	// The metadata is checked against the buffer the commit will show,
	118	+	// including when a client changes representation without attaching a new
	119	+	// buffer: then the one of the last queued commit that attached, or the
	120	+	// current one.
119	121		func (s *surface) checkRepresentationCommit() bool {
120	122		rep := s.pendingRepresentation
121	123		if rep.coefficients == 0 && rep.chroma == 0 {
122	124		return true
123	125		}
124	126		buffer := s.current
	127	+	for i := len(s.queue) - 1; i >= 0; i-- {
	128	+	if s.queue[i].attached {
	129	+	buffer = s.queue[i].buffer
	130	+	break
	131	+	}
	132	+	}
125	133		if s.attached {
126	134		buffer = s.pending
127	135		}

 	h.surf.pendingRepresentation.chroma = uint8(chroma)
 }
-// The metadata is checked against the committed buffer, including when a
+// The metadata is checked against the buffer the commit will show,
-// client changes representation without attaching a new buffer.
+// including when a client changes representation without attaching a new
+// buffer: then the one of the last queued commit that attached, or the
+// current one.
 func (s *surface) checkRepresentationCommit() bool {
 	rep := s.pendingRepresentation
 	if rep.coefficients == 0 && rep.chroma == 0 {
 		return true
 	}
 	buffer := s.current
+	for i := len(s.queue) - 1; i >= 0; i-- {
+		if s.queue[i].attached {
+			buffer = s.queue[i].buffer
+			break
+		}
+	}
 	if s.attached {
 		buffer = s.pending
 	}

M internal/adapters/wayland/colorrepresentation_test.go +35 -0


@@ -149,6 +149,41 @@ func TestColorRepresentationQueuedCommit(t *testing.T) {
149	149		})
150	150		}
151	151
	152	+	// A representation-only commit is checked against the buffer of the last
	153	+	// queued commit, not the applied one it will not keep.
	154	+	func TestColorRepresentationChecksQueuedBuffer(t *testing.T) {
	155	+	s, c, _ := colorServer(t)
	156	+	manager := bindProtocol(t, c, "wp_color_representation_manager_v1")
	157	+	registerProtocol(t, c, manager)
	158	+	comp := bindProtocol(t, c, "wl_compositor")
	159	+	registerProtocol(t, c, comp)
	160	+	wl := c.AllocateID()
	161	+	requestProtocol(t, c, comp, wayland.CompositorRequestCreateSurface, wl)
	162	+	registerProtocol(t, c, wl)
	163	+	obj := c.AllocateID()
	164	+	requestProtocol(t, c, manager, cr.WpColorRepresentationManagerV1RequestGetSurface, obj, wl)
	165	+	registerProtocol(t, c, obj)
	166	+	buf := shmBuffer(t, c)
	167	+	requestProtocol(t, c, wl, wayland.SurfaceRequestAttach, buf, int32(0), int32(0))
	168	+	if err := c.Roundtrip(); err != nil {
	169	+	t.Fatal(err)
	170	+	}
	171	+	s.display.Do(func() {
	172	+	for res, surf := range s.surfaces {
	173	+	if res.ID() != wl {
	174	+	continue
	175	+	}
	176	+	// The RGB commit waits in the queue; the next one only sets
	177	+	// YUV coefficients.
	178	+	surf.queueUpdate()
	179	+	surf.pendingRepresentation.coefficients = coefficient709
	180	+	if surf.checkRepresentationCommit() {
	181	+	t.Error("YUV coefficients accepted for a queued RGB buffer")
	182	+	}
	183	+	}
	184	+	})
	185	+	}
	186	+
152	187		func TestColorRepresentationRejectsRGBWithYUVCoefficients(t *testing.T) {
153	188		_, c, _ := colorServer(t)
154	189		manager := bindProtocol(t, c, "wp_color_representation_manager_v1")


@@ -149,6 +149,41 @@ func TestColorRepresentationQueuedCommit(t *testing.T) {
149	})	149		})
150	}	150		}
151		151
		152	+	// A representation-only commit is checked against the buffer of the last
		153	+	// queued commit, not the applied one it will not keep.
		154	+	func TestColorRepresentationChecksQueuedBuffer(t *testing.T) {
		155	+	s, c, _ := colorServer(t)
		156	+	manager := bindProtocol(t, c, "wp_color_representation_manager_v1")
		157	+	registerProtocol(t, c, manager)
		158	+	comp := bindProtocol(t, c, "wl_compositor")
		159	+	registerProtocol(t, c, comp)
		160	+	wl := c.AllocateID()
		161	+	requestProtocol(t, c, comp, wayland.CompositorRequestCreateSurface, wl)
		162	+	registerProtocol(t, c, wl)
		163	+	obj := c.AllocateID()
		164	+	requestProtocol(t, c, manager, cr.WpColorRepresentationManagerV1RequestGetSurface, obj, wl)
		165	+	registerProtocol(t, c, obj)
		166	+	buf := shmBuffer(t, c)
		167	+	requestProtocol(t, c, wl, wayland.SurfaceRequestAttach, buf, int32(0), int32(0))
		168	+	if err := c.Roundtrip(); err != nil {
		169	+	t.Fatal(err)
		170	+	}
		171	+	s.display.Do(func() {
		172	+	for res, surf := range s.surfaces {
		173	+	if res.ID() != wl {
		174	+	continue
		175	+	}
		176	+	// The RGB commit waits in the queue; the next one only sets
		177	+	// YUV coefficients.
		178	+	surf.queueUpdate()
		179	+	surf.pendingRepresentation.coefficients = coefficient709
		180	+	if surf.checkRepresentationCommit() {
		181	+	t.Error("YUV coefficients accepted for a queued RGB buffer")
		182	+	}
		183	+	}
		184	+	})
		185	+	}
		186	+
152	func TestColorRepresentationRejectsRGBWithYUVCoefficients(t *testing.T) {	187		func TestColorRepresentationRejectsRGBWithYUVCoefficients(t *testing.T) {
153	_, c, _ := colorServer(t)	188		_, c, _ := colorServer(t)
154	manager := bindProtocol(t, c, "wp_color_representation_manager_v1")	189		manager := bindProtocol(t, c, "wp_color_representation_manager_v1")

M internal/core/action.go +3 -1


@@ -154,7 +154,9 @@ func (c *Core) applyAction(a Action) Effect {
154	154		} else {
155	155		dir = 1
156	156		}
157		-	edge := w.columnToward(dir) < 0
	157	+	// The first move from a float leaves it for the columns (FocusColumn),
	158	+	// unless a covering fullscreen window pins the focus.
	159	+	edge := w.columnToward(dir) < 0 && (!w.floatFocus \|\| w.pinned())
158	160		if i := c.neighbor(dir); edge && i >= 0 {
159	161		c.focusScreen = i
160	162		return Effect{}

 		} else {
 			dir = 1
 		}
-		edge := w.columnToward(dir) < 0
+		// The first move from a float leaves it for the columns (FocusColumn),
+		// unless a covering fullscreen window pins the focus.
+		edge := w.columnToward(dir) < 0 && (!w.floatFocus || w.pinned())
 		if i := c.neighbor(dir); edge && i >= 0 {
 			c.focusScreen = i
 			return Effect{}

M internal/core/core.go +1 -0


@@ -53,6 +53,7 @@ type Channels struct {
53	53		// fullscreen and hide the windows it opens next. The user can still
54	54		// fullscreen it (bind or taskbar), and an app can ask again later.
55	55		const fullscreenGrace = time.Second
	56	+
56	57		type binding struct {
57	58		mods ports.Mods
58	59		key string


@@ -53,6 +53,7 @@ type Channels struct {
53	// fullscreen and hide the windows it opens next. The user can still	53		// fullscreen and hide the windows it opens next. The user can still
54	// fullscreen it (bind or taskbar), and an app can ask again later.	54		// fullscreen it (bind or taskbar), and an app can ask again later.
55	const fullscreenGrace = time.Second	55		const fullscreenGrace = time.Second
		56	+
56	type binding struct {	57		type binding struct {
57	mods ports.Mods	58		mods ports.Mods
58	key string	59		key string

M internal/core/floating_test.go +24 -0


@@ -322,3 +322,27 @@ func TestFloatingSceneDimAndConfigure(t *testing.T) {
322	322		w.SetFullscreen(3, false)
323	323		check(0.3)
324	324		}
	325	+
	326	+	// Activating a hidden float shows the floats and focuses it; the
	327	+	// fullscreen window it hid behind leaves fullscreen.
	328	+	func TestActivateHiddenFloat(t *testing.T) {
	329	+	w := workspace()
	330	+	w.AddWindow(1)
	331	+	w.AddFloating(2, 20, 10)
	332	+	w.ToggleFloatingVisible()
	333	+	w.FocusID(1)
	334	+	w.SetFullscreen(1, true)
	335	+	w.Activate(2)
	336	+	if id, _ := w.Focused(); id != 2 \|\| w.floatsHidden \|\| w.fullscreen != 0 {
	337	+	t.Fatalf("focused %d hidden %v fullscreen %d", id, w.floatsHidden, w.fullscreen)
	338	+	}
	339	+	// The float's own fullscreen, asked while hidden, comes back with it.
	340	+	w.FocusID(2)
	341	+	w.ToggleFloatingVisible()
	342	+	w.SetFullscreen(2, true)
	343	+	w.SetFullscreen(1, true)
	344	+	w.Activate(2)
	345	+	if id, _ := w.Focused(); id != 2 \|\| w.floatsHidden \|\| w.fullscreen != 2 {
	346	+	t.Fatalf("own fullscreen: focused %d hidden %v fullscreen %d", id, w.floatsHidden, w.fullscreen)
	347	+	}
	348	+	}


@@ -322,3 +322,27 @@ func TestFloatingSceneDimAndConfigure(t *testing.T) {
322	w.SetFullscreen(3, false)	322		w.SetFullscreen(3, false)
323	check(0.3)	323		check(0.3)
324	}	324		}
		325	+
		326	+	// Activating a hidden float shows the floats and focuses it; the
		327	+	// fullscreen window it hid behind leaves fullscreen.
		328	+	func TestActivateHiddenFloat(t *testing.T) {
		329	+	w := workspace()
		330	+	w.AddWindow(1)
		331	+	w.AddFloating(2, 20, 10)
		332	+	w.ToggleFloatingVisible()
		333	+	w.FocusID(1)
		334	+	w.SetFullscreen(1, true)
		335	+	w.Activate(2)
		336	+	if id, _ := w.Focused(); id != 2 \|\| w.floatsHidden \|\| w.fullscreen != 0 {
		337	+	t.Fatalf("focused %d hidden %v fullscreen %d", id, w.floatsHidden, w.fullscreen)
		338	+	}
		339	+	// The float's own fullscreen, asked while hidden, comes back with it.
		340	+	w.FocusID(2)
		341	+	w.ToggleFloatingVisible()
		342	+	w.SetFullscreen(2, true)
		343	+	w.SetFullscreen(1, true)
		344	+	w.Activate(2)
		345	+	if id, _ := w.Focused(); id != 2 \|\| w.floatsHidden \|\| w.fullscreen != 2 {
		346	+	t.Fatalf("own fullscreen: focused %d hidden %v fullscreen %d", id, w.floatsHidden, w.fullscreen)
		347	+	}
		348	+	}

M internal/core/layout.go +7 -0


@@ -631,6 +631,13 @@ func (w *Workspace) ToggleFullscreen() {
631	631		// Activate focuses a window and makes it visible: it leaves another
632	632		// window's fullscreen, which hides everything else.
633	633		func (w *Workspace) Activate(id WindowID) {
	634	+	if w.floatsHidden && w.floatIndex(id) >= 0 {
	635	+	// A hidden float must be seen: show the floats with it. Another
	636	+	// window's fullscreen leaves first, so the float's own pending
	637	+	// fullscreen comes back.
	638	+	w.fullscreen = 0
	639	+	w.showFloats()
	640	+	}
634	641		if w.fullscreen != 0 && w.fullscreen != id {
635	642		w.fullscreen = 0
636	643		}


@@ -631,6 +631,13 @@ func (w *Workspace) ToggleFullscreen() {
631	// Activate focuses a window and makes it visible: it leaves another	631		// Activate focuses a window and makes it visible: it leaves another
632	// window's fullscreen, which hides everything else.	632		// window's fullscreen, which hides everything else.
633	func (w *Workspace) Activate(id WindowID) {	633		func (w *Workspace) Activate(id WindowID) {
		634	+	if w.floatsHidden && w.floatIndex(id) >= 0 {
		635	+	// A hidden float must be seen: show the floats with it. Another
		636	+	// window's fullscreen leaves first, so the float's own pending
		637	+	// fullscreen comes back.
		638	+	w.fullscreen = 0
		639	+	w.showFloats()
		640	+	}
634	if w.fullscreen != 0 && w.fullscreen != id {	641		if w.fullscreen != 0 && w.fullscreen != id {
635	w.fullscreen = 0	642		w.fullscreen = 0
636	}	643		}

M internal/core/screens_test.go +25 -0


@@ -854,3 +854,28 @@ func TestTerminalTokenClaimedOnce(t *testing.T) {
854	854		t.Fatalf("second claim: %v", got)
855	855		}
856	856		}
	857	+
	858	+	// From a focused float, the first column focus move leaves the float for
	859	+	// the columns, even when the column has no neighbor on that side: it does
	860	+	// not jump to the neighbor monitor.
	861	+	func TestFocusColumnFromFloatStaysOnMonitor(t *testing.T) {
	862	+	r := startMulti(t, nil, left, right)
	863	+	r.mapWindow(t, 1)
	864	+	r.client <- ports.WindowMapped{ID: 2, Floating: true, Width: 20, Height: 10}
	865	+	receive(t, r.scenes)
	866	+	set := r.key(t, "Right", ports.ModAlt)
	867	+	var focused []ports.WindowID
	868	+	for _, s := range set {
	869	+	for _, w := range s.Windows {
	870	+	if w.Focused {
	871	+	focused = append(focused, w.ID)
	872	+	if s.Output != "DP-1" {
	873	+	t.Fatalf("focused %d on %s", w.ID, s.Output)
	874	+	}
	875	+	}
	876	+	}
	877	+	}
	878	+	if !slices.Equal(focused, []ports.WindowID{1}) {
	879	+	t.Fatalf("focused %v", focused)
	880	+	}
	881	+	}


@@ -854,3 +854,28 @@ func TestTerminalTokenClaimedOnce(t *testing.T) {
854	t.Fatalf("second claim: %v", got)	854		t.Fatalf("second claim: %v", got)
855	}	855		}
856	}	856		}
		857	+
		858	+	// From a focused float, the first column focus move leaves the float for
		859	+	// the columns, even when the column has no neighbor on that side: it does
		860	+	// not jump to the neighbor monitor.
		861	+	func TestFocusColumnFromFloatStaysOnMonitor(t *testing.T) {
		862	+	r := startMulti(t, nil, left, right)
		863	+	r.mapWindow(t, 1)
		864	+	r.client <- ports.WindowMapped{ID: 2, Floating: true, Width: 20, Height: 10}
		865	+	receive(t, r.scenes)
		866	+	set := r.key(t, "Right", ports.ModAlt)
		867	+	var focused []ports.WindowID
		868	+	for _, s := range set {
		869	+	for _, w := range s.Windows {
		870	+	if w.Focused {
		871	+	focused = append(focused, w.ID)
		872	+	if s.Output != "DP-1" {
		873	+	t.Fatalf("focused %d on %s", w.ID, s.Output)
		874	+	}
		875	+	}
		876	+	}
		877	+	}
		878	+	if !slices.Equal(focused, []ports.WindowID{1}) {
		879	+	t.Fatalf("focused %v", focused)
		880	+	}
		881	+	}