contrib/ports/ajaguar

[djipi]

This package is mostly linked to the Atari Jaguar support, which is limited to video rendering at the moment. It adds also compilation directives, palette functions, and a potential fix for a string manipulation. The palette functions seems working fine but would like to have a second look. Updated readme files and a screenshot bring additional information to the port.

[ghaerr]

Hello @djipi,

Very nice job on the Atari Jaguar port, as well as the Win32 palette routines!

There are a few issues with accepting the PR as-is, but I'd like to get most of it into the repository. I can talk about these things depending on what your objective is.

First item, it appears that you've done the work for both compilation using the normal "make" mechamism that uses src/config, Makefile.rules, Arch.rules, etc, as well as the much-harder-to-maintain "make -f Makefile_nr" approach, which has historically only been used for those development systems that don't have a compatible make, etc. Which are you using here, or have you got both to work?

Thank you!

[djipi]

Hi @ghaerr,

Thank you for your comments, feel free to ask me questions you may have. The Atari Jaguar is based on a 68000, with 2M of RAM, has a GPU/DSP/Blitter & OP capabilities; no Operating System, no keyboard and no mouse. For the mouse, there are some solutions to handle one but requires external HW.

I did the port for 2 main reasons, the first one is related to a retro-gaming project and the sources use Win32 functions which are present in microwindows (except the palette ones). The second reason was to give it a try.

I should have been clearer about the makefiles; I got one error when using them, tried to fix it but was out of luck with any make version I got the hand on MSYS2 or MinGW (I still would like to make it work); after that, I decided to move on the makefile_nr files and it worked just fine. Because of the nature of the Jaguar, it is necessary to handle everything (video init, runtime, etc.), so using the microwindows library require additional work. I have provided a screenshot showing examples running on the system.

[georgp24]

I think Makefile_nr is much easier to maintain than the usual group of Makefiles.

