-
Notifications
You must be signed in to change notification settings - Fork 15
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
[WIP] Improve knightos/display.h #23
base: master
Are you sure you want to change the base?
Conversation
Implemented in C since I don't know if the kernel supports it natively.
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.
Honestly, it'd probably be better to write this in asm and expose a small C interface. SDCC 4 is a lot better with codegen, but it still reeks compared to hand-written (and we're not on it, just yet).
If you'd rather not redo this, I get it, and I'm happy to merge as is after testing a bit.
} | ||
|
||
void draw_float(SCREEN* screen, unsigned char x, unsigned char y, float value){ | ||
/* Implementation is weird and slow because of non-working snprintf. Indeed, %f format strings are not yet implemented */ |
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.
@aviallon what was wrong with snprintf?
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.
%f doesn't work at all (it's ignored). I don't know if it is standard, but every libc I know supports it. And it really is useful.
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.
I can't remember why we didn't implement %f, and there's probably a good reason, but you may want to experiment with https://github.com/KnightOS/libc/blob/master/src/format.c
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.
Worst case, I'd like to see %f support using this function as a base.
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.
Actually, this function has a major issue :
it does not support floats whose integer part is greater than INT_MAX
...
In order to have full float support, I would have to go char by char... which may be even slower 😞
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.
Let me know when this PR is done.
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.
Yup, I will do. If you have any idea on a fast implementation for that, I'm all ears 😝
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.
First idea: don't worry about fast. It can be sped up later if we need to.
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.
Let me know when this is ready for another review :)
I may try to do the signed integer one in ASM. But for floats it may be a little hard. |
@aviallon fyi @pixelherodev and I are active on IRC. #knightos on freenode.net |
Are you interested in a Matrix group? (IRC bridges are easy to set up and officially supported) I'm coming soon. |
Needs to be updated now that e.g. log10 is in libc. |
WTF happened (somehow I had random commits in my PR) |
6355594
to
a35754c
Compare
Updated to use log10u. I am in the process to try to make a draw_float able to display any floats, but it certainly is not trivial ! I got some ideas though... |
* Draws a long at x, y | ||
* NYI |
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.
TODO is better, it's more commonly used
value = -value; | ||
} | ||
draw_short(screen, x, y, value); | ||
return; |
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.
Why the explicit return?
} | ||
|
||
void draw_float(SCREEN* screen, unsigned char x, unsigned char y, float value){ | ||
/* Implementation is weird and slow because of non-working snprintf. Indeed, %f format strings are not yet implemented */ |
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.
Let me know when this is ready for another review :)
Implemented several functions in C since I don't know if the kernel supports it natively.