Skip to content

Ametric percent #714

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
cubekii wants to merge 5 commits into
base: develop
Choose a base branch
from
Open

Ametric percent #714

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
22c83df
add % metric as _pr
cubekii Mar 19, 2026
2f7460b
clean example
cubekii Mar 19, 2026
70fda5c
gemini fixed
cubekii Mar 19, 2026
097bd7c
hide __aui/icon_512x512.png err for GTEST
cubekii Mar 21, 2026
f187277
dolbaeb fix
cubekii Mar 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions aui.image/src/AUI/Image/AImage.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,12 @@ _<AImage> AImage::fromUrl(const AUrl& url) {
if (auto raster = AImageLoaderRegistry::inst().loadRaster(buffer))
return raster;
} catch (const AException& e) {
#if AUI_BUILD_TESTS
if (url.full()!="builtin://__aui/icon_512x512.png")
Copy link
Copy Markdown
Collaborator

@Alex2772 Alex2772 Mar 25, 2026

Choose a reason for hiding this comment

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

thank you

ALogger::err("Could not load image: " + url.full() + ": " + e.getMessage());
#else
ALogger::err("Could not load image: " + url.full() + ": " + e.getMessage());
#endif
}
return nullptr;
}
Expand Down
6 changes: 5 additions & 1 deletion aui.views/src/AUI/ASS/Property/BorderRadius.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,9 @@
#include "BorderRadius.h"

void ass::prop::Property<ass::BorderRadius>::applyFor(AView* view) {
view->setBorderRadius(mInfo.radius);
if (mInfo.radius.getUnit() == AMetric::T_PR) {
view->setBorderRadius((glm::min(view->getWidth(), view->getHeight())) * (mInfo.radius.getValuePx() * 0.01f));
Copy link
Copy Markdown
Collaborator

@Alex2772 Alex2772 Mar 25, 2026

Choose a reason for hiding this comment

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

i'm not sure this would work perfectly fine but your solution is definitely small and simple. Give me some time to test please.

} else {
view->setBorderRadius(mInfo.radius.getValuePx());
}
}
1 change: 1 addition & 0 deletions aui.views/src/AUI/Util/AMetric.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ AMetric::AMetric(const AString& text)
{ "px", T_PX },
{ "em", T_DP },
{ "pt", T_PT },
{ "pr", T_PR },
};

if (auto x = unitMapping.contains(unitName))
Expand Down
22 changes: 17 additions & 5 deletions aui.views/src/AUI/Util/AMetric.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,14 +67,16 @@ class AString;
* <!-- aui:index_alias _dp -->
* <!-- aui:index_alias _pt -->
* <!-- aui:index_alias _px -->
* <!-- aui:index_alias _pr -->
*
* Currently supported units:
*
* | Unit | Enum | Literal | Value |
* | -------------------------- | ---- | ------- | --------------------------- |
* | Density-independent Pixels | T_DP | _dp | px * `scale_factor` |
* | Typography point | T_PT | _pt | px * `scale_factor` * 4 / 3 |
* | Pixels | T_PX | _px | px |
* | Unit | Enum | Literal | Value |
* | -------------------------- | ---- | ------- | ---------------------------------------- |
* | Density-independent Pixels | T_DP | _dp | px * `scale_factor` |
* | Typography point | T_PT | _pt | px * `scale_factor` * 4 / 3 |
* | Pixels | T_PX | _px | px |
* | Percent | T_PR | _pr | reference_dimension * value / 100 |
*
* It's highly recommended to use only Density-independent Pixel unit (_dp). DP guarantees that your application
* will correctly handle systems with hidpi screens.
Expand All @@ -88,6 +90,7 @@ class API_AUI_VIEWS AMetric
T_PX,
T_DP,
T_PT,
T_PR,
};

private:
Expand Down Expand Up @@ -141,6 +144,7 @@ class API_AUI_VIEWS AMetric
[[nodiscard]] float getValuePx() const;
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Mar 19, 2026

Choose a reason for hiding this comment

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

high

Calling getValuePx() on an AMetric with T_PR unit is ambiguous as it requires a reference dimension to be converted to pixels. Currently, it falls through to the default case in AMetric.cpp and returns the raw percentage value. This is misleading and error-prone, especially with operator float() implicitly using this conversion.

It would be safer to make getValuePx() on a T_PR metric an error to enforce the use of the context-aware getValuePx(float referenceDimension) overload. You could add an assertion in getValuePx() for the T_PR case.

Example for AMetric.cpp:

// in AMetric::getValuePx()
switch (mUnit) {
    // ... other cases
    case T_PR:
        AUI_ASSERT_NOT_REACHED("AMetric with T_PR unit cannot be converted to pixels without a reference dimension. Use getValuePx(float) instead.");
        return 0.f;
    default:
        return mValue;
}