From: Gregory Haerr Sent: Monday, August 17, 2020 11:57 PM To: ghaerr/microwindows Cc: Subscribed Subject: Re: [ghaerr/microwindows] contrib/ports/ajaguar (#41)

Hello @djipi,

Very nice job on the Atari Jaguar port, as well as the Win32 palette routines!

There are a few issues with accepting the PR as-is, but I'd like to get most of it into the repository. I can talk about these things depending on what your objective is.

First item, it appears that you've done the work for both compilation using the normal "make" mechamism that uses src/config, Makefile.rules, Arch.rules, etc, as well as the much-harder-to-maintain "make -f Makefile_nr" approach, which has historically only been used for those development systems that don't have a compatible make, etc. Which are you using here, or have you got both to work?

Thank you!

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

[rofl0r]

I have provided a screenshot showing examples running on the system.

the screenshot is almost 2MB. since it uses only a few colors, i suppose it could be saved as an indexed .gif occupying a lot less space (note that once checked into the main git repo, the blob will remain in it even when replaced later)

[ghaerr]

Hello @djipi,

I did the port for 2 main reasons, the first one is related to a retro-gaming project and the sources use Win32 functions which are present in microwindows (except the palette ones). The second reason was to give it a try.

Two great reasons!! You've done a great job with it.

I will attempt here to provide feedback on which portions of the original commit can be made, and have discussion then on the best way to proceed.

I understand now that you tried using the normal config system, but could not because of make compatibility reasons. From first glance, it looks like all of the modifications you made to the config system for Jaguar can be kept, even though makefile_nr is currently required to build.

From @georgp24: I think Makefile_nr is much easier to maintain than the usual group of Makefiles.

That's fine Georg, if you're ok with it. You'll notice that all of the mods required to make Jaguar work will undo all of the settings you've made for DJGPP and other builds. That's the problem with the Makefile_nr approach, one must carefully go through and change tons of settings in order for their particular build to work. IMO, it would be far easier to have separate Makefile-xxx_nr files instead of having to re-do changes for each port that requires Makefile_nr.

Since you're the maintainer of Makefile_nr, if you want me to accept the changes to it and the other makefiles as is, that's ok with me, but the working port it replaces will stop compiling - FYI. This is why makefile_nr is harder to maintain. For systems that can't use GNU make, we need a redesign of Makefile_nr that allows specific port options to be changed without removing options for other working ports.

Following are comments on the current commit:

• Restore src/config to unmodified. (to compile, one will "cp Configs/config.jaguar config".
• src/drivers/fb.c: We can't keep the #if's per MWPIXEL_FORMAT, as fb.c (and other parts of Microwindows) are designed to allow frame buffer format switching on the fly.
• The #if MWPIXEL_FORMAT tests later in the file also cannot be kept, since Microwindows also supports image formats other than the frame buffer. Is the reason for these exclusions due to size constraints in the Jaguar?
• For the specific malloc & size ~7, the psd->size field should be changed where it is inited, rather than in the malloc statement. I don't quite understand the size & ~7, and then the adds + 8 in other routines... Does the Jaquar frame buffer start 8 bytes past the malloc'd area?
• The NOSTDPALX ifdefs are probably OK to keep, as it is usually not possible to switch between 1/2/4/8 bpp modes when running, except the latter ones in fb.c for the image conversion will need to be removed, as palette conversions of images to screen are supported, regardless of the compiled-in pixel format.
• src/drivers/kbd_ajaguar.c - we have a nul kbd routine which could be used. If you're thinking of adding keyboard support later, then this is fine for a stub.
• src/drivers/mou_ajaguar.c - same as kbd above.

For the various src/include files that are not required for Microwindows or it's Win32 API implementation, I think it better to keep these files in another directory, that can be searched when compiling various applications of your own. This would include headers like fileapi.h, the additional to winbase.h, wincon.h, the include in windef.h, the new includes in windows.h (possibly with a single "extra" include to make things work), new structures in wingdi.h (except new palette routines), and winnt.h.

The reason for the above exclusions is that there needs to be a quick way of knowing whether a function is in fact implemented within Microwindows Win32 or not. The current header files do not define routines or structures that are not used, and instead all the zillions of "extra" headers have been placed in contrib/winextra, and WINEXTRA=Y defined in config, to automatically search that directory.

• The changes in wingdi.c look good, with the exception of the "#pragma message XXX needs to be verified", which should be removed. (What is the reason for these?) Thank you for adding the Palette routines!

Let me know what you think and how you'd like to proceed, and thank you!

[djipi]

Hello @ghaerr,

Thank you again for your feedbacks.

I'm sorry about the makefiles issues, I wanted to minimize the changes as much as possible. I will simply follow your decisions about this.

• Restore src/config to unmodified. (to compile, one will "cp Configs/config.jaguar config".

Sure. I should have done it at first glance.

• src/drivers/fb.c: We can't keep the #if's per MWPIXEL_FORMAT, as fb.c (and other parts of Microwindows) are designed to allow frame buffer format switching on the fly.

Sure. It is probably possible on the jaguar too but will require a specific runtime. User has to program the video chipset to create the resolution stuff. Usually, developer tend to program for one resolution and then keep it.

• The #if MWPIXEL_FORMAT tests later in the file also cannot be kept, since Microwindows also supports image formats other than the frame buffer. Is the reason for these exclusions due to size constraints in the Jaguar?

I tried to reduce as much as code/data possible, since the library on M68000 need more room than on x86. This is not a problem to remove the #if

• For the specific malloc & size ~7, the psd->size field should be changed where it is inited, rather than in the malloc statement. I don't quite understand the size & ~7, and then the adds + 8 in other routines... Does the Jaquar frame buffer start 8 bytes past the malloc'd area?

The Object Processor requires objects to be on a 16 byte boundary, usually the video buffer is allocated directly inside the bss and the code tells the OP to uses it. The specific malloc is in relation with the blitter, it requires to be on a 8 bytes boundary. I wanted to give it a try but my knowledge of the blitter is too limited for the moment. I thought the define dedicated to blitter could be useful to another people, but no problem to simply wipe out this change.

• The NOSTDPALX ifdefs are probably OK to keep, as it is usually not possible to switch between 1/2/4/8 bpp modes when running, except the latter ones in fb.c for the image conversion will need to be removed, as palette conversions of images to screen are supported, regardless of the compiled-in pixel format.

Sure.

• src/drivers/kbd_ajaguar.c - we have a nul kbd routine which could be used. If you're thinking of adding keyboard support later, then this is fine for a stub.

Someone did one keyboard prototype for the system; unfortunately, I was not able to get the hand on it. I was also thinking to use the specific keypad (12 keys) available on the gamepad to simulate a minimal keyboard entry. I have no plan to do it in the near future, so I guess it is better to use the null stub.

• src/drivers/mou_ajaguar.c - same as kbd above.

I have a mouse adapter, the code to handle it need to runs in the IRQ. I do not plan to do it soon, so I guess it is better to use the null stub.

For the various src/include files that are not required for Microwindows or it's Win32 API implementation, I think it better to keep these files in another directory, that can be searched when compiling various applications of your own. This would include headers like fileapi.h, the additional to winbase.h, wincon.h, the include in windef.h, the new includes in windows.h (possibly with a single "extra" include to make things work), new structures in wingdi.h (except new palette routines), and winnt.h.

The reason for the above exclusions is that there needs to be a quick way of knowing whether a function is in fact implemented within Microwindows Win32 or not. The current header files do not define routines or structures that are not used, and instead all the zillions of "extra" headers have been placed in contrib/winextra, and WINEXTRA=Y defined in config, to automatically search that directory.

I understand, and I'm fine with your solution.

• The changes in wingdi.c look good, with the exception of the "#pragma message XXX needs to be verified", which should be removed. (What is the reason for these?) Thank you for adding the Palette routines!

You welcome. I added these pragma because I wanted a second look at the code. As you feel the changes are fine, such pragma can be removed.

Let me know what you think and how you'd like to proceed, and thank you!

I'm ok with your conclusion. I can either let you handle the modifications or I can redo a pull request.

[ghaerr]

I can either let you handle the modifications or I can redo a pull request.

How about lets leave this PR open (for reference), and you submit a sequence of separate PRs, in related pieces, to ease the review process and allow some more detailed discussion.

I'm thinking the first PR could include all readme file changes/additions, config file changes, and the Makefile_nr changes, but rename Makefile_nr to Makefile_jaguar. Then we'd keep the older Makefile_nr unchanged for the time being, I'd like to rename it to Makefile_djgpp, etc to avoid the problems mentioned previously. The changes in contrib/makefiles_nr/ can stay. All of the Arch.rules, etc modifications setting up the options specific for Jaguar stay.

Also include the screen, kbd and mouse drivers (since you're thinking of possibly enhancing those at some point, there's no problem using them instead of the null drivers).

In general, in the first commit we'll put almost all the items that are outside the Microwindows engine, and then we can more carefully discuss the changes to the core routines.

I've tried to keep the design so that separate platforms do not have to change the core engine/ (and other) routines with special #ifdefs per-platform unless absolutely necessary. In that way, platform-specific changes are made without having to worry about core routines becoming buggy.

The 2nd PR could include all of the C routines that you have enhanced, including all those that have the new MW_FEATURE_PORTRAIT changes (thanks, that's a useful addition to reduce size when not needed), the osdep.c changes, and the changes to winfont.c and wingdi.c.

A 3rd PR could include the extra windows files, which would be moved from the src/include directory to contrib/winextra/.

Does this sound like too much work? If so, I could take a pass at only including the items from this PR using a diff where I exclude everything that needs to get added seperately, and then have you add the remaining items in subsequent PRs. Let me know what you think.

The Object Processor requires objects to be on a 16 byte boundary, usually the video buffer is allocated directly inside the bss and the code tells the OP to uses it. The specific malloc is in relation with the blitter, it requires to be on a 8 bytes boundary. I wanted to give it a try but my knowledge of the blitter is too limited for the moment. I thought the define dedicated to blitter could be useful to another people, but no problem to simply wipe out this change.

Ah, I understand now. You're welcome to leave that change (the +8 and ~7 stuff) for now, it helps creating a faster driver. We can do a little cleanup afterwards. But those changes should only actually have to be made in your own screen driver and genmem.c.

The change in genmem.c for (pitch * height) + 8 should not use a general name of HAVE_BLITTER_SUPPORT, but just #if AJAGUAR, if that's what is needed for the OP. IMO there should only be one image format for the Jaguar, I guess you are still implementing this?

We will need to make a core change to Microwindows, as it currently only supports alignment to DWORD (genmem.c line 290). Perhaps a change to the pitch using pitch = (pitch + 7) & ~7 there, along with the screen driver to force the screen allocation to a 16 byte boundary might work better. I need to think about this a bit. I'm not sure you need 16-byte alignment on all images, just the screen buffer. Setting pitch doesn't change alignment unless we also wrap all the image allocation routines as well. But the Microwindows blitters only require DWORD alignment.

Microwindows can support either 1) all drawing into a malloc'd buffer, which in your case would be on a 16 byte boundary, or 2) drawing directly into a hardware frame buffer. I don't fully understand the Jaguar OP fully, but it sounds like you're having Microwindows draw into a malloc'd buffer, then having the OP use that memory to update the hw screen at update time.

What is the HAVE_BLITTER_SUPPORT option? Should that be confined only to your screen driver or are you looking for MIcrowindows blitter support as well? You'll want the mwin internal blitter support for fast drawing.

Since you've written your own screen driver, it's probably best just to drop the changes in scr_fb.c, which are used for Linux and systems that support changing the frame buffer format at runtime.

[ghaerr]

Oops - correction:

If Object Processor requires a 16 byte boundary, then pitch should be calculated as (pitch + 15) & ~15, and effectively the same for the screen memory alloc in the Jaguar screen driver.

[djipi]

I should be able to handle it, I'm not a github professional but I think I have to create separate branches to dispatch the works. It may take some delays thought. @rofl0r has notice the size of the screenshot, I can also try to reduce it.

I set the HAVE_BLITTER_SUPPORT believing it could also be used on system with blitter such as the Amiga, Atari STe/Falcon. I do not have a Falcon unfortunately, but a system equipped with a CT60 card (68060) would be an interesting experience. As the Jaguar's blitter support is not yet functional, may I suggest to put it away and I will later have a look at the mwin internal blitter?

*I don't fully understand the Jaguar OP fully, but it sounds like you're having Microwindows draw into a malloc'd buffer, then having the OP use that memory to update the hw screen at update time.

This is a good summary; please note the OP is based on objects, so it is possible to make it draw lot of stuff. In our case, the OP handles only the screen; used this option to have microwindows running on the system in a reasonable time frame.

[djipi]

Hi @ghaerr , I have removed the changes not related to the Atari Jaguar. Please let me know, it this is fine. I will create new PRs for the changes we have previously discussed.

[ghaerr]

Hello @djipi,

Nice work, thank you!

I have some concern about src/config being in .gitignore, I like to see changes in that file, and also possibly the *.c in src/images/.gitignore, I don't quite understand the reason for that. But this is minor, so I'm going to commit this now, and look forward to your other PRs required for the Jaguar or to improve Microwindows.

Thank you!

[rofl0r]

@rofl0r has notice the size of the screenshot, I can also try to reduce it.

unfortunately, as this PR has not been squashed, now both and old version of the screenshot made it into the commit history, and therefore both blobs are now saved in the repo (and everyone pulling the repo will have to download them).

the only way to undo this is to reset HEAD to what it was before merging the PR, and then force-pushing with a squased commit, or doing the same with git rebase -i.

[rofl0r]

btw the new screenshot is still half a megabyte, saved as an indexed png gif it would be 250KB: https://0x0.st/iglq.png

[ghaerr]

Hi @rofl0r,

Thanks for your comments. I'm definitely a better C programmer than a git manager.

I'm traveling right now, on a laptop where I have not yet pulled the last merged PR. Admittedly, I've been managing the ELKS project and normally merge commit over there, this repo doesn't see many PRs and I merged rather than squashed, inadvertently making things worse.

If you can give me the exact way I could fix this, I'll do it from the laptop (I don't have the merge commit pulled yet).

Not sure how I could grab/use your PNG instead... but am willing to try that as well.

Finally, is your recommendation that for this repo, all PRs should be squashed and merged, or rebased and merged?

[rofl0r]

If you can give me the exact way I could fix this, I'll do it from the laptop (I don't have the merge commit pulled yet).

i'm currently pushing a fixed version, but it is really going slow because the repo is already pretty big and upload speed is meager: Writing objects: 46% (6149/13292), 22.54 MiB | 47.00 KiB/s i'll tell you when the upload completes how you can merge the fixed version.

in the meantime, what i did was:

git clone https://github.com/ghaerr/microwindows 
git rebase -i cbf6e10556634c187e8e85004d9130e773d488ea
(change all but the first lines that open in the editor from "pick" to "f")
wget https://0x0.st/iglq.png
mv iglq.png screenshots/Microwindows0.94pre_Demos_AtariJaguar.png
git rm screenshots/Microwindows0.94pre_Demos_AtariJaguar.jpg 
git add screenshots/Microwindows0.94pre_Demos_AtariJaguar.png 
git commit --amend

Finally, is your recommendation that for this repo, all PRs should be squashed and merged, or rebased and merged?

depends on the PR, but usually most PRs only do one "logical change", so it's fine to squash them into a single commit.

[rofl0r]

i'll tell you when the upload completes how you can merge the fixed version.

on the repo version that is on cbf6e10556634c187e8e85004d9130e773d488ea, execute

git remote add atari_fixed git://github.com/rofl0r/microwindows-tmp
git fetch atari_fixed
git merge atari_fixed/atari_squashed
(output should tell you "fast-forward", i.e. no merge commit was created)
(review commit in gitk or similar local frontend)
git push -f

[ghaerr]

Ugh. My laptop last commit is actually back from June 8, 2019 (56041759161132a94df3e575ae995e53d6b6e1d8).

That means I need to fetch about 9 commits through Apr 12, 2020. Is there an easy way to do this, or should I just git pull then try to back off HEAD from that? I don't want to screw everything up, I'm pretty good screwing things up with git.

[rofl0r]

That means I need to fetch about 9 commits through Apr 12, 2020. Is there an easy way to do this, or should I just git pull then try to back off HEAD from that?

if you pull my branch as descibed in https://github.com/ghaerr/microwindows/pull/41#issuecomment-679424224 it will already include those commits.

in order not to screw things up i'd highly recommend to install the gitk gui frontend to review the commits before pushing (git log -p is also an option, but it doesn't show you merge trees).

