search

ghaerr / elks

Embeddable Linux Kernel Subset - Linux for 8086
Other
1.02k stars 108 forks source link

[basic cga] CGA Graphic support for IBM-PC ELKS Basic #2003

Closed tyama501 closed 2 months ago

tyama501 commented 2 months ago

Hello @ghaerr

This is the first attempt to add CGA PLOT/DRAW for IBM-PC ELKS Basic as we have been talking on https://github.com/ghaerr/elks/discussions/1939#discussioncomment-10605658

I have added ia16 for the assembler codes and I have added empty int_10 and memclrw() to the else case for now.

After I merged upstream, I realized I cannot use seg_t with compile error. So, I used unsigned int instead. (Something changed recently?)

Thank you!

ghaerr commented 2 months ago

Hello @tyama501,

In general this looks very good, but there are some changes I'd like to see before commit. I will comment below and in other cases by the code itself.

I have added ia16 for the assembler codes and I have added empty int_10 and memclrw() to the else case for now.

In general, one can't assume that #if !__ia16__ this means Watcom. Thus, better to #ifdef __WATCOMC__ the unfinished portions of the int_10 and memclrw routines. However - in this case I would recommend removing the blank routines entirely as the Watcom build uses host-stub.c and thus CGA isn't supported anyways. I feel it is better to leave the routines completely out than to add them which do nothing, as this sets up a source of bugs where one forgets the functions are not in fact implemented. The routines can be added in when they are written.

I realized I cannot use seg_t with compile error. So, I used unsigned int instead. (Something changed recently?)

Yes, and unsigned int is fine. During the Doom port to ELKS, it was found that all the kernel types (seg_t, sector_t, etc) were being exposed even though libc because the C library headers include <linuxmt/types.h>. Long story short, both Doom and ELKS uses sector_t differently and it was a mess. I made a change that stops all kernel types from leaking out to C library. This change also necessitated a change in fmemalloc to use unsigned int rather than seg_t as well.

toncho11 commented 2 months ago

Would this work on VGA monitor or it is only CGA?

tyama501 commented 2 months ago

Hi @ghaerr

I have added asm-ibmpc.S and copied fmemsetw and also added int_10 for now.

It worked. I think I have done all you mentioned but if I missed something let me know.

Hi @toncho11 It is for 320x200, 4 color mode (CGA) but most ibm compatible pc supports this mode, doesn't it? (Sorry if it doesn't)

Thank you.

ghaerr commented 2 months ago

Hello @tyama501,

This now looks quite nice and very clean, thank you! We are better off with these functions directly in ASM, as they are now quite easy to understand and read, even though we haven't (yet) solved the problem of ASM portability between ia16 and Watcom!

I do have a comment about using ARG0 etc in the int_10 function which I will address with the code.

Thank you!

ghaerr commented 2 months ago

It is for 320x200, 4 color mode (CGA) but most ibm compatible pc supports this mode, doesn't it?

I uploaded snakecga.bas and started @tyama501's improved basic snakecga on QEMU, and it operates correctly :)

@tyama501, you're welcome to upload your final version of snakecga.bas into elkscmd/basic so that others can play with this too, thank you.