Conversation
Whether a toolbar button is available, and whether it looks pressed, came from running a predicate apiece in draw. Most of those predicates look a floater up in the registry by name, which is two string-keyed map lookups and a key comparison, and there are eighteen buttons on a default layout. Every frame, to arrive at what was already on screen. It is settled on an idle callback now, and draw has none of it. Idle runs ahead of the display for the frame, so nothing shown is older than it was. Three things fell out of moving it that a gate in draw would not have given: a read-only bar never registers at all, rather than reaching a test every frame to decide to do nothing; being drawn was the visibility check, so that had to become one it makes for itself; and setToggleState reshapes a button through autoResize, which was happening in the middle of a draw and now happens where layout belongs. The pass runs when something says the answer may have moved. There is no timer behind it. A button's state comes from a predicate named in commands.xml and looked up in the enable-callback registry, so a toolbar cannot know what any of them reads -- but they can be read, and the eleven the shipped commands use bottom out in six things, every one of which already had a signal: floater visibility, which LLFloater::setVisible now says on a transition; the push-to-talk state, which was already firing a signal of its own; the voice channel and connection, through the status observer; the parcel, through a parcel observer; RLV behaviours, through the toggle callback; the HUD attachment flag and the marketplace status, each said where it is set, both of which have exactly one place a person changes them. The watcher that holds those connections lists them, because a command whose predicate reads something not on that list gets a button that never changes. Extending it is part of adding one. Voice is a parameterised singleton, so it is asked for rather than assumed -- it is made in LLAppViewer::init and the toolbars at STATE_WORLD_INIT, but nothing here would notice if that order changed. RLV connects whether or not it is on, since it can be turned on afterwards and the signal costs nothing while it is off. Two other things left draw while it was open. Buttons hold the command they were built from, which createButton already had and dropped, so neither the draw nor the three other places that wanted it look it up in the manager by id -- setCommandId takes both together so they cannot drift, and commands are registered once from commands.xml and never removed. And laying out says whether it moved the bar, so the three UI-origin matrix calls that follow it happen only when it did; the panel's own visibility went into that same pass, where half of it already was. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Laying a toolbar out translates and reshapes the bar itself, not only its buttons, and it did that from inside draw -- after the parent had pushed a matrix translated to where the bar was a moment earlier. Three lines stood after it to repair that: pop the parent's matrix, push another, translate again with the new rectangle. Balanced against the parent's own pop, and correct, but a view reaching into its caller's frame to undo a translation the caller had made in good faith. Laid out on the idle callback instead, beside the button states that moved there for their own reasons, and in that order: a button that gains or loses its pressed look resizes itself, and the layout pass is what places it. The repair is deleted rather than skipped. It also settles the rectangle before anything reads it. LLToolBarView::draw sizes each toolbar's containing panel from that rectangle, and did so before the toolbar's own draw relaid itself out -- so the container was sizing to where its contents had been on the previous frame. Three things were checked before moving it, because layout timing is the kind that breaks quietly: Layout does not now run every frame. LLToolBar::reshape raises the flag whatever it is handed, and LLToolBarView::draw reshapes each containing panel every frame -- but LLView::reshape does nothing at all for a size it already has, children included, so the flag stays down. The auto-hide slide does not raise it either. Those offsets are applied with translate, which leaves the flag alone. Had they reshaped, every frame of every slide would have laid the bar out again. And layout still runs with the bar where it was: from draw it ran after the offset had been applied, and from idle it runs with the offset the last frame left. The same place either way. What does change: a resize raising the flag during a draw is served on the next idle rather than within that draw, so a bar being dragged smaller trails by a frame, as the rest of the UI does. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The location readouts rebuilt their text on every frame the coordinate
setting was on: a format, five allocations, a shaping pass, and for the top
info bar a segment rebuild of the text box and a reshape cascade into the
chiclet bar. The string it produced was the same one it produced last frame
for all but a couple of frames a second.
It could not simply be gated, because four of the things it names had no
event behind them and were kept current by that rebuild alone:
- onRegionBoundaryCrossed only re-armed the navmesh listener, so the
region name and maturity rating survived a crossing by way of
onAgentParcelChange happening to fire too;
- nothing at all covered the handshake, so an estate manager changing a
region's rating or renaming it reached the UI through no code path;
- both parcel observers were wired to the icons and not to the text, so a
parcel rename reached the readout the same way;
- health lived on LLStatusBar, which does not display it, and its setter
notified nobody -- a function-local static in each of two pollers was
the whole of the change detection.
So the callbacks land first and the gate second.
Health moves to LLAgent, where the standing FIXME asked for it, and gains a
signal. That is not only tidiness: initWorldUI builds the navigation bar,
and therefore both readouts, before gStatusBar exists, so a signal on the
status bar could not be connected from either constructor.
LLViewerRegion gains a static region-info-changed signal, fired from
unpackRegionHandshake only when the name or the rating actually moved -- a
handshake arrives for every neighbour that comes into view. Static because
its listeners display the agent's region, and a per-instance signal would
have to be reconnected on every crossing.
The gate is three integers. The rounding moves into LLAgentUI::getDisplayPos,
which buildLocationString itself calls, so the two cannot disagree about what
a coordinate rounds to. A second gate compares the built string, because at
2 m and 4 m buckets the integers can move while the text does not -- and it
asks the widget rather than the last string built, since the field may be
showing the SLURL the user clicked for or the text they typed before giving
up focus, and putting the readable text back over both is the same
function's other job.
Splitting refreshMaturityButton exposed two older faults. positionMaturityButton
sets the button's own visibility when the text leaves no room for it, and its
caller guarded on getVisible() before calling it -- so narrowing the field
once hid the icon until the agent crossed into a differently-rated region.
mLastSimAccess started at SIM_ACCESS_MIN, which a region can really report,
so the first update could be skipped.
setParcelInfoText and layoutParcelIcons each compared the panel's rect and
each fired the resize signal, but only the second ever changes that rect, so
one text change fired twice into a full reshape of the chiclet bar.
llformat is gone from these files. buildLocationString formats into the
caller's buffer, so the local copy and the copy out both go; the parcel and
region names are views, every source of them handing back a reference that
outlives the call; getSimAccessString returns its reference rather than a
copy of it. "%.100s" truncated a name at a byte and could halve a multi-byte
character -- utf8str_truncate cuts on a character boundary instead, which is
a behaviour change.
LL_PROFILE_PLOT is new, alongside LL_PROFILE_ALLOC. The plots record what
fraction of rebuilds produced different text, and how much text the UI
reshaped per frame rather than replaying; the zones say where the time in
the strip goes. Both were unanswerable before.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
LLLocationInputCtrl and LLPanelTopInfoBar each carried the same enum, the same parcel queries, the same visibility rules, the same notifications and the same health readout, laid out in opposite directions. Two copies is how the health readout came to be keyed on a process-wide static in both, and how the parcel observer came to be wired to the icons but not to the location text in both. ALParcelIconStrip owns the behaviour. It is handed its controls rather than owning them, because that is the one thing the two panels genuinely do differently: the navigation bar builds them from parameter blocks on its own widget, the top info bar takes them as XUI children. Slots may be left empty, which is how the top info bar fills seven of the nine. The pathfinding pair keeps its click handler in the navigation bar -- rebaking needs a notification callback bound to the panel that owns the region listener. Laying the strip out from either end is not a matter of pinning the other edge of each control: the list has to be walked the other way too, or the strip comes out mirrored. Padding is now spent only on controls actually placed, where the navigation bar used to charge it for hidden icons as well -- nine icons, most of them hidden most of the time, each taking two pixels of nothing. The maturity icon is measured through the text cache. Where it sits depends on how wide the location text came out, and extracting that from a font shapes the whole string; the window and the navigation bar's own splitter both reshape the control on every frame of a drag, asking the same question of the same text each time. LLLineEditor exposes its text's generation so the cache can be told what it is holding. The trailing space is left uncached: it is one glyph, and a second slot keyed on a different string would break the cache's one-source rule. The status bar's six pull-downs are built on first hover. Six XUI documents were parsed and six widget trees built during login for panels that stay invisible until the pointer reaches their button, and most of which a given session never opens. Their six near-identical show handlers collapse into one: the two alignments they used differ by a division. getNearbyMediaPanel hands out one of the six and its only caller already reads null as "the user has not touched the media controls", which is exactly true of one never built. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
LLTextBox::setText clears the document, re-parses it into segments and
reflows, every time it is called. LLUIString::assign already declines to
announce a value it is being handed twice, but the call below it happened
regardless, so the guard one layer down was defeated by the layer above.
The question is asked of the document rather than of a remembered version,
so it stays right whatever else wrote the text. Every way it can be wrong is
the safe way: removeCRLF, truncate and URL-label substitution can each leave
the document differing from what was handed in, which makes this rebuild
when it need not have, never skip when it must not.
The style half of the question has no answer by comparison. LLInitParam
blocks cannot be compared, so a LLStyle::Params holding nothing but defaults
is indistinguishable from one a caller meant, and BaseBlock::isProvided
reports whether the block was provided as a nested parameter -- not that.
LLStyle::defaultParams is one instance that every default argument resolves
to, so "no style supplied" is answerable by address. LLTextBase::setValue and
LLTextBox::setTextArg each built their own static default and so would have
been invisible to the guard; both go through the shared one now. A caller
that supplies a style always rebuilds, and an unstyled write over text a
styled write put there rebuilds too -- it has a style to take back off.
The text itself reaches these by view. Every implementation copies what it is
given, because it has to validate and truncate it, so an owning parameter
meant the caller copied it first for the callee to copy it again --
LLStringExplicit derives from std::string and its converting constructor is
a copy.
That parameter cannot simply become std::string_view, though. LLStringExplicit
is not there to own anything; it is there to refuse a bare literal, so that
English text in code is caught and moved to a translatable XUI file, and the
tree's setText(LLStringExplicit("12:00 AM")) call sites are that working.
A plain view drops the bar along with the copy.
ALStringViewExplicit keeps the bar and drops the copy: explicit from
const char*, implicit from std::string and std::string_view, and taken by
value like the string_view it is. setText takes it from LLTextBase down
through LLTextBox, LLTextEditor, LLLineEditor, LLSearchEditor,
LLScrollListCell, LLExpandableTextBox and the location input. The existing
escape-hatch call sites are untouched: LLStringExplicit is a std::string, so
it reaches the new parameter in one conversion.
Refusing the literal is the only thing that type does, and it does it through
the absence of a conversion, which nothing in the build would notice the loss
of. Three static_asserts next to the class say so instead.
LLStringExplicit itself is left owning. It is held by value from temporaries
-- llviewermenu has both a local initialised from LLSD::asString and a
function returning one -- and a view would leave those dangling with nothing
to say so.
LLUIString::assign takes a plain view, which loosens nothing: it already
accepted a literal through const std::string&. Only assign takes one, not the
constructor or the assignment -- a constructor from a view cannot be reached
from std::string without two user conversions, and an operator= taking one
ties with copy-assignment for the same argument.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ding ALParcelIconStrip placed each control by writing one edge and then deriving the other from getWidth() -- which is mRight - mLeft, and so was answering for a rect half-way through being moved. Every icon in both the location input and the mini location bar came out the width of the gap between where it was and where it was going. The two routines this replaced each read the width before touching either edge. Consolidating them kept the shape of the code and lost the ordering that made it correct, which is the only thing either of them was careful about. Moved by translation now, which has no way to express the mistake: it takes a delta computed from the rect it is given and cannot change the size at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
buildLocationString was two jobs in one function: reading the region, the parcel and the agent, and arranging what it found into one of twelve strings. The first half needs an LLAgent to exist, which is why the second half had no test while all twelve of its branches were rewritten and its truncation rule changed from bytes to characters. Split, the second half is a function of its arguments and needs no mocks at all. ALAgentLocationFormat covers every shape against a named and an unnamed parcel, the separator that must not appear when a region has no rating, signed and four-digit coordinates, that the buffer is cleared rather than appended to -- including on the two branches that assign whole rather than going through the sink -- and that a landmark name is cut on a character boundary, which is the fix "%.100s" needed and the one thing about that change nothing else would have noticed. llagentui.h had to be made self-contained to be included from outside the viewer's precompiled header. It spelled everything in std::string, LLVector3 and S32 while including none of them. Also here, from the same sweep: llstatusbar.h forward-declared seven classes it does not name, llstatusbar.cpp defined five constants nothing reads, and refresh() built an LLRect it never used. Two things deliberately left alone. LLMenuBarGL::arrange already reshapes the bar to its content width, so the status bar's poll of the rightmost menu edge looks redundant -- but the only thing that marks the bar as needing arranging is the menu search filter, nothing outside the status bar reads the bar's width to notice if it went stale, and the poll is a walk that stops at the first visible item. It is commented now rather than removed. And updateClock's LLSD is not a container standing in for a struct: it is how the translatable format string from the XUI receives its datetime. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
drawChildren asked each visible child for its screen rect, and calcScreenRect answered by walking the parent chain to the root -- twice, once per corner, though both corners are offset from the screen by the same amount and the second walk was only there to add the width and height to a number it already had. The parent's own screen origin is the whole of what those walks recompute. A child's screen rect is its own rect plus the sum over its ancestors, and its ancestors are its parent and its parent's, so the origin is taken once per parent and each child's rect is translated by it. O(children x depth) becomes O(depth + children), in the function that is the whole of the UI's per-frame cull. calcScreenRect keeps the same shape for its other sixty-odd callers, minus one of the two walks, and moves its rect by translation rather than by assigning corners. localPointToScreen was copying an LLRect per level of the tree to read two of its fields. Checked rather than assumed, because this bypasses a call: calcScreenRect is not virtual and nowhere overridden. localPointToScreen is virtual, but all four of its overriders are LLMouseHandler implementations rather than LLViews and so cannot appear in a child list. And a child list only ever holds children of its owner -- addChild reparents before inserting, the reorder helpers guard on parentage and only reorder, removeChild nulls the pointer -- which the loop now asserts. Not measured. It cannot be slower, being strictly fewer operations for the same result, but what it is worth depends on the shape of the tree. One case is a small regression: a parent all of whose children are invisible now walks its own chain once where it did nothing before. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
getPeriodMedianPerSec collected one entry per period that had a duration, sorted, and then read buf[buf.size() / 2] unconditionally. When no period qualified that indexes an empty vector. It is reachable: a stat bar with show_median set on a count stat calls exactly this overload, and every period is skipped whenever the recording has not yet accumulated any elapsed time. The SampleAccumulator overload in the .cpp already returns 0 for that case; the template never learned to. The declared return type and the type the body constructed were spelled differently -- RelatedTypes<T::value_t>::fractional_t against RelatedTypes<T>::fractional_t -- and agreed only because the generic RelatedTypes falls back to F64. Named once, so they cannot drift. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
calc_num_rapid_changes resets time_since_value_changed when the sampled value moves and compares it against a threshold on the next move, but nothing ever added to it. It was zero at every comparison, so every change counted as rapid and the sample bars fell back to the mean far sooner than the ten-per-second rule says they should. The periods it walks carry the durations it needed, so it now accumulates them. The threshold it compared against was a local shadowing the file-scope one declared for this purpose; the shadow is gone and the documented constant is live for the first time. The rest of the pass over the stat widgets: calc_tick_value took logf of a magnitude that can be zero. That is -inf, llceil of it is not representable, and the negated result then indexed the digit loop. Guarded: a magnitude that small has one whole digit. calc_auto_scale_range derived its upper bound from the lower bound it had just overwritten. Both now come from the incoming pair. That leaves min at or below zero and max at or above it, which is what the first of its two RANGES loops tested the negation of -- it could never run, and is gone. drawTicks put its termination test at the end of the loop body, past a continue that skips ticks drawn too close together, so the test could be bypassed indefinitely. It is latched at the top now, and the walk carries its own bound for the case where the spacing is too small beside the values it steps through to advance them in F32 at all. Tick labels were formatted and measured for every tick. The label is only drawn when it clears the spacing rule, and its width is only read in the vertical layout; both moved to where they are used. setStat left the previous stat bound when a name resolved to nothing, so a bar could keep reporting a stat it no longer names. LLStatGraph never initialized mLabel, so its tooltip dropped the label half. It divided by an empty range, picked a threshold color by a normalization the fill height does not use, ran lower_bound over a vector nothing had sorted, and read past the end of an empty one. LLContainerView warned about hidden children and then laid out rows for them anyway; they reserve no row now. Its label row and the strip that toggles a section were fixed at 20 and 10 pixels while LLStatBar sizes its rows from the live monospace metrics -- both derive from the same metrics, which reproduces 20 exactly on the face those constants were chosen for. Formatting goes through fmt. The value line and tick labels are rebuilt every frame and the font takes a view, so they format into a stack buffer and never reach the allocator; LLStatGraph's tooltip stops building a printf spec to feed to a second printf. Profiling zones on the draw, layout and measurement paths, so the draw call count these widgets spend has something to be read against. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
gl_rect_2d opens with getTextureSlot(0)->unbind(), and unbind() flushes unconditionally. Nothing about a filled rect needs that, but it means the vertices the previous rect left buffered get drawn right then -- so every rect a stat bar draws ends up its own draw. begin()/end() would have batched them; the unbind between them is what stops it. A bar drawing its bar spends one on the background, one on the min/max span, one on the current or the history run, one on the mean line, one per tick mark, and two more around each tick label as the font atlas is bound and the next mark unbinds it -- plus two for the scissor, which flushes on the way in and again on the way out. floater_stats holds ninety of them. The geometry now goes into one begin(TRIANGLES)/end() through batchRect, which emits the same winding without touching the texture slot, and the strings are drawn once it closes. Tick labels are collected during the mark walk and drawn after it rather than between the marks. The colour rides on the vertices. That holds because the nested-UI pass runs under gUIProgram, whose vertex shader takes diffuse_color as an attribute; a program without it routes gGL.color4f to a uniform and the whole batch would come out one colour. Written down where the batch is built, because nothing else would catch it. The label and value move from before the bar to after it. They sit in the row reserved above the bar and do not overlap it, except where the widget is too short to give them separate rows -- and there, over reads better than under. The scissor is gone with them. It was doing real work: the min/max span, the current and mean lines, the history run and the tick marks are all derived from stat values and none of them were bounded by anything else. They are clamped to the widget now, per axis, so a rect handed its edges the other way round stays that way round. The two strings that could run past an edge take a pixel cap and ellipsize instead. Ordering across that boundary holds: the font atlas goes through the flushing bind path, so the batch is drawn with the white placeholder still on the slot before any glyph is bound. What this does not do is merge across bars. Each one still unbinds for its geometry and binds back for its strings, so it is two draws a bar rather than the dozen-odd it was. Merging further means draining every child's geometry before any child's text, which is a change to how the container draws its children, not to the bar. Runtime-verify owed, against UI Verts/Calls with floater_stats expanded. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
drawChildren decides a child is worth drawing on two tests: its screen rect meets the root view's rect, and it meets the dirty region. A view scrolled out of a scroll container fails neither. What stops it appearing is the container's scissor, and that applies to what it has already drawn. floater_stats puts ninety stat bars in a content view some two thousand pixels tall inside a window of about four hundred. Most of them are drawn in full and then thrown away by the scissor. LLView gains getChildCullRectScreen: a screen-space bound a view can add to those two tests when it knows something they cannot. It is asked once per parent, not once per child, and returns null -- no extra cull -- for everything that does not override it. LLContainerView overrides it with the content window of the scroll container it sits in. That container is found in postBuild rather than handed over. These views are built straight from XUI with no code in between to introduce them, which is why setScrollContainer has never had a caller anywhere in the tree and the two branches reading the member have never run. Only a container that a scroll container scrolls *directly* takes it. The branch in reshape that now runs sizes the view to the scroll window, which is what the scrolled view wants and would tell a nested section that the whole window's height is its minimum -- every section would fill the window. Both container_views in the tree are direct children of their scroll container, and every stat_view is nested inside one of those, so sections keep the sizing they had. What this does not reach: a section only partly in view still draws every bar it holds. Culling those means nested sections carrying a cull rect of their own, which is a different question from the one the member answers -- the container that scrolls me, against the container I am scrolled inside. Runtime-verify owed. The reshape branch has not run before, so the sizing of both stats floaters wants a look, scrollbar appearing and disappearing included. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
min, max and mean each walk every recorded period -- up to two hundred of them -- and every bar did all three on every frame. The line they feed has been rate limited to MEAN_VALUE_UPDATE_TIME for as long as that limit has existed. The walk ran anyway, and fifteen results out of sixteen were computed and thrown away. floater_stats holds ninety bars. Two cadences now. min and max place the bar's span and drive its auto-scaling, and the bar reads them every frame it is drawn. Sampling those at the line's rate would step the range where it currently eases, and would miss a spike that lands between samples -- the floating target snaps up to a new maximum the frame it appears. So they stay per-frame while the bar is shown. While it is not, nothing reads them at all and they are not computed: show_bar defaults off, which is most of the rows in floater_stats. Everything else follows the line. The mean, the median, and the count of rapid changes that decides which of the three a sample row displays. That decision is held between refreshes rather than retaken every frame; it turns on a count over a window a second wide, which does not move faster than the line it selects for. A row that is not changing fast still shows its live value frame by frame, as it did. The unit label was rebuilt on every draw, and for a count stat that meant concatenating "/s" onto the stat's name -- an allocation per bar per frame for a string that follows only from the stat and this widget's override. Built when the stat is bound. The value's width is measured to reserve room for the label beside it, and measuring is a shaping pass. The bytes it is taken from change on the refresh cadence, so the measurement is kept until they do. The string standing in for a value that is not a number was a function static resolved on the first draw of the first bar, which also put it out of reach of a language change. It belongs to the widget now. calcTickValue and calcAutoScaleRange become statics on the class. They were free functions with external linkage sitting in a .cpp, reachable only by someone willing to declare them. The three walks are still three. Folding them into one means either restating what PeriodicRecording means by each aggregate -- which differs per accumulator type and is exactly the kind of thing that drifts -- or adding a folded accessor beside the existing ones. After this and the scroll cull there is not much left in them to fold, so that wants a measurement before it is paid for. Runtime-verify owed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Ten cases over the two range helpers, which between them held two of the defects this pass fixed and are the only part of the bar that can be tested without a GL context. The one covering the logarithm of zero took three attempts to make real, and the two failures are worth the record. It first used a range centred on zero, which never reaches the bad path at all: the trigger is min == -(max - min) / divisor, and no such range is centred. Then it used -1 to 5, which does reach it -- 6 wide, first divisor 6, first tick at -1 + 1 -- and still passed against the unguarded code in the same 0.8 seconds. Unguarded, the digit count comes back as the bottom of S32 and negating it wraps to near the top, which is where the loop below starts counting from; but its first step finds min already whole and breaks before any of that matters. min has to be fractional for the walk to happen. -0.5 to 2.5 has both properties, and against the unguarded code the test took 11.97 seconds where it takes 0.83 with the guard. It still passed: those two billion steps arrive back at the divisor they started from, so both versions answer 0.5 and there is no wrong value to assert on. The only thing that separates them is the time, so the time is what is asserted -- loose by four orders of magnitude and still decisive. That case is the one the test exists for. The whole-numbered ranges beside it pass either way and are there to hold the boundary between the two. Verified in both directions: the assertion fails against the code before the guard and passes after it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
calcTickValue raises ten to its loop variable on every step of an inner loop it runs once per divisor, five of them. Every bar in floater_scene_load_stats reaches that every frame: all forty-nine provide exactly one of bar_min and bar_max, and one auto-scaled bound is the case that recomputes tick spacing here rather than taking it from the range scaler. floater_stats has two such bars out of ninety. The table is filled from pow itself, once, so an entry is the value the loop used to compute rather than one walked up by repeated multiplication. Tick spacing decides what numbers the bar prints beside itself, and the test for a good spacing is whether a candidate lands on a whole number -- a last-bit difference is exactly the kind that changes the answer, and not worth a multiply. Its range is what the search can ask for. The walk starts at one minus the count of whole digits in a float and runs to five, and a float has at most thirty-nine of them. Outside that the answer still comes from pow, which is also what answers calcAutoScaleRange's single call when its exponent is the bottom of S32. The two comments recording what the vertical layout constants used to be, before they were taken from the font, are gone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
arrange places its rows from the top down and reshaped each one on the way. LLView::reshape does nothing when neither dimension changed -- it guards on exactly that -- but a row that is itself a container overrides reshape and re-arranges its whole subtree without consulting the deltas. floater_stats puts fourteen sections in one container, so opening any of them shifted every section below it, and each of those re-laid all of its own bars for a move. Rows whose size did not change are no longer reshaped. Their contents sit in their own coordinates, so moving the row does not disturb them. The question has to be asked before setRect, which applies the new size itself and would leave nothing to compare against -- a guard placed after it fires every time and skips the relays that are needed. sForceReshape still puts every row through. The heights are asked for once rather than twice. Placing a row needs the total height before it can put the first one down, so two walks over the children is inherent; but getRequiredRect recurses the whole subtree under each child, and asking twice walked all of it twice. The rows are carried along with their heights rather than the child list being filtered a second time, so the placing walk cannot come to a different conclusion than the measuring one about which children are visible. Runtime-verify owed: resizing either stats floater, and opening and closing a section with other sections open below it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Both location readouts cache the rounded position their text was built from, and draw() rebuilds only when the agent's position has left that bucket. Both recorded the position before knowing whether the text would be written. The path that does not write it is a region crossing: buildLocationString finds no parcel to name and returns false, and rather than flash a placeholder the function keeps the string already on screen. The position had been recorded a dozen lines earlier, so draw() then believed the readout matched where the agent was standing, and stopped coming back. An agent that stops moving before the parcel arrives keeps the previous region's coordinates until it moves a full bucket -- 2 m at a walk, 4 m in flight. The position is read into locals now and recorded only once the string is known to be the one that position produces. The two exits that do write the text are unaffected; the one that does not now leaves the cache alone, which is what makes draw() come back. The navigation bar's plot was also measuring a different question than the gate it documents itself as sizing: it compared against the string this function last built, while the gate compares against what is in the field. Those part company exactly when something else put text there -- a clicked SLURL, or what the user typed before giving up focus -- which is the case the gate exists for. One local feeds both now. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Moving button state and layout out of draw() and onto an idle callback registered the callback only for editable bars -- a read-only bar has no button whose availability can change, so it seemed to have no reason to ask. But draw() had been doing two things, and only one of them was about button state. The other was updateLayoutAsNeeded, which every bar needs and which draw() called outside the !mReadOnly guard it kept around the first. So the read-only bars were left setting mNeedsLayout from reshape and createButtons with nothing in the process that would ever consume it. floater_toybox declares its toolbar read_only, and only survives because LLFloaterToybox populates it through addCommand, which lays out on the spot. Resizing it did not. Every bar registers now, and the callback keeps draw()'s division: states only for an editable bar, layout for all of them. Reached by way of a worse fix. Having reshape() call updateLayoutAsNeeded for read-only bars looks like the smaller change and is a stack overflow: updateLayoutAsNeeded ends by calling reshape on itself to size the bar to its buttons, and reshape sets mNeedsLayout before calling out, so the `if (!mNeedsLayout) return` at the top of updateLayoutAsNeeded can never be what ends it. It compiled and linked. Separately, getToolTip dereferenced mCommand where callIfEnabled and onCommit had both been given a null check for the same pointer. It reads null for a button whose command id is not in commands.xml. Guarded to match its two siblings. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
LL_PROFILE_PLOT's build-without-Tracy expansion was a pair of casts to void, one after the other and neither wrapped. Under an unbraced `if` the second runs whatever the condition said, and an `else` after it does not compile at all -- and the builds affected are the ones without Tracy, which is the configuration a developer looking at plots never has selected. Wrapped in do/while(0), as a macro standing where a statement stands has to be. LL_PROFILE_PLOT_CONFIG had the same shape. Latent: every call site today is inside braces. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The last tick label on a vertical bar is slid left so its end stays inside the bar, by a fraction of its own width. A label wider than the bar slides past the left edge doing it, and the scissor that used to cut the overhang is gone. The cap that replaced the scissor was measured from where the label asked to be rather than from where it can go, so a negative position asked for more room than the widget has and the glyphs ran out over the row beside it. Both ends are held now: the start is pulled back to the edge, and the cap is measured from there. Reachable with four decimal digits in a bar narrower than the number -- around ninety pixels of text in under ninety pixels of bar. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
setIcon asserted its index and then wrote the array either way. llassert is nothing in a release build, so what stood in front of a write into a fixed-size member was, where it matters, nothing. Every caller passes an enum constant today. The class exists to be filled by two panels with different subsets of the icons, which is the shape that eventually gets a computed index. Refused and returned instead, with the assert kept for the build that can still say so. layout() also drops its const. It was true only of the array of pointers: everything they point at is moved, which is the whole of what the function does. A reader takes const on a function returning an edge to mean it worked one out without disturbing anything. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
findHelpTopic walks a control's ancestors looking for a panel that owns a help topic, and at each one searches that panel for a sub-panel or an open tab carrying one. The search was written beginTreeBFS(), unqualified, which is this->beginTreeBFS() -- the control the walk started from, not the ancestor it has reached. So the first turn of the loop was right by coincidence, ctrl being this, and every turn after it re-searched the same subtree and reached the same answer. An ancestor panel's own sub-panels and tabs were never looked at; only its own help topic, tested further down, could ever answer for it. The inner LLPanel* shadowed the outer one it is compared against, which is what let the wrong receiver read as deliberate. Named apart. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
focusFirstItem ran the tab order query, returned if it found anything, and then ran two more queries over the same tree in the same frame: one with a text-input prefilter, and one identical to the first. Neither could ever answer. A prefilter narrows a query, so the text-input pass returns a subset of the pass that just came back empty, and the third pass is the first one written out again. Reaching either means the tree holds no tab stop, and the two further walks of every view under the control confirmed that at length. The preference the parameter names cannot be honoured by running the general query first, and reordering it to run the text pass first would move focus in every panel that has both. Left as it behaves; the parameter stays for its callers. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
removeChild's updateBoundingRect sat below the branch, so the path that only warns that the view is not a child of this one still recomputed the bounding rect and, through it, claimed a screen region to repaint. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
focusNext takes a control only if it has a tab stop. focusPrev took any control at all. Through focusNextItem and focusPrevItem the difference is invisible: the tab order query prefilters on tab stop, so both are handed a list that already has none of the others in it. focusPrevRoot runs the focus roots query instead, which filters on isFocusRoot and nothing else -- so Ctrl-Shift-Tab could land on a focus root that Ctrl-Tab would step over. Both ask for a tab stop now. Floaters, the roots this actually cycles, carry one by default, so the cycle they walk is unchanged. The cast beside it stops being a C cast while the line is being read. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Both places that ask whether a view is in sPreviewHighlightedElements ran std::find across it, walking the tree in order to reach an answer the container reaches by descending it. The floater's is a find followed by an insert if it was absent, which is what insert does on its own. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
sSelectID is declared in the class and defined nowhere, so anything that tried to use it would not link. GL_NAME_UI_RESERVED names a GL select buffer reservation from a picking scheme the UI no longer has. lluistring.h is included twice, two lines apart. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
updateMouseEventInfo drops everything it is given on its first line when logging is off, which it is in every session that is not being recorded. Seven call sites built its last argument with getPathname(), which recurses to the root of the view tree assembling a string through an ostringstream, on every mouse down and up the UI handles. It takes the view now and builds the path itself, on the far side of that first line. The same reasoning was already written down beside logVisibilityChange, which had the same shape; a signature that cannot be called eagerly says it better than a comment asking callers not to. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
visibleAndContains is the hit test. It runs once per child per level of the view tree for every mouse move, hover, tooltip and drag the window sees, and it asked its question through sDrilldown -- a std::function holding a boost::bind of pointInView, installed as a default so that a test could replace it. sDrilldown is empty now and the default answer is the call itself. A test installing one still gets asked first; nothing else pays an indirect call to reach a rect comparison. The two tests also swap. Visibility is a bool read where containment is four comparisons against a rect that may be a bounding rect of a whole subtree, and most of the children of a large tree are hidden -- an open inventory keeps the items of its closed folders that way. Both are pure, so the order does not change the answer. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
postBuild asks every child of every panel the viewer builds whether it is a control, and asked with dynamic_cast, which walks the RTTI graph to reach the answer isCtrl returns as a constant from a vtable slot. Pairing isCtrl with static_cast is how the rest of llui asks it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The view classes in llui that nothing in the tree derives from -- searched across every header, source and test in llui and newview -- are final, as newview's were. That says what each is, and lets the compiler resolve a call on one without a table. The classes newview builds on, and the abstract ones, stay open. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The walk that attaches a badge to the nearest holder asked whether each ancestor was a panel, which is true of every holder in the tree and of nothing the compiler could hold anyone to. LLView answers the question itself now: asBadgeHolder() is null on a view and the panel on a panel, so a second holder has an override to write, and the walk asks nothing else. The twenty-six casts in Alchemy's own floaters and panels ask as<T>() like the rest of the UI: parent lookups through getParentAs<T>(), pointers that may be null through the null-safe static, a child looked up by name as the panel it is registered as and then asked -- getChild<T>() must be able to build a dummy T, and a class without a parameter-block constructor cannot be one -- and a list item through the typed lookup the list already had. Two of them were not asking a view's kind at all but for an interface two panels mix in, which no kind can answer and as<T>() refuses; the interface answers for itself now, by asking each of its implementers by type, which is the whole of the RTTI walk without the names. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Thirty-seven of the viewer's casts asked whether a view was a control, a folder or a floater, and every one took the RTTI walk to find out. They ask as<T>() now, as llui does. Nine asked the focus manager for the focused element and cast it to a control; the manager keeps the focused view and answers that question itself, so they take its answer. Four found a folder by asking the inventory panel for an item by id and casting; the panel has had getFolderByID() all along, which is those two steps, and it answers with one compare now. Four walked the parents looking for a floater: the URL handler's walk was getParentByType<LLFloater>() and is written as that, and the three with work to do inside the loop keep the loop and ask each parent by type. The rest ask a live view directly, or a pointer that may be null through the null-safe static. Two casts remain in text that does not compile, one in a comment block and one under #if 0. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Dragging a floater dropped the framerate hard, and Tracy put the time in LLUIImage::genDisplayList. The display-list key carried the image's screen position and the UI translation, so every image in a moving floater missed the cache every frame. Each miss recorded a fresh vertex buffer, and the stale recordings sat out the two-second timeout before being torn down eight images per frame. The recording no longer knows where it was made. It is captured at the origin under the UI scale alone and placed through the modelview at draw time, the way ALTextTransform already places text, so one recording serves an image wherever it appears. The key keeps size, colour, the solid flag, UI scale and the texture name; its hash mixes those instead of xoring fields that share bit positions. Last-used stamps come from LLFrameTimer::getTotalSeconds() rather than a clock read on every hit. At a fractional UI scale the nine-slice centre rect is now rounded relative to the image rather than the screen, so a seam may sit a sub-pixel from where it did. The outer edges were never snapped. The new llrender test pins that a second position or UI offset replays the first recording, and that both the plain-quad and nine-slice recordings span exactly the image's size from the origin. Reading those vertices back goes through glGetBufferSubData: on this platform the upload streams straight to GL and never fills the CPU-side copy. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Forty-five casts to four view types ask as<T>() now. The side-panel container's typed getPanel<T>() and findPanel<T>() went first, since they are where seven of the appearance lookups belong: each spelled the container's untyped accessor and a cast by hand, and each is the typed accessor now, whose warning on a wrong type is the one check those sites lacked. The conversation container's session lookups and the outfit lists' wearable-list lookups go through the null-safe static, since a map value, an accordion's content view and a callback's control may each be absent. One pair was not asking a kind at all: the media control chose a parent for its context menu with two upward casts, which are conversions the language makes on its own, and they are written as the conversion they are. The UI preview's container test asks the same way for the layout stack beside the panel, since one expression does not want two idioms. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Seventy-five casts to twelve view types ask as<T>() now. Six profile lookups spelled the floater registry's untyped show or find and a cast by hand, and are the registry's typed twins. The alert toast and the window asked the focus manager for an element or a mouse handler and cast it to a view; the manager keeps both views and answers directly. The spellcheck menu handlers walk to their context menu through getParentAs<T>(), the side-tray panels to their container the same way, and the inventory floaters, notification buttons and favorites buttons ask each child as it is visited. The chiclet panel compared a child to its scroll area through an upward cast, which is no cast at all. Seven view classes defined in source files rather than headers had no declaration, since the sweep that gave every header its line did not read sources: the emoji picker's grid row, divider, icon and preview panel, and the favorites bar's button, menu item and menu. Each declares its place now and is final, which is what let the grid and the bar ask for them at all. One cast to LLView stays, in the search floater: it asks whether the edit-menu handler is a view, and two things that are not views answer that interface, so the question is the interface's to answer, not a kind's. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The sweep that gave every newview view class its declaration read headers, so the forty-nine defined in sources or nested inside another class -- the inspectors, the snapshot panels, the texture-view bars, the local-asset tabs, the legacy profile's tabs, the expandable text box's editor -- answered as their nearest declared base and could not be asked for by name. Each declares its place now, and each that nothing derives from is final. The two local-asset bases stay open, since their tabs derive from them, and the apply-asset base is abstract besides. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The search floater asked whether the global edit-menu handler was a view by cross-casting it, and two things that are not views -- the selection manager and the land brush -- answer that interface, so the question was a real one and RTTI was the only thing answering it. asView() on the interface is pure virtual now: a view implementer returns itself, a manager returns null, and a new implementer has to say which it is before it compiles. The floater asks the handler. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Seventy-six casts to seventeen view types ask as<T>() now, and the null-safe static gained a const overload, since the list comparators take their items as const panels. Four inventory lookups that found the side panel by name and cast it use findChild<T>(), which asks the same question inside; three outfit-edit lookups use the container's typed accessor; the toolbars find their layout panel through getParentAs<T>(). Selected list items, tab panels, handles and toast panels go through the static, since each may be absent; children visited in a loop, a floater just created and a floater already checked are asked directly. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Twenty-three casts to seven view types ask as<T>() now. The poser and preferences lookups spelled the floater registry's untyped show or find and a cast, and are the registry's typed twins. The day-cycle editor found three sky panels by name and cast each, and uses findChild<T>(), which asks inside. The group panels find their hosting floater through getParentAs<T>(). A session tab's host, a search floater's editor handle and a text segment's target are asked as they are: the first two may be absent and go through the static, the last is a reference and is asked directly. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Fifty-five casts to twenty-eight view types ask as<T>() now, two sites apiece but for the avatar render settings floater, whose source CMake does not build and which is left as it is. The shapes are the ones the earlier batches settled: registry and side-panel lookups through their typed accessors, a parent through getParentAs<T>(), a child visited in a loop or an instance iterated from the registry asked directly, and a handle, a tab panel, a selected item or a callback's control through the null-safe static. Two sites were not quite that. The translation settings floater was handed a focusable element and cast it to a line editor, which is a cross-cast; it asks the element for its view first, then asks the view. The teleport history panel asked a child whether it was an accordion tab and then C-cast it regardless; one question now yields the typed pointer. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Forty-eight casts to forty-eight view types ask as<T>() now, one site each. What is left of the viewer's view-typed casts after this is the notification channel family -- the screen channel, the IM chiclet and the notification list item -- and two sites in text the compiler never sees. The shapes are the settled ones. Three are worth a word. The alert handler cast its own freshly built alert panel up to a toast panel, which is a conversion the language makes unasked, so it is an assignment. The teleport history tab asked a child whether it was a flat list and then C-cast it; one question yields the pointer. And the main inventory's incremental find walks the inventory floaters asking each whether it is the main inventory panel, which none of them is, since they are the side-panel containers that hold it; the question is asked the same way it was and answers the same way, which is never. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
LLPanelMainInventory::incrementalFind walked the inventory floater list asking each floater whether it was the main inventory panel, which none of them is: the floater is the side-panel container that holds the panel. It has been so since the 2009 inventory side-panel merge ported the function from the old inventory floater, whose registry entries were the panel, and nothing has called it since -- no C++ caller, no XUI command. A search that returns false before it searches is not worth repairing for a caller that does not exist. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Twenty casts in the notification channel family ask as<T>() now: the screen channel from the channel manager, the toast handlers, the toast inspector, the script floater and persistent storage; the IM chiclet from the chiclet panel's walks; and the notification list item from the tabbed notifications floater. A channel is reached through a handle or a lookup by id and may be absent, so those go through the null-safe static; the chiclets and list items are visited in loops over live children and are asked directly. With these, no dynamic_cast in the viewer names a view, save two in text the compiler never sees. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Three casts to the experience list item, missed by every count that decided view-ness from a name: the selected item, asked directly since the list already checked it, and the two items a comparator takes as const panels, through the null-safe static. The count that found them decides by declaration instead -- a target with an AL_VIEW_TYPE line is a view -- and it puts the viewer's remaining view-typed casts at three, all in text the compiler never sees. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The scroll-list cells are not views, so the ancestor table on LLView could not answer for them, and thirteen casts in the viewer kept asking RTTI whether a column was text, a check box or a bar. The cell hierarchy is eight classes two deep, which is the case the table was made for: it keeps the same table now, each cell class declares its place, the seven leaves are final, and a cell answers as<T>() in one compare. Since every one of the thirteen took a column from an item and asked, the item asks for it: getColumn<T>(i) is the column as a T, or null when the column is absent or another kind. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… past it processObjectUpdate copied at most 2048 bytes of a compressed object block and then told the packer the block was as long as the sim had declared. The reader clamps that declaration to the packet, so the bound is MAX_BUFFER_SIZE, and a well-formed block of 2049 to 8180 bytes made every downstream bounds check trust a size the stack buffer did not have: the unpack calls read past it, and cacheFullUpdate copied up to six kilobytes of adjacent stack into a heap entry that was then written to the object cache on disk. A missing block returned a negative sentinel that reached the same path as a size. The buffer is now the reader's own bound, the three places that spelled 2048 take it from sizeof, and a block that still does not fit -- or has no size at all -- is skipped with a warning instead of decoded. The object cache version moves from 18 to 19 so that an entry already written with stack bytes in it is discarded rather than reloaded. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Contributor
|
Important Review skippedToo many files! This PR contains 688 files, which is 588 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. Usage-priced reviews support at most 300 files. ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (688)
You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
c/cpp
llrender
llui
llcommon
cmake
llcorehttp
viewer
skins
labels
LLFontTextCache cannot compare the text it is handed, so the caller names it through setSource and the cache fingerprints it. LLLineEditor named it in exactly one place -- textWidth -- and never on the path that draws. A password field swaps mText for bullets at the top of draw() and puts the real text back at the end, so the two paths are about two different strings. setCursor runs outside draw() and measures through the pre-selection buffer, fingerprinting the password; the next draw() renders the bullets through that same buffer without naming them, and the fingerprint check fires. That check is not decoration: the width slots are keyed on the span rather than the bytes, so a cache holding one of those two strings answers for the span of the other. Name the source once, on all three buffers, before anything is drawn. Widths asked for later in the same draw name it identically and so cost nothing, and the version bump the swap carries invalidates the buffers on the way in and on the way out. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Description
Related Issues
Issue Link:
Checklist
Please ensure the following before requesting review:
Additional Notes