Skip to content

View: Limit and auto-fit table column widths (#1023) - #1037

Open
dhingora-amd wants to merge 4 commits into
mainfrom
dhingora/limit-table-column-width
Open

dhingora-amd wants to merge 4 commits into
mainfrom
dhingora/limit-table-column-width

Conversation

@dhingora-amd

Copy link
Copy Markdown
Contributor

Motivation

Closes #1023.

Table columns sized themselves to their raw content, so a single very long
value (e.g. a demangled kernel name, which can be multiple KB) would stretch a
column to thousands of pixels and force endless horizontal scrolling, making
tables hard to read. The goal is to keep columns to a sensible width, elide
overflowing text while keeping the full value accessible, and auto-fit columns
to what's on screen without fighting the user's manual resizes.

Technical Details

  • Clamped auto-fit: columns are fitted to the widest loaded value, clamped
    to [MIN_COLUMN_WIDTH_EM, MAX_COLUMN_FIT_WIDTH_EM] (font-size multiples, so it
    scales with DPI). The min keeps the filter row usable; the max stops runaway
    columns.
  • Fit timing: a full fit runs on content changes (track selection / filter);
    scroll page-ins only ever grow columns (never shrink mid-browse); sorting and
    plain scrolling don't refit.
  • Respects manual resizing: manual column drags are detected and skipped by
    the auto-fit. Sizing state is keyed by column name, so adding or removing
    tracks (and their columns) no longer resets widths you set yourself; only a
    full clear (all tracks deselected) restores default sizing.
  • Text elision: cell values that exceed the column width are elided with an
    ellipsis; the full value remains available in the hover tooltip and in copy
    actions. Elision is centralized in a shared gui_helpers helper.
  • Filter hints: replaced the verbose per-column placeholder strings with a
    simple "Filter" placeholder plus a hover tooltip that shows the supported
    operators (> < = >= <= !=) and an example, styled to match existing tooltips.

Long values (e.g. demangled kernel names up to a few KB) previously
stretched auto-sized table columns to thousands of pixels. Cap each
column's fitted width and elide overflowing cell text to the live column
width, keeping the full value in a hover tooltip (copy/export unaffected).

Re-fit columns to their content when the result set changes (track
select/deselect, filter, time-range selection), leaving columns the user
manually resized alone and resetting to auto-fit once the table is
emptied. Sorting and scrolling no longer re-fit, so widths stay
consistent while reordering/paging.

Also make gui_helpers' ElideWithEllipsis measure in a single pass
(instead of popping one char at a time), give it an optional character
cap, and share a TEXT_ELLIPSIS constant.

Fixes #1023
- Key column sizing by name so adding/removing tracks keeps manual widths
- Clamp fit width to [min, max] and only grow on scroll page-in
- Replace verbose filter placeholders with a "Filter" hint + hover help tooltip
…column-width

# Conflicts:
#	src/view/src/widgets/rocprofvis_infinite_scroll_table.h

// A content change (select/filter) re-fits the columns fresh; any other
// page-in of this table's data just lets columns grow to newly loaded values.
std::shared_ptr<TableDataEvent> table_event =

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

^

Comment thread src/view/src/widgets/rocprofvis_infinite_scroll_table.cpp Outdated
@tomk-amd

Copy link
Copy Markdown
Collaborator
image

Please limit the width of the tool tip to something reasonable, there are examples of this being done in other areas of the UI for example on the Kernel Details tab of the Compute view:

image

Long cell values (e.g. kernel names) now wrap at a fixed max width instead of stretching across the screen, matching the Compute view's Kernel Details table tooltip behaviour.
Comment on lines +952 to +957
// WidthRequest only diverges from our applied value on a user drag.
const float delta = table->Columns[c].WidthRequest - it->second;
if(delta > 0.5f || delta < -0.5f)
{
m_user_sized_columns.insert(name);
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will capture a font size or time format change and include those as user sized as well, so you lose auto fit for affected columns.

Comment on lines +981 to +990
if(is_elided && ImGui::IsItemHovered())
{
ImGui::SetNextWindowSizeConstraints(ImVec2(0, 0),
ImVec2(TOOLTIP_MAX_WIDTH, FLT_MAX));
BeginTooltipStyled();
ImGui::PushTextWrapPos(ImGui::GetCursorPosX() + TOOLTIP_MAX_WIDTH);
ImGui::TextUnformatted(cell_text->c_str());
ImGui::PopTextWrapPos();
EndTooltipStyled();
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For time columns, up one level from this is another tooltip that just shows it in ns. When a time column is cut off, that existing one seems to take affect instead of this one. I don't know which one we would rather have, so just FYI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Defect]: Limit column size in tables

4 participants