Remove _member_access from geometry #1910
Merged
Conversation
This file contains 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
github-actions
bot
added
sources
✅ Deploy Preview for conkyweb canceled.
|
Caellian
changed the title
Caellian
force-pushed
the
fix/geometry-accessors
branch
from
df61cec
to
90b0923
Compare
Caellian
force-pushed
the
fix/geometry-accessors
branch
2 times, most recently
from
0e14543
to
9bb3061
Compare
Caellian
commented
May 13, 2024
Comment on lines
+183
to
+188
| #pragma GCC diagnostic push | ||
| #pragma GCC diagnostic ignored "-Wfree-nonheap-object" | ||
| // *_allocated takes care of avoiding freeing unallocated objects | ||
| if (name_allocated) delete[] error_name; | ||
| if (code_allocated) delete[] code_description; | ||
| #pragma GCC diagnostic pop |
There was a problem hiding this comment.
This produces a (new) warning in GCC 14.1.1
Comment on lines
+54
to
+57
| add_compile_options( | ||
| $<$<COMPILE_LANG_AND_ID:CXX,Clang>:-stdlib=libc++> | ||
| $<$<COMPILE_LANG_AND_ID:CXX,Clang>:-Wno-unknown-warning-option> | ||
| $<$<COMPILE_LANG_AND_ID:CXX,GCC>:-Wno-unknown-warning>) |
There was a problem hiding this comment.
Had to add these to avoid warning about new warnings with older compiler versions.
| @@ -226,7 +226,7 @@ char *find_and_replace_templates(const char *inbuf) { | |||
| outlen += len; | |||
| *o = '\0'; | |||
| outbuf = static_cast<char *>(realloc(outbuf, outlen * sizeof(char))); | |||
| strncat(outbuf, tmpl_out, len); | |||
| strcat(outbuf, tmpl_out); | |||
There was a problem hiding this comment.
This was also a warning because the new compiler can tell that len is derived from strlen(tmpl_out) which is then the same as using strcat directly.
Caellian
commented
May 13, 2024
Comment on lines
-1142
to
+1183
| int display_width = workarea[2] - workarea[0]; | ||
| int display_height = workarea[3] - workarea[1]; | ||
| int display_width = workarea.width(); | ||
| int display_height = workarea.height(); | ||
|
|
||
| switch (horizontal_alignment(align)) { | ||
| case axis_align::START: | ||
| sizes[*x11_strut::LEFT] = std::clamp( | ||
| window.geometry.x + window.geometry.width, 0, *workarea.width); | ||
| window.geometry.x() + window.geometry.end_x(), 0, display_width); | ||
| sizes[*x11_strut::LEFT_START_Y] = | ||
| std::clamp(*window.geometry.y, 0, *workarea.height); | ||
| std::clamp(window.geometry.y(), 0, display_height); | ||
| sizes[*x11_strut::LEFT_END_Y] = std::clamp( | ||
| window.geometry.y + window.geometry.height, 0, *workarea.height); | ||
| window.geometry.y() + window.geometry.height(), 0, display_height); | ||
| break; | ||
| case axis_align::END: | ||
| sizes[*x11_strut::RIGHT] = | ||
| std::clamp(workarea.width - window.geometry.x, 0, *workarea.width); | ||
| std::clamp(display_width - window.geometry.x(), 0, display_width); | ||
| sizes[*x11_strut::RIGHT_START_Y] = | ||
| std::clamp(*window.geometry.y, 0, *workarea.height); | ||
| std::clamp(window.geometry.y(), 0, display_height); | ||
| sizes[*x11_strut::RIGHT_END_Y] = std::clamp( | ||
| window.geometry.y + window.geometry.height, 0, *workarea.height); | ||
| window.geometry.y() + window.geometry.height(), 0, display_height); | ||
| break; | ||
| case axis_align::MIDDLE: | ||
| switch (vertical_alignment(align)) { | ||
| case axis_align::START: | ||
| sizes[*x11_strut::TOP] = | ||
| std::clamp(window.geometry.y + window.geometry.height, 0, | ||
| *workarea.height); | ||
| std::clamp(window.geometry.y() + window.geometry.height(), 0, | ||
| display_height); | ||
| sizes[*x11_strut::TOP_START_X] = | ||
| std::clamp(*window.geometry.x, 0, *workarea.width); | ||
| sizes[*x11_strut::TOP_END_X] = std::clamp( | ||
| window.geometry.x + window.geometry.width, 0, *workarea.width); | ||
| std::clamp(window.geometry.x(), 0, display_width); | ||
| sizes[*x11_strut::TOP_END_X] = | ||
| std::clamp(window.geometry.x() + window.geometry.width(), 0, | ||
| display_width); | ||
| break; | ||
| case axis_align::END: | ||
| sizes[*x11_strut::BOTTOM] = std::clamp( | ||
| workarea.height - window.geometry.y, 0, *workarea.height); | ||
| display_height - window.geometry.y(), 0, display_height); | ||
| sizes[*x11_strut::BOTTOM_START_X] = | ||
| std::clamp(*window.geometry.x, 0, *workarea.width); | ||
| sizes[*x11_strut::BOTTOM_END_X] = std::clamp( | ||
| window.geometry.x + window.geometry.width, 0, *workarea.width); | ||
| std::clamp(window.geometry.x(), 0, display_width); | ||
| sizes[*x11_strut::BOTTOM_END_X] = | ||
| std::clamp(window.geometry.x() + window.geometry.width(), 0, | ||
| display_width); |
There was a problem hiding this comment.
This was buggy. I didn't use display_width and display_height at all, and instead workarea.width which was in fact an absolute end x position and not width.
Comment on lines
-355
to
-358
| workarea[0] = ps->x_org; | ||
| workarea[1] = ps->y_org; | ||
| workarea[2] = workarea[0] + ps->width; | ||
| workarea[3] = workarea[1] + ps->height; |
There was a problem hiding this comment.
As can be seen here.
It was difficult ensuring temporary `this` was correct when combined with composition. Signed-off-by: Tin Švagelj <tin.svagelj@live.com>
Add some convenience setters. Signed-off-by: Tin Švagelj <tin.svagelj@live.com>
This allows source pragmas for future warnings which are in a newer version of the compiler but not implemented in an older one. Signed-off-by: Tin Švagelj <tin.svagelj@live.com>
strncat that concatenates to a buffer whose length was determined from source strlen can be replaced by strcat - if the buffer is invalid (unterminated), using strlen will result in same behavior. Signed-off-by: Tin Švagelj <tin.svagelj@live.com>
Signed-off-by: Tin Švagelj <tin.svagelj@live.com>
brndnmtthws
approved these changes
May 13, 2024
4 tasks
Caellian
pushed a commit
that referenced
this pull request
May 15, 2024
Fixes incorrect member accessing left behind after #1910.
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.
It was difficult/too costly ensuring temporary
thiswas correct whenvecs contained inrects were accessed directly.This PR also adds:
rectfor sized & absolute rectangles, and fixes another (unreported) bug with previous PR (Introduce geometry primitives #1862) where I used absolute end positions instead of width/height.Fixes #1908.