Skip to content
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

Restore support for the Nokia N-Gage #12148

Open
wants to merge 52 commits into
base: main
Choose a base branch
from

Conversation

mupfdev
Copy link
Contributor

@mupfdev mupfdev commented Jan 31, 2025

As described in #11243 and #6691, support for the Nokia N-Gage has been dropped due to technical limitations that have now been resolved. The custom-built C-Compiler now supports C99. The C++ compiler is still outdated and has only been retained for compatibility reasons.

Description

  • A completely new port that uses a native rendering backend instead of SDL's software renderer.
  • Limited but working audio support!
  • Built-in FPS display for performance control during development #.
  • EKA2L1 can output messages via SDL_Log().
  • A demo application is currently being developed. It can be found in the ngage-toolchain repository.
  • The backlight is kept on while the SDL application is running.

Existing Issue(s)

  • It is currently not possible to put the application that has been launched into the background. It continues to run and can then no longer be closed. This is a bug in the new rendering backend.

  • If the application is put in the background while sound is playing, some of the audio is looped until the app is back in focus.

  • It is recommended initialising SDLs audio sub-system even when it is not required. The backend is started at a higher level. Initialising SDLs audio sub-system ensures that the backend is properly deinitialised.

@slouken slouken added this to the 3.4.0 milestone Jan 31, 2025
Comment on lines 123 to 127
target_compile_options(
ngage_test
PUBLIC
-O3
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Configuring with -DCMAKE_BUILD_TYPE=Release will add -O3 automatically.

Suggested change
target_compile_options(
ngage_test
PUBLIC
-O3
)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed it from my demo application and I believe this is not the case. I used -DCMAKE_BUILD_TYPE=Release, but my performance dropped slightly (5-10 FPS). Where does this information come from?

Copy link
Contributor

Choose a reason for hiding this comment

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

This is added to CMAKE_C_FLAGS_RELEASE when using a GNU toolchain.
But since I posted the message above, I have used your toolchain file where I'm not sure the compiler is detected as GNU.
I have a fix for this, but am seeing a link error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Symbian requires a multi-step linking procedure. Have a look at build_exe. I try to replicate your problem ASAP.

Copy link
Contributor Author

@mupfdev mupfdev Feb 2, 2025

Choose a reason for hiding this comment

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

I added a minimal exe-application to the toolchain: minimal

Does that help?

Copy link
Contributor

@madebr madebr Feb 2, 2025

Choose a reason for hiding this comment

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

I'll try that. I'll also try to get it working with a pure add_executable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is definitely one of the topics that should be resolved before the PR is closed. Optimisation obviously makes a big difference here.

@@ -0,0 +1,133 @@
cmake_minimum_required(VERSION 3.16)
Copy link
Contributor

Choose a reason for hiding this comment

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

This file should be integrated into CMakeLists.txt, but once you get ci running for the currently supported SDL3 platforms, I'll try to do this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ngagesdk/ngage-toolchain#13

Apparently you tried this before. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mupfdev
Copy link
Contributor Author

mupfdev commented Feb 1, 2025

Some of the more recent changes is causing an error message when I close the running SDL application. I look into this.

source = dest;
}

Mem::Copy(phdata->bitmap->DataAddress(), source, pitch * h);
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks like something that should be done during texture update/unlock, rather than each time the texture is rendered.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It probably doesn't matter. NGAGE_UpdateTexture() is called before each call to this function internally.

return SDL_GetScancodeFromKey(keycode, NULL);
}

void CRenderer::HandleEvent(const TWsEvent& aWsEvent)
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be possible to separate event handling from the rendering code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In SDL2 it was exactly where it should be, in the video driver implementation, but since all the window management is now handled by the dedicated renderer, I moved it there. I wanted to keep it as simple as possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Speaking of code placement: In my example application I also pointed out that SDLs audio sub-system should be initialised, even when it is not needed. This ensures that all resources are properly released when the application is closed.

Why? Because the rendering and audio backends are initialised early in E32Main() before the active scheduler is activated. There may be other ways to do this, but it's not easy to find good example code for such an old platform.

break;
}

return SDL_GetScancodeFromKey(keycode, NULL);
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd suggest directly using the SDL_SCANCODE_ values here rather than converting from a fixed set of SDLK_ values.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How do you mean that? I am currently translating the scan codes from the Symbian API into the identifiers used by SDL.

@icculus
Copy link
Collaborator

icculus commented Feb 9, 2025

What's the status on this? Are we ready to merge?

(Tweaks and additional fixes/improvements can come later, but I'd rather this not have to live in a branch if it's Good Enough at this point.)

@mupfdev
Copy link
Contributor Author

mupfdev commented Feb 9, 2025

@madebr made great progress with CI. @madebr what's your take on this?

A merge would be fine with me, but N-Gage shouldn't be the only platform without a working CI.

@madebr
Copy link
Contributor

madebr commented Feb 9, 2025

@madebr made great progress with CI. @madebr what's your take on this?

A merge would be fine with me, but N-Gage shouldn't be the only platform without a working CI.

I can't comment on the N-Gage C/C++ code, which seems to be in ok state.
I'm not so happy with the current CMake integration: N-Gage's CMake toolchain file uses hacks and does not allow to build any SDL tests or examples.
I have a wip ngage-toolchain branch in which I modified the Emscripten wrappers to let the ngage toolchain behave in a way that cmake (meson?) likes.

Short term, I can live with this but long term this needs to be fixed.

mupfdev and others added 25 commits March 2, 2025 21:03
Co-authored-by: Cameron Cawley <[email protected]>
Also flipped the return value of readRlePixels() to match convention.

Fixes libsdl-org/sdl2-compat#308
…can be called to allow the backlight to turn off. respectively SDL_DisableScreenSaver() keeps it on.
…bian API, this fixes several compiler warnings.
… added to get rid of several implicit declaration warnings. The warnings are perfectly valid and should not be suppressed.
[sdl-ci-filter ngage]
@icculus
Copy link
Collaborator

icculus commented Mar 2, 2025

Let's try to get the CMake stuff up to standards and merge this, so @mupfdev doesn't have to keep working out of a branch.

@icculus
Copy link
Collaborator

icculus commented Mar 2, 2025

(Whether that's @madebr or @mupfdev, or me, that needs to do that, let's figure it out.)

@mupfdev
Copy link
Contributor Author

mupfdev commented Mar 4, 2025

@madebr started working on a compile wrapper to streamline CMake support. AFAIK it's in this branch: https://github.com/madebr/SDL/tree/cmake-ngage-extra-support

If there's anything I could do to help, just let me know. I can also modify the toolchain if necessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants