Rebuild Displays around the canvas, and let the transaction keep color
Claude-Session: https://claude.ai/code/session_01Ms2FbjQy31TVf3CEvQhGM8
This commit is contained in:
Gabriel Brown
@@ -28,10 +28,34 @@ rg -Fq 'position = "${record.x}x${record.y}"' "$service" \
|
||||
|| fail 'the compositor payload does not include explicit positions'
|
||||
rg -Fq 'generation === root.operationGeneration' "$service" \
|
||||
|| fail 'stale monitor readback can settle a newer transaction'
|
||||
|
||||
# The extended record. Every one of these is carried through currentLayout by
|
||||
# readback, which is the whole clobber fix: an apply that changes the scale
|
||||
# must not silently drop the bit depth monitors.lua asked for.
|
||||
for contract in \
|
||||
'function applyRecord(output:' \
|
||||
'vrrMode' \
|
||||
'colorProfile' \
|
||||
'bitdepth' \
|
||||
'sdrBrightness' \
|
||||
'sdrSaturation' \
|
||||
'mirrorOf'; do
|
||||
rg -Fq "$contract" "$service" \
|
||||
|| fail "the extended display record is missing: $contract"
|
||||
done
|
||||
rg -q 'vrrMode\s*(!==\s*-1|>\s*-1|>=\s*0)' "$service" \
|
||||
|| fail 'nothing in the service decides when the vrr key is omitted, so following the global policy would be written as an override'
|
||||
|
||||
rg -Fq 'function applyLayoutFixture(' "$harness" \
|
||||
|| fail 'the fixture cannot exercise complete layout transactions'
|
||||
rg -Fq 'function injectReadback(' "$harness" \
|
||||
|| fail 'the fixture cannot prove stale readback isolation'
|
||||
rg -Fq 'function applyRecordFixture(' "$harness" \
|
||||
|| fail 'the fixture cannot exercise a partial record change'
|
||||
rg -Fq 'function layoutMatch(' "$harness" \
|
||||
|| fail 'the fixture cannot probe the readback carve-outs'
|
||||
rg -Fq 'function persistedEntryValid(' "$harness" \
|
||||
|| fail 'the fixture cannot prove an old stored blob still validates'
|
||||
|
||||
fixture="$(mktemp -d /tmp/panama-display-transaction.XXXXXX)"
|
||||
state_home="$fixture/state-home"
|
||||
@@ -47,11 +71,15 @@ cat >"$monitor_state" <<'JSON'
|
||||
{
|
||||
"name":"DP-2","description":"Primary fixture","width":4500,"height":3000,
|
||||
"refreshRate":60,"scale":1.5,"transform":0,"x":0,"y":0,
|
||||
"currentFormat":"XRGB8888","colorManagementPreset":"auto",
|
||||
"sdrBrightness":1.0,"sdrSaturation":1.0,"mirrorOf":"none","vrr":false,
|
||||
"availableModes":["[email protected]"]
|
||||
},
|
||||
{
|
||||
"name":"HDMI-A-1","description":"Second fixture","width":2560,"height":1440,
|
||||
"refreshRate":60,"scale":1,"transform":0,"x":3000,"y":0,
|
||||
"currentFormat":"XRGB8888","colorManagementPreset":"auto",
|
||||
"sdrBrightness":1.0,"sdrSaturation":1.0,"mirrorOf":"none","vrr":false,
|
||||
"availableModes":["[email protected]"]
|
||||
}
|
||||
]
|
||||
@@ -78,7 +106,7 @@ if [[ -f "$fixture/fail-once" ]]; then
|
||||
fi
|
||||
[[ -f "$fixture/no-apply" ]] && exit 0
|
||||
|
||||
python3 - "$fixture/monitors.json" "$payload" "$fixture/wrong-y" <<'PY'
|
||||
python3 - "$fixture/monitors.json" "$payload" "$fixture/wrong-y" "$fixture/odd-format" <<'PY'
|
||||
import json
|
||||
import pathlib
|
||||
import re
|
||||
@@ -87,6 +115,7 @@ import sys
|
||||
state_path = pathlib.Path(sys.argv[1])
|
||||
payload = sys.argv[2]
|
||||
wrong_y = pathlib.Path(sys.argv[3]).exists()
|
||||
odd_format = pathlib.Path(sys.argv[4]).exists()
|
||||
monitors = json.loads(state_path.read_text(encoding="utf-8"))
|
||||
by_name = {monitor["name"]: monitor for monitor in monitors}
|
||||
|
||||
@@ -97,10 +126,17 @@ for block in re.findall(r"hl\.monitor\(\{([^}]*)\}\)", payload):
|
||||
raise SystemExit(f"missing field {pattern}: {block}")
|
||||
return match.group(1)
|
||||
|
||||
def optional(pattern: str):
|
||||
match = re.search(pattern, block)
|
||||
return match.group(1) if match else None
|
||||
|
||||
name = field(r'output\s*=\s*"([A-Za-z0-9_.-]+)"')
|
||||
mode = field(r'mode\s*=\s*"(\d+x\d+@\d+(?:\.\d+)?)"')
|
||||
position = re.search(r'position\s*=\s*"(-?\d+)x(-?\d+)"', block)
|
||||
if not position or name not in by_name:
|
||||
# A mirrored rule asks for "auto" instead of coordinates, because the
|
||||
# compositor is the one that decides where a mirror lands.
|
||||
placement = field(r'position\s*=\s*"([^"]+)"')
|
||||
position = re.match(r"^(-?\d+)x(-?\d+)$", placement)
|
||||
if name not in by_name or (position is None and placement != "auto"):
|
||||
raise SystemExit(f"bad output or position: {block}")
|
||||
width, height, refresh = re.match(r"(\d+)x(\d+)@(\d+(?:\.\d+)?)", mode).groups()
|
||||
monitor = by_name[name]
|
||||
@@ -110,9 +146,44 @@ for block in re.findall(r"hl\.monitor\(\{([^}]*)\}\)", payload):
|
||||
"refreshRate": float(refresh),
|
||||
"scale": float(field(r"scale\s*=\s*([0-9.]+)")),
|
||||
"transform": int(field(r"transform\s*=\s*(\d+)")),
|
||||
"x": int(position.group(1)),
|
||||
"y": int(position.group(2)),
|
||||
})
|
||||
if position is not None:
|
||||
monitor["x"] = int(position.group(1))
|
||||
monitor["y"] = int(position.group(2))
|
||||
|
||||
# The compositor answers about colour with the names Hyprland uses in
|
||||
# `hyprctl -j monitors`, not with the keys the rule was written in.
|
||||
preset = optional(r'cm\s*=\s*"([a-z]+)"')
|
||||
if preset is not None:
|
||||
monitor["colorManagementPreset"] = preset
|
||||
depth = optional(r"bitdepth\s*=\s*(\d+)")
|
||||
if depth is not None:
|
||||
monitor["currentFormat"] = "XRGB2101010" if depth == "10" else "XRGB8888"
|
||||
if odd_format:
|
||||
# A format Panama has no mapping for. The request is not wrong; the
|
||||
# readback is simply not evidence either way.
|
||||
monitor["currentFormat"] = "XBGR16161616F"
|
||||
sdr_brightness = optional(r"sdrbrightness\s*=\s*([0-9.]+)")
|
||||
if sdr_brightness is not None:
|
||||
monitor["sdrBrightness"] = float(sdr_brightness)
|
||||
sdr_saturation = optional(r"sdrsaturation\s*=\s*([0-9.]+)")
|
||||
if sdr_saturation is not None:
|
||||
monitor["sdrSaturation"] = float(sdr_saturation)
|
||||
|
||||
# Mirroring is where the compositor stops taking instructions: a mirrored
|
||||
# output lands on top of its target whatever position the rule carried.
|
||||
# Panama must not hold the requested x/y against it.
|
||||
# Hyprland says "none" rather than an empty string, and Panama has to know
|
||||
# that is not the name of a monitor.
|
||||
mirror = optional(r'mirror\s*=\s*"([A-Za-z0-9_.-]*)"')
|
||||
monitor["mirrorOf"] = mirror or "none"
|
||||
if mirror and mirror in by_name:
|
||||
monitor["x"] = by_name[mirror]["x"]
|
||||
monitor["y"] = by_name[mirror]["y"]
|
||||
|
||||
# vrr is deliberately not reflected here: the readback field is live
|
||||
# adaptive-sync state, and nothing in the service may verify against it.
|
||||
|
||||
if wrong_y and name == "HDMI-A-1":
|
||||
monitor["y"] += 1
|
||||
|
||||
@@ -251,6 +322,133 @@ mv "$fixture/reconnected.json" "$monitor_state"
|
||||
run ipc --pid "$harness_pid" call displays-test refresh >/dev/null
|
||||
wait_for '.layout | length == 2' >/dev/null
|
||||
|
||||
# ── The extended record ──────────────────────────────────────────────────────
|
||||
#
|
||||
# Colour, bit depth, the SDR trim and mirroring ride the same keep-or-revert
|
||||
# transaction as geometry. vrr cannot: `hyprctl -j monitors` reports whether
|
||||
# adaptive sync is live at this instant, not what the config asked for, so it
|
||||
# is applied and never verified -- and a display that follows the global gaming
|
||||
# policy must not emit the key at all, because emitting it IS the override.
|
||||
|
||||
b64() { printf '%s' "$1" | base64 -w0; }
|
||||
apply_record() {
|
||||
run ipc --pid "$harness_pid" call displays-test applyRecordFixture "$1" "$(b64 "$2")"
|
||||
}
|
||||
probe() {
|
||||
run ipc --pid "$harness_pid" call displays-test "$1" "$(b64 "$2")" "$(b64 "$3")"
|
||||
}
|
||||
settle() { wait_for '.busy == false and .awaiting == false and .reverting == null' >/dev/null; }
|
||||
|
||||
[[ "$(apply_record HDMI-A-1 '{"colorProfile":"srgb","bitdepth":10,"sdrBrightness":1.4,"sdrSaturation":1.0,"vrrMode":-1}')" == "true" ]] \
|
||||
|| fail 'an extended display record was refused'
|
||||
wait_for '.canConfirm == true' >/dev/null
|
||||
color_payload="$(tail -1 "$eval_log")"
|
||||
if rg -q 'vrr\s*=' <<<"$color_payload"; then
|
||||
fail "a display following the global VRR policy emitted a vrr override anyway: $color_payload"
|
||||
fi
|
||||
# Neutral saturation is the absence of a request. A rule that names 1.0 pins
|
||||
# the display to it, which is not the same thing as leaving it alone.
|
||||
if rg -q 'sdrsaturation\s*=' <<<"$color_payload"; then
|
||||
fail "neutral SDR saturation was written as a rule: $color_payload"
|
||||
fi
|
||||
rg -Fq 'cm = "srgb"' <<<"$color_payload" \
|
||||
&& rg -q 'bitdepth\s*=\s*10' <<<"$color_payload" \
|
||||
&& rg -q 'sdrbrightness\s*=\s*1\.4\b' <<<"$color_payload" \
|
||||
|| fail "the colour fields never reached the compositor payload: $color_payload"
|
||||
# The clobber this exists for: an apply that changes one field must carry the
|
||||
# rest of the record, or monitors.lua's 10-bit request dies on the next apply.
|
||||
rg -Fq 'output = "DP-2"' <<<"$color_payload" && rg -q 'bitdepth\s*=' <<<"$color_payload" \
|
||||
|| fail "the untouched display lost its colour fields on somebody else's apply: $color_payload"
|
||||
|
||||
[[ "$(run ipc --pid "$harness_pid" call displays-test confirmChange)" == "true" ]] \
|
||||
|| fail 'a verified colour change could not be kept'
|
||||
jq -e '.displays["HDMI-A-1"] | .colorProfile == "srgb" and .bitdepth == 10
|
||||
and .vrrMode == -1 and .mirrorOf == "" and (.sdrBrightness - 1.4 | fabs) < 0.001' \
|
||||
"$store" >/dev/null \
|
||||
|| fail 'confirmation did not persist the extended record'
|
||||
|
||||
# vrr applied, never verified: Keep must still become available.
|
||||
[[ "$(apply_record HDMI-A-1 '{"vrrMode":2}')" == "true" ]] \
|
||||
|| fail 'a VRR override was refused'
|
||||
wait_for '.canConfirm == true' >/dev/null
|
||||
rg -q 'vrr\s*=\s*2' <<<"$(tail -1 "$eval_log")" \
|
||||
|| fail "an explicit VRR override did not emit the vrr key: $(tail -1 "$eval_log")"
|
||||
run ipc --pid "$harness_pid" call displays-test revertChange >/dev/null
|
||||
settle
|
||||
|
||||
# A framebuffer format with no 8/10 mapping is not evidence against the
|
||||
# request. Asserting it anyway would leave Keep permanently unavailable on
|
||||
# hardware that reports a format Panama has never heard of.
|
||||
touch "$fixture/odd-format"
|
||||
[[ "$(apply_record HDMI-A-1 '{"bitdepth":8}')" == "true" ]] \
|
||||
|| fail 'a bit depth change was refused'
|
||||
wait_for '.canConfirm == true' >/dev/null
|
||||
rm -f "$fixture/odd-format"
|
||||
run ipc --pid "$harness_pid" call displays-test revertChange >/dev/null
|
||||
settle
|
||||
|
||||
# Mirroring: the compositor puts a mirrored output on top of its target and
|
||||
# ignores the position the rule carried, so x/y are the two assertions that
|
||||
# must be skipped for that record -- and only for that record.
|
||||
[[ "$(apply_record HDMI-A-1 '{"mirrorOf":"DP-2"}')" == "true" ]] \
|
||||
|| fail 'a valid mirror request was refused'
|
||||
wait_for '.canConfirm == true' >/dev/null
|
||||
mirror_payload="$(tail -1 "$eval_log")"
|
||||
rg -Fq 'mirror = "DP-2"' <<<"$mirror_payload" \
|
||||
|| fail "the mirror key never reached the compositor: $mirror_payload"
|
||||
# The mirrored rule asks for "auto", not for the coordinates on file: the
|
||||
# position it was stored with is not one the compositor will honour.
|
||||
rg -q 'output = "HDMI-A-1", mode = "[^"]+", position = "auto"' <<<"$mirror_payload" \
|
||||
|| fail "a mirrored display still asked for a position of its own: $mirror_payload"
|
||||
jq -e '.[1].mirrorOf == "DP-2" and .[1].x == .[0].x and .[1].y == .[0].y' \
|
||||
"$monitor_state" >/dev/null \
|
||||
|| fail 'the fixture compositor did not stack the mirrored output on its target'
|
||||
run ipc --pid "$harness_pid" call displays-test revertChange >/dev/null
|
||||
settle
|
||||
|
||||
# The carve-out is exactly one record wide. Same readback, same layout, with
|
||||
# the mirror flag removed: now the position disagreement is a real one.
|
||||
#
|
||||
# Both fixtures are in the shape the service holds after parsing a readback,
|
||||
# not in hyprctl's own: this probes the comparison, not the parser.
|
||||
mirrored_readback='[{"name":"DP-2","width":4500,"height":3000,"refreshRate":60,"scale":1.5,"transform":0,"x":0,"y":0,"primary":true,"colorPreset":"auto","bitdepth":8,"sdrBrightness":1.0,"sdrSaturation":1.0,"mirrorOf":""},{"name":"HDMI-A-1","width":2560,"height":1440,"refreshRate":60,"scale":1,"transform":0,"x":0,"y":0,"primary":false,"colorPreset":"auto","bitdepth":8,"sdrBrightness":1.0,"sdrSaturation":1.0,"mirrorOf":"DP-2"}]'
|
||||
mirrored_request='[{"name":"DP-2","mode":"[email protected]","scale":1.5,"transform":0,"x":0,"y":0,"primary":true,"colorProfile":"auto","bitdepth":8,"sdrBrightness":1.0,"sdrSaturation":1.0,"mirrorOf":"","vrrMode":-1},{"name":"HDMI-A-1","mode":"[email protected]","scale":1,"transform":0,"x":3000,"y":100,"primary":false,"colorProfile":"auto","bitdepth":8,"sdrBrightness":1.0,"sdrSaturation":1.0,"mirrorOf":"DP-2","vrrMode":-1}]'
|
||||
unmirror='(.[] | select(.name == "HDMI-A-1") | .mirrorOf) = ""'
|
||||
[[ "$(probe layoutMatch "$mirrored_readback" "$mirrored_request")" == "true" ]] \
|
||||
|| fail 'a mirrored output was held to the position the compositor overrode'
|
||||
[[ "$(probe layoutMatch \
|
||||
"$(jq -c "$unmirror" <<<"$mirrored_readback")" \
|
||||
"$(jq -c "$unmirror" <<<"$mirrored_request")")" == "false" ]] \
|
||||
|| fail 'the position carve-out leaked to a record that is not mirrored'
|
||||
|
||||
# Anything the table does not allow is refused before the desktop moves.
|
||||
while IFS='|' read -r output patch reason; do
|
||||
[[ "$(apply_record "$output" "$patch")" == "false" ]] || fail "$reason"
|
||||
[[ "$(transaction_status | jq -r .awaiting)" == "false" ]] \
|
||||
|| fail "$reason (and it left a change pending)"
|
||||
done <<'CASES'
|
||||
HDMI-A-1|{"mirrorOf":"HDMI-A-1"}|a display was allowed to mirror itself
|
||||
DP-2|{"mirrorOf":"HDMI-A-1"}|the primary display was allowed to mirror another
|
||||
HDMI-A-1|{"mirrorOf":"NOPE-1"}|a display was allowed to mirror an output that is not connected
|
||||
HDMI-A-1|{"vrrMode":7}|an out-of-range VRR mode was accepted
|
||||
HDMI-A-1|{"colorProfile":"neon"}|an unknown colour profile was accepted
|
||||
HDMI-A-1|{"bitdepth":12}|an unsupported bit depth was accepted
|
||||
HDMI-A-1|{"sdrBrightness":4}|an out-of-range SDR brightness was accepted
|
||||
CASES
|
||||
|
||||
# Every machine that has this installed already has a settings.json with none
|
||||
# of the new fields in it. Those entries must keep validating, or a docking
|
||||
# station restores nothing on the next start.
|
||||
old_shape='{"mode":"[email protected]","scale":1,"transform":0,"x":3000,"y":100,"primary":false}'
|
||||
new_shape='{"mode":"[email protected]","scale":1,"transform":0,"x":3000,"y":100,"primary":false,"vrrMode":-1,"colorProfile":"srgb","bitdepth":10,"sdrBrightness":1.0,"sdrSaturation":1.0,"mirrorOf":""}'
|
||||
bad_shape='{"mode":"[email protected]","scale":1,"transform":0,"x":3000,"y":100,"primary":false,"vrrMode":7}'
|
||||
[[ "$(run ipc --pid "$harness_pid" call displays-test persistedEntryValid "$(b64 "$old_shape")")" == "true" ]] \
|
||||
|| fail 'a settings.json written before the colour fields existed stopped validating'
|
||||
[[ "$(run ipc --pid "$harness_pid" call displays-test persistedEntryValid "$(b64 "$new_shape")")" == "true" ]] \
|
||||
|| fail 'a stored entry carrying the extended record was rejected'
|
||||
[[ "$(run ipc --pid "$harness_pid" call displays-test persistedEntryValid "$(b64 "$bad_shape")")" == "false" ]] \
|
||||
|| fail 'a stored entry with an impossible VRR mode was accepted'
|
||||
|
||||
# A revert that exits zero but reads back wrong remains an explicit manual
|
||||
# recovery error rather than pretending the desktop was restored.
|
||||
[[ "$(run ipc --pid "$harness_pid" call displays-test applyLayoutFixture 3000 400)" == "true" ]] \
|
||||
|
||||
Reference in New Issue
Block a user