[ghaerr]

Hello @djipi,

I finally looked at the screenshot. BTW, have you tried turning on the config option NUKLEARUI=Y, which gives you the much-updated window draw code? It would look pretty nice on the Jaguar.

[djipi]

Hi @ghaerr

The resolution I'm using is 320x240 palletized. I may try a higher one, but the video registers are rather special to handle. I have notice the config option NUKLEARUI, I was wondering about his usage. I will try it, probably after I'm done with the PRs.

I'm doing the next PR about the palette functions, beside the updated and new code; my understanding is the 4 additional headers file must be added in the contrib\winextra directory. I'm still learning the ropes of Github, and try to minimize the problems.

[rofl0r]

if you pull my branch ...

well, please let me know whether i can remove my temporary repo or not.

btw, one can test the git commands mentioned / play around safely by checking out a copy of the repo to another location, such as /tmp.

[ghaerr]

Hi @rofl0r,

I'm sorry, I'm traveling and haven't had time to download gitk and mess with this right now.

I haven't pulled your repo yet, and am likely going to have to wait until I return home next week before I can try it.

Rather than leaving your temp repo out, perhaps leave this 2Mb blob in, and I will more carefully check image sizes before committing in the future, so you won't have to wait so long to download. I don't want to rush learning gitk nor risk damaging the master repo, until I have backups and can review it all.

