Polish profiler launch & remote file browser UI - #1041
dhingora-amd wants to merge 1 commit into
Conversation
- Make Launch Profiler and Advanced Options frameless (in-body header + close) to match the app's other dialogs. - Remote file browser: readable selection tint, rounded padded table, centered breadcrumb chevrons, 12px window rounding. - Center "(?)" markers; align Browse/Add buttons; draw input hints as a non-layout overlay (removes stray box that shifted buttons). - Handle high-DPI / narrow panes: font-relative widths, auto-sized buttons, stacked OUTPUT FORMAT / TRACE WINDOW. - Indent Advanced Options; name shared layout constants.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| dl->PushClipRect(ImVec2(mn.x + pad, mn.y), ImVec2(mx.x - pad, mx.y), true); | ||
| dl->AddText(ImVec2(mn.x + pad, mn.y + ((mx.y - mn.y) - th) * 0.5f), | ||
| ImGui::GetColorU32(ImGuiCol_TextDisabled), hint); | ||
| dl->PopClipRect(); |
There was a problem hiding this comment.
The layout fix is correct (a dummy/child after InputText really did shove the following SameLine() Browse/Add button), but a draw-list overlay is a weaker substitute than ImGui::InputTextWithHint.
rocprofvis_stickynote.cpp already does the resize-callback + InputTextWithHint pattern. Using that here would keep the hint inside the input (no extra layout item), clip/elide with the field, and not paint on top of the caret while the empty field is focused.
This also drops the old ElidedText hover tooltip. Several callers pass hints that will not fit a compressed/high-DPI pane ("leave empty to use the remote $ROCM_PATH or $PATH", "e.g. --iterations 100 --input data.bin"), so the full hint is now unreadable with no fallback.
| ImGui::TextUnformatted(help); | ||
| ImGui::PopTextWrapPos(); | ||
| ImGui::EndTooltip(); | ||
| } |
There was a problem hiding this comment.
This bypasses the item system that HelpTip above just started using.
Dummy(q_sz)is submitted at theSameLinecursor (top-aligned with the title), while the glyph is drawn attitle_cy. Hit-testing uses the drawn rect viaIsMouseHoveringRect, which does not honor window hover/occlusion — a window stacked over this card can still spawn the tooltip.BeginTooltip()shows immediately and skipsBeginItemTooltip/BeginTooltipStyled(), so delay, padding, and colors disagree with every other(?)in the launcher.
An InvisibleButton (or Dummy after SetCursorScreenPos(q_at)) plus BeginItemTooltipStyled() would match HelpTip and keep the vertical centering.
Review: Polish profiler launch & remote file browser UINice, focused polish pass. The readable selection tint, right-anchored Browse buttons, font-relative label columns, and stacked OUTPUT FORMAT / TRACE WINDOW split are the right fixes, and CI is green. The one change I would not merge as-is is making Launch Profiler and Advanced Profiling Options frameless Other notes are in line comments. None of them are blockers if the drag-handle issue is fixed. Verdict: request changes for the unmovable windows; the rest is solid UI cleanup. |
| // Indent to line up with the cards' inner content (inset by card padding). | ||
| const float adv_indent = SettingsManager::Get().GetDefaultStyle().WindowPadding.x; | ||
| ImGui::Indent(adv_indent); | ||
| if (ImGui::Button("Advanced Options...", ImVec2(180, 0))) |
There was a problem hiding this comment.
Nit: the PR description says action buttons become auto-sized / font-relative, but this is still a hardcoded 180 px, and kLaunchActionButtonWidth is still 84.0f px. Label columns grow with CalcTextSize, so at large UI fonts “Browse” / “Add” / “Advanced Options...” can clip while the labels beside them do not.
ImVec2(0, 0) (or CalcTextSize + frame padding, floored at the named constant) would match the rest of the high-DPI work.
|
|
||
| ImGui::SameLine(); | ||
| ImGui::SetCursorPosX(ImGui::GetWindowContentRegionMax().x - ImGui::GetFrameHeight()); | ||
| bool close_clicked = XButton("##dialog_close", "Close", &settings); |
There was a problem hiding this comment.
ConfigWindowsMoveFromTitleBarOnly is enabled globally (src/app/src/main.cpp), so a NoTitleBar ImGui::Begin window has no move grip. These two dialogs used to be draggable from the native title bar; after this change they are stuck wherever SetNextWindowPos(..., ImGuiCond_Appearing) placed them.
The in-body header is also all real items (TextUnformatted + XButton), so even turning that flag off would not make the header draggable.
This is the same helper used for Advanced Options, which opens centered over the launcher — without a drag region the user cannot get it out of the way.
Please add a drag handle on this header (InvisibleButton over the title band, then SetWindowPos(GetWindowPos() + io.MouseDelta) while it is active), and keep the close button outside that hitbox. Other frameless UI in this app is BeginPopupModal and does not have this problem; these two are regular windows and need the extra work.
1 participant
Motivation
The profiler launch and remote file browser UI had several inconsistencies with the rest of the app: the Launch Profiler and Advanced Options windows used native ImGui title bars (every other dialog is frameless), the remote file browser's row selection was hard to read, help
(?)markers and the Browse/Add buttons were misaligned, and the layout broke on high-DPI displays and when the pane was compressed.Technical Details
(?)help markers consistently, and align the Browse/Add buttons by drawing input placeholder hints as a non-layout draw-list overlay (a hidden child window previously added a stray grey box and shifted the following button).