Skip to content
This repository has been archived by the owner on Feb 10, 2024. It is now read-only.

[SDL1.2][WIP] AmigaOS 3.x Support #128

Open
wants to merge 23 commits into
base: SDL-1.2
Choose a base branch
from
Open

[SDL1.2][WIP] AmigaOS 3.x Support #128

wants to merge 23 commits into from

Conversation

MBeijer
Copy link
Member

@MBeijer MBeijer commented Feb 17, 2020

@capehill If you see something that won't work, just let me know. This is extremely WIP and will take some time to fix. I will add AmigaOS4 support to CMake as well

@MBeijer MBeijer added enhancement New feature or request help wanted Extra attention is needed OS:amigaos3 labels Feb 17, 2020
@MBeijer MBeijer self-assigned this Feb 17, 2020
MBeijer
MBeijer commented Feb 17, 2020
View reviewed changes
src/main/amigaos/SDL_main.h Outdated Show resolved Hide resolved
MBeijer
MBeijer commented Feb 17, 2020
View reviewed changes
include/SDL_stdinc.h Outdated Show resolved Hide resolved
capehill
capehill reviewed Feb 22, 2020
View reviewed changes
src/video/SDL_video.c
}
if ( saved_colors ) {
pal->colors = saved_colors;
}

/* Fall through to video surface update */
#ifndef __AMIGA__
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please exclude amigaos4

Copy link
Member Author

@MBeijer MBeijer May 14, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ALL
Shouldn't be that AMIGA ifdef be removed, in favor to have all flavors necessary define? I mean:

__amigaos3__
__amigaos4__

Because things like #if defined (AMIGA) && !defined(__amigaos4__) not reads very well, instead #if defined (__amigaos3__) && !defined(__amigaos4__) reads much better and cleaner (that how we do back in past for dopus5 port).

@kas1e @capehill
I have sent in a suggestion to @bebbo to add __amigaos3__ as a default define for the OS3.x toolchain.

I will also add this in the os3-config file, so it works on older toolchains:

#ifndef __amigaos3__
#define __amigaos3__
#endif

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It got merged! bebbo/gcc#156
🥳

capehill
capehill suggested changes Feb 22, 2020
View reviewed changes
Copy link
Collaborator

@capehill capehill left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have tested most SDL tests on AmigaOS 4 now. Double buffer test didn't work (there was flicker), I suspect changes in SDL_video.c area.

capehill
capehill reviewed Feb 22, 2020
View reviewed changes
src/video/SDL_yuv_sw.c
unsigned char* row2;
unsigned char* lum2;
int x, y;
int cr_r;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Compiler warnings about set but not used vars 368-370

MBeijer
MBeijer commented Feb 22, 2020
View reviewed changes
src/video/SDL_surface.c Outdated Show resolved Hide resolved
MBeijer and others added 3 commits February 22, 2020 16:08
@MBeijer @capehill
Update src/video/SDL_video.c
62baa83 
Co-Authored-By: capehill <juha.niemimaki@gmail.com>
@MBeijer @capehill
Apply suggestions from code review
55f1a21 
Co-Authored-By: capehill <juha.niemimaki@gmail.com>
@MBeijer
Update src/video/SDL_video.c
4a54350
@kas1e
Copy link

kas1e commented Feb 22, 2020

@ALL
Shouldn't be that AMIGA ifdef be removed, in favor to have all flavors necessary define? I mean:

__amigaos3__
__amigaos4__

Because things like #if defined (AMIGA) && !defined(__amigaos4__) not reads very well, instead #if defined (__amigaos3__) && !defined(__amigaos4__) reads much better and cleaner (that how we do back in past for dopus5 port).

@MBeijer
Copy link
Member Author

MBeijer commented Feb 22, 2020

@ALL
Shouldn't be that AMIGA ifdef be removed, in favor to have all flavors necessary define? I mean:

__amigaos3__
__amigaos4__

Because things like #if defined (AMIGA) && !defined(__amigaos4__) not reads very well, instead #if defined (__amigaos3__) && !defined(__amigaos4__) reads much better and cleaner (that how we do back in past for dopus5 port).

is amigaos3 a default toolchain define? if not, that's not a choice we can make.

@kas1e
Copy link

kas1e commented Feb 23, 2020

is amigaos3 a default toolchain define? if not, that's not a choice we can make.

Why not ? We can.

I do not remember, you may check it on your os3 gcc as os3 version mantainer :) Even if not, it can be introduced . I just do not remember how it with os3 anymore. But we use that defne it on dopus5.

It can be just like #if detect_amigaos3_dunno_how_it_now, then #define __amigaos3__ 1 ,somewhere in the main includes, if it didn't there by default. And all code will be reads fine.

Its just will read beter.

But not that i care much, i only care that os4 version will be not broken by replaced files.

At first place all the changes was in needs be placed by single commts, step by step, via amigaos3 ifdefs, without total replacing of some files.