Thank you!

[rofl0r]

I don't want to rush learning gitk

well, there's not really much to learn: you just run "gitk" from the shell after cd'ing to your repo and then enjoy a GUI which is kinda like the "commits" link in github, but much more responsive. gitk is a TCL/TK script distributed together with git sources, if you have TCL/TK installed it should be as simple as copying the gitk script from git-x.y.z/gitk-git/ to /usr/local/bin

[djipi]

I haven't pulled your repo yet, and am likely going to have to wait until I return home next week before I can try it.

I guess it is better to wait your return home before I send you any new PR?

[ghaerr]

I guess it is better to wait your return home before I send you any new PR?

No, keep submitting them, please. I may need more time to review them, depending on how technical they get, anyways. Would you like to submit multiple PRs, or prefer to have each committed before the next?

well, there's not really much to learn: you just run "gitk" from the shell

@rofl0r, thanks, I'm running macOS and I'm reading up on the homebrew installation, which seems to have some kinks in it. I remain concerned for this last commit whether its worth me endangering the master repo, given my propensity to screw git things up... I have done it before. I will definitely ask you about images before the next one though!

[djipi]

@ghaerr

Would you like to submit multiple PRs, or prefer to have each committed before the next?

I use branches, and try to avoid dependencies between them. So, I will submit multiple PRs with some delay between them.

