Opened 15 years ago

Closed 14 years ago

Last modified 12 months ago

#8425 closed patch

EPOC/Symbian port

Reported by: SF/sumthinwicked Owned by: anotherguest
Priority: normal Component: Port: Symbian
Keywords: Cc:
Game:

Description

Hey folks, here is the first version of the first patch I
have ever submitted. Basically it's an RFC, so don't
hesitate :P

There are some #ifdefs in weird places, but without it
gcc/UIQ will barf all over itself :(

Also please not that I am not the author of the original
ports (0.5.1, 0.6.1 & 0.7.1, done by Andreas 'Sprawl'
Karlsson), I am only the one that is trying to get it up to
current ScummVM standards and into CVS.

If you want to try to build this puppy yourself using the
UIQ2.1 framework, then ask me for some pointers first,
so you don't have to invent the wheel all over again.
(And trust me: there's a lot of wheels to be invented!)

I'll be seein' ya!
SumthinWicked

Ticket imported from: #1188433. Ticket imported from: patches/530.

Attachments (3)

ScummVM-EPOC.v1.diff (207.0 KB ) - added by SF/sumthinwicked 15 years ago.
Symbian port patch version 1
ScummVM-EPOC.v2.diff.gz (27.6 KB ) - added by SF/sumthinwicked 15 years ago.
Symbian port patch version 2
ScummVM-EPOC.v3.diff.gz (31.3 KB ) - added by SF/sumthinwicked 15 years ago.
Symbian port patch version 3

Download all attachments as: .zip

Change History (46)

by SF/sumthinwicked, 15 years ago

Attachment: ScummVM-EPOC.v1.diff added

Symbian port patch version 1

comment:1 by sev-, 15 years ago

http://www.scummvm.org/documentation.php?view=conventions

And if possible, compress your patch before submitting.

comment:2 by fingolfin, 15 years ago

Some comments/questions on the patch (besides what Eugene
already said :-)

* The mods in common/scaler.cpp don't make sense to me. Why
are they there? Looks not like something I'd accept into
CVS...

* printf uses are commented out. But OTOH, printf is
#defined. Why do both? Anyway, we can try to replace most
usages of printf with warning/debug calls, if that helps.

* g_sysfont_huge: why that? you really need a *bigger* font?
Last night you made the impression that you need a *smaller*
font... huh?

* The hint about ttf2bdf is useful; I guess it would be a
good idea to add this, as well as some rough info on how to
use convbdf (and its output), somewhere. Not sure where,
though, maybe to tools/README

* You modify the MSVC8 project files, is that on purpose? If
so, what do the changes do?

* All your new source files must have a GPL / copyright
header. This is important. You probably will want to give
credit to the original author, yourself, but also the
ScummVM team. Look at e.g. backends/sdl/sdl.cpp

* The copyright header in backends/fs/symbian/symbian-fs.cpp
is obviously wrong

* The change in OSystem_SDL::setupIcon (allocating 'icon' on
the heap) was made due to stack size limitations, I assume?
If so, I'd explain that in a comment next to the code,
otherwise somebody might revert it. Also, out of curiosity,
is that code actually used on Symbian?

* Why did you add a commented out WRITE_UINT16 in scummsys.h?

* What about the RandomSource::RandomSource() mods?

* What does this strange "releaseTimerResource" code in
scumm/smush/smush_player.cpp do ? It looks a lot like a
deeply flawed attempt to provide thread sync... does the
Symbian port feature 'real' threads, or are they emulated?

by SF/sumthinwicked, 15 years ago

Attachment: ScummVM-EPOC.v2.diff.gz added

Symbian port patch version 2

comment:3 by SF/sumthinwicked, 15 years ago

First: thanks for the comments, second: this is not originally
my port: so a few of the changes to 0.7.1 I don't even
understand, they just work and need to be there for it to
compile on gcc/UIQ. I am open to better suggestions to
handle some of the hacks. Let's discuss them. I am in the
process of trying to figure out why each mod was made and
possibly working around them.

* common/scaler.cpp: it's a way not to compile the scalers in
and still be able to use SDL, this removes the dependencies
on scalebit.cpp & friends. Since the UIQ build system can't
use DISABLE_SCALERS... Any suggestions?

* printf: true: it's a combination of both.. at first when I was
looking through the code I noticed that Andreas had
commented these out, without documenting why: later I
found out that EScummVM crashes when something is printf
()'ed. Later I 'constructed' the neater #define, so the
outcommenting can go now. I added this yesterday. Night.
Uhh.. morning :) debug calls would be the best solution, then
the entire #define hack can go. I changed them to debug
(1, ...) for now...

* g_sysfont_huge: yes: bigger and more to the point: fatter:
you can't read the normal g_sysfont_big font @ around
320x200. If there is a better way to make the font more
readable for small screens without introducing this font
please let me know.

* ttf2bdf: invent a wheel: tell people about it :)

