Hdplus and fullhd perf - #16
Draft
jamaliki wants to merge 10 commits into
Draft
Conversation
Three independent costs dominated the FullHD path: 1. Ribbon meshes were over-tessellated. 14 spline subdivisions x 12 coil segments produces 336 triangles per residue -- 368,976 for 1AOI against 1.18M pixels, so most triangles were sub-pixel. The existing LOD only engaged above 5000 residues, so nearly every real structure paid full tessellation. LOD is now graduated: <=300 residues keep 12x10, the middle tier drops to 8x8, >5000 keeps the existing 4x6. Measured at 1600x736, 8x8 renders 2.1x faster than 14x12 for a mean per-channel error of 0.40/255, with differences confined to silhouette edges. 2. Triangle projection was serial. render_cartoon_tiled projected and shaded every triangle single-threaded -- the resolution-independent floor under every frame, and its largest stage. Now par_iter. The tile merge and the depth tint (including its min/max scan) are parallelized too; tiles cover disjoint screen rectangles, so the merge partitions cleanly over rows. 3. The main loop redrew unconditionally ~30x/second while idle, re-running the whole pipeline and retransmitting ~225 KB/frame of escape sequences to reproduce an identical image. Redraws are now gated on input, auto-rotate, a pending clear, a mesh rebuild, background analysis completing, or the SSH warning countdown. Per frame, 1AOI at 1600x736 with the mesh cached: 23.09 ms -> 12.04 ms. Idle cost is now ~0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Map4Nz8qwRcPi5iXQ9kpwd
…cost HD (HalfBlock) is left byte-identical; HDplus is a new tier alongside it, reached with `m` (Braille -> HD -> HDplus) or `--render hdplus` (aliases hd+, halfblockplus, half-block-plus). A braille cell addresses a fixed 2x4 dot grid, so extra spatial resolution is not available in this render path. What is available is a better decision about each dot. HDplus rasterizes into a 2x supersampled framebuffer and box-filters it back down: each dot is backed by a 2x2 sample block and lights at >=50% coverage rather than on a single sampled pixel, and the cell color averages every covered sample of its lit dots. This anti-aliases silhouettes and stops thin ribbons crawling between dots as the camera rotates. The emitted character grid is identical to HD's, so nothing extra reaches the terminal. Supersampling is close to free here because the HD framebuffer is small enough that cost is dominated by the resolution-independent per-triangle projection stage. Measured on 4HG6 (1402 residues, 11029 atoms) at 200x50: Cartoon HD 6.45 ms -> HDplus 6.75 ms Backbone HD 1.64 ms -> HDplus 2.63 ms Wireframe HD 3.34 ms -> HDplus 5.64 ms (33 ms frame budget) Two details that are easy to get wrong, both covered by tests: - Line thickness must be derived from the output resolution and only then scaled by the supersampling factor. Deriving it from the supersampled width lets the clamp(1.0, 3.0) floor absorb the factor and renders strokes thin. - The camera must be scaled by the same factor, because zoom and pan are in framebuffer pixel units. Otherwise the protein covers the same pixel count in a buffer twice as wide and downsamples to half the size of Braille and HD. Color quantization (8 over SSH, 4 local) merges run-length spans that supersampling would otherwise break, cutting SGR color escapes ~18% on 4HG6. Adds 8 tests, including a viewport-level test that drives render_viewport through a TestBackend so the camera-scaling wiring is covered, not just the rasterizer in isolation. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Map4Nz8qwRcPi5iXQ9kpwd
A braille cell carries a single foreground color, but its eight dots routinely straddle several structures at different depths. Averaging every covered sample blended a pink helix, a yellow sheet and a green coil into brown, so the interior of any dense cartoon render came out muddy in both HD and HDplus. The z-buffer already knows which fragment is in front, so the cell now takes the color of its frontmost covered sample instead of the mean. It falls back to the mean when no covered sample carried a finite depth, which keeps framebuffers written without z (as in tests) behaving as before. Measured on 4HG6 at 160x44, mean cell saturation: Braille 1.000 (flat and unshaded -- the reference for vividness) HD 0.872 -> 0.970 HDplus 0.859 -> 0.969 Lambert shading and the depth fog are unchanged, so the depth cue survives; only the choice of which covered sample colors the cell changes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Map4Nz8qwRcPi5iXQ9kpwd
Every fixed color was a `Color::Rgb(...)` literal buried in a match arm in color.rs, including the eight-entry chain cycle written out twice. They now come from a `Palette` resolved once at startup. ProteinView reads ~/.config/proteinview/palette.toml (or under $XDG_CONFIG_HOME) when it exists; --palette <FILE> overrides that. Covers the Structure, Chain, Element, B-factor, pLDDT and Interface schemes, nucleic acid bases, and small molecules. Rainbow's HSV sweep and the depth fog are still fixed. Design notes: - Every key is optional and falls back to its built-in default, so a file containing one color is valid. Element symbols merge onto the CPK table rather than replacing it, so overriding carbon does not silently drop every other element; the chain list replaces outright, since it is an ordered cycle. - Unknown keys are rejected. A typo naming a color you cannot otherwise verify should say so, not quietly do nothing: `helics` reports "unknown field `helics`, expected one of `helix`, `sheet`, `turn`, `coil`". - Chain colors live under a [chain] section rather than a bare top-level `chains` key, because in TOML a bare key written after any [table] header binds to that table instead of the document root -- a trap the first draft of the example file fell into. A test covers it. - The palette is process-wide read-only state in a OnceLock rather than being threaded through every renderer, since element_color and plddt_color are associated functions with no self to hang it on. - The B-factor gradient is written as `low * (1 - t) + high * t` specifically so the default blue-to-red endpoints reproduce the previous hardcoded arithmetic bit for bit. Defaults reproduce the previous colors exactly: every pre-existing color assertion in color.rs passes untouched, and rendering 4HG6 with the fully populated docs/palette.example.toml is byte-identical to rendering it with no config at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Map4Nz8qwRcPi5iXQ9kpwd
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
FullHD re-encoded and re-transmitted the whole framebuffer every frame. On a Retina panel a full-screen viewport is ~2550x1435 device pixels -- 3.7 MP, over four times the area the cell grid suggests -- so each frame meant zlib over 14 MB of RGBA (12.7 ms, as much as the entire rasterizer) and 656 KiB of base64 down the PTY, which the terminal then had to inflate and re-upload as a texture. Kitty's t=s transmission medium removes the transfer instead of speeding it up: the pixels go into a POSIX shared memory object and the escape sequence carries only its name, so the terminal maps the same pages we wrote. No compression, no base64, nothing for the terminal to decode. Support is established at startup with the query-action + device-attributes handshake the protocol documents, never assumed; SSH sessions and terminals that answer no keep the zlib path. Alongside that: - Rasterize into horizontal bands written directly into the framebuffer, which drops the per-tile scratch buffers and the merge pass entirely. Each scanline solves the barycentric half-planes for its x-span instead of scanning the bounding box -- ribbon triangles are slivers whose box is several times their area -- and walks those bounds incrementally, so the three divisions per scanline become three per triangle. The projected-triangle array is reused across frames; reallocating it was the largest allocator cost in a profile. - Write RGBA straight into the shared mapping, in parallel. That removes the intermediate RgbaImage, the DynamicImage wrapper, and to_rgba8()'s full clone of every frame. - Render at half resolution whenever the camera is moving, not just when auto-rotating a structure over 5000 residues, and draw one full-resolution frame the moment it settles. Motion hides the softness; standing still no longer costs anything. One function now owns FullHD sizing for both the renderer and the zoom calculation, so they cannot drift apart, and it caps very large displays at 4 MP. - Default --threads to one core each rather than 4. Holding cores back for the terminal made sense when it had a frame to inflate every tick. 4HHB at 2550x1435: 22.9 ms/frame (43 fps) -> 10.3 ms (98 fps), and 656 KiB per frame down the PTY -> 79 bytes. While rotating: 22.9 ms -> 5.0 ms. Snapshot PNGs across six structure/mode combinations are byte-identical to before, so this is speed only. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011eVRPRPVdfAcHv25ThbxWW
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fc898420a3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let must_redraw = had_input | ||
| || interacting | ||
| || settled | ||
| || app.ssh_hd_warning |
There was a problem hiding this comment.
When FullHD is entered over SSH and the user stops providing input, the last warning frame is drawn while ssh_hd_warning is still true, and app.tick() then clears the flag. On the next iteration this condition is false, so idle redraw suppression leaves ⚠ Full HD over SSH may be slow visible indefinitely until another input event. Track the warning transition or request one final redraw when its countdown reaches zero.
Useful? React with 👍 / 👎.
Comment on lines
+908
to
+910
| let c = fb.color[row + sx]; | ||
| if c != [0, 0, 0] { | ||
| covered += 1; |
There was a problem hiding this comment.
The new palette accepts "000000", but HD and HDplus treat an RGB-black rasterized sample as uncovered even when its depth is finite. Consequently, black-configured geometry—especially planar geometry for which depth tinting is skipped—produces blank cells rather than black braille glyphs and disappears even on a light terminal background. Use the depth buffer as the coverage/background indicator so every valid palette color remains renderable.
Useful? React with 👍 / 👎.
jamaliki
force-pushed
the
hdplus-and-fullhd-perf
branch
from
September 3, 2026 23:07
fc89842 to
e97cc67
Compare
`cargo fmt --check` was failing on one over-wrapped `parse(...)` call in the tests. Formatting only; no behaviour change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011eVRPRPVdfAcHv25ThbxWW
The cap was set at 4 MP on an estimate of the cell size. Measured on a real full-screen kitty at font_size 14 on a 2560x1600 Retina panel, cells are 20x43 device pixels over a 144x36 viewport -- a 4.46 MP framebuffer, which tripped the cap by 10%. That is the worst case for capping: the terminal scales the render back up by 1.056x, so every still frame paid a non-integer resample to save a tenth of the pixels. Raise it to 12 MP and say what the cap is actually for. It is a backstop against a framebuffer large enough to cost real memory, not a frame-rate control -- a still frame is drawn once before the loop goes idle, and frames drawn while the view moves are already quartered. 12 MP clears 4K and full-screen HiDPI laptops with room to spare, leaving the cap to meet 5K and above, where the framebuffer would otherwise run past a hundred megabytes. Tests now use the measured geometry rather than the estimate, and pin both a 4K and a full-screen HiDPI viewport as rendering natively. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011eVRPRPVdfAcHv25ThbxWW
jamaliki
marked this pull request as draft
September 3, 2026 23:25
Reading a structure means moving between the sequence and the geometry, and ProteinView had no way to do that: you could cycle chains, but never see what a chain *is*, let alone point at one residue in it. `S` opens a panel listing every chain's sequence in one-letter codes -- amino acids and nucleotides alike -- wrapped to the panel width and scrolled as a single list. Rows carry the residue number of their first residue rather than a running count, so the numbering gaps that fill deposited structures stay honest, and letters take their color from the active scheme so the panel and the structure read as one picture. Arrows move a residue cursor, `Shift`+arrow extends a range, `Enter` picks one residue and `A` a whole chain. Picked residues are drawn as ball-and-stick over whatever mode is active, z-buffered against it, with the selection color on carbons and CPK elsewhere -- the standard way to make a fragment legible without recoloring chemistry. `b` drops back to one marker sphere per residue, pushed forward by a ribbon half-width so a marker is never hidden by the very residue it marks. `z` centres the view on the selection, which is the difference between finding twelve residues in a mitoribosome and not. The panel takes only the arrow keys: h/j/k/l still rotate, so the structure stays steerable with the sequence in front of you. The selection outlives the panel, and the status bar keeps its count. Design notes: - Membership is a flat [chain][residue] bitmap, not a hash set. The renderer asks about every residue every frame; this makes that an index. - One `SequenceLayout` per panel width feeds both the renderer and the cursor keys, so what is on screen and what the arrows move through cannot disagree. Cursor position maps to a row by arithmetic rather than a search. - The layout is synced before *and* after input handling: the key that opens the panel arrives after the first sync, and without the second the opening frame would draw an empty panel. - Renderers take `Option<SelectionView>`; the snapshot and panel-server paths pass `None` and are unchanged. - Selection colors come from a new `[selection]` palette section. Verified against 8XT3 (52 chains, 9758 residues, 100k atoms) by driving the real TUI in a pty, including at 16x44 and alongside the interface sidebar. Thirteen new tests cover the layout/cursor round-trip, header-skipping vertical moves, cross-chain range extension, camera centring, the panel-size floor, and that the overlay reaches all four render modes while touching only picked residues. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WWJUeNvJK9gt3iBy9fzg3v
Three things, all aimed at the same complaint: a big structure renders as a flat mat of colour with no sense of what is in front of what. Depth fog looked like it had stopped working. It had not -- an 8XT3 render is byte-identical back to before any of the recent performance work -- but the blend ramps linearly across the structure's own depth range, so the contrast between two features a fixed distance apart shrinks as the structure gets deeper. A 10 A separation is 0.065 of blend in a small protein and 0.015 in a ribosome: invisible, and invisible exactly when depth cues matter most, because that is when everything overlaps. The strength now scales with the depth span up to a cap, which restores roughly constant discrimination per angstrom. Structures at or below a reference depth are untouched; 8XT3 gets 2.1x the contrast it had. Atoms were drawn as flat discs of a single colour. Tens of thousands of them overlapping have no form at all, which is most of why a large structure reads as confetti. Each disc is now shaded as the silhouette of a sphere -- one square root per pixel -- so it has a highlight and a shaded limb. This also pinned down a convention the code had never had to settle: the triangle rasterizer shades with abs(dot), so the sign of the view axis never mattered, and taking it at face value put the highlight on the wrong limb. Ribbon triangles are now wound consistently. Every cross-section ring comes from a reference frame carried along the spline, and that frame can flip partway down a chain; when it did, the ring's vertex order reversed and so did the winding of every triangle built from it. The stored normals were therefore arbitrarily signed, which is why shading had to use abs(dot) to look right at all. Orienting each ring from its own signed area -- rather than guessing per triangle, which fails on a sheet arrowhead's barb, where the face points along the axis and the radial test is reading noise -- fixes the whole mesh. A test pins the invariant. Back-face culling is deliberately still off. The winding is now good enough for it, but the surface is not watertight: a sheet's arrowhead and the coil after it meet at a T-junction. Sealing the gaps balances the edge counts and the T-junctions still crack into hairline slivers once the faces behind them stop being drawn, and making the seam watertight properly cost 28% more triangles on 1UBQ. That is a poor trade for a few milliseconds on a frame that already fits its budget three times over. The blocker is recorded where the cull would go. Cartoon geometry is byte-identical; what changes is fog on deep structures and the shading of atoms and ions. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011eVRPRPVdfAcHv25ThbxWW
The lighting term ran `0.55 + 0.45 * (abs(dot) * 0.4 + 0.6)`, which spans 0.82..1.00. An 18% swing between a surface facing the light and one facing away is close to no shading at all, so a cartoon render came out looking flat and washed out however good the geometry underneath it was. That formula was not a style choice, it was compensation. Cross-section rings inherited their winding from a reference frame carried along the spline, and that frame can flip partway down a chain, so the stored normals were arbitrarily signed. `abs(dot)` was the only way to shade them without half the ribbon going dark, and clamping the range hid what was left. With the mesh now wound consistently outward, the true normal can be used. Shading is now a half-Lambert wrap over 0.35..1.00. The wrap keeps the terminator soft, which suits a ribbon turning through a helix better than a hard Lambert edge, and nothing reaches black. A one-sided term was tried at 0.25..1.00: more dramatic, but the shadowed side loses so much saturation that it reintroduces the muddiness this is meant to fix. This also settles which way the view axis points. Depth grows away from the camera, so a normal facing the viewer has negative z while the light sits in front of the scene; the two were never in the same frame, and `abs` meant it never showed. Left as it was, the highlight lands on the wrong side. Atom spheres move from 0.70 to 0.50 for the same reason, staying a little flatter than the ribbon because an atom is often just a few pixels across, where the full contrast reads as speckle instead of form. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011eVRPRPVdfAcHv25ThbxWW
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.