@capehill
Copy link
Collaborator

capehill commented Feb 23, 2020 •
edited
Loading

It can be just like #if detect_amigaos3_dunno_how_it_now, then #define __amigaos3__ 1 ,somewhere in the main includes, if it didn't there by default. And all code will be reads fine.

#if defined(AMIGA) && !defined(__amigaos4__) && !defined(__MORPHOS__)
#define __amigaos3__ // Or whatever...
#endif

Might work.

EDIT: MorphOS...

@kas1e
Copy link

kas1e commented Feb 23, 2020 •
edited
Loading

@capehill
Yeah, having that block one time somewhere in os3 specific files of sdl1.2 and ___amigaos3___ in all other parts , for sure will made it all reads better and fixing/add things more easy, without messing hacks and stuff all together so other ones will in needs to retest what was broken in working before code :)

@MBeijer
Copy link
Member Author

MBeijer commented Feb 23, 2020

@kas1e What you don't get, is that, that custom define would need to be redefined in all other application sources. I won't do it just because you don't find it aesthetically pleasing.

if everyone made up their own OS-defines, code wouldn't be very maintainable.

@kas1e

This comment has been minimized.

Sign in to view
@capehill
Copy link
Collaborator

capehill commented Mar 7, 2020

@MBeijer I tested the latest, now double-buffer test works again, thank you :)

@MBeijer
Copy link
Member Author

MBeijer commented Mar 7, 2020 •
edited
Loading

@MBeijer I tested the latest, now double-buffer test works again, thank you :)

@capehill
Thank you for testing! It seems to run beautifully on AmigaOS3.x as well, so it's getting close to a merge. I want to clean up some more stuff from the generic sources first though!

@capehill
Copy link
Collaborator

capehill commented Mar 8, 2020

@MBeijer Please enable compile warnings too. We use -Wall on OS4. Should actually add -Werror as well :)

@MBeijer
Copy link
Member Author

MBeijer commented Mar 8, 2020

@MBeijer Please enable compile warnings too. We use -Wall on OS4. Should actually add -Werror as well :)

In CMake, or?

@capehill
Copy link
Collaborator

capehill commented Mar 9, 2020

In CMake, or?

I meant originally Makefile.amigaos3 but I cannot see any warning flags in CMakelists either. Not sure if they are pulled from some other source? I haven't tried CMake to build SDL1.

@MBeijer MBeijer changed the title [WIP] SDL1.2 AmigaOS 3.x [SDL1.2][WIP] AmigaOS 3.x Support Mar 26, 2020
@MBeijer
Copy link
Member Author

MBeijer commented Mar 26, 2020 via email

@MBeijer
Copy link
Member Author

MBeijer commented Jun 21, 2020

@capehill Fixed AHI audio for OS3, and removed the junk from SDL_audio.c. I basically backported OS4's AHI code to OS3 compatible API calls. So perhaps we should merge that backend and have one common AHI one and just do OS4/OS3 specific stuff with ifdefs?

@MBeijer
Copy link
Member Author

MBeijer commented Jun 21, 2020

@kas1e @capehill rather than defining amigaos3, I intend to clean up OS-specific stuff from the general files. And if a fix in the general files are necessary, they should use the defines that comes with SDL_config.h:
#if SDL_VIDEO_DRIVER_CYBERGRAPHICS, #if SDL_AUDIO_DRIVER_AHI etc, like you're supposed to do. :P

@capehill
Copy link
Collaborator

capehill commented Jul 1, 2020

@capehill Fixed AHI audio for OS3, and removed the junk from SDL_audio.c. I basically backported OS4's AHI code to OS3 compatible API calls. So perhaps we should merge that backend and have one common AHI one and just do OS4/OS3 specific stuff with ifdefs?

I have missed (at least) these comments.

It sounds OK to me. Of course if there will be lots of 68K optimization tricks, then maybe separate files would be better.

@MBeijer
Copy link
Member Author

MBeijer commented Jul 1, 2020

@capehill to be honest, the backported AHI backend feels quicker and more responsive than the old one, so I'm unsure if any hacks would be needed.

My point is that, it'll be easier to maintain one codebase than several (MOS, WOS, OS4, OS3, AROS) since they all use AHI and have pretty much the same API.

perhaps compile-time ifdef macros would be a cleaner approach, so we won't have to clutter up the code too much with ifdefs.

There's also some unused code in the OS4 AHI code from what I can remember.

@MBeijer MBeijer mentioned this pull request May 15, 2021
Open
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@capehill capehill capehill requested changes

Assignees

@MBeijer MBeijer

Labels
enhancement New feature or request help wanted Extra attention is needed OS:amigaos3
Projects
None yet
Milestone
Unified SDL-1.2 for all platforms
Development

Successfully merging this pull request may close these issues.

[SDL1.2] Merge in changes from libSDL12 version
3 participants
@MBeijer @kas1e @capehill