* MSVC8: I shouldn't have included these: all-nighter-oopsy.
The files on CVS do miss some .cpp files by now, especially
for gob & saga I thought.

* GPL: true: all header stuff that is in there was done/omitted
by Andreas. I think I did one file: ScummApp.cpp. I added
headers to all the files now.

* OSystem_SDL::setupIcon: that was another one of those
undocumented changes. Actually: Symbian can do without
it, so I removed the changes and added another !defined()
around the call of setupIcon(); (MacOSX doesn't use it either)

* WRITE_UINT16: because in CVS it's not there at all: the
LE/BE READ/WRITE macros are incomplete: maybe
because they are not used (yet). Don't we want to complete
the READ/WRITE-16/32 set? I added the missing macros.

* RandomSource::RandomSource(): another undocumented
one by Andreas. I added a comment explaining this: //
SumthinWicked says: hmmm.. weird.. I could swear time(0)
used to cause problems on Symbian/WINS... It seems to be
compiling ok now, so I decided to remove the mod entirely.

* releaseTimerResource: in short: I haven't the faintest... we
would have to ask Andreas. I'll send him a message to ask
him if he would look at my work so far...

* coding conventions: backends/epoc code is all by Andreas:
I will update these later...

Cheers,
SumthinWicked

comment:4 by fingolfin, 15 years ago

* Your build system can't use DISABLE_SCALERS ? Well, no
problem, simply do not compile those files *at all*, that
should be sufficient. Or do you need some select
scalers/functions from scaler.cpp? If so, tell us which, and
we can discuss whether it makes sense to split the file

* One problem with #define'ing printf probably is that the
Debugger has a method with that name. Well, we could rename
that method, no big deal.

* In some cases, we would replace printf with debug(0,...)
or debug(1,...), in others with warnings; there are a few
cases, though, when we really want to print something... hmm

* You can't read g_sysfont_big at 320x200 ?? Hu? You
definitely can over here. And you can also read the small
font. Maybe your problem is not the pixel size, but rather
that you are looking at a tiny screen? I imagine if 320x200
was displayed at the size of a small stamp, I'd have
troubles reading the font...

* ttf2bdf -> I put a README file into the tools/ directory
which mentions it

* Regarding WRITE_UINT16: We use the READ_* macros for LE/BE
specific access, nothing else. READ_UINT32 is an
anachronism, used together with MKID, that's all. There's no
reason for a WRITE_UINT16 macro, or simililar "native
endian" read/write macros.

* releaseTimerResource: good

I'll now take a look at your new patch.

comment:5 by fingolfin, 15 years ago

Owner: set to fingolfin

comment:6 by fingolfin, 15 years ago

* what is 'backends/epoc/build/.placeholder' good for?

* why were empty destructors added to various ~Debugger
classes

* The change in simon/vga.h is possibly needed on other
systems, too. I extended it and commited it to CVS.

* In OSystem_SDL_Symbian::OSystem_SDL_Symbian, you do this:
ConfMan.set("FM_high_quality", false);
ConfMan.set("FM_medium_quality", true);
That seems odd, you always override these values. Either you
really meant to use registerDefault here; or, if the values
are hard coded anyway, don't store them into the config
manager, and instead use a #define for them...

* Our source files should end with a newline; many of the
files in the epoc/ dir do not.

* Of course, you two guys should be covered in the credit
lists.

* I am astonished that g_sysfont_huge works for you,
considering that our font renderer currently only supports
fonts with a width of up to 16 pixels, while the huge font
is 24 pixels wide...

by SF/sumthinwicked, 15 years ago

Attachment: ScummVM-EPOC.v3.diff.gz added

Symbian port patch version 3

comment:7 by SF/sumthinwicked, 15 years ago

* DISABLE_SCALERS: true: not compiling is already what I
do, but graphics.cpp/.h uses stuff in scaler.cpp. Symbian
only uses GFX_NORMAL, so only Normal1x is used. I
copied the trivial Normal1x() to SymbianOS.cpp together with
gBitFormatand and InitScalers(). Now scalers.cpp is no
longer nescessary.

* printf: there are situations where you want scummvm to
report errors, just not warnings/notices, hmm yes: we should
introduce a notice(...) fcn in the style of the warning() and
debug() fcns. Output could then easily be disabled for
__SYMBIAN32__. We could leave the Debugger printf() alone
then.

* WRITE_UINT16: I removed my additions.
sky/rnc_deco.php: I remember now: there was a
WRITE_UINT16() in here: it was used by Andreas in a part of
fcn: RncDecoder::makeHufftable()
if (huffLength[i] == bitLength) {
WRITE_UINT16(table, (1 << bitLength) -
1);
table++;
this seemed no longer nescessary, so I removed it, I'll have
to ask Andreas if there was a reason for the macro.

* 'backends/epoc/build/.placeholder' is used in the build
process to create the .sis Symbian installer package. It
ensures that the directory 'C:\documents\EScummVM' is
created, so scummvm.ini can be saved here by the app on
first execution. If omitted, the dir is not created & the save
fails. We could add a createDir() for this, but I think this is
better: this means the dir & .ini are removed on app uninstall.
The installer is configured to not remove this file on an
upgrade.

* empty destructors for ~Debugger classes: without them
compilation fails :( They do no harm, right? I added
comments to signify their importance? Hmm .. weird: I tried
moving the destructor implementations from the .cpp to
the .h files, but gcc/ARMi b0rked, so I had to move them
back.

* ConfMan.set("FM_*_quality", *): you have found another :) I
assume this is to preserve processing speed: there might be
a better way to handle this though... I guess we need to ask
Andreas again :)

* newlines: fixed.

* coding conventions: fixed.

* credits.pl: would that be me under ScummVM team and
Andreas under Contributors? I'm not sure where and if
Andreas wants to be... something to add in the mail I'll send
him I guess :)

