Topics

1

Programming / SurfaceSet::loadPck()

« on: February 03, 2011, 07:01:12 am »

I figured out the bug that was causing setPixelIterative() to walk past the end of the buffer.  I submitted a patch earlier that prevents setPixel() from going past the end of the buffer, but this fixes the actual problem.

Here's what was going on, keep in mind this is only apparent on Android.

In SurfaceSet::loadPck(), after the first read happens and we skip x amount of pixels via two nested for loops, then we go in to a while loop.  This while loop continues until we hit the end of the spite, represented by 0xFF in the file.  The problem is on Android we never hit the if or else if branch and the while loop runs to the end of the file.  Then we start the next iteration of the main for loop at the end of the file.  When we do that first read, we get back 0xFF, and attempt to skip more pixels than we have.  This is where the crash in setPixelIterative()/setPixel() happens.  The reason we never hit the if or else if branches on Android is because char is unsigned, so 0xFF and 0xFE get get converted to 255 and 254 when compared to -1 and -2.  On the desktop, char is signed, so 0xFF and 0xFE get correctly converted to -1 and -2.  My patch casts both -1 and -2 as chars before comparing them to the value we read.  This fixes works both on Android and the desktop.

2

Builds & Ports / Android Port (Old)

« on: January 23, 2011, 01:49:46 am »

I have been working on porting OpenXcom to Android.  I am happy to report that I have OpenXcom running on Android!  With the except of tracking down a crash due to a bug in Surface::setPixel(), this wasn't too difficult, it was done in a couple evenings, less than a week.  In the YouTube video below you will see it running on a Samsung Fascinate.  It will be interesting to see how it performs on slower hardware.

This port uses a SDL port for android:
https://www.anddev.org/code-snippets-for-android-f33/sdl-port-for-android-sdk-ndk-1-6-t9218.html.

Repository is on github:
https://github.com/sirnacnud/openxcom-android

Progress of the port:

• Virtual keyboard support. The SDL port has some support for a virtual keyboard, but not for so how we want to handle this.  I faked out the keyboard input in the video for naming a new base since the Fascinate doesn't have a physical keyboard.
• UI usability with touch screen.  You will notice in the video I have a magnifying mouse cursor, this is provided by the SDL port.  The buttons are so small it makes them very hard to use with your finger, the magnification helps out some.  In the end I think a better solution would be to make the UI more finger friendly, while maintaining a look that is true to the original.
• Wide chacter support. Android doesn't support wide characters.  With a recent change, openxcom now uses wide characters for Unicode.  This doesn't work on Android.

Here is a video of the port in action:
https://www.youtube.com/watch?v=J073Ej_g9gM

3

Programming / Surface::setPixel() bug

« on: January 22, 2011, 06:39:52 pm »

I found a bug in Surface::setPixel(), the checks to see if x and y are valid allow x to be equal to width and y equal to height.  If x or y are width or height, the pixel set will be wrong, and if both are width and height, you will go past the end of the pixel buffer.  See attached patch for the fix.

We actually do go past the end of the buffer when loading spks files.  The culprit is Surface::setPixelIterative() in conjunction with Surface::loadSpk().  I am not for sure why people haven't been seeing a crash, I guess we got lucky with the memory past the pixel buffer.  I will be looking in to why we are trying to write more pixel data than we allocated.  I will also probably be rewriting the loadSpk() and loadScr(), as using setPixelIterative() and setPixel() aren't efficient ways to load the pixel data.

4

Programming / Moved primitive drawing calls in to Surface

« on: December 04, 2010, 10:42:50 pm »

I moved the primitive drawing calls in to the Surface class as discussed here: https://openxcom.ninex.info/forum/index.php?topic=124.0

I have added the following functions to Surface:
drawRect()
drawLine()
drawCircle()
drawPolygon()
drawTexturedPolygon()
drawString()

These functions should be used instead of directly calling SDL_gfx functions.  Let me know if there is anything I have missed.  See attached patch.

5

Suggestions / Move primitive drawing calls in to Surface

« on: November 18, 2010, 04:11:25 am »

I have been looking over the code base and noticed there are calls to SDL_gfx functions in multiple places.  Given there has been talk of switching to a different library for primitive rendering, this would involve changing multiple files.  If we were to have these primitive drawing functions in the Surface class, switching would involve updating one file.   Also if we wanted to support multiple rendering libraries, it would be simple as using a different implementation of Surface or something along those lines.  It is also probably a good idea to keep all code for doing pixel manipulation on SDL Surfaces in one place, again allowing easy switching of rendering solutions.

I am proposing we have functions in Surface like:

Surface::drawCircle()
Surface::drawPolygon()
Surface::drawTexturedPolygon()
Surface::drawLine()
...

SupSuper, if you are ok with this change, I will implement the necessary changes and submit a patch.

6

Programming / Debug Makefile option

« on: November 02, 2010, 04:07:51 am »

I have added support to the Makefile for a debug build.  I am not a master of Makefiles, so there may be a better way to do it, I'm not for sure.  See attached patch.

For debug builds type:

Code: [Select]

$ make debug
Keep in mind for a full debug build, you will need to do a 'make clean' first.

7

Programming / Problems with r189

« on: November 02, 2010, 03:07:21 am »

I got a compile error on Linux.  #include <string> needs to be added to RuleSet.h.

I also noticed a warning with SavedBattleGrame.cpp, _terrian needs to be the first variable initialized in the constructor.

Lastly I ran into a problem where terrain data was failing to load due to case sensitive filenames.

The attached patch fixes all of these issues.

8

Programming / Surface copy constructor

« on: October 28, 2010, 06:39:50 am »

I noticed the Surface copy constructor doesn't copy the _hidden variable.  I am going to guess this is a bug and not by design.

9

Programming / Improved cursor drawing

« on: October 20, 2010, 03:35:20 am »

The cursor was being rendered to its own surface every time its position changed.  This is unnecessary as the appearance of the cursor doesn't change when it is moved.  The only times it changes are either when its color is changed or it's palette is changed.  The color change case was already accounted for.  I added code to re-rendering the cursor when its palette changed.  See attached patch.

10

Programming / Fixed build error with r146

« on: September 29, 2010, 03:04:50 am »

MovingTarget.cpp needed cmath included for sqrt().  Attached patch fixes this.

I noticed you had defined PI yourself in a couple of places in the code.  There is a define for PI in cmath, called M_PI.  Just a heads up in case you didn't know about it.

11

Programming / Fixed Virtual Destructors

« on: September 25, 2010, 10:18:06 pm »

The Target and ImageButton classes needed their destructors to be virtual.  While I didn't see any places in the code base where having these base classes' destructors non-virtual caused a memory leak, it could have caused one in the future.

Attached patch contains the fixes.

12

Programming / Fixed memory leak in Globe

« on: September 19, 2010, 07:57:13 pm »

The dynamically allocated blink and rotate timers were not being deleted in the Globe destructor.

Attached patch fixes memory leak.

13

Programming / Fixed PCK resources failing to load when lowercase

« on: September 13, 2010, 06:05:51 am »

On Linux, basebits.pck and graphs.pck were failing to load due to the file extensions being lower case.

The problem was SurfaceSet::loadPck() was assuming uppercase file extensions.  I added a call to insensitive() before trying to open these files.  In order for this to work, I moved insensitive() out of ResourcePack and in to it's own file called Util.  Seems to me insensitive() is a utility function and doesn't really belong in ResourcePack.