Just merge it xd #4
Conversation
update from aromadev @160121
This commit is from my personal development, and never open to public, because it was inside my corporate source (my lazyness) But now it's time to update the source periodically if there is any update in source. Not much core function change, no new controls, or any new major feature in this source. But I did alot of optimize, including usage of jpeg-turbo, and add support to save canvas into jpeg file. Many improvement in framebuffer and input driver, mainly to fit my need (in my projects). Example: we now can set the framebuffer to request new resolution :) It also add device blacklist, some memory leaks and corrupt access, improve some performance, and many tweaks that I forgot (need to check all in my main project commit history :( ). Anyway..... Thanks.
will add gitignore
And remove old jpeg-turbo simd
|
why did you close the PR again? |
Small bugs make big disasters :) This should fix character rendering issues, as the font locking never worked until now.
text_common: fix font mutex locking
| static inline int __MAX(int a, int b){ | ||
| return (a>b)?a:b; | ||
| } | ||
| static inline int __MIN(int a, int b){ | ||
| return (a<b)?a:b; | ||
| } | ||
| static inline float __FMAX(float a, float b){ | ||
| return (a>b)?a:b; | ||
| } | ||
| static inline float __FMIN(float a, float b){ | ||
| return (a<b)?a:b; | ||
| } | ||
| static inline double __DMAX(double a, double b){ | ||
| return (a>b)?a:b; | ||
| } | ||
| static inline double __DMIN(double a, double b){ | ||
| return (a<b)?a:b; | ||
| } |
There was a problem hiding this comment.
those names violate the C standard: http://port70.net/~nsz/c/c11/n1570.html#7.1.3
All identifiers that begin with an underscore and either an uppercase letter or another underscore are always reserved for any use.
Also, what's the point of e.g. calling it __MAX and then have a macro MAX alias it? Why not give it that name directly?
Also, MIN and MAX are defined in many headers and there's a reason non of them are implemented as C functions: so they work with all types.
While the prefixed DMIN/DMAX and FMIN/FMAX might be a good idea, MIN/MAX is not, since most people would expect them to work for all types.
Also, I'd suggest using lowercase names for functions because uppercase function names are generally accepted to be macros which users would expect to support all types.
| @@ -153,5 +153,6 @@ byte libaroma_hid_get(LIBAROMA_HID_EVENTP e); | |||
| */ | |||
| byte libaroma_hid_config(char * name,char * svalue,dword dvalue); | |||
|
|
|||
| void libaroma_hid_restart(); | |||
There was a problem hiding this comment.
you should enable -Wstrict-prototypes. While void is optional for C++, in C it has a different, quite dangerous semantic.
| @@ -158,8 +158,11 @@ byte libaroma_start() { | |||
|
|
|||
| #ifdef LIBAROMA_TTY_KDSETMODE | |||
| ALOGI("KDSETMODE = KD_GRAPHICS"); | |||
| int tty1 = open("/dev/tty1", O_RDWR); | |||
| char ttydev[32]; | |||
| snprintf(ttydev,32,"/dev/tty%i",LIBAROMA_TTY_KDSETMODE); | |||
There was a problem hiding this comment.
| snprintf(ttydev,32,"/dev/tty%i",LIBAROMA_TTY_KDSETMODE); | |
| snprintf(ttydev,sizeof(ttydev),"/dev/tty%i",LIBAROMA_TTY_KDSETMODE); |
| @@ -158,8 +158,11 @@ byte libaroma_start() { | |||
|
|
|||
| #ifdef LIBAROMA_TTY_KDSETMODE | |||
| ALOGI("KDSETMODE = KD_GRAPHICS"); | |||
| int tty1 = open("/dev/tty1", O_RDWR); | |||
| char ttydev[32]; | |||
There was a problem hiding this comment.
that length seems arbitrary
/dev/tty1== 9- max value of
%i== is platform specific. - 0-terminator
I suggest doing this so you know the length is always 1
uint8_t value = LIBAROMA_TTY_KDSETMODE ? 1 : 0;With that, you can use a length of 11.
| @@ -158,8 +158,11 @@ byte libaroma_start() { | |||
|
|
|||
| #ifdef LIBAROMA_TTY_KDSETMODE | |||
| ALOGI("KDSETMODE = KD_GRAPHICS"); | |||
| int tty1 = open("/dev/tty1", O_RDWR); | |||
| char ttydev[32]; | |||
| snprintf(ttydev,32,"/dev/tty%i",LIBAROMA_TTY_KDSETMODE); | |||
There was a problem hiding this comment.
while I generally prefer using snprintf, in this case you know the length so you could just use sprintf.
Alternatively (what I'd prefer) since the function libaroma_start has an error code return value already anyway you could properly check the return value just in case we overlooked something.
A proper check would be:
int rc = snprintf(buf, len, ...);
if (rc < 0 || (size_t)rc >= len) {
// error
}
| int tty1 = open("/dev/tty1", O_RDWR); | ||
| char ttydev[32]; | ||
| snprintf(ttydev,32,"/dev/tty%i",LIBAROMA_TTY_KDSETMODE); | ||
| int tty1 = open(ttydev, O_RDWR); | ||
| ioctl(tty1, KDSETMODE, KD_GRAPHICS); |
There was a problem hiding this comment.
I know you didn't change this line in this PR but I just noticed: you forgot to check the return value of ioctl.
|
@amarullz Okay so I just realized that the issues I brought up(and many more) were already present before. So I guess it'd make sense to Also, a lot has happened between 2016 and today in terms of coding-standards and how to write safe, qualitative code. Also, I've learned a lot in the meantime 😋 Would you be fine with us realizing the following things before we start working on new things? I'd be willing to help a lot on this.
What I'd appreciate but is a lot of work and needs your consent:
|
New build environment will only compiling/building
libexternals (zlib, png, jpeg, freetype, etc) and
libaroma as static object ( libexternals.a and libaroma.a ).
With that 2 static object, we can easily make simple app using
libaroma+external, with just something like this:
gcc -O2 \
main.c libaroma.a libexternals.a \
-Ilibaroma/include -o main
This build system is simple. There is 4 main files:
/build-config.ini
This file is your configuration for building libaroma.
We can set LIBAROMA_BUILD_FLAGS, LIBAROMA_ROOT_PATH,
and BUILD_ARCH
/build.sh
Is command that we can run to build the static libraries, with arguments
example like this:
# ./build.sh
# ./build.sh clean
# ./build.sh armv7
# ./build.sh armv7 clean
# ./build.sh aarch64
# ./build.sh aarch64 clean
# ./build.sh x86_64
# ./build.sh x86_64 clean
/build/build-ARCH.sh
This script will prepare any arch config, like compiling flags,
cross-compile, and which special arch-only file to compile.
/build/builder.sh
The real script that do all build process. Receiving all configuration
from others 3 scripts.
----
Ideal build environment is Linux or Windows+WSL.
since now I don't support and recommended building
it with bat scripts 😥😥
For cross-compiling in ubuntu/ubuntu-wsl, we can use:
ARMv7
sudo apt install gcc-arm-linux-gnueabihf
AARCH64 / ARM64
sudo apt install gcc-aarch64-linux-gnu
-----
This commit also remove all un-necessary files and libraries.
My plan before is to move all this file into /deprecated folder,
but the git won't simply move it, but act as it was deletion
+addition (which is wrong).
The script actually safe :), we can restore it again if we needed.
|
Yea. I agree... The problem with this code is, it didn't progress in so long time, only some place tweaked for some need. I also use and learn a lot of new coding standard, and used it in my newer c project. For libaroma, I don't know yet what the best thing I should do. I still need it and library still need backward compatible, Maybe I should move it to libaroma2? With whole new API, no backward compatibility. But all optimized code in current I will try my best... |
|
IMO it's not worth preventing progress due to backwards compatibility. With that being said, yes, every time you break API compatibility you should increase the major version, but the version should not be part of the projects name and does not need a new git repository or anything. If you wanna keep maintain the old version you can simply do that in a release branch. And while you probably have bigger ideas in mind, I'd be able to do most of the things in my list within a weekend (combined, not each) because I've done them many times in past. So if you want me to help you, just say so. What I'd ask you to do first, would be to decide on a clangformat-supported codestyle though since that'd change every single line in the repo, making it hard to merge other features once you started working on them. |
|
about versioning: https://semver.org/ |
|
Here it is my little apport: |
|
I think I can implement new rewriting libaroma version that a lot cleaner. to maintain backward compatibility, I can add wrapper to call new functions. I have a plan to make calls/naming nicer for every feature. usage likes libaroma_canvas_new or libaroma_listitem_option is something that I want to avoid (its become annoying when the project become bigger). I will try my best to follow standard. If you have any suggestion for func/struct/types naming, please tell me. Something like al_canvas * cv = alCanvas(x,y);maybe.. still don't know.... (al -> aroma lib / la -> lib aroma), camel case or snake_case or combination???? 🤔🤔 |
|
And maybe I should start using libraries (zlib, png, etc) from external source, and not including in this source. |
|
About byte, word, dword, ... Those are fine as names or typedefs and I don't mind anybody using them but:
And just a related piece of information that might be interesting to you: There are platforms where 1 byte != 8 bits 😉 |
|
I use this as vscode setting for formatting, and I like google format without modification 😉 {
"editor.formatOnPaste": true,
"editor.formatOnSave": true,
"editor.formatOnType": true,
"editor.codeActionsOnSave": {
"source.fixAll": true
},
"clang-format.executable": "clang-format-10",
"clang-format.style": "google",
"clang-format.language.c.enable": true,
"[c]": {
"editor.defaultFormatter": "xaver.clang-format",
"editor.wordBasedSuggestions": false,
"editor.suggest.insertMode": "replace",
"editor.semanticHighlighting.enabled": true
}
} |
|
If you create a |
|
Let me know what do you think about this new source tree: |
|
about src/:
about src/arch/:
about libaroma/ctl inside of src and include:
about src/platform:
|
|
For platform, It can use But for The naming still subject to change, previously the folder is not I explain about source file naming: I use I will try change it with Here: I think, |
|
IMO the headers shouldn't have any prefix since they already are in their own directory. And as for sources why does there have to be an "aroma" directory? This is the aroma git repository, what else would it be? |
|
You right, we don't need prefix for filename 😁, its cleaner. About "aroma" directory, I like it managed in category, it makes me focus when coding it. The |
|
It's beautiful now! Just comfortable to my eyes, and makes faster to know what are you working on without needing to read the prefix (also wastes less screen space) :D |
I'm not sure if I understand how that helps, but It's fine by me. That code structure LGTM. |
I do alot switching between one source to another, if whole source keep in one directory without any clear separation, it will take me a while to browse the file that I needed. I think @MLXProjects know what I mean (difficult to explain it 😜) |
|
Yeah @amarullz, I understand pefectly! I also work on up to three projects at same time and the folder makes it easier for me to identify which project I'm working at. |
|
@MLXProjects maybe discord or something?, just invite me I will join. |
|
I'd prefer discord over telegram since it's easier to use on PC |
|
I'm mainly a Telegram user, but will use a discord server and maybe bridge it to some Telegram group. |
* Fix text processing when double tags not rendered * Add function libaroma_wm_message_noclear(), to prevent next libaroma_wm_set_active_window from clearing message queue
Image control previously only support scalexy. now it accept scale mode with this flags: LIBAROMA_CTL_IMAGE_MODE_COVER LIBAROMA_CTL_IMAGE_MODE_FIT LIBAROMA_CTL_IMAGE_MODE_NOSCALE and some drawing flags: LIBAROMA_CTL_IMAGE_MODE_CENTER LIBAROMA_CTL_IMAGE_MODE_NOSMOOTH
While updating just a region and not the full framebuffer, there was distortion because of using canvas width instead of line size (which is 4x the width). This was most notably affecting SDL usage.
fb: fix non-fullscreen framebuffer update
* Add button left alignment
* Need -DLIBJPEG_HASRGB565_OUT in gcc flags * LIBJPEG_HASRGB565_IN for encoding still not works (jpeg-turbo bugs?)
No description provided.