* g_sysfont_huge: I experimented a bit with _enableScaling &
different fonts. This seems to work for now. I'll work on it a bit
more lateron: maybe we don't need to add this font at all... I
also figured out why you thought there wouldn't be problems
with g_sysfont_big @ 320x200: in graphics.cpp there is a
piece of code in setGraphicsMode() that changes to
GFX_DOUBLE if (_screenWidth * newScaleFactor <
_overlayWidth). If you comment out this code, scummvm
does not even work correctly anymore: game elements are
ok, but the UI still renders 2x! My P900 has 208x320, so it's
pixel perfect!

* added a comprehensive README with full build
instructions, links, kudos & the like

* I'm trying to find out why this is: if you compile in config-
file.cpp, then the application will fail to start in the WINS UIQ
Emulator with an "Application couldn't be started" error. Any
ideas? Any unhandled exceptions in the new config-file.cpp?

comment:8 by fingolfin, 15 years ago

* "empty destructors for ~Debugger classes: without them
compilation fails" -> interesting what error do you get?

* g_sysfont_huge: I still don't understand how it ever could have
worked for you. The font renderer (!) doesn't support wide fonts.
I.e. the code responsible for drawing the font, no matter whether
it is scaled or not.

* config-file.cpp is only used by some SCUMM-HE code at this
point. Any run time failures make no sense.

comment:9 by fingolfin, 15 years ago

What's the status of this? I'd like to get this integrated
soon, but I still need to hear some answers:

* g_sysfont_huge is still unclear. As I explained, our font
renderer will not even work with that font, so I don't see
how it was working for you. Can you elaborate?

* config-file.cpp had some missing code in older revisions,
maybe that caused your problems. Does it work with latest
CVS?

comment:10 by fingolfin, 15 years ago

In the meantime, some additional OSystem methods were added, you may
have to slightly adapt some of your code (maybe not, though, since you
are subclassing the SDL backend).