[[nodiscard]] float getValueDp() const;


static float fromPxToMetric(float value, Unit unit);

operator float() const {
Expand Down Expand Up @@ -247,6 +251,10 @@ constexpr inline AMetric operator""_pt(unsigned long long v)
{
return AMetric(static_cast<float>(static_cast<long long>(v)), AMetric::T_PT);
}
constexpr inline AMetric operator""_pr(unsigned long long v)
{
return AMetric(static_cast<float>(static_cast<long long>(v)), AMetric::T_PR);
}

inline std::ostream& operator<<(std::ostream& o, const AMetric& value) {
o << value.getRawValue();
Expand All @@ -260,6 +268,9 @@ inline std::ostream& operator<<(std::ostream& o, const AMetric& value) {
case AMetric::T_PT:
o << "_pt";
break;
case AMetric::T_PR:
o << "_pr";
break;

default:
break;
Expand All @@ -279,6 +290,7 @@ template <> struct fmt::formatter<AMetric> {
case AMetric::T_PX: suffix = "px"; break;
case AMetric::T_DP: suffix = "dp"; break;
case AMetric::T_PT: suffix = "pt"; break;
case AMetric::T_PR: suffix = "pr"; break;
default: suffix = ""; break;
}

Expand Down
3 changes: 3 additions & 0 deletions cmake/aui.build.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,9 @@ macro(aui_enable_tests AUI_MODULE_NAME)
target_sources(Tests PRIVATE ${TESTS_SRCS}) # append sources
endif()

# add compile definition to the module when building with tests
target_compile_definitions(${AUI_MODULE_NAME} PUBLIC AUI_BUILD_TESTS=1)

if (TARGET Tests)
# if the specified target is an executable, we should add it's srcs to the Tests target, otherwise just link it.
get_property(_type TARGET ${AUI_MODULE_NAME} PROPERTY TYPE)
Expand Down
6 changes: 6 additions & 0 deletions examples/ui/bordercheck/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
cmake_minimum_required(VERSION 3.10)

get_filename_component(_t ${CMAKE_CURRENT_SOURCE_DIR} NAME)

aui_executable("aui.example.${_t}")
aui_link("aui.example.${_t}" PRIVATE aui::core aui::views)
62 changes: 62 additions & 0 deletions examples/ui/bordercheck/src/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
/*
* AUI Framework - Declarative UI toolkit for modern C++20
* Copyright (C) 2020-2025 Alex2772 and Contributors
*
* SPDX-License-Identifier: MPL-2.0
*
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/.
*/

#include <AUI/Platform/Entry.h>
#include <AUI/Platform/AWindow.h>
#include <AUI/Util/UIBuildingHelpers.h>

using namespace ass;
using namespace declarative;


AUI_ENTRY {
auto window = _new<AWindow>("Checkbox", 300_dp, 100_dp);
window->setContents(
Vertical {
Centered {
_new<ALabel>("25%") AUI_OVERRIDE_STYLE {
BorderRadius { 25_pr },
FixedSize { 200_px , 40_px },
BackgroundSolid { 100_rgb },
},

_new<ALabel>("50%") AUI_OVERRIDE_STYLE {
Margin { 100_px, 0, 0, 0},
BorderRadius { 50_pr },
FixedSize { 200_px , 40_px },
BackgroundSolid { 150_rgb },
},

_new<ALabel>("75%") AUI_OVERRIDE_STYLE {
Margin { 200_px, 0, 0, 0},
BorderRadius { 75_pr },
FixedSize { 200_px , 40_px },
BackgroundSolid { 200_rgb },
},
_new<ALabel>("100%") AUI_OVERRIDE_STYLE {
Margin { 300_px, 0, 0, 0},
BorderRadius { 100_pr },
FixedSize { 200_px , 40_px },
BackgroundSolid { 300_rgb },
},
//Circle!
_new<ALabel>("") AUI_OVERRIDE_STYLE {
Margin { 400_px, 0, 0, 0},
BorderRadius { 100_pr },
FixedSize { 40_px },
BackgroundSolid { 5148_rgb },
}
}
}
Comment on lines +23 to +58
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Mar 19, 2026

Choose a reason for hiding this comment

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

medium

The layout structure here is a bit confusing. Centered layout stacks all its children on top of each other at the center. You are then using Margin to offset them vertically. A much simpler and clearer way to achieve a vertical arrangement would be to make the labels direct children of the Vertical layout.

    window->setContents(
        Vertical {
            _new<ALabel>("10%") AUI_OVERRIDE_STYLE {
                BorderRadius { 10_pr },
                FixedSize { 200_px , 40_px },
                BackgroundSolid { 100_rgb },
            },
            // ... other labels as direct children of Vertical
        }
    );

This would make the example easier to understand and maintain.

);
window->show();
return 0;
}
Loading