Implement runtime UI workflow and editor integration - #819
Conversation
adriengivry
left a comment
adriengivry
left a comment
There was a problem hiding this comment.
Found a bunch of issues that I reported to you directly on Discord.
Didn't dive into the code yet, as it's subject to change.
Some design issues to figure out:
Transformshould be "adaptive", depending on the context (2D/3D).- When in 2D mode, width/height/anchor preset/anchor pivot should be displayed
- When in 3D mode, nothing should change from what was previously shown
- There should be no need for an extra component, the transform should store 2D-related properties under an
std::optionaldata struct.
- Font rendering: currently fonts are rasterized with a fixed font size, so scaling a font will lead to blurry text. This could be fixed by implemented either SDF or dynamic glyph rendering. Dynamic glyph might be easier for now.
CText::m_textshould be displayed as a multiline string. Right now it's not possible to add line breaks in texts.
Some bugs I found so far:
- Anchor presets not working properly (partially addressed, the bounds in scene view are rendering incorrectly)
- Constant-size/scale with screen size issue, if I set the Canvas to 1920x1080, and add an image of size 1920x1080, it doesn't exactly fit the screen. When using scale with screen size (which should be default), we should be able to choose when the ratio is mismatching, to either match Width or Height, shrink, or expand. (see Unity documentation)
- When using multiple Canvas, composition order should be based on their position in the hierarchy.
|
Maybe we should also document that you need to add a canvas (add a message)? |
When we gray out the Anchored Position, we could also disable the gizmo
|
bool OvCore::ECS::Components::CTransform::IsHorizontalUIPositionEditable() const
{
return IsHorizontalUIPositionEditable(GetUIAnchorPreset()) && !OvCore::ECS::Components::UI::UITransformResolver::IsDrivenByLayout(owner);
}
bool OvCore::ECS::Components::CTransform::IsVerticalUIPositionEditable() const
{
return IsVerticalUIPositionEditable(GetUIAnchorPreset()) && !OvCore::ECS::Components::UI::UITransformResolver::IsDrivenByLayout(owner);
}Isn't enough here as it would block us from scaling too, maybe we would also add some logic here |
The horizontal layout also doesn't count the scale
|
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
All four points are fixed : canvas warning in the inspector, translate gizmo axes hidden when the (I gave Claude a try for these changes, reviewed and tested before pushing) |
Maybe we should fix that (the message is not fully visible)
|
Nice catch ! |
| m_mesh = std::make_unique<OvRendering::Resources::Mesh>(vertices, indices); | ||
| } | ||
|
|
||
| void OvCore::ECS::Components::UI::CImage::SynchronizeTextureState() const |
There was a problem hiding this comment.
Revision counters instead of per-component listeners: catches texture deletion and reload in one place
|
|
||
| // Raises the smallest slots first until every slot shares the same size, then splits what is | ||
| // left evenly. Slots are never shrunk below the size they requested. | ||
| void ExpandAlongMainAxis(std::vector<float>& p_sizes, float p_availableSize) |
There was a problem hiding this comment.
Mirrors Clay's grow: smallest slots raised first, then the rest split evenly. Never shrinks below preferred
| return GetUIDataOrDefault().position; | ||
| } | ||
|
|
||
| void OvCore::ECS::Components::CTransform::SetUIRotation(float p_rotation) |
There was a problem hiding this comment.
Rotation and scale stay in the local transform, so the 2D and 3D views never drift apart
| p_result.drawables.push_back(drawable); | ||
| } | ||
|
|
||
| void AppendHierarchyUIDrawables( |
There was a problem hiding this comment.
Draw order follows the hierarchy walk rather than the material, so canvases layer like in the panel
|
|
||
| for (auto& actor : m_actors) | ||
| std::unordered_set<ECS::Actor*> serializedActors; | ||
| const std::function<void(ECS::Actor&)> serializeHierarchy = [&](ECS::Actor& p_actor) |
There was a problem hiding this comment.
Actors are serialized depth-first now, because the scene order drives UI draw order
Gopmyc
left a comment
Gopmyc
left a comment
There was a problem hiding this comment.
Honestly, once I looked at it properly, we were only really using Clay for slot sizes, all the actual positioning was being redone by hand right after, since Clay works from a top-left origin and our elements are centered. That's ~5000 vendored lines, the 29 build warnings, and an arena per CLayoutGroup, for something we were already computing ourselves
So I pulled it out and wrote a small solver in LayoutSolver.cpp that does the same fit/grow/expand work. Layouts should come out pixel-identical. It also makes this part of the PR much easier to review, the whole layout logic now fits in one readable file instead of being spread across a vendored library
Happy to put Clay back if you'd rather keep it as a base for richer layouts later 🙌
|
nice, It works fine for me |
3 participants
Description
CCanvas,CTransform2D,CImage,CText.CLayoutGroup,CHorizontalLayout,CVerticalLayout.controlChildrenWidth/Heightsizing.Related Issue(s)
Fixes #631
Review Guidance
CLayoutGroup(controlChildrenWidth/Height).CTransform2Dstretch presets and inspector behavior.CText/Fontmaterial path (EnsureEmbeddedMaterial+ color override).Reviewer Notes (Key integration details)
1) Layout behavior (
CLayoutGroup)controlChildrenWidth/Heightnow effectively writes computed sizes to supported child UI components (CImage,CTextextents,CTransform2Dsize).2) Dedicated layout components
CHorizontalLayoutandCVerticalLayoutare concrete specializations ofCLayoutGroup.3) Transform2D anchor/stretch presets
HORIZONTAL_STRETCH_TOP/MIDDLE/BOTTOMVERTICAL_STRETCH_LEFT/CENTER/RIGHTSTRETCH_BOTH4) Text/Font material flow
Fontnow owns an embedded UI material (EnsureEmbeddedMaterial/GetEmbeddedMaterial).CTextreuses this embedded material and only overrides dynamic text color.5) Editor rendering mode behavior
GameView: always includes UI in screen space.SceneView: always includes UI; toolbar toggle switches screen-space mode on/off for Scene View display.6) Lua exposure
CHorizontalLayoutandCVerticalLayout.Transform2Danchor preset enum values.Screenshots/GIFs
AI Usage Disclosure
Generated new code / Refactored code / Debugging
Build Warnings
The changes generate build warnings, specifically 29 new warnings originating from the
Claylibrary itself.These correspond to typical internal conversions on MSVC side: pointer to
uint32_t(hash),doubletofloat,float/__int64toint32_t.These are true narrowing warnings, but not relevant to our current integration as long as we remain within normal layout/UI sizes.
Note
You can find an Overload project on the
631-demobranch of my Overload fork. It includes scenes and scripts used to test the Lua API, as well as the added and modified source code.Checklist