Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion contrib/vcpkg
Submodule vcpkg updated 946 files
10 changes: 7 additions & 3 deletions samples/06_Widgets/Widgets.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,17 +33,20 @@ CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
#include "UI/UiService.h"
#include "UI/Button.h"
#include "UI/Panel.h"
#include "UI/Text.h"

using namespace OSRE;
using namespace OSRE::App;
using namespace OSRE::RenderBackend;
using namespace OSRE::Common;

static Ui::WidgetBase *createUi() {
Rect2i r1(10, 10, 200, 100);
Rect2i r1(100, 100, 400, 400);
Ui::Panel *panel = Ui::UiService::getInstance()->createPanel("root", r1, nullptr);
Rect2i r2(20, 20, 160, 40);
panel->addWidget(new Ui::Button("Quit", r2, panel));
Rect2i r2(120, 120, 160, 40);

Ui::Text *text = new Ui::Text("Hello World", r2, panel);
panel->addWidget(text);
return panel;
}

Expand All @@ -61,6 +64,7 @@ class WidgetsApp final : public AppBase {

/// @brief
WidgetsApp(int argc, char *argv[]) : AppBase(argc, const_cast<const char **>(argv)) {
// empty
}

/// The class destructor.
Expand Down
2 changes: 2 additions & 0 deletions src/Engine/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,8 @@ SET(ui_src
UI/Panel.h
UI/UiService.cpp
UI/UiService.h
UI/Text.cpp
UI/Text.h
UI/TextPanel.cpp
UI/TextPanel.h
UI/WidgetBase.h
Expand Down
97 changes: 83 additions & 14 deletions src/Engine/RenderBackend/2D/CanvasRenderer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,86 @@

DECL_OSRE_LOG_MODULE(CanvasRenderer)

static constexpr size_t NumQuadVert = 4;
static constexpr ui32 NumQuadIndices = 6;

// will rescale coordinates from absolute coordinates into model space coordinates
inline void mapCoordinates(const Rect2i &resolution, i32 x, i32 y, f32 &xOut, f32 &yOut) {
xOut = (2.0f * static_cast<f32>(x) / static_cast<f32>(resolution.width)) - 1.0f;
xOut = (2.0f * static_cast<f32>(x) / static_cast<f32>(resolution.width)) - 1.0f;
yOut = (2.0f * static_cast<f32>(y) / static_cast<f32>(resolution.height)) - 1.0f;
yOut = -1.0f * yOut;
}

static void generateTextBoxVerticesAndIndices(const Rect2i &resolution, i32 x, i32 y, i32 textSize, i32 layer, const String &text,
Vec3Array &positions, Vec3Array &colors, Vec2Array &tex0, cppcore::TArray<ui16> &textIndices) {
using namespace ::OSRE::Common;

const size_t NumTextVerts = MeshUtilities::getNumTextVerts(text);
positions.resize(NumTextVerts);
colors.resize(NumTextVerts);
tex0.resize(NumTextVerts);
textIndices.resize(MeshUtilities::getNumTextIndices(text));

const f32 invCol = 1.f / 16.f;
const f32 invRow = 1.f / 16.f;
ui32 textCol(0), textRow(0);
for (size_t i = 0; i < text.size(); i++) {
const i32 colOffset = textCol * textSize;
const c8 ch = text[i];
if (Tokenizer::isLineBreak(ch)) {
textCol = 0;
++textRow;
continue;
}

const ui16 VertexOffset = static_cast<ui16>(i) * static_cast<ui16>(NumQuadVert);
const i32 rowHeight = -1 * textRow * textSize;

mapCoordinates(resolution, x + colOffset, y + rowHeight, positions[VertexOffset + 0].x, positions[VertexOffset + 0].y);
positions[VertexOffset + 0].z = static_cast<f32>(-layer);

mapCoordinates(resolution, x + colOffset, y + textSize + rowHeight, positions[VertexOffset + 1].x, positions[VertexOffset + 1].y);
positions[VertexOffset + 1].z = static_cast<f32>(-layer);

mapCoordinates(resolution, x + textSize + colOffset, y + rowHeight, positions[VertexOffset + 2].x, positions[VertexOffset + 2].y);
positions[VertexOffset + 2].z = static_cast<f32>(-layer);

mapCoordinates(resolution, x + textSize + colOffset, y + textSize + rowHeight, positions[VertexOffset + 3].x, positions[VertexOffset + 3].y);
positions[VertexOffset + 3].z = static_cast<f32>(-layer);

const i32 column = (ch) % 16;
const i32 row = (ch) / 16;
const f32 s = column * invCol;
const f32 t = (row + 1) * invRow;

tex0[VertexOffset + 0].x = s;
tex0[VertexOffset + 0].y = 1.0f - t;

tex0[VertexOffset + 1].x = s;
tex0[VertexOffset + 1].y = 1.0f - t + 1.0f / 16.0f;

tex0[VertexOffset + 2].x = s + 1.0f / 16.0f;
tex0[VertexOffset + 2].y = 1.0f - t;

tex0[VertexOffset + 3].x = s + 1.0f / 16.0f;
tex0[VertexOffset + 3].y = 1.0f - t + 1.0f / 16.0f;

colors[VertexOffset + 0] = glm::vec3(0, 0, 0);
colors[VertexOffset + 1] = glm::vec3(0, 0, 0);
colors[VertexOffset + 2] = glm::vec3(0, 0, 0);
colors[VertexOffset + 3] = glm::vec3(0, 0, 0);
const ui32 IndexOffset = i * NumQuadIndices;
textIndices[0 + IndexOffset] = 0 + VertexOffset;
textIndices[1 + IndexOffset] = 2 + VertexOffset;
textIndices[2 + IndexOffset] = 1 + VertexOffset;

textIndices[3 + IndexOffset] = 1 + VertexOffset;
textIndices[4 + IndexOffset] = 2 + VertexOffset;
textIndices[5 + IndexOffset] = 3 + VertexOffset;
++textCol;
}
}
Comment on lines +50 to +118
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

generateTextBoxVerticesAndIndices – eliminate unused arrays & potential overflow

  1. colors is completely filled but never used later. Remove it or use it to drive color0.
  2. Vertex/index offsets are cast to ui16. For very long strings (> 6553 chars) this overflows. Either:
    • enforce OSRE_ASSERT(text.size() < 6553) or
    • switch the public interface to ui32.
  3. Consider reserve() instead of resize() + manual assignment to avoid default-construction overhead.
-    positions.resize(NumTextVerts);
-    colors.resize(NumTextVerts);
-    tex0.resize(NumTextVerts);
-    textIndices.resize(MeshUtilities::getNumTextIndices(text));
+    positions.reserve(NumTextVerts);
+    tex0.reserve(NumTextVerts);
+    textIndices.reserve(MeshUtilities::getNumTextIndices(text));

This keeps RAM the same but skips pointless writes.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In src/Engine/RenderBackend/2D/CanvasRenderer.cpp lines 50 to 118, remove the
unused colors array or integrate it properly to drive color0 since it is
currently filled but not used. Replace the ui16 casts for vertex and index
offsets with a wider type like ui32 or add an assertion to ensure text.size() is
less than 6553 to prevent overflow on long strings. Change positions, colors,
tex0, and textIndices from resize() to reserve() to avoid unnecessary default
construction and improve performance.


inline void clip(const Rect2i &resolution, i32 x, i32 y, i32 &x_out, i32 &y_out) {
x_out = x;
y_out = y;
Expand Down Expand Up @@ -146,13 +219,7 @@
}

CanvasRenderer::CanvasRenderer(i32 numLayers, i32 x, i32 y, i32 w, i32 h) :
mDirty(true),
mPenColor(1, 1, 1, 0),
mActiveLayer(0),
mNumLayers(numLayers),
mFont(nullptr),
mMesh(nullptr),
mText(nullptr) {
mDirty(true), mPenColor(1, 1, 1, 0), mActiveLayer(0), mNumLayers(numLayers) {
setResolution(x, y, w, h);
}

Expand All @@ -165,6 +232,7 @@

void CanvasRenderer::preRender(RenderBackendService *rbSrv) {
if (rbSrv == nullptr) {
osre_error(Tag, "Render-Backend is nullptr.");
return;
}

Expand All @@ -176,6 +244,7 @@

void CanvasRenderer::render(RenderBackendService *rbSrv) {
if (rbSrv == nullptr) {
osre_error(Tag, "Render-Backend is nullptr.");
return;
}

Expand Down Expand Up @@ -432,15 +501,15 @@
usedSize = mFont->Size;
}

f32 x_model = 0.0f;
f32 y_model = 0.0f;
mapCoordinates(mResolution, x, y, x_model, y_model);
Vec3Array positions;
Vec3Array colors;
Vec2Array tex0;
TArray<ui16> indices;
const f32 fontSize = static_cast<f32>(usedSize) / static_cast<f32>(mResolution.getWidth());
MeshUtilities::generateTextBoxVerticesAndIndices(x_model, y_model, fontSize, text, positions, colors, tex0, indices);
f32 xModel{ 0.f }, yModel{ 0.f };
mapCoordinates(mResolution, x, y, xModel, yModel);

//MeshUtilities::generateTextBoxVerticesAndIndices(xModel, yModel, usedSize, text, positions, colors, tex0, indices);

Check notice

Code scanning / CodeQL

Commented-out code Note

This comment appears to contain commented-out code.

Copilot Autofix

AI 8 months ago

The best way to fix this issue is to either remove the commented-out code entirely or reinstate it if it is still relevant and functional. In this case, the commented-out line appears to be an alternative implementation of the generateTextBoxVerticesAndIndices function. Since the active implementation is already present on the next line (line 512), the commented-out code is redundant and should be removed to improve code clarity.

Suggested changeset 1
src/Engine/RenderBackend/2D/CanvasRenderer.cpp

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/Engine/RenderBackend/2D/CanvasRenderer.cpp b/src/Engine/RenderBackend/2D/CanvasRenderer.cpp
--- a/src/Engine/RenderBackend/2D/CanvasRenderer.cpp
+++ b/src/Engine/RenderBackend/2D/CanvasRenderer.cpp
@@ -510,3 +510,2 @@
 
-    //MeshUtilities::generateTextBoxVerticesAndIndices(xModel, yModel, usedSize, text, positions, colors, tex0, indices);
     generateTextBoxVerticesAndIndices(mResolution, x, y, usedSize, mActiveLayer-2, text, positions, colors, tex0, indices);
EOF
@@ -510,3 +510,2 @@

//MeshUtilities::generateTextBoxVerticesAndIndices(xModel, yModel, usedSize, text, positions, colors, tex0, indices);
generateTextBoxVerticesAndIndices(mResolution, x, y, usedSize, mActiveLayer-2, text, positions, colors, tex0, indices);
Copilot is powered by AI and may make mistakes. Always verify output.
generateTextBoxVerticesAndIndices(mResolution, x, y, usedSize, mActiveLayer-2, text, positions, colors, tex0, indices);

DrawCmd *drawCmd = alloc();
drawCmd->PrimType = PrimitiveType::TriangleList;
Expand All @@ -455,7 +524,7 @@
drawCmd->Vertices[posIndex].color0 = mPenColor.toVec3();
drawCmd->Vertices[posIndex].position.x = positions[posIndex].x;
drawCmd->Vertices[posIndex].position.y = positions[posIndex].y;
drawCmd->Vertices[posIndex].position.z = static_cast<f32>(-mActiveLayer);
drawCmd->Vertices[posIndex].position.z = positions[posIndex].z;
drawCmd->Vertices[posIndex].tex0.x = tex0[posIndex].x;
drawCmd->Vertices[posIndex].tex0.y = tex0[posIndex].y;
}
Expand Down
6 changes: 3 additions & 3 deletions src/Engine/RenderBackend/2D/CanvasRenderer.h
Original file line number Diff line number Diff line change
Expand Up @@ -163,9 +163,9 @@ class OSRE_EXPORT CanvasRenderer : public IRenderPath {
Rect2i mResolution;
i32 mActiveLayer;
i32 mNumLayers;
Font *mFont;
Mesh *mMesh;
Mesh *mText;
Font *mFont = nullptr;
Mesh *mMesh = nullptr;
Mesh *mText = nullptr;
};

inline Font* CanvasRenderer::getActiveFont() const {
Expand Down
10 changes: 5 additions & 5 deletions src/Engine/UI/Button.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,15 @@ namespace OSRE::Ui {
using namespace ::OSRE::RenderBackend;

Button::Button(const String &label, const Rect2i &rect, WidgetBase *parent) :
WidgetBase(rect, parent), mLabel(label) {
WidgetBase(rect, parent), mText(nullptr) {
setDirty();
if (!label.empty()) {
setLabel(label);
}
}

Button::~Button() {
// empty
delete mText;
}

void Button::onUpdate() {
Expand All @@ -48,9 +51,6 @@ void Button::onRender(CanvasRenderer *renderer) {
renderer->setColor(Color4(0.5f, 0.5f, 0.5f, 1.0f));
const Rect2i &r = getRect();
renderer->drawRect(r.x1, r.y1, r.width, r.height, true);
if (!mLabel.empty()) {
renderer->drawText(r.x1+1, r.y1+1, 40, mLabel);
}
setClean();
}
}
Expand Down
19 changes: 12 additions & 7 deletions src/Engine/UI/Button.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,9 @@ IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
-----------------------------------------------------------------------------------------------*/
#include "WidgetBase.h"
#include "UI/Text.h"

namespace OSRE::Ui {
namespace OSRE::Ui {

//-------------------------------------------------------------------------------------------------
/// @ingroup Engine
Expand All @@ -41,20 +42,24 @@ class OSRE_EXPORT Button : public WidgetBase {
void onRender(RenderBackend::CanvasRenderer *renderer) override;

private:
String mLabel;
Text *mText;
};

inline void Button::setLabel(const String &text) {
if (text == mLabel) {
if (mText == nullptr) {
mText = new Text(text, getRect(), this);
return;
}

mLabel = text;
setDirty();

mText->setLabel(text);
}

inline const String &Button::getLabel() const {
return mLabel;
if (mText == nullptr) {
return String();
}

return mText->getLabel();
}
Comment on lines 57 to 63
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Potential issue with returning reference to temporary object.

The getLabel method returns a reference to a temporary String object when mText is nullptr, which is dangerous. The temporary object will be destroyed after the function returns, leaving the reference pointing to invalid memory.

Fix this by creating a static empty string or returning by value:

inline const String &Button::getLabel() const {
    if (mText == nullptr) {
-       return String();
+       static const String empty;
+       return empty;
    }
    
    return mText->getLabel();
}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
inline const String &Button::getLabel() const {
return mLabel;
if (mText == nullptr) {
return String();
}
return mText->getLabel();
}
inline const String &Button::getLabel() const {
if (mText == nullptr) {
static const String empty;
return empty;
}
return mText->getLabel();
}
🤖 Prompt for AI Agents
In src/Engine/UI/Button.h around lines 57 to 63, the getLabel method returns a
reference to a temporary String object when mText is nullptr, causing a dangling
reference. Fix this by either returning a static empty String object reference
or changing the return type to return by value instead of by reference to ensure
the returned object remains valid after the function returns.


} // namespace OSRE::Ui
64 changes: 64 additions & 0 deletions src/Engine/UI/Text.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/*-----------------------------------------------------------------------------------------------
The MIT License (MIT)

Copyright (c) 2015-2025 OSRE ( Open Source Render Engine ) by Kim Kulling

Permission is hereby granted, free of charge, to any person obtaining a copy of
this software and associated documentation files (the "Software"), to deal in
the Software without restriction, including without limitation the rights to
use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of
the Software, and to permit persons to whom the Software is furnished to do so,
subject to the following conditions:

The above copyright notice and this permission notice shall be included in all
copies or substantial portions of the Software.

THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER
IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
-----------------------------------------------------------------------------------------------*/
#include "UI/Text.h"

namespace OSRE::Ui {

Text::Text(const String &label, const Rect2i &rect, WidgetBase *parent) :
WidgetBase(rect, parent), mLabel(label) {
setDirty();
}

Text::~Text() {
// empty
}

void Text::setLabel(const String &text) {
if (text == mLabel) {
return;
}
mLabel = text;
setDirty();
}

const String &Text::getLabel() const {
return mLabel;
}

void Text::onUpdate() {
// empty
}

void Text::onRender(RenderBackend::CanvasRenderer *renderer) {
osre_assert(renderer != nullptr);

if (isDirty()) {
renderer->selectLayer(1);
renderer->setColor(Color4(0.5f, 0.5f, 0.5f, 1.0f));
const Rect2i &r = getRect();
renderer->drawText(r.x1 + 1, r.y1 + 1, 40, mLabel);
setClean();
}
}

} // namespace OSRE::Ui
50 changes: 50 additions & 0 deletions src/Engine/UI/Text.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/*-----------------------------------------------------------------------------------------------
The MIT License (MIT)

Copyright (c) 2015-2025 OSRE ( Open Source Render Engine ) by Kim Kulling

Permission is hereby granted, free of charge, to any person obtaining a copy of
this software and associated documentation files (the "Software"), to deal in
the Software without restriction, including without limitation the rights to
use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of
the Software, and to permit persons to whom the Software is furnished to do so,
subject to the following conditions:

The above copyright notice and this permission notice shall be included in all
copies or substantial portions of the Software.

THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER
IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
-----------------------------------------------------------------------------------------------*/
#pragma once

#include "UI/WidgetBase.h"
#include "RenderBackend/2D/CanvasRenderer.h"

namespace OSRE::Ui {

//-------------------------------------------------------------------------------------------------
/// @ingroup Engine
///
/// @brief Todo!
//-------------------------------------------------------------------------------------------------
class OSRE_EXPORT Text : public WidgetBase {
public:
Text(const String &label, const Rect2i &rect, WidgetBase *parent = nullptr);
~Text() override;
void setLabel(const String &text);
const String &getLabel() const;

protected:
void onUpdate() override;
void onRender(RenderBackend::CanvasRenderer *renderer) override;

private:
String mLabel;
};

} // namespace OSRE::Ui
Loading
Loading