Conversation
…sizing Pulls together the four architecture points from the GUI review: - Hit-testing & input routing: Widget gains HitTest/GetChildCount/GetChildAt (ForEachChild is now built on these, non-virtual) and handlers return SInputReply instead of void, so Manager can bubble a press/release/move along the actual top-to-bottom hit path with mouse capture, instead of broadcasting to every widget under the cursor. EVisibility grows HitTestInvisible/SelfHitTestInvisible/Collapsed alongside Visible/Hidden. - Measure pass: ComputeDesiredSize now takes the available space and is driven through a cached, non-virtual Widget::Measure, so containers no longer re-measure clean subtrees on every arrange. TextBlock/FontManager gain wrap-aware measurement (ETextOverflow, MeasureWrapped). - Z-order runs: RenderBatch::Sort groups commands into per-type SBatchRun ranges after sorting by ZOrder, and Renderer walks them in that order (SDrawItem/AppendRange/firstInstance) instead of one pass per command type, so quads and text interleave correctly by z instead of text always drawing on top. - Typed slot sizing: LayoutSlot's m_FillRatio becomes SSizeParam (Auto/Fill/Fixed) with corrected space distribution (fixed space first, remaining space split by fill ratio), EHorizontalAlignment/ EVerticalAlignment gain Fill for the cross axis, and Panel/TPanel<TSlot> replace the untyped Ref<Slot> vector + static_pointer_cast in every container with a typed slot list (TPanel<LayoutSlot>/TPanel<CanvasSlot>, explicitly instantiated and exported so client code can derive from them across the DLL boundary). Test fixtures (WidgetTestUtils, DrawCacheTest, ForEachChildTest, InvalidationTest, DirtyTrackingTest, WidgetLifetimeTest) updated to match: new ComputeDesiredSize signature, TPanel<LayoutSlot> instead of hand-rolled Panel subclasses, GetSlotCount() instead of GetSlots(), and slot-level SetFillSize() instead of the removed per-panel SetStretching().
- Clip stack: Widget::ClipsChildren (default false) and a clipRect param threaded through CollectDrawCommands, intersected at RenderBatch::Append time rather than baked into BuildDrawCommands - so scrolling a ScrollBox doesn't have to invalidate every descendant's cached draw commands, only reassemble the frame. SRect::Intersect added to Definitions.h as the supporting primitive. - ScrollBox (new): a ContentWidget that clips its content, measures it unconstrained on the scrolling axis/axes and capped to the viewport on the other, clamps the scroll offset to content-minus-viewport, and draws an optional track+thumb scrollbar. Routes MouseScrolledEvent through the same SInputReply convention as the rest of the input handlers. - Popup layers: Manager gains a layer stack (SLayer) above the always- present root layer - PushPopup/PopPopup/ClearPopups, hit-tested and dismissed independently (click outside closes it), arranged and rendered through the same per-subtree z-banding as everything else so a popup always ends up on top without a magic z offset. A layer-stack version counter covers the case a freshly opened popup starts dirty by construction without ever calling MarkLayoutDirty.
The if-init-statement's condition was `++i`, incrementing the loop counter a second time (the for-loop already does it) and using the post-increment's truthiness as the guard instead of checking whether GetChildAt actually returned a widget. Net effect: every other child was skipped, and a null child would never have been filtered anyway. Drop the second increment and check the child itself.
- ClipStackTest: clip only propagates from a ClipsChildren() container, nested clips intersect rather than replace, an inherited clip narrows an existing ad-hoc scissor instead of overwriting it, and a direct regression test that ScrollBox actually scissors oversized content end to end via AssembleFrame (ClipsChildren() on it was silently false at one point, with nothing catching it). - ScrollBoxTest: desired size never exceeds the configured viewport but shrinks to smaller content, LayoutChildren offsets content by the scroll offset, SetScrollOffset clamps both directions, and HandleMouseScrolled moves the offset within bounds (Handled) or defers to an ancestor at the edge (Unhandled). - PopupLayerTest: push/pop/clear track the popup count correctly (including no-op pop on an empty stack), a freshly pushed popup is arranged immediately against the last known extent, its draw commands sort strictly above the root layer's after AssembleFrame - the core guarantee of the popup stack - and a clip on one layer doesn't leak into another. - WidgetLifetimeTest: RemoveChild is protected on Widget/Panel with no outside production caller; the one test exercising it now drives it through a local TPanel<LayoutSlot> test double that promotes it via `using`, the same idiom ForEachChildTest already uses, instead of relying on VerticalBox exposing it publicly.
…ren does VerticalBox/HorizontalBox::ComputeDesiredSize summed each child's raw Measure() result along the main axis regardless of its slot's sizing rule, while LayoutChildren correctly used the slot's configured pixel size for Fixed children and gave Fill children no intrinsic contribution. A Fixed child whose content measured smaller than its configured size made the container under-report its own desired size. Found by live-debugging a real symptom: a ScrollBox wrapping a VerticalBox of fixed-height rows measured the list as shorter than its actual laid-out height, concluded nothing overflowed the viewport, and silently refused to scroll or show a scrollbar - even though the rows were visibly clipped. Fix: ComputeDesiredSize now switches on GetSizeRule() the same way LayoutChildren already does. Regression tests confirm the old code was wrong (3 of 4 new tests fail against it) and pass against the fix.
… space - TPanel<TSlot>::AddChild is now virtual, so a concrete container (Canvas) can override it instead of just hiding the base via name lookup. - Canvas/ScrollBox's configured-size setter is now SetSize on both (was SetDesiredSize), matching the rest of the widget setter naming; backing fields renamed to m_Size for the same reason. Canvas's default changed from 800x600 to 100x100. - Canvas::ComputeDesiredSize now clamps to glm::min(m_Size, availableSize) instead of returning m_Size unconditionally - Canvas was the one container that never checked whether the parent actually had room, unlike ScrollBox which already followed this rule. Test fixtures and the Editor's example dropdown menu updated for the renamed setter; CanvasTest gains coverage for the default size, the setter (including the same-value no-invalidate guard), and the new available-space clamp - confirmed the clamp test fails without the fix.
…etry Outline previously expanded past the widget's own m_Geometry in the vertex shader and drew in the [0, thickness] band outside the shape, so two Fill-sized siblings with zero gap between them would have their borders bleed into each other. Flip the outline band to [-thickness, 0] (inside the shape) and draw it after the fill/inset-shadow so it sits on top like a real border, and stop expanding the quad for it in the vertex shader - only drop shadow still needs that. Widget::Measure now budgets outline thickness into the desired size it reports, so a widget that wants a 10x10 content box with a 1px outline actually occupies 12x12 when auto-sized, instead of the outline eating into its own content.
Content was always measured and arranged at the ScrollBox's full width/height, never leaving room for the scrollbar it draws over the edge - so content with a background reaching that edge (e.g. a section header) rendered on top of the scrollbar instead of under it, and z-order meant it always would, since scrolled content is a child and children draw above their parent's own commands. Add ScrollBox::CrossAxisSpace, subtracting the scrollbar's thickness from whichever axis it actually occupies whenever m_ShowScrollbar is set, and use it in both ContentMeasureConstraint (what content thinks it has) and LayoutChildren (what it's actually arranged into) - the second one was missing before, so even content that measured itself narrower was stretched back out to the full width at arrange time.
m_DPIScale was stored but never read - QuadRenderPass and TextRenderPass both scale cmd.Geometry.Position/Size by it before building vertex data, but DebugRenderPass used the raw logical-point geometry, so a debug rect (e.g. the focus ring) landed at half its intended screen position on a 2x display.
Widget gains an IsFocusable/SetFocusable flag, independent of mouse- click focus (Manager::ProcessMousePress already focuses whatever widget consumes a mouse-down regardless of this flag) - it only controls membership in the keyboard-navigable Tab order. TextField opts itself in, since it already reacts visibly to focus. Manager::HandleKeyPressed intercepts Tab/Shift+Tab and Escape before bubbling to the focused widget, not after: TextField::HandleKeyPressed unconditionally returns Handled() for every key code, so a focused field would swallow Tab silently forever if this checked the bubble result first. FocusNext/FocusPrevious walk a lazily-rebuilt, depth- first focus order cached behind the same dirty-epoch + layer-stack- version key NeedsRebuild already uses, scoped to the topmost popup layer so Tab never escapes an open popup into whatever is underneath it - and deliberately don't clear focus when that scoped order is empty, so a popup with no focusable content of its own can't silently steal focus from the layer below. The focus ring itself reuses AddDebugRect's existing DEBUG_Z_ORDER sentinel (always-on-top, immune to any ScrollBox/popup clip in effect) rather than inventing a second z-layering mechanism alongside the popup stack's. (Widget::SetFocusable's definition landed in the prior outline-fix commit by accident - a `git commit <pathspec>` pathspec commits the working tree content of that path, not what was staged via `git apply --cached`, so it pulled in more than intended. No functional impact, just a messier split than planned.) Promotes SetFocusedWidget/ProcessMousePress/HandleKeyPressed from private to protected on Manager so tests can drive them directly, matching the existing AssembleFrame/NeedsRebuild/MarkRebuilt pattern in ManagerTestUtils.h, and adds FocusTest.cpp covering Tab ordering, wrap-around both directions, Escape, popup scoping (both directions - staying inside an open popup, and not leaking out when it has nothing focusable), and the pre-existing click-outside-clears-focus behavior as a regression guard.
Focus visualization is a per-widget concern, not something the Manager should impose uniformly: a widget that wants to show its own focus state already has everything it needs (IsFocused(), already invalidated on change via HandleFocus/HandleLostFocus calling MarkRenderDirty) to render it inside its own BuildDrawCommands, the same way Button already reacts to m_Hovered - color swap, outline, background texture, whatever fits the widget. No opt-in/opt-out flag on Widget either: a widget that doesn't render anything for its own focused state simply has no focus visual, rather than the base class carrying a default every widget has to actively suppress.
Splits the background texture into m_NormalBackground/ m_FocusedBackground and adds a separate m_FocusedOutline, then BuildDrawCommands picks between them based on m_Focused - the same per-widget pattern Button already uses for m_Hovered, and the first real consumer of the "no default focus visual" principle from the prior commit: TextField opts in by drawing something specific for its own focused state, texture or outline, instead of relying on any Manager-level fallback.
…sabled) Adds StyleSet on the Widget base: four style layers (Normal < Hovered < Pressed < Disabled) composed field-by-field, so a later active layer only overrides what it explicitly declares - a Pressed override that changes just the color still shows whatever Hovered or Normal set for texture, borders, corner radius, etc. BackgroundTexture uses optional<Ref<Texture2D>> specifically so a layer can distinguish "I didn't touch this" from "clear the inherited texture" (a non-null but empty Ref). GetInteractionState()/GetResolvedStyle() live on Widget as protected, built from the widget's own IsHovered/IsPressed/IsEnabled - available to any concrete widget's BuildDrawCommands, not just Button. The mutators (SetStyle/ClearStyle/SetBackgroundColor/SetForegroundColor/ SetBackgroundTexture/ClearBackgroundTexture) are public on Widget, following the same precedent as SetOutline/SetInsetShadow/ SetDropShadow already being public and simply inert on widgets that never draw with them. Widget also gains IsEnabled/SetEnabled: disabling marks render-dirty, cancels an in-progress press, and blocks HandleMouseDown/HandleClick - without touching EVisibility or layout. Outline/InsetShadow/DropShadow stay single-value widget properties (via the pre-existing SetOutline/SetInsetShadow/SetDropShadow) rather than becoming per-layer overrides: GetResolvedStyle() overlays them from the widget's own getters after resolving, so they don't vary by interaction state. Avoids a second source of truth for the same value, at the cost of those three fields in SStyleOverride never actually taking effect if set through SetStyle directly. Migrates Button off its old m_NormalColor/m_HoverColor/ m_NormalBackground fields onto this API, keeping only its own thin wrappers (SetCornerRadius, SetBackgroundBorders, SetTextColor) on top of the inherited one; no compatibility shims kept, since the only three call sites (Application.cpp) get updated directly in the same change. Button::HandleMouseDown also gains the IsEnabled() check it was missing before. Adds StyleTest.cpp: StyleSet::Resolve in isolation (Normal-only, partial Hovered override, Pressed-over-Hovered, Disabled-over-both, Disabled falling back when it declares nothing, explicit texture clear, an inactive layer never leaking into the result) plus Widget's public surface (style/hover/press/enabled changes mark render-dirty; disabling blocks input that would otherwise be handled).
…errides GetResolvedStyle() previously overlaid Outline/InsetShadow/DropShadow from Widget's own single-value getters after resolving m_Styles, so those three SStyleOverride fields never actually took effect - setting them through SetStyle/SetOutline/SetInsetShadow/SetDropShadow was a silent no-op. Removes the single-value m_Outline/m_InsetShadow/ m_DropShadow fields and their old setters/getters entirely; every setter (SetOutline, SetOutlineColor, SetOutlineThickness, SetInsetShadow and its per-component variants, SetDropShadow and its per-component variants) now takes an EStyleLayer, reads the layer's current override, patches just the one field it owns, and writes it back - so a widget can genuinely give Hovered a different outline than Normal. Also adds SetCornerRadius/SetBackgroundBorders directly on Widget for the same reason Button no longer needs its own copies of them. Migrates the only call sites (Application.cpp, Button's own SetTextColor wrapper) to the new per-layer signatures.
Both had their own single-value background/corner-radius/text-color fields, parallel to (and now stale next to) the base Widget's StyleSet - Panel::BuildDrawCommands and TextField::BuildDrawCommands were still reading m_Outline/m_InsetShadow/m_DropShadow directly, which the previous commit emptied out entirely (those became per- layer-only), so any Outline set through the new API on a Canvas, Overlay or TextField was silently never drawn. Panel (base of Canvas/Overlay/HorizontalBox/VerticalBox) drops its own m_Background/m_CornerRadius and their accessors; BuildDrawCommands now resolves through GetResolvedStyle() like Button already does. TextField drops m_TextColor/m_BackgroundColor/m_CornerRadius/ m_BackgroundBorders/m_NormalBackground and their setters, resolving the same way for its Normal/Hovered/Pressed/Disabled state; keeps m_FocusedBackground/m_FocusedOutline as its own fields, since Focused isn't one of StyleSet's four layers (see EStyleLayer) - the resolved style still supplies background color/corner radius/borders/shadows even while focused, only the texture/outline branch differs. Keeps a thin SetTextColor(layer, color) wrapper renaming SetForegroundColor, matching Button's own precedent. Fixes the three existing tests that called the old single-argument Widget::SetBackground.
Precedence becomes Normal < Hovered < Pressed < Focused < Disabled -
Disabled still wins even over a widget that stays focused after being
disabled (nothing clears focus just because IsEnabled() went false).
Widget::GetInteractionState() now includes IsFocused().
TextField drops m_FocusedBackground/m_FocusedOutline and their setters
entirely: the constructor sets a default outline on the Focused layer
via SetStyle the same way it already does for Normal, and
BuildDrawCommands loses its whole if (m_Focused) {...} else {...}
branch - GetResolvedStyle() already picks the right layer on its own.
Background color, corner radius, borders and shadows stay whatever
Normal/Hovered/Pressed resolved to while focused, since Focused's
default only declares Outline.
Widget::SetEnabled(false) has no way to clear Manager::m_FocusedWidget - Widget has no back-reference to the Manager - so a widget disabled while focused stays focused, just Disabled-styled. That left two real gaps in its "stops accepting input events" contract: - Manager::HandleKeyPressed/HandleKeyTyped kept bubbling from m_FocusedWidget regardless of IsEnabled(), so a disabled TextField that was focused before being disabled would keep taking keystrokes. - Manager::CollectFocusOrder never checked IsEnabled(), so Tab could land inside a disabled widget even though a mouse click already can't (Widget::HandleMouseDown's own IsEnabled() check). Both now guard on IsEnabled(). FocusNext/FocusPrevious already handle the resulting case correctly for free: once a focused widget drops out of the (now enabled-only) order, they treat it as "not in the order" and jump to the first entry instead of stepping from a stale position. Verified both new FocusTest cases actually catch the bug: reverted the Manager.cpp guards, confirmed both failed, restored the fix.
…alette Give Button and TextField a real default visual identity across every StyleSet layer (Normal/Hovered/Focused/Disabled) instead of placeholder dev colors, matching chakra-ui.com's "subtle" zinc button. Disabled uses SColor alpha directly since the engine has no render-blended opacity. Panel is intentionally left untouched - it backs Canvas/Overlay/H-V Box as transparent layout scaffolding across the Editor.
Fix GUI typed style tests
Fix/fixes gui
Fix/fixes gui
Fix/fixes gui
Fix/fixes gui
Feature/editor gui
|
Important Review skippedToo many files! This PR contains 139 files, which is 39 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. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (10)
📒 Files selected for processing (139)
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 |
💡 Codex Review Apply the simulated particle color in particle shaders
When an emitter uses  Define Fresnel inputs for particle shader permutations
When a material containing Widen scalar inputs before applying a component mask
When Elixir/Shaders/Material/Material.ps.hlsl Lines 123 to 127 in f2be3cc Use the graph-provided normal for surface lighting
When a graph drives Elixir/Elixir/Source/Engine/Core/Application.h Lines 38 to 42 in f2be3cc Document the new public application hooks
The newly introduced public AGENTS.md reference: AGENTS.md:L9-L9 Elixir/Elixir/Source/Engine/Material/MaterialSystem.cpp Lines 81 to 84 in f2be3cc Keep material frame data separate for in-flight frames
When two frames are in flight and their material values differ, this overwrites the one persistently mapped Elixir/Elixir/Source/Engine/Material/MaterialSystem.cpp Lines 74 to 76 in f2be3cc Handle material-table overflow before building the snapshot
When a scene contains more unique material proxies than Elixir/Elixir/Source/Engine/Material/MaterialGraph.cpp Lines 74 to 76 in f2be3cc Validate graph connections and channel roots
When an editor or deserializer supplies a stale source ID, a missing channel node, or a cycle, this validation checks only each node's parameter references and returns success. Apply authored textures to ribbon and mesh materials
When an effect-authored ribbon or mesh material includes a Bundle or remove the runtime DXC dependency
When a built application runs on a machine without the Vulkan SDK or Elixir/Elixir/Source/Engine/Aether/Emitter.cpp Lines 64 to 68 in f2be3cc Reject systems whose emitter material cannot render its mode
When an emitter is assigned a material that fails compilation or does not support its selected render mode, this only logs an error and leaves ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
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
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.
Summary
Testing