If you can provide a new patch which applies to CVS, and clarify the
remaining questions, I am sure we can have this in CVS very soon (and
you can get commit access etc.)

comment:11 by SF/sumthinwicked, 15 years ago

hey folks, just to let you know: I'm in Mexico now on an
Internship for my University degree. I'll be here for about
5 months. After I settle in and get all my things in order
(like my room here, my new notebook, P900 docking
station/software, VS2005, UIQ2.1 build environment, etc) I
will resume work on the EPOC port. I suspect this to be in
about a week.

Further, I haven't heard form Sprawl/Andreas yet, and I
would really like him to take a look at my work so far, as
he still knows more about the EPOC platform than I do...

so, hope to post here again soon!

regards,
SumthinWicked

comment:12 by anotherguest, 15 years ago

Hi! Well.. actually it is me that has made the latest ports &
updates to the EPOC/Symbian port. It seems like some of
my patches has been going into the platform which I think is
great. But still there are some heaving patching todo,as the
Symbian platform is in deed more sensitive to alignment
problems compares to the Pocket PC version.

So have you grabbed the latest 0.7.1 version from
Sprawls/Andreas homepage where I have made some
improvements, as recommended by Max!

comment:13 by anotherguest, 15 years ago

*printf.. On UIQ it brings up a textconsole on top of the
screen, even as a different process, this does n't look very
good.
The best way for scummvm would be to use defines, so
different platforms could call different functions
*scalers - Well on a 320x208 platform, there is no reason to
have any other scalers except the 1x in there. There is no
current way to disable the
others except the 1x scaler or?
* RandomSource::RandomSource The time(0) is crashing the
Symbian emulator when called. Why I dont know.. it just
happens.. Works fine on target
* There are stackproblems in ScummVM at least for the Wins
platform, so I have removed as many as I can.
* MP3/Movie support. A great deal of patching has gone into
this, as Symbian can't share filehandles between different
threads. Some of this was fixed for 0.7.1 but still some
patching was needed.
* Fix the alignment problems in Beneath a steel sky. I know it
can be made a lot cleaner, and I am not sure if the game is
completable in the current version.
* Same for Queen ...

comment:14 by anotherguest, 15 years ago

Also notice the removal of <TEMPLATE> on some of the
destructors. They just dont compile with the version of GCC
used for Symbian OS right now.

Also.. A note on a port to S60. One reason it do work as well
as it does on UIQ is that all current devices allow direct
screen memory writes. S60 does not support this, and as
bitmap data used for S60 is not sharable between threads, it
would probably crash for all those games animating under a
timerdriven thread. (Beneath a steel sky does so in the
emulator version, where a second bitmap is used for drawing).

And the MACRO in Beneath a steel sky was used to handle
data alignment. Somehow all scumm games have been given
a much better alignment handling than other additional
games.

comment:15 by anotherguest, 15 years ago

And should n't the SDL source be separated from the
SCUMMVM source!? Two different diffs I guess!

comment:16 by anotherguest, 15 years ago

And ZLib is mostly already implemented in the Symbian OS
ver 7, the whole lib is NOT needed!

comment:17 by anotherguest, 15 years ago

SO rnc_deco.cpp indeed needs modification, but I guess
adopting it the proper way is the way to go.
(Current diff ver 3 is not enough. Will lockup/crash on the
device)

comment:18 by anotherguest, 15 years ago

Also the Symbian BSearch is not working very well, so I
replaced it with a copy, i.e the bsearch2 implementation.

comment:19 by SF/sumthinwicked, 15 years ago

* I'm set up to do dev for my P900 now, so development has
resumed. I'm in contact with Lars & we are figuring things out :P
I hope to be able to release Patch.v4 in the near future.
* Solved the whole printf to console problem by redirecting stdout
& stderr to files. This also gives more info in case of problems.
I'm removing other printf-related edits.
* Safe to say I'm very happy with the new DISABLE_ macros, as
newer HE & Scumm CVS code was grinding the ancient GCC
version the UIQ framework uses to a halt.
* The scaling / huge font problem has been solved too.
_force1xOverlay = true; in gameDetector.cpp does wonders for
Symbian :). Removed the silly font changes.

saludos,
SumthinWicked

comment:20 by SF/sumthinwicked, 15 years ago