I have done some sort of plan. 1) PR for the palette functions and the NOSTDPALX compilation directive (it won't affect the other platforms, as it only detects the presence of the NOSTDPALX defines). 2) PR for a BMP format compilation fix, add Visual Studio / Win32 support in the .BMP file converter souce (demo tool / makebmp) and a (potential) fix for a string manipulation in the fonts. 3) PR for portrait compilation directives, and for the international characters. 4) PR for one additional Win32 function timeGetTime(), and headers updates / additions. Additional headers: fileapi.h, winbase.h, wincon.h, and winnt.h

I have some concern about src/config being in .gitignore, I like to see changes in that file, and also possibly the *.c in src/images/.gitignore, I don't quite understand the reason for that.

5) config file is back to his original condition. Sorry about this. 6) The .c in src/images/demos/mwin directory are not in your repository, so I've thought it was unnecessary to commit them as well. I have generated most of the .c so I can commit them if you want to.

[ghaerr]

@djipi,

Sounds good.

For #6 above, no, I don't think we need to commit those demos, my concern was just that we should not ignore them all in .gitignore.

[ghaerr]

I have notice the config option NUKLEARUI, I was wondering about his usage.

As I think about it, one of the problems with the nicely-colored NuklearUI option is that is uses a number of slightly-different colors in order to achieve its effect. That will have issues with a fixed palette. Microwindows does support auto-filling the palette, currently only during image display, using the gr_nextpalentry in-use palette pointer. It is possible that could be extended to allow the palette to be auto-filled during the window frame drawing, which should allow this to work. The other option would be a NuklearUI pre-filled pallette in devpal8.c.

