2010-01-19

A Bugfix Bonanza

If you have been following the comments for the previous few posts, or the forum, you’ll have seen work progressing at a frantic pace. Most of the thanks for this goes to Lakedaemon, who has been zapping bugs throughout Woopsi’s text rendering system. Working so quickly and not regularly blogging about it means that I’ve probably got a lot of changes to list here. Let’s see if I can collate them into some sort of order…

The WoopsiString class has seen a lot of changes. I’ve added indexOf(), lastIndexOf() and subString() methods. The first will find the first index of a particular character in the string and return its index. The second does the same thing, but in reverse - it finds the last index of a character. The last will return a pointer to a WoopsiString containing a subsection of the original string.

WoopsiStrings are no longer null-terminated. They already included a member variable that stores the string length, so appending a terminator was a waste of a byte.

The Text class’ wrap() function has had some long-standing bugs fixes. The strangest was probably an invalid comparison between a line index and a char index that was supposed to prevent the function from wrapping the entire text, but actually just screwed up the function most of the time, and made it slower the rest of the time. Not sure how that stayed in there for so long.

The next most significant bugfix was to the GraphicsPort’s clipScroll() method. This is used primarily in the ScrollingPanel gadget. It performs two functions. If it is possible to copy any of the existing visible region to the new location, the function copies it. This prevents redrawing, which is slow, if the required visual data is already available. Secondly, it remembers any rectangular regions that require redrawing and passes the list back to its caller. The ScrollingPanel uses the list to redraw any non-copied regions of itself.

The clipScroll() method worked perfectly if scrolling in just one plane, but as soon as horizontal and vertical movement was attempted simultaneously, it fell apart. Rectangular regions were missed out or copied to the wrong locations, particularly in the corners.

The function was originally tackling the problem backwards. It calculated the size of the source and destination rectangles then clipped those rectangles. However, this introduces two problems:

  • The rectangles could become different sizes;
  • The rectangles could acquire different offsets from their original locations.

Neither problem is too difficult to solve, but it does mean that if either problem arises, the function will not be able to copy the regions and must therefore fall back on the much slower redraw option.

The “correct” approach to the problem is to clip before calculating the sizes of the source and destination rectangles. That will ensure that neither of the two highlighted issues arises, which means that the copy can be used in preference to redrawing much more frequently.

I’ve finally got around to removing the glyphs from the Topaz and NewTopaz fonts. As the glyphs are now an entirely separate font, keeping that data in the bitmaps was a waste of space. Neither font requires colour now that the glyphs are gone, to I’ve converted Topaz from a Font to a MonoFont, and converted NewTopaz from a PackedFont16 to a PackedFont1. This has made both fonts considerably smaller.

Related to this, PackedFontBase::isCharBlank() returns the correct value (false) if the requested character is outside the range of the available bitmap data.

Dragging a background screen below the level of the topmost screen no longer crashes the system. The Screen class was attempting to copy a region with a negative height, which the DMA hardware really doesn’t like.

The FileRequester has had yet more fixes since the last post. When running in SDL mode, it now shows a dummy list of files and directories. I had intended to get it to show a real list of directories, but I ran into The Windows Problem - every major operating system but Windows has POSIX-compliant file system access. Unfortunately, of those major operating systems Windows is the most prevalent. I really couldn’t be bothered to write the POSIX code if most Woopsi developers are using Windows, and I couldn’t be bothered to write Windows code when I only run the SDL version in OSX. The compromise, which will work on all platforms, POSIX or not, was just to create the dummy list.

Finally, both the MultiLineTextBox and the standard TextBox respond to d-pad repeats. Holding down left or right on the d-pad when either of these two gadgets has focus and a visible cursor will cause the cursor to move repeatedly until the d-pad is released.

2009-04-26

Scrolling Changes

Building on the GraphicsPort::copy() method reported last time, I’ve added a new GraphicsPort::scroll() method. This takes in x and y co-ordinates as the location of the region to scroll, in addition to the width and height of the region to be scrolled. This information represents a rectangle that will be shifted through a specified distance both vertically and horizontally, which are also parameters to the function.

The method uses the copy() method to do the actual pixel moving, but does some crazy clipping calculations in order to limit the region being copied to within the bounds of the visible regions of the relevant gadget. To be specific, it uses the copy() routine where possible, but there are a number of situations where no data is actually available to copy (ie. destination falls outside the bounds of the clipping region). To cater for this, the function expects to be passed a pointer to an empty WoopsiArray of rects. Where the copy cannot be performed, a rect representing the region to be refreshed is appended to the array. These can be redrawn (using a loop around the draw(Rect) method) once the scroll function exits.

In addition to this, the function adds all revealed regions to the array too. So, if you scroll a gadget horizontally by 10 pixels, the array will contain a rect 10 pixels wide representing the newly-visible area that needs to be redrawn.

The scroll() method rendered much of the code in the ScrollingPanel redundant. In fact, it reduced it from ~500 lines of code down to ~200, and removed another set of functions that dealt directly with the framebuffer and recalculated visible regions. Unfortunately, it doesn’t appear to have made any noticable difference to the speed of the ScrollingPanel’s dragging function - it’s still flickery. The only way around that would appear to be double-buffering the display, which is counter-productive given the way I’m using the DMA hardware to copy pixels around.