you always forget stuff you've done:
* Wrote a Perl script that converts the list of .o object files in the
module.mk files to the incredibly dense UIQ MMP makefile
system. It automatically adds and disables the correct source
files in the different .mmp files. This way it's easier to keep track
of new/modified source files.
* Added Gob project file & lib: it compiled on the first run :-) Now
hope that the current version will start running games again... I
just got it up to running the chooser & config dialogs again
without crashing today... I think/hope Lars will be a big help in
this department ... hehe

/wicked out.

comment:21 by fingolfin, 14 years ago

Glad to hear your work is progressing.

BTW, if you are able to split the patch into small
independant parts, then it'll be easier to commit it, step
by step. E.g. if you need to make changes to particular
engines, or the GUI code, etc., then you may want to add a
seperate patch for these, as they should be independant of
your backend code. Likewise for any changes which might be
required in the SDL backend.

comment:22 by anotherguest, 14 years ago

Just a thought about the mp3 integration. Is it possible to
make the design so that the filehandles is always opened in
the mp3 thread and not in the main thread, which then is read
by the mp3/sound thread. The current symbian patch is
working, but not the prettiest in the world.

comment:23 by fingolfin, 14 years ago

Hm, that might be possible, but it would be quite tricky.
We'd have to pass the filename and an offset to the audio
playback class, and then it would have to be recoded to only
open the file and read data from it the first time its sound
callback is invoked. That's going to be quite dirty, I am
afraid :-/

comment:24 by anotherguest, 14 years ago

Yes it is definatly not the best solution for all platforms. I am
thinking about adding a separate mp3 implementation just for
those with this special need, and thus not messing up the
current mp3 implementation.

comment:25 by fingolfin, 14 years ago

Owner: changed from fingolfin to anotherguest

comment:26 by fingolfin, 14 years ago

It would be nice if we were able to start integrating this.

I would strongly recommend to not worry about the MP3
problem for now. First, let's get the basic port into CVS.
Improvements can then be based on this.

Future work would e.g. include refactoring the SDL backend
to make subclassing a bit easier; this would have to be
coordinated with Arisme for the WinCE backend, of course.

comment:27 by anotherguest, 14 years ago

I think that SomethinWicked is getting ready for a Patch nr 4.
We have been adding support for S60, and restructuring the
buildsystem so it will suite the different Symbian platforms.
We have also been adopting SDL for S60 so it will cater for all
of ScummVM needs. Also Joystick handling has to be
improved for SDL, so Sumthin will try to improve that. I am
looking into some alignment problems for Gob, found a
couple of bugs which stopped it from working, now we have a
couple of strange sprites.

comment:28 by fingolfin, 14 years ago

OK. However, please keep in mind: It is preferable to
perform changes in small independent sets. So, if you have
to modify the SDL backend, it would be good if that was
applied to CVS independently of all the other changes. Else,
if anything breaks, we'll have a much harder time to track
down the cause.

comment:29 by anotherguest, 14 years ago

We are aware of that, and I think that we can make the
submissions in small sets, i.e getting the symbian build
structure in there, adding the primary changes to SDL, and
other bugfixes. And as for Gob, I guess we are doing some of
the alignment fixes, which the Gob team has n't been able to
do yet. And luckily those are not in the way of hacks. :-) Btw.
what assumptions and usage of "char" should be made?

I noticed that different platforms as different default of char
beeing signed or unsigned signed, which lead to a bug in
Gob.

comment:30 by sev-, 14 years ago

char if not related to strings should be converted into
either byte or int8. We've did that for gob in all places
which we could spot but of course there could be some
missing like you report.

And another thing Gob engine alignment/char fixes has
nothing to do with Symbian port and should be submitted
different. Or:

I had an impression that anotherguest is already in the
team, right? So gob fixes should go directly into CVS. As
well as this patch, no need for v4 just split not
backends/symbian changes into smaller commits and dump whole
thing in just like Fingolfin tells. Then do fixes. Just make
sure you don't break anything. And come to #scummvm :)

comment:31 by anotherguest, 14 years ago

