ghaerr / microwindows

The Nano-X Window System
Other
648 stars 92 forks source link

Support & fix (re-upload of #44) #45

Closed rofl0r closed 3 years ago

rofl0r commented 3 years ago

1) Add BMP format compilation in case of HAVE_BMP_SUPPORT is set to N. 2) Add Visual Studio / Win32 support in the BMP file converter souce. 3) Fix in gen_getscreeninfo() to add a break within 'case MWIF_RGBA8888'. 4) Potential fix for a string manipulation in the fonts.

ghaerr commented 3 years ago

@rofl0r, thanks for fixing #44, I'll now review this.

ghaerr commented 3 years ago

Hello @djipi,

Thanks for finding these bugs and providing fixes.

For the lfFaceName bug, the solution still doesn't ensure the last byte is a NUL char, but using strzcpy (a special routine already in Microwindows that is strncpy but guarantees last byte is NUL) will. Using strzcpy instead of strncpy with sizeof(lf.LfFaceName) will fix this. Would you like to make that change, and then I'll commit this also.

Thank you!

djipi commented 3 years ago

Hi @ghaerr I have done it at my side, but due to the recent PR actions done, my branches seem no longer in sync. I cannot do a proper PR from my develop branch to your repository. It will be better if you can do the modifications at your side.

ghaerr commented 3 years ago

@djipi,

Lets try this - I'll commit this now, since its the last PR since @rofl0r's rebase, and the strzcpy change can be made in a future PR. Let me know if you you have further troubles with syncing or PR generation, perhaps something can be done.

rofl0r commented 3 years ago

using strzcpy (a special routine already in Microwindows that is strncpy but guarantees last byte is NUL) will. Using strzcpy instead of strncpy with sizeof(lf.LfFaceName) will fix this

the C99 way to properly do a zero-terminated, non-overflowing strcpy is snprintf(buf, sizeof buf, "%s", string_to_copy);

my branches seem no longer in sync. I cannot do a proper PR from my develop branch to your repository.

sorry for the complications. if you have local changes not yet merged you can copy the changed files to a temporary location, do a fresh clone of the repo here and copy the files back in place.

ghaerr commented 3 years ago

the C99 way to properly do a zero-terminated, non-overflowing strcpy is snprintf(buf, sizeof buf, "%s", string_to_copy);

Thank you. For Microwindows, since we don't want to force in the C library *printf routines, it would be better to use the simpler strzcpy in this case.

If you have local changes not yet merged you can copy the changed files to a temporary location, do a fresh clone of the repo here and copy the files back in place.

If @djipi has separate branch(s) of development based on the previous repo contents before a forced push, is there a way for them to get synced up with the current, or will they need to be rebased using special git magic to enable PRs from that branch to GitHub?

rofl0r commented 3 years ago

If @djipi has separate branch(s) of development based on the previous repo contents before a forced push, is there a way for them to get synced up with the current, or will they need to be rebased using special git magic to enable PRs from that branch to GitHub?

if he's got only changed files, the easiest way is to copy them over a new repo clone, but if he already did some commits he can e.g. do git format-patch HEAD~X where X is the number of commits made, which will create a series of patch files which can then be applied to another branch/clone using git am 0001-xxx.patch etc

djipi commented 3 years ago

It's not really a big issue, I have sent you almost all my modifications. If I do more, I will send you a new PR. Hope my contribution didn't created too much problems and will be useful.

ghaerr commented 3 years ago

@djipi,

Your contributions are very useful, thank you. Please continue to send so that we can support the jaguar port from this repo.

Also, what are you thinking about the modifications required for the hw object processor, those that might need Microwindows to align on an 8-byte boundary? I'm thinking the Microwindows screen device memory will have to be aligned that way, but not necessarily any other Microwindows images.

djipi commented 3 years ago

@ghaerr

I have created a specific branch to explore my venues for the Jaguar. If something is useful, or multi-platform, I will extract the stuff and will create a specific PR.

The alignment boundary is on my list, including the blitter support you already did for Microwindows. I had a brief look on Nuklear too, looks pretty nice, but it requires probably a much higher resolution than 320x240.