[ghaerr]

Hello @rofl0r,

Since I've asked @djipi to submit more PRs, would you like to fix this multi-blob repo issue from your end, since I'm still a bit hesitant about it? I could add you for write access and you could perform the git force push and gitk inspection, before the next commits. I have the repo cloned/backed up.

Thank you!

[rofl0r]

would you like to fix this multi-blob repo issue from your end

thanks, i checked the insights/traffic tab and because there were only 3 git clones since the PR merge, i decided to forcepush the cleaned up commit i had prepared. should be all good now!

[ghaerr]

Thanks @rofl0r,

Hello @djipi, can you take a quick look at #42 and #43 again? It seems the .gitignore issue with src/ and demos/ has popped up again after @rofl0r's fix. We're ready to commit both those after that.

[rofl0r]

i've pushed PR's #44 #45 #46 as rebased versions of @djipi 's last 3 PRs (since they were based on master pre-force push)

It seems the .gitignore issue with src/ and demos/ has popped up again after @rofl0r's fix.

what are you refering to ? master's contents should be identical to what it was after this PR was merged originally.

[ghaerr]

@rofl0r,

The conflicts with .gitignore popped up after your force push, but then went away after your rebase changes, thanks.

It seems, for some reason, that the jaguar screenshot PNG is not displaying on the main page... is it working on your system?

[rofl0r]

@ghaerr oops, the filename in Readme slipped through my radar, addressed in #48