Ohh yes. Well. that is true, I am already a member of the
team, but since some of the changes are pretty.. well.. big..
the patches gives us a way for Fingolfin to see if the are
good. As for Gob fixes, we will submit them, as we go along.
As far I have made 2 changes, one is a GCC_PACK for a
palette structure, and another is a compare for a -5.. where
Symbian was returning a 251. And 251 was n't == -5. :-) So
expect Gob fixes going in. Specially since it is one of
Sumthinwickeds favorite games. :-)

comment:32 by fingolfin, 14 years ago

Yo folks,

seems the patch went in w/o most people noticing, since the
commit mailing list wasn't working for some reason. Glad to
see this.

Now, I am currently reviewing the changes. One thing I
noticed is that printf was changed in some places despite
discussion taking place here. I am going to revert those
changes for now; if printf causes troubles on Symbian which
aren't solvable on a port specific base, we can discuss
solving this properly; but just changing printf to
debug(1,...) is not correct, I am afraid.

comment:33 by anotherguest, 14 years ago

Hi! Well.. the reason for the debug solution is that you want
to be able to route the print outs where ever you like, and
debug gave you a way of doing this. I guess that we can
override the printf implementation so it does n't end up in the
wrong place. The problem for symbian is that a virtual text
console is started (which is fullscreen btw) each time a
default stdout or stderr is used. And its not very nice. But as I
said, we can try to remove it by replacing printf with our own
implementation. Also I have been working on the MP3 patch,
so it works for symbian and multithread environment without
makeing it too much different for the other ports (would be
that the code would be to cluttered to read). As for smush
support, I will make my own chunk.cpp as the patches in that
is just to great to look nice. We are also looking at making
the WINCE keymapping support a part of a more wide small
screen, special device configuration.

ANd it seems like all the nightly, well at least *nix and
windows is building nicely.

All comments are welcome and we will replace printf on a
port basis instead.

comment:34 by fingolfin, 14 years ago

I'd prefer a port based printf replacement, aye -- in
particular, people will keeping using it in their engines,
so you'll run into troubles with it anyway.

As for replacing chunk.cpp -- that file may undergo a
rewrite soon anyway, Just so you know. When you say you
replace it by your own file, I am not sure what exactly you
mean; might be good to discuss this before commiting.

comment:35 by anotherguest, 14 years ago

Ok.. then we are aligned then. :-) As for chunk.cpp well, I am
changing the readfunctions to indicate if they are used from
the timer thread or from the main thread. And then I have 2
filehandles to read from , depending on the thread. Just to get
smush running. But chunk.cpp has already changed some
from 0.7.1 to 0.8 so have to rethink some things.

Waiting for your cvs updates then. :-)

comment:36 by anotherguest, 14 years ago

I have now submitted the MP3 patch to CVS. I think it is
pretty clean eventhough it is patched in a number of places.
But does n't redesign the class as such and should still be
good to work with.

My biggest concern about the mp3 playing, is that ideally you
should guarantee that the file is read from one only thread. As
it is now it is opened and first read done by the main thread,
then is additional reads done by the sound thread. What is
the possibilities for the Sound thread it all along?

comment:37 by anotherguest, 14 years ago

BTW I saw the last change to the Smush_Player. Great
work, I will test to see if it works as expected later today. :-)
Single thread filehandling is just what we where looking for. :-)

comment:38 by anotherguest, 14 years ago

* Latest Smush_Player + chunk seems to work well with the
FT demo I tested with.
* Thinking about a generic SDL key-remapping in ScummVM.
I.e map any event to any other event. For example X->F5. The
easiest way I see to do that is to call a RemapEvent function
after the pump event in SDL. This could also be used for
WINCE key remapping or any other sutiable SDL platforms
where extra keys is available but not mapped
as "standard"keys.

comment:39 by fingolfin, 14 years ago

Do we need to keep this tracker item open? The port is in
CVS now, and discussion about it could easily go on on the
mailing list. If you need to record / write down stuff, I
propose you use the ScummVM wiki for that (I can make you
accounts if you need 'em).

http://wiki.scummvm.org/

comment:40 by sev-, 14 years ago

I think this could be closed too. Putting in pending state
with resolution "Accepted".

comment:41 by sev-, 14 years ago

Status: newpending

comment:42 by anotherguest, 14 years ago

Status: pendingclosed

comment:43 by digitall, 12 months ago

Component: Port: Symbian
Note: See TracTickets for help on using tickets.