reports: show entry names as host over a tenant subline
Session history and the most-used table showed "host (Tenant)" names
wrapped across three lines; they now read as the host over a quieter
tenant line, as on Connections and Recordings.
Part of #254
sessions, recordings: show the session type with the shared badge (#254)
The sessions page printed the type as plain text and the recordings page
as a coloured word; connections and reports already used .type-badge
from rustguac.css. All four now use the same badge. The Proxmox badge
colour moves into the shared stylesheet with the others.
Shared facet filters (static/filters.js) on the sessions, recordings,
reports and connections pages: a dropdown per facet (Type, Folder, User,
Owner, Status as each page has them), multi-select with counts that narrow
as other facets apply, All/None, a find box for long lists, a Clear filters
link, and selections kept per page in sessionStorage. They sit beside the
existing text search and combine with it; the buttons are compacted to the
height and padding of the other controls and the dropdown reserves room for
its scrollbar so it no longer sits over the counts.
The list pages share one toolbar: a borderless row with the search box, the
facets and the page action on the right (New ad-hoc session, Export CSV),
placeholders standardised to 'Search <noun> (fields…)'. Sessions gains a
search (user, host, owner, ID, type) and a User facet; Reports moves from
its own .filter-bar to the shared one. The redundant page titles go — the
nav already names and marks the active page — and section headings centre
their heading and action button on one line.
Connections: the folder header is sticky at the top of the entries pane, so
it stays under the nav and the active-sessions strip while a long table
scrolls. All four pages show the session type with the shared .type-badge
(the Proxmox colour moves into rustguac.css), reports and recordings show
"host (Tenant)" names as the host over a quieter tenant line, and the key
glyph before Credentials in the nav is sized to the text.
Part of #254
tests: shared facet filters and the list toolbar on every list page
The ad-hoc session form sat in a collapsible framed box above the session
list. It now opens as a modal from a button on the right of the Sessions
heading, dropping the page's framed form box inside the dialog, with the
primary button first and Cancel at the same height as Connect, matching the
other modals. Section headings centre their heading and action button on
one line.
Part of #254
tests: the ad-hoc session form opens as a modal
(cherry picked from commit 872cc61fce)
pages: bake the caller's role into the served page, version the logo
The header flashed on every navigation: gated links (Sessions, Reports,
Tokens, Admin) were hidden in the markup and shown after /api/me
answered, the logo was revalidated on each page load, and the nav
differed between pages, so switching tabs reflowed the header.
Serve each page in five variants, one per role level, that differ only
in a data-role-level attribute on <html>; optional auth now runs on the
page routes so a logged-in request gets its variant with the navigation
already right on first paint. The ETag is per variant. The logo joins
scripts and stylesheets in content-addressed, immutable caching.
Part of #254
pages: one navigation for every page, gated by role in CSS
Every page carries the same eight links in the same order, including
the Credentials entry (which opens the dialog on the connections page
and deep-links to it from elsewhere), with data-min-role on the gated
ones instead of inline display:none. rustguac.css shows them from the
data-role-level the server writes on <html>; the pages set that
attribute themselves from /api/me for API-key users, whose page GET
carries no credential. Admin-only controls follow the same gate, so
nothing pops in after the first paint. Recordings shows "host (Tenant)"
entry names as the host over a quieter tenant line.
Part of #254
tests: one navigation on every page, gated by data-role-level
The sticky sidebar was sized from the viewport rather than from where
it starts under the header, so at the top of the page its bottom hung
off screen and the page had to be scrolled before the list could be
scrolled to its end. The page is now an app shell: the header stays,
and the sidebar and the entries area are two independent scroll panes
that always fit the viewport. Selecting a folder scrolls the entries
pane, not the page.
The sidebar is a bordered panel: one pinned header band on a surface
tone with the Folders label, a compact new-folder button and the filter
box, then the list with hairline dividers, 46px rows and one 12px edge
for every band. "host (Tenant)" names read as the host over a quieter
tenant line in the entries and search tables, and a role-gate
reconciliation covers API-key users.
Part of #254
connections: scroll only the entries pane on selection, clip the body
jumpToEntriesHeader() used scrollIntoView(), which scrolls every
scrollable ancestor, and a body with overflow hidden can still be
scrolled by script: selecting a folder pushed the brand row and the nav
off the top of the page. Scroll the entries pane to its top instead, and
make the body overflow: clip, which no script can scroll. The body's
border box now spans the full height so the page padding stays inside
the viewport.
Part of #254
tests: app shell with two independent scroll panes and a clipped body (#254)
The sidebar lists one folder per customer and gets long. Add a live
filter box (name, path, description; loaded subfolders included, with
ancestors of a match kept visible) mounted in a new #sidebar-tools div,
persisted in sessionStorage so live refreshes keep it. Make the tree
keyboard-navigable (arrows, Home/End, Enter/Space, Left/Right to
collapse/expand) with role=tree/treeitem, aria-selected/expanded and a
roving tabindex, and add an 'f' shortcut that mirrors the '/' one.
Implemented by post-processing the DOM renderFolders() builds and by
wrapping that function; its body is unchanged.
(cherry picked from commit 267ca7092d)
With a long folder list, clicking a folder appeared to do nothing: the
entries table was off screen, the only loading state was a text swap,
and nothing drew the eye to the changed title. loadEntries() also
called renderFolders() just to update one count badge, rebuilding the
whole sidebar on every click.
- Jump to the entries header when the shown folder changes (smooth,
'auto' under prefers-reduced-motion); never on initial load, live
refresh, forceRefresh after a mutation or search clear.
- .changed on #entries-header replays a 350ms title slide-in and an
accent underline flash on the action bar; cleared on animationend.
- Skeleton table (4 shimmering rows, same geometry as the real table)
while entries are fetched from the server; the cached path renders
immediately with no skeleton.
- updateFolderCount() swaps the number in the existing .folder-count
badge in place; renderFolders() only as a fallback when the badge
does not exist yet.
- renderFolders() is wrapped (not edited) to preserve the sidebar
scroll position across rebuilds and to reveal the selected row within
the sidebar (nearest, no page scroll) when the selection changes or
after loadFolders() rebuilds the tree.
(cherry picked from commit 6fd18394a7)
With 20+ customer folders the sidebar shared the page scroll with the
entries table, so picking a folder far down the list changed content off
screen, and every leaf row was pushed ~90px in by a hidden chevron
placeholder while names were ellipsised.
- .sidebar is position: sticky with its own thin scrollbar and
max-height tied to the viewport; .main keeps scrolling with the page.
- The FOLDERS header is wrapped in .sidebar-pin together with a new
empty #sidebar-tools slot, both pinned at the top of the sidebar.
- buildFolderNode() renders a .tree-chevron only for folders with
children; leaves get no placeholder (removed the .tree-chevron.leaf
rule), so rows start right after the 3px accent bar.
- Name, entry-count pill (.folder-count, now just the number with a
data-count attribute and "N entries" title) and a smaller scope glyph
share one line; description sits beneath at --fz-xs; rows are ~40px.
- Nested .tree-children indent 14px behind the existing dashed guide.
All new CSS is in one block at the end of the page's <style>.
(cherry picked from commit 198226c4b7)
The 1710px spec from the fork passed against main as well as against the
change it was meant to pin: its fixture names are short enough to fit
either way under Linux's monospace font, and the overrun on the reporter's
Mac came from wider glyphs and longer real names. With realistic data
(dotted FQDNs, service accounts, long emails and folder paths, which have
no natural break point) main overran by 370px, and the tighter padding on
its own still overran Connections and Recordings.
Table cells on Connections, Recordings and Reports now wrap long text
inside the cell (overflow-wrap: anywhere), with the Connect and action
buttons kept on one line. The spec gains a long-data test per page: all
three fail on main, two fail with the padding change alone, all pass now.
The e2e README lists only the specs that exist.
The 1800px breakpoint set a tighter page gutter on the connections page
only, so switching to Sessions or any other tab snapped the content
sideways. Move it to rustguac.css so every page moves together.
Part of #254
(cherry picked from commit b9db3ca2d2)
The 18px tables with 20px cell padding ran past the right edge on a
1710px MacBook display, the recording link cut off. Same treatment as
the connections table: 16px cells with tighter padding, 14px headers,
actions kept on one line. Nothing is hidden.
Part of #254
(cherry picked from commit 1b93b0c8c1)
The 18px table with 20px cell padding and a separate cell each for
edit, clone and delete overran the main column by ~300px on a 1710px
MacBook display, cutting "delete" off the right edge.
Put the three admin actions in one cell, and add a max-width: 1800px
breakpoint on the connections page: 20px page padding, 240-280px
sidebar, 16px cells with tighter padding, 14px headers, a 110px
Connect button and a smaller folder title. The icon-only "open in new
window" button keeps its compact size. Nothing is hidden; at 1710px the
table fits with about 250px to spare.
Fixes#252
(cherry picked from commit e7358ab9c9)
The comments on the #251 fit said Windows RDP without GFX rounds the
width down. It is our own guacd: patches/007-rdp-disp-mod16 rounds both
dimensions to a multiple of 16, with or without GFX, to stop H.264's
padding showing as a green band along the edge. Say so where the margin
is handled, and that the rounding must not be relaxed to make the margin
go away, since that brings the band back.
With fullscreen-on-connect, the first mousedown after CONNECTED is the
browser's fullscreen gesture, but Guacamole.Mouse forwarded it to guacd
as well, and the transition that followed swallowed its release. guacd
was left with the button held and the terminal selected text on every
pointer move; the same with a key press as the gesture.
Stop the gesture event before the display's handlers see it, and on
entering fullscreen release everything explicitly: keyboard.reset() and
a mouse state with no buttons at the last pointer position, so a release
lost for any other reason during the transition cannot stick either.
Fixes#256
(cherry picked from commit ee03b3e65c)
Stretching the display to fill a mod-16 shortfall costs sharpness when the
picture is already drawn one framebuffer pixel per physical pixel, and
nearly every RDP session hits the rounding. So the fill now applies only
when the picture is already being resampled (scale x devicePixelRatio != 1),
where it costs nothing; a pixel-exact session is never stretched, and the
framebuffer is centred in the available area so the small margin is split
evenly in the dark background instead of showing as a bare strip down one
edge. offsetLeft includes the centring margins and ignores the fill
transform, so the pointer maths that divides out fillX/fillY is unchanged.
Fixes#251
(cherry picked from commit d562946d0b)
Windows RDP without GFX rounds the requested width down to a multiple
of 16, so a 1710-wide window gets a 1696-wide desktop. The uniform fit
in scaleDisplay() then keeps the scale at 1 (the height matches) and
the missing 14px shows as a strip of bare page down the right edge.
After the uniform fit, stretch the display element by the remaining
fraction on each axis when it is at most 2.5%, and divide the same
factors out of pointer coordinates before sendMouseState. A larger gap
is a real aspect mismatch and keeps the letterbox. Multi-monitor
pop-outs reset the fill.
Fixes#251
(cherry picked from commit ebfab63ea7)
VeNCrypt, RealVNC and macOS Screen Sharing need a username as well as a
password. Pass it to guacd's VNC `username` arg and expose it in the
connection editor, the connect-time credentials prompt and ad-hoc
sessions.
(cherry picked from commit fac8d0aec5)
Folder access was by OIDC group only, so giving one person one VM meant
a group per person. Folders now also take allowed_users, a list of
login emails, next to allowed_groups. A user can open a folder if a
group matches or their email is listed (ignoring case). They need not
have logged in yet: access is decided from their identity when they
use rustguac. The list lives in the folder's config in Vault, so it
applies on every instance sharing that Vault.
Every folder access decision goes through resolve_folder_access, which
now takes the user (email and groups) rather than just groups, so the
compiler enforced the change at every call site: the tree, listings,
search, credential variables and connect.
The tree now also shows a folder the user cannot open when it leads to
one they can, at the top level and in subfolder listings, as the code
already intended one level down. Without that, a grant on users/jsmith
was unreachable from the tree when users itself was admin-only (this
also applied to group grants). Such a folder is listed without entries,
and opening it says it has nothing shared with you directly.
The allowed_users list is only returned to admins (folder config);
listings that operators read leave it out, so a user does not learn who
else has access. Saving a folder without the field keeps the stored
list, so older clients cannot wipe it. Entries are trimmed, repeats
dropped, and anything with spaces or control characters refused, up to
500 per folder. The folder dialog gains an "Allowed users" box, and the
bulk move dialog's access warning compares users as well as groups.
Docs: folder access rewritten, including the correction that a folder
with no allowed groups is admin-only (the docs said it was open to
every authenticated user), and the API reference.
Requested by @3zAlb.
Re-filing more than a handful of entries meant opening each one. Admins
now get a checkbox column on the entries table (select all, shift-click
for ranges) and an action bar with Move to, Copy to and Delete. One
dialog handles all three: a filterable list of every folder, a preview
naming any entries that already exist in the target, a choice to leave
those or give them a new name, warnings when the move crosses between
shared and instance or the folders have different access groups, and a
result per entry afterwards.
POST /api/addressbook/bulk does the work, up to 1000 entries a request,
8 at a time. Each entry goes through the same stored-entry copy as the
single move: credentials included, Vault check-and-set so nothing is
overwritten, and a move removes the original only after the copy is
written. Clashes are skipped or renamed (name-2 .. name-99, within the
64-character limit); there is no overwrite option. Repeated items are
acted on once. Each entry is audited on its own, headline only. The
route alone accepts a 256KB body, since 1000 entries is about 100KB;
it is admin-only and authentication happens before the body is read.
GET /api/addressbook/folder-paths lists every folder for the picker; the
existing "Move to folder" list only knew about folders already expanded
in the tree.
Delete now checks the entry exists first, in both the bulk and single
endpoints. Vault reports success for deleting a path that is not there,
so a missing entry was answered, and audited, as deleted.
Moving an entry to another folder was done in the browser as "create
a copy from the edit form, then delete the original". The page never
receives an entry's secrets, so the copy had no password, private key,
jump host credentials, Proxmox token or container password, and a
$variable reference stored in the password field was lost the same way.
The original was then deleted permanently. Clone had the same flaw.
(#232)
Two admin-only endpoints now do it from the stored entry:
- POST .../entries/{entry}/move copies the full entry to the target and
only then removes the original. If that removal fails it says the
entry was copied, rather than reporting a plain failure.
- POST .../entries/{entry}/copy copies it under a new name. Clone uses
it and then applies the form's edits as an ordinary update, which
keeps stored secrets it is not given.
The target is written with Vault check-and-set (cas 0), so an existing
entry is never overwritten, even by a concurrent writer. Scope, folder
and names go through the existing Vault validation (only shared and
instance scopes, no traversal, .config reserved, so a copy cannot
replace a folder's access settings), and those failures are 400s. Both
actions are audited, headline only. A note on the shared helper records
that any future per-folder editing (#231) must require rights on both
the source and the target folder.
A busy main thread does not delay pointer movement, it discards it: moves
arriving while a handler runs are merged into one, and what is dispatched
afterwards carries only the last position. A drag feels that immediately --
the path between two points is gone, so a dragged scrollbar thumb is dropped
rather than merely lagged -- while a hover or a video degrades gracefully
under the same load.
getCoalescedEvents() returns what was merged. Replaying it in order restores
the path, though not the timing: nothing running on a blocked thread can do
that, and the positions still arrive together once the thread is free. What
changes is that the remote end sees where the pointer went rather than only
where it ended up.
Deliberately additive. The mousemove listener is untouched and still delivers
every position it did before, so touch, the compatibility mouse events it
produces and the counter that swallows them all behave as they did; the
dispatched position arrives twice and the second is dropped by the
unchanged-coordinates check already in move(). Migrating this file to pointer
events outright is the tidier shape and a far larger change -- every listener
would have to move together, or synthetic events from touch would arrive with
nothing left counting them down.
Only while a button is held, because the path matters for a drag and the
endpoint is all that matters for a hover, and replaying every merged move of
an idle pointer would multiply a session's upstream mouse traffic for movement
nobody can perceive. Capped and evenly sampled, because a pointer reporting at
1000Hz behind a thread blocked for 100ms offers a hundred positions for one
drag, and answering a rendering problem with a traffic one is no answer. An
ordinary pointer never reaches the cap.
The pop-out monitor windows get the same replay. They bypass Guacamole.Mouse
entirely -- it does not track the X axis correctly in a popup -- and map the
pointer themselves, so a drag on a second head would otherwise lose its path
exactly as the first one did. The sampler moves to a static so there is one
definition of the rule rather than two that can drift, and that path needs one
thing the main display does not: it sends unconditionally, where move() drops
an unchanged position, so it keeps what it last sent and drops a repeat --
keyed on the buttons as well as the position, since a press and a release
happen at one place and are not repeats of each other. Feature-detected on the
event rather than on PointerEvent, because a pop-out window is a realm of its
own and the constructor this page can see is not the one that made the event
it is handed.
That pointer handling lives in Guacamole.MonitorPointer rather than inline in
client.html, because these windows cannot be opened without a live
multi-monitor session: while it sat in the page the only thing a test could do
was match the source text and hope, and the two things most worth checking
there are a coordinate mapping and a suppression rule -- exactly the kind that
look right in a diff and are off by a term. It takes the canvas, a function
returning this monitor's rectangle in the combined framebuffer (a call rather
than a captured value, since the layout changes under a running window) and a
function to send a state.
Writing those tests found a real defect in the wheel, which sends a momentary
press and release: it built one state object, sent it, mutated it and sent it
again, which works only while every consumer copies synchronously. It sends
two states now.
Tested rather than tried, because this path only runs once the browser has
already thrown something away: the case it exists for is the one that cannot
be reliably produced, and a broken version feels like the behaviour it
replaced. Twenty tests cover the mapping, the button mask, the repeat
suppression, the wheel's reset of it and the replay, plus a harness driving
real pointer input through the debugger with the main thread deliberately
blocked -- because the unit tests feed events shaped the way the code expects,
and that assumption is the one worth checking against a browser.
(cherry picked from commit d8ce743fe0801bd8c776281ee5541abf8040a339)
A client with no hardware H.264 decoder lost H.264 altogether, and lost it in
the worst available way. Chrome reports a configuration carrying
hardwareAcceleration 'prefer-hardware' as unsupported outright rather than
falling back, so configure() failed asynchronously, the decoder closed, every
frame was then held waiting for a keyframe that cured nothing -- and since
guacd suppresses ordinary image operations for a layer carrying H.264, what
the user saw was a permanently black screen rather than a degraded picture.
Measured on Chrome 153 against an Intel UHD 770 with no VA-API decode
available: isConfigSupported() answers supported for avc1.4d4020 with no hint
and with 'prefer-software', and not supported with 'prefer-hardware'. The
same box then decoded the stream perfectly once the hint was dropped.
So ask for hardware once and, if that is refused, build again without asking.
Dropping the hint unconditionally is the smaller change and the wrong one: it
hands the choice to the browser on every client, including the ones this
whole path exists for, where a software decode costs far more than it saves.
It says so when it happens. A session quietly decoding H.264 in software is
worth knowing about -- it is the difference between the cost this feature was
built to avoid and the cost it was built around -- so the fallback reports
decoder_software_fallback rather than only appearing as a slow session.
Tested, because the hint reads like a preference and restoring it would look
harmless. The test drives the shape Chrome actually presents -- configure()
accepted, the refusal arriving later through the error callback -- and checks
that hardware is asked for first, asked for only once, that no frame is
stranded by the refusal, and that the fallback is audible.
Reachable on a VM, a machine whose driver is blocklisted, or a browser with
acceleration switched off, none of which are exotic.
(cherry picked from commit 59fc416e80daab20ac07bdac5dbebb6ff7eddf75)
Upstream: ported by hand, as the fork's decoder has diverged. The
fallback reports through console.warn only, since the fork's diagnostic()
channel is not carried here. tests/h264-hardware-fallback.mjs passes
unchanged (node tests/h264-hardware-fallback.mjs).
Guacamole.Display puts its software cursor layer in the DOM from
construction, and client.html never handed the pointer over -- no oncursor
handler, nothing hiding the browser's arrow -- so both were always drawn.
They overlap at rest and separate during fast movement: the browser's is
composited by the OS, while the software one is moved by the page on each
mousemove and trails by however long the main thread takes to get to it.
The remote pointer image now becomes the CSS cursor and the layer is hidden,
falling back to the layer only where a CSS cursor is refused. Declared
through image-set() at the display's scale rather than via
Guacamole.Mouse.setCursor(), which assumes one image pixel per CSS pixel
and would draw a HiDPI host's pointer at twice its size under Native
Resolution. Re-applied from scaleDisplay(), since the scale changes under
a pointer that does not.
(cherry picked from commit 9cc6f77f2ee3e48a10d38055d6fe8debffd37ef8)
Unrelated fixes carried by the fork: session times and date filters in local
time, no local resolution of the target when a jump host is set, no resize
request that changes no pixels, and rustfmt on three hunks left unformatted.
(cherry picked from commit d9b361db6eb216d871f3fdac41c423a586e2d559)
Upstream changes on pick: the client.html resize skip is kept but
without the native-resolution sizing (displayScale/deviceSize), which
belongs to the HiDPI commit not taken here, so sizes stay in CSS pixels.
The oidc.rs hunk was formatting only and is superseded.
Provider discovery is lazy with retry, so SSO recovers on its own once the
provider comes up, and /api/auth/status reports availability separately from
enablement. Includes the callback diagnostics for state-cookie mismatch.
(cherry picked from commit dffe34660601a223021819c8efbed2ad503bfc14)
Upstream changes on pick: resolved against the ID token diagnostics from
#237 (the provider key ids now travel with the cached client and are
logged on rediscovery), and the unauthenticated /api/auth/status warm-up
is single-flight via OidcState::warm_in_background, so polling it while
the provider is down cannot queue up unbounded discovery tasks.
Wake-on-LAN (per entry): new wol_send_packet/wol_mac_addr/wol_broadcast_addr/
wol_udp_port/wol_wait_time fields on AddressBookEntry, threaded through
CreateSessionRequest into the SSH/VNC/RDP guacd param structs and emitted as
guacd's native wol-* connection args (guacd sends the magic packet and polls the
target before connecting). New 'Wake-on-LAN' section in the entry editor;
Guacamole imports carry existing wol-* params over.
SSH terminal font: new per-entry ssh_font_size (default 12), replacing the
hardcoded font-size. SSH DPI is now pinned to 96 so terminal text no longer
renders oversized on HiDPI displays — the client auto-scales the canvas, so a
fixed baseline keeps text size consistent across normal and HiDPI screens.
(RDP/VNC keep the device-scaled DPI.)
Onboarding modal: add a corner close button that permanently dismisses the
welcome tour (re-show still available via Settings -> Welcome Tour).
(cherry picked from commit e881bbc1e8933b46f7f2a8de691c86343dbbf15b)
The first WebSocket to a pending session takes its guacd stream, already
logged in to the target. Any operator could claim another user's pending
session that way, and session ids are visible in reports. Now only the
session's creator may make that connection, with no admin bypass: an admin
"shadowing" a pending session used to silently become its owner and lock
the real owner out, so shadow now refuses sessions that have not started
(409). handle_ws only takes the stream when the owner check passed at
upgrade time.
POST /api/sessions accepts an admin-only `owner` field so a backend using
an API key can create a session for a user who then connects with their
own login. That user is recorded as the creator.
Share links and shadow tokens gain an optional read-only mode; the default
stays read/write. Each shareable session gets a second token
(share_url_readonly), and shadow takes {"read_only": true}. guacd enforces
it per joining user via the read-only connect arg, which was previously
hardcoded to false. The Share dialog offers both links, the Sessions page
offers watch and shadow, and docs no longer claim a read-only mode that
did not exist.
Supersedes the cherry-picked 2449aa5 (install wrappers once), which stopped
the recursion crash but left passthrough silently dead after the RDPGFX
channel reconnect that xrdp performs at login. b3b2cce guards per callback
on what is actually installed, covering both failure directions.
Also brings 869c738, which releases the sync gate at the pipeline depth
waitForPending() gates on rather than at zero. Releasing only at zero meant
a continuously decoding session never drained fully, so every sync waited
out its 200ms timeout: choppy audio and a framerate throttled by guacd
pacing against an inflated lag figure.
# Conflicts:
# patches/004-h264-passthrough.patch
This reverts 8988165, restoring the unconditional full-range BT.709
conversion. The commit it undoes identified a real inconsistency and
resolved it toward the wrong side.
Its reasoning was that the 4:2:0 path never reaches the shader, so the
combined 4:4:4 picture converted differently from the 4:2:0 one beside
it, and that adopting the decoded frame's colour space would make the two
agree. The two did then agree -- as limited range, on hosts that send
full range. Before it, 4:4:4 was correct and 4:2:0 was crushed; after it,
both were crushed. The symptom it describes, "the combined 4:4:4 picture
renders visibly flatter", was the correct picture beside a crushed one.
What every RDP host actually sends is now read out of the SPS rather than
assumed. Windows writes video_full_range_flag=1 with no colour
description. The xrdp fork writes it with a complete BT.709 description.
Stock xrdp 0.10.6 omits the video_signal_type block altogether -- it
passes x264 no VUI parameters -- while naming its own conversion
XRDP_yuv444_709fr. All three encode full-range BT.709, which is what
MS-RDPEGFX defines.
What the browser reports for those streams is a different question.
Chrome's hardware decoder ignores a bare range flag with no description
beside it and reports limited for a host that plainly declared full, so
a shader that adopts the frame's colour space expands 16-235 to 0-255 on
full-range samples: blacks crushed to zero, whites clipped, chroma
over-saturated by 255/224.
The inconsistency this reinstates is real and is not being dismissed: a
session can render 4:4:4 correctly and 4:2:0 crushed. One path right
beats both wrong, and the repair belongs upstream of the browser --
completing the host's SPS on the wire, so that neither path has to guess
and third-party clients benefit too. That follows separately; it is
server-side and does not belong in this PR.
The coefficients are exact BT.709 rather than the 8-bit approximations
the original cited from FreeRDP's prim_internal.h. Under passthrough
FreeRDP never decodes these streams, so matching its rounding was never
the point.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013Rxg4cT87G9czw4nUxKhHG
waitForPending() holds the Guacamole sync response while pendingDecodes
exceeds MAX_PIPELINE_DEPTH, but resolveIfIdle() only fired its resolvers at
pendingDecodes <= 0. So once the backlog crossed the depth it had to drain
completely before a sync could go out, and a session decoding continuously
never reaches zero. Every sync therefore waited out its full timeout,
logging "sync wait timeout" and reporting that much processing lag however
fast the client was really going.
guacd paces frames against the client's processing lag, so the session runs
slow while the client is not in fact behind. The function's own
documentation already said the callback fires "when the backlog is within
the allowed pipeline depth"; only the implementation disagreed.
It shows up first on AVC444, where a picture is two access units and the
backlog therefore runs twice as deep for the same frame rate, making it far
less likely to touch zero between frames. That presents as AVC444 being
expensive rather than as a threshold mismatch, so an A/B that varies the
server between AVC444 and AVC420 while holding the client constant cannot
tell the two apart.
The colour-space change introduced "float y" for the scaled luma, but y is
already the int pixel row in that scope. GLSL rejected the redefinition and
then every arithmetic line that mixed the two, so the shader failed to
compile and the renderer fell back to 4:2:0 for the whole session:
ERROR: 0:89: 'y' : redefinition
ERROR: 0:93: '+' : no operation exists that takes highp int and highp float
Name it luma.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The combining shader hardcoded BT.709 at full range, citing the coefficients
FreeRDP decodes these streams with. But in passthrough guacd never decodes --
the browser does, and the 4:2:0 path never reaches the shader at all: that
VideoFrame is drawn by the browser, which applies the frame's own colour
space including the limited-range expansion of 16-235 to 0-255.
So the two paths converted differently on the same session. An H.264 stream
carrying no VUI signalling is limited range, and treating it as full leaves
black at RGB 16 and white at 235, with chroma unscaled to match: the combined
4:4:4 picture renders visibly flatter than the 4:2:0 one beside it, depending
only on which codec the server chose.
Take the range and matrix from the decoded frame's colorSpace instead, read
once from the first main view, and default to limited range BT.709 when the
decoder reports nothing -- which is what the absence of signalling means.
VideoFrame.colorSpace was available in scope and never consulted.
Restores GfxAVC444=TRUE, skips the server-side H.264 decode, forwards both
AVC444 views (new 'view' field on the h264 instruction + Yuv444.js WebGL2
4:4:4 reconstruction), and fixes blank H.264 recording playback. Review
findings folded in (f16eed7). guacd CPU ~100%/90% of a core -> ~2%.
Adds a per-entry RDP keyboard layout, wiring guacd's `server-layout`
parameter (previously hardcoded to empty) through the address-book entry,
the connect request, and the guacd handshake, with a dropdown in the
connections editor and a migration mapping from Guacamole's `server-layout`
on import. Empty keeps guacd's default (en-us-qwerty).
Review notes from the upstream issue thread:
- The auxiliary-view snapshot was sized to the auxiliary frame's own
dimensions. The v1 chroma layout pads that view to a multiple of 16
rows, so the snapshot could be taller than the picture the renderer
draws; with no rects the whole thing is blitted, painting a blank
strip below the image. Size it from the rendered canvas instead.
- The per-frame watchdog was cleared before combineFrame() ran, so the
4:4:4 path had no timeout despite the comment saying otherwise: a
copyTo() that never settled would hold the ordered display queue with
nothing able to release it. Clear it once the frame is done with
instead -- in the finally for the synchronous paths, and after the
copy settles for the combine.
- Drop the unreachable view != 0 branch after the combine hand-off.
- '0' no longer coerces to false in override(), so ?h264ChromaFilter=0
sets threshold zero rather than switching the filter off. Boolean
overrides are unaffected: they coerce, and 0 is falsy.
- Drop the stale stats() reference from client.html; the decoder has no
such method.
Recordings capture the raw protocol stream, so a recording of an H.264
session contains h264 instructions. recordings.html loaded neither
H264Decoder.js nor Yuv444.js, so playback of those sessions rendered
nothing.
H.264 passthrough had two independent defects. On Windows hosts it
negotiated no H.264 at all, and where it did engage it delivered about
half the saving it should.
GfxAVC444 = FALSE was set to make the client advertise AVC420-only,
which is not what the combination does. FreeRDP gates the entire RDPGFX
V10+ capability block on
!GfxH264 || GfxAVC444
(rdpgfx_main.c:254, rdpgfx_send_caps_advertise_pdu), so with GfxH264 set
and GfxAVC444 clear no V10 capability set is sent at all -- the client
advertises only V8 and V8.1. Windows will not offer H.264 at V8.1: it
falls back to CLEARCODEC and CAPROGRESSIVE, which guacd then decodes and
re-encodes as JPEG/WebP, exactly the cost the feature exists to avoid.
The display stays correct, so the regression is invisible without
measuring CPU or counting codecs on the wire. Over a minute of 1080p
video on Windows 11 Pro: 4050 CLEARCODEC + 1447 CAPROGRESSIVE + zero
H.264 frames before, all frames codec 15 after. xrdp is unaffected,
which is why this went unnoticed -- it accepts AVC420 at V8.1 and never
needed the V10 capsets.
AVC444 was disabled because forwarding it was broken, not because
AVC420 was wanted: only bitstream[0] reached the client, rendering as a
green/magenta split. Forwarding one view cannot work -- both bitstreams
decode through the same H264_CONTEXT in FreeRDP's avc444_decompress, so
they are one H.264 sequence with alternating views. Dropping either
breaks reference frames for the other, and the decoder reports no error
because what arrives is well formed. Both views are now forwarded,
tagged with a view field on the h264 instruction; the client decodes
both, draws only view 0, and combines the auxiliary view's chroma into
full 4:4:4 in a WebGL2 pass, falling back to 4:2:0 if it cannot.
The GDI decode is now skipped for captured H.264 commands. Previously
the wrapper always called through to the original handler, so only the
re-encode was saved. RemoteFX, planar and progressive still decode
normally, so a server mixing codecs within a frame stays correct.
guacd CPU with 1080p video playing, sampled per-thread over 30s:
xrdp (AVC420) ~100% of a core -> 2.0%
Windows 11 (AVC444) 90.6% of a core -> 2.1%
Configuration is unchanged: the per-connection enable-h264 argument is
the only switch, and there are no environment variables.
Dismissing the "N credentials to set up" banner now persists in
localStorage instead of a page-lifetime flag, so it stays gone across
refreshes. It records exactly which credentials were dismissed and
re-appears only if a genuinely new unset credential shows up — so a
partial save that shrinks the set won't re-nag, but a new requirement
still surfaces.
The collapsible group caret used the small-triangle glyph (U+25BE) like
the old folder chevron did, so it read faint. Switch to the full-size
U+25BC and bump its size, matching the folder-tree chevron.
The credential editor rendered a flat list inside a modal that scrolled as
a whole, so past a few variables the Save/Close buttons and context
scrolled off and it got unusable. Restructure it: the modal header (title,
description, filter box) and the Save/Close footer are pinned and only the
credential list scrolls. Add a filter box (shown once there are more than a
handful of variables) that live-filters rows by name/domain, and group each
domain into a collapsible section. Focused single-connect mode is unchanged
(few rows, no filter box).
Fetch the user's set credential variables on load and cross-reference
against each entry's credential_variables (already on EntryInfo). Entries
that reference variables the user hasn't set get a "credentials needed"
pill, and a dismissible banner reports how many credentials are still to
set up (aggregated across the search index). Both link to the editor, and
refresh after a save. Turns the old reactive "connect fails, then hunt for
the menu" flow into something visible up front.
When a connect fails for missing credential variables, open the My
Credentials editor focused on just the missing variables ("credentials
needed to connect to X") and, on save, re-run that exact connect instead
of dead-ending the user in the full editor with a toast. openMyCredentials
now takes { onlyVars, heading, onSaved }.
Also promote My Credentials to a visible "Credentials" nav link (kept in
the Settings menu too) so people can find it without hitting a failure
first.
Each per-user credential variable can live in the shared backend (fleet-
wide) or the local backend (this instance) - its location is the scope.
Reads merge both backends (local wins); writes partition by a per-key
scope sent from My Credentials, moving a key between backends when its
toggle changes and honouring "leave blank to keep". A "Shared across
sites" toggle appears per credential.
Quick-start preserved: with one Vault the shared and local cells alias the
default, creds_split() is false, the toggle is hidden, and everything is
written to the single store in one call - byte-identical to before. New
config user_credentials_default_scope (default "local") seeds new toggles.
The write partition is a pure function with unit tests covering single-
store, blank-keeps-existing, scope routing, cross-scope moves, and the
default-scope fallback.
list_all_folders now returns (folders, unavailable_scopes); a scope whose
dedicated backend is down is reported instead of silently dropped, and
GET /api/addressbook includes unavailable_scopes. The Connections tree
renders a non-interactive greyed "<scope> connections unavailable" node
so a partial outage is visible while other scopes stay usable; the 15s
/api/me poll refreshes it.
Single-Vault is unaffected: every scope aliases the one backend, so
unavailable_scopes is always empty while it is up (full outage still
returns 503 and the existing banner). Adds a test asserting a lone
[vault] block is sufficient (quick-start invariant).
The folder expand/collapse indicator used the small-triangle glyphs
(U+25BE/U+25B8), which render as faint, hard-to-see ink even at 20px,
while every other collapsible on the page uses the full-size triangles
(U+25BC/U+25B6). Switch the folder chevron to match so it is clearly
visible.
fit-on-connect: a session opened in a new window (at a fixed size), or otherwise
created at dimensions that differ from the viewing window, now fits the window
without a manual resize. The client previously only sent a size on a
window-resize event. The one-time fit is retried past the point where the resize
path becomes ready (RDP Display Control channel connected / SPICE agent and
display ready) because an early send is silently dropped; guacd coalesces the
retries into a single resize since the requested size is unchanged.
pop-out teardown: a monitor pop-out is only a satellite of the main window (it
blits from that window's canvas and sends input through its client), so it
cannot outlive it. Closing the main window now closes its pop-outs instead of
leaving a frozen orphan, and a session disconnect shows a "Disconnected" overlay
in the pop-out rather than a stale live image.
Adds RDP multi-monitor, reusing the protocol-agnostic client work already used
by SPICE (secondary-monitors advertisement, multimon-layout cropping, pop-out
windows, native-mouse mapping).
guacd (patch 010): a "secondary-monitors" arg enables multi-monitor. The
Display Update module now tracks a per-monitor layout (tiled left-to-right,
top-aligned, widths rounded to RDP-valid geometry) and sends the full
DISPLAY_CONTROL_MONITOR_LAYOUT array via SendMonitorLayout rather than a single
monitor. The host extends its desktop across the monitors and streams one
combined framebuffer, so there is no client-side compositing (unlike SPICE).
guacd advertises secondary-monitors on user join and publishes the
multimon-layout so the client can split the framebuffer into per-monitor
windows.
rustguac: RdpParams gains secondary_monitors, wired from the entry's
max_monitors and sent as the secondary-monitors connect arg (resize-method is
already display-update, which the Display Control channel requires). The
address-book entry editor gains a Monitors field for RDP.
Validated against a Windows RDP host: ticking a second monitor extends the
desktop, the second head renders and resizes, and clicks land on both.
Each entry gains a compact icon button beside Connect that launches the
session in a separate browser window (which can be dragged to another
physical display, and its secondary monitors popped out from there). Connect
is unchanged (opens a new tab).
The window is opened synchronously in the click handler so it is not
popup-blocked after the async connect fetch, then navigated once the session
exists; it also works through the credential prompt. The icon is a compact,
vertically-centred sibling that stays on one line with Connect.
guacd (patch-008): composite multiple SPICE display channels into one
framebuffer. A Windows/PVE guest exposes each head as a separate QXL device
(a separate display channel with its own primary surface); the previous code
tracked a single display channel and shared surface, so a second head
clobbered it and the session showed black or dropped. Now each channel is
tracked by id, tiled left-to-right into the combined default layer, and the
multimon layout enumerates every head with its offset. The SPICE pointer is
mapped from the combined coordinate to the owning head's local coordinate
(clamped to bounds so an edge value maps to the correct head).
client: the main window now crops to monitor 0 instead of showing the whole
combined framebuffer, and rescales to fit on resize. Secondary monitors open
in their own window that fills and rescales; that window's pointer uses native
mouse events mapped from the canvas's live rect, because Guacamole.Mouse does
not track the X axis correctly in a popup window. The per-frame blit clamps to
the source framebuffer bounds so a resize cannot read past it and produce a
corrupt image.
Validated on a Windows PVE console (VMID 300): both heads render composited,
clicks land on target on both monitors, and resizing the second monitor is
stable.
client.html: per-monitor tick-boxes (contiguity-enforced) that open each
secondary monitor in its own window, blitting that monitor's region of the
combined framebuffer (rAF) and forwarding mouse (combined coords) + keyboard;
untick/close releases the monitor. Requests floor to a sane size if a popup
reports zero dimensions.
Entry storage: AddressBookEntry/EntryInfo gain max_monitors, ab_connect_entry
passes it through, and the Proxmox editor gets a Monitors field. Lets a saved
Proxmox/SPICE entry offer multiple monitors (previously only the ad-hoc API
path could).
KNOWN ISSUE: enabling multi-monitor (secondary-monitors>0) intermittently drops
the SPICE connection ~2-6s after connect, independent of the request. Under
investigation; single-monitor is unaffected. A3 end-to-end (tick -> activate ->
render) still needs a real-browser verification (Playwright can't size popups).
Jad Seifeddine