-
Notifications
You must be signed in to change notification settings - Fork 0
Just do it #12
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.
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
Just do it #12
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,7 +1,7 @@ | ||||||||||||||||||||||||||||||||||||||||||||
| /* | ||||||||||||||||||||||||||||||||||||||||||||
| MIT License | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| Copyright (c) 2022-2025 Kim Kulling | ||||||||||||||||||||||||||||||||||||||||||||
| Copyright (c) 2022-2024 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 | ||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -56,16 +56,19 @@ static Image *loadIntoImageCache(Context &ctx, const char *filename) { | |||||||||||||||||||||||||||||||||||||||||||
| return image; | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| int w = 0; | ||||||||||||||||||||||||||||||||||||||||||||
| int h = 0; | ||||||||||||||||||||||||||||||||||||||||||||
| int bytesPerPixel = 0; | ||||||||||||||||||||||||||||||||||||||||||||
| image = new Image; | ||||||||||||||||||||||||||||||||||||||||||||
| if (image == nullptr) { | ||||||||||||||||||||||||||||||||||||||||||||
| return nullptr; | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+59
to
+63
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Prevent memory-leak when - unsigned char *data = stbi_load(filename, &w, &h, &bytesPerPixel, 0);
- if (data == nullptr) {
- return nullptr;
+ unsigned char *data = stbi_load(filename, &w, &h, &bytesPerPixel, 0);
+ if (data == nullptr) {
+ delete image; // reclaim the allocation
+ return nullptr;
}📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||
| int w, h, bytesPerPixel; | ||||||||||||||||||||||||||||||||||||||||||||
| unsigned char *data = stbi_load(filename, &w, &h, &bytesPerPixel, 0); | ||||||||||||||||||||||||||||||||||||||||||||
| if (data == nullptr) { | ||||||||||||||||||||||||||||||||||||||||||||
| return nullptr; | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
| image = new Image; | ||||||||||||||||||||||||||||||||||||||||||||
| int pitch = w * bytesPerPixel; | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| int32_t pitch = w * bytesPerPixel; | ||||||||||||||||||||||||||||||||||||||||||||
| pitch = (pitch + 3) & ~3; | ||||||||||||||||||||||||||||||||||||||||||||
| image->mSurfaceImpl = Renderer::createSurfaceImpl(data, w, h, bytesPerPixel, pitch); | ||||||||||||||||||||||||||||||||||||||||||||
| image->mX = w; | ||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -110,8 +113,18 @@ static Widget *createWidget(Context &ctx, Id id, Id parentId, const Rect &rect, | |||||||||||||||||||||||||||||||||||||||||||
| return widget; | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| void eventDispatcher() { | ||||||||||||||||||||||||||||||||||||||||||||
| void eventDispatcher(Context &ctx, int32_t eventId, EventData *eventData) { | ||||||||||||||||||||||||||||||||||||||||||||
| if (ctx.mFocus == nullptr) { | ||||||||||||||||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| if (eventId == Events::KeyDownEvent || eventId == Events::KeyUpEvent) { | ||||||||||||||||||||||||||||||||||||||||||||
| if (eventData != nullptr) { | ||||||||||||||||||||||||||||||||||||||||||||
| if (ctx.mFocus->mType == WidgetType::TextField) { | ||||||||||||||||||||||||||||||||||||||||||||
| ctx.mFocus->mText.append((const char*)eventData->data[0]); | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+116
to
128
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unsafe cast when appending a single key to ctx.mFocus->mText.append((const char*)eventData->data[0]);This is undefined behaviour and usually crashes because the pointer is interpreted as an address. Append the character itself: - ctx.mFocus->mText.append((const char*)eventData->data[0]);
+ ctx.mFocus->mText.push_back(static_cast<char>(eventData->data[0]));Additional thought: you probably want to filter non-printable characters and support UTF-8. |
||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| ret_code Widgets::container(Context &ctx, Id id, Id parentId, const char *text, const Rect &rect) { | ||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -190,6 +203,22 @@ ret_code Widgets::label(Context &ctx, Id id, Id parentId, const char *text, cons | |||||||||||||||||||||||||||||||||||||||||||
| return ResultOk; | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| ret_code Widgets::textfield(Context &ctx, Id id, Id parentId, const Rect &rect, Alignment alignment) { | ||||||||||||||||||||||||||||||||||||||||||||
| if (ctx.mRoot == nullptr) { | ||||||||||||||||||||||||||||||||||||||||||||
| return ErrorCode; | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| Widget *widget = createWidget(ctx, id, parentId, rect, WidgetType::TextField); | ||||||||||||||||||||||||||||||||||||||||||||
| if (widget == nullptr) { | ||||||||||||||||||||||||||||||||||||||||||||
| return ErrorCode; | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| widget->mAlignment = alignment; | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| return ResultOk; | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| ret_code Widgets::button(Context &ctx, Id id, Id parentId, const char *text, const char *image, const Rect &rect, CallbackI *callback) { | ||||||||||||||||||||||||||||||||||||||||||||
| if (ctx.mSDLContext.mRenderer == nullptr) { | ||||||||||||||||||||||||||||||||||||||||||||
| return ErrorCode; | ||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -227,20 +256,6 @@ ret_code Widgets::box(Context &ctx, Id id, Id parentId, const Rect &rect, bool f | |||||||||||||||||||||||||||||||||||||||||||
| return ResultOk; | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| ret_code Widgets::toolbar(Context &ctx, Id id, Id parentId, const Rect &rect) { | ||||||||||||||||||||||||||||||||||||||||||||
| if (ctx.mSDLContext.mRenderer == nullptr) { | ||||||||||||||||||||||||||||||||||||||||||||
| ctx.mLogger(LogSeverity::Error, "TUI-Renderer is nullptr."); | ||||||||||||||||||||||||||||||||||||||||||||
| return ErrorCode; | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| Widget *child = createWidget(ctx, id, parentId, rect, WidgetType::ToolBar); | ||||||||||||||||||||||||||||||||||||||||||||
| if (child == nullptr) { | ||||||||||||||||||||||||||||||||||||||||||||
| return ErrorCode; | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| return ResultOk; | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| ret_code Widgets::panel(Context &ctx, Id id, Id parentId, const char *title, const Rect &rect, CallbackI *callback) { | ||||||||||||||||||||||||||||||||||||||||||||
| if (ctx.mSDLContext.mRenderer == nullptr) { | ||||||||||||||||||||||||||||||||||||||||||||
| ctx.mLogger(LogSeverity::Error, "TUI-Renderer is nullptr."); | ||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -302,7 +317,6 @@ ret_code Widgets::progressBar(Context &ctx, Id id, Id parentId, const Rect &rect | |||||||||||||||||||||||||||||||||||||||||||
| ctx.mUpdateCallbackList.push_back(callback); | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| return ResultOk; | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -360,7 +374,15 @@ static void render(Context &ctx, Widget *currentWidget) { | |||||||||||||||||||||||||||||||||||||||||||
| const uint32_t fillRate = state->filledState; | ||||||||||||||||||||||||||||||||||||||||||||
| const uint32_t width = r.width * fillRate / 100; | ||||||||||||||||||||||||||||||||||||||||||||
| Renderer::drawRect(ctx, r.x1, r.y1, width, r.height, true, ctx.mStyle.mTextColor); | ||||||||||||||||||||||||||||||||||||||||||||
| } break; | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
| break; | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| case WidgetType::TextField: | ||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||
| Renderer::drawRect(ctx, r.x1, r.y1, r.width, r.height, true, ctx.mStyle.mFg); | ||||||||||||||||||||||||||||||||||||||||||||
| Renderer::drawRect(ctx, r.x1+2, r.y1+2, r.width-4, r.height-4, true, ctx.mStyle.mBorder); | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
| break; | ||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+380
to
+385
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Render the text inside the Renderer::drawRect(ctx, r.x1+2, r.y1+2, r.width-4, r.height-4, true, ctx.mStyle.mBorder);
+ if (!currentWidget->mText.empty()) {
+ Color4 fg = ctx.mStyle.mTextColor, bg = ctx.mStyle.mBg;
+ Renderer::drawText(ctx, currentWidget->mText.c_str(),
+ ctx.mSDLContext.mDefaultFont,
+ currentWidget->mRect, fg, bg,
+ currentWidget->mAlignment);
+ }📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| case WidgetType::Container: | ||||||||||||||||||||||||||||||||||||||||||||
| case WidgetType::Box: | ||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -428,7 +450,12 @@ void Widgets::onKey(Context &ctx, const char *key, bool isDown) { | |||||||||||||||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| eventDispatcher(); | ||||||||||||||||||||||||||||||||||||||||||||
| if (isDown) { | ||||||||||||||||||||||||||||||||||||||||||||
| EventData eventData; | ||||||||||||||||||||||||||||||||||||||||||||
| eventData.data[0] = *key; | ||||||||||||||||||||||||||||||||||||||||||||
| eventDispatcher(ctx, Events::KeyDownEvent, &eventData); | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| void recursiveClear(Widget *current) { | ||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -475,6 +502,17 @@ bool Widgets::isEnabled(Context &ctx, Id id) { | |||||||||||||||||||||||||||||||||||||||||||
| return false; | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| ret_code Widgets::setFocus(Context &ctx, Id id) { | ||||||||||||||||||||||||||||||||||||||||||||
| Widget *widget = findWidget(id, ctx.mRoot); | ||||||||||||||||||||||||||||||||||||||||||||
| if (widget == nullptr) { | ||||||||||||||||||||||||||||||||||||||||||||
| return ErrorCode; | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| ctx.mFocus = widget; | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| return ResultOk; | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| Widget *Widgets::getWidgetById(Context &ctx, Id id) { | ||||||||||||||||||||||||||||||||||||||||||||
| return findWidget(id, ctx.mRoot); | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Removed nullptr initialization for mSurface.
The explicit nullptr initialization for the mSurface member was removed. While the destructor and clear() method check for nullptr before freeing, there's a risk of undefined behavior if the member is accessed before being properly initialized.
📝 Committable suggestion