2011-04-03

WoopsiGfx v1.3 Released

Version 1.3 of WoopsiGfx, my Woopsi-derived 2D graphics library for the DS, has been released. Download it now from the BitBucket page:

This version adds in all of the latest Woopsi goodness, including all of the new string functionality, the floodfill bugfix, font and bitmap refactoring, etc. The full changelog can be found in the archive.

2010-07-25

Removing Code

I promise I’m not stealing the subject matter for today’s posts from Hacker News. This story got mentioned there today, but it’s something that I’ve kept in the back of my mind when coding for years.

It was this story that lead to the latest Woopsi changes. I’ve been trawling through the code trying to get rid of things that no longer serve any purpose or just clutter things up.

The first change is the removal of the FixedWidthFontBase class. It was needed by the Font and MonoFont classes, which were themselves removed in the last release. Other than in those two classes the FixedWidthFontBase class wasn’t used anywhere. It’s now in the “extras” folder in the SVN repository.

I’ve tidied up some of the Graphics class. The drawHorizLine() and drawVertLine() methods are now protected, and the drawLine() method will automatically detect if a horizontal or vertical line is being drawn and will call the faster method appropriately. All the benefits of the faster routines remain, but Woopsi now handles calling the right method without any developer intervention. The drawCircle() and drawFilledCircle() are similarly called automatically by the drawEllipse() and drawFilledEllipse() methods. These changes meant that the drawVertLine(), drawHorizLine(), drawCircle() and drawFilledCircle() methods in the GraphicsPort class were unnecessary and have been removed.

The putSDLPixel() and getSDLPixel() methods have been moved into the FrameBuffer class as they aren’t used anywhere else.

Finally, the TinyFont class is broken in Woopsi 0.99.2. That has been fixed.

2010-02-19

GraphicsPort Refactored

There have been quite a few changes and improvements since the last blog post. The biggest one is another redesign of the GraphicsPort class. Back in November last year I stripped the drawing code out of the bitmap class and separated it into a new hierarchy. The GraphicsUnclipped class contained drawing methods that were not clipped. The Graphics class was a subclass of GraphicsUnclipped and clipped to the confines of a bitmap. The GraphicsPort also subclasses GraphicsUnclipped and clipped to the visible regions of a Gadget.

This seemed like a reasonable design, but it did have its downsides. As the Graphics and GraphicsPort classes were peers within the inheritance hierarchy, rather than one subclassing the other, there was inevitably some repeated code. More disturbing was the spaghetti-like integration between the classes, in which GraphicsPort would call a method in GraphicsUnclipped which would in turn call a method in GraphicsPort.

In addition to the inheritance problems, the GraphicsPort had its own internal problems. Its drawing methods used a variety of different co-ordinate systems. Some used the co-ordinates of the entire UI as their reference point, whilst some used the co-ordinates of the gadget, and others used the co-ordinates of the GraphicsPort itself.

The GraphicsPort is designed to work in two ways - it can be constructed and used within a gadget’s draw methods, or it can be created outside of the gadget and used to draw over the gadget. This meant that it needed to retain either a single clipping rectangle, used when drawing from within a draw method, or a list of clipping rectangles, used when drawing from outside of the gadget’s code.

All of these problems led to a horrible mess, which I have now put right. I’ve even introduced some new features in the process.

First of all, I’ve merged the Graphics and GraphicsUnclipped classes into a single class: “Graphics”. It contains all of the drawing methods available to the entire Woopsi system. The class can be given a clipping rectangle in which it can draw, which means that all of the drawing methods now clip. Any potential speed loss is negligible, since the drawing methods were already clipping to the confines of the bitmap being drawn to. I’ve just made that clipping area user-definable.

The GraphicsPort class no longer inherits from either of the other Graphics classes. Instead, it includes an instance of the Graphics class and presents a facade over the top. The GraphicsPort no longer has a convoluted set of responsibilities; it now:

  • Maintains a list of clipping rects for the gadget it relates to;
  • Receives drawing instructions;
  • Converts the co-ordinates from “GraphicsPort space” to framebuffer space;
  • Uses its Graphics object to draw to all clipping rects in its list.

The GraphicsPort does not maintain a single clipping rect in addition to a separate list of clipping rects. It now adds that single clipping rect to its list, erasing any previous data in its list. In this way, it achieves the same functionality without the extra complexity of two separate data storage mechanisms.

I mentioned GraphicsPort space and framebuffer space in the above descrition. I’ve put some work into trying to formalise the different co-ordinate systems that Woopsi uses. Descriptions of these will be included in the documentation whenever I get around to finishing it.

The GraphicsPort class is now entirely separate from the Gadget class. Previously the GraphicsPort included a pointer to the gadget that it was drawing to. This is no longer necessary, which should result in a (negligible) speed increase, since the GraphicsPort no longer needs to query the gadget’s Woopsi space co-ordinates using its recursive getX() and getY() methods.

I’ve removed the OutlineType enum from the Graphics class. This was not relevant to all gadgets so should not have been in the base class. This resulted in the addition of a new CalendarDayButton class and some changes to the WoopsiKey class so that they could remain “stuck down” when selected, which is represented by their outlines changing from bevelled out of the screen to bevelled into the screen.

I removed the padding variables from a few classes a week or so ago. I’ve now done the same to the MultiLineTextBox and replaced it with larger border sizes. This change, coupled with the rationalisation of the GraphicsPort’s co-ordinates, finally enabled me to identify the bug that caused the textbox to have graphical glitches when using a padding of greater than 12. That’s the first bug I’ve closed in the SourceForge tracker in months. The textbox’s vertical top alignment option works correctly, too.

Lastly, I’ve improved the cursor-following code in the TextBox again. When deleting characters from a string that is wider than the textbox, it is no longer possible to create a large gap between the end of the string and the right edge of the textbox.

2010-02-13

Simplifying the API Part 5

More tidying up and throwing away. A while ago I altered the TextBox so that it would scroll to follow the cursor. The code worked but it was ugly, and the scrolling effect was ugly too. The text was always aligned to the left of the textbox, so if the cursor scrolled to the right partial characters would appear on the right-hand side of the box. What a user would expect to happen is see the characters on the right aligned nicely whilst the characters on the left were partially visible. I ripped out the existing code and replaced it with a much improved system.

I’ve removed the “_padding” variable from the Label, Requester, FileRequester and Alert classes. This variable was used to force the gadgets’ children to leave a border between themselves and the real gadget border. I’ve replaced it with the newly improved border code. Related to this, a number of gadgets have improved and simplified drawing routines.

Finally, I’ve added a new set of XOR drawing routines. These accept a colour parameter to XOR against, allowing for more interesting XOR effects.

2010-02-11

Simplifying the API Part 4

The basic Woopsi gadget includes the concept of a border. This is one pixel wide and surrounds the entire gadget. It can have one of a variety of appearances, such as bevelled in, bevelled out, bevelled depending on whether or not the gadget is clicked, and so on. Other gadgets, such as the AmigaWindow and TextBox, have wider borders. The AmigaWindow is particularly unusual because its top border has a different size to the other borders. It eschews the standard border appearance, opting for a border built out of other gadgets instead.

Up until now, the border system has not been implemented very well. Most of the gadget calculate the amount of space taken up by their border using something like this:

u8 width = (!_flags.borderless) << 1;

If the gadget has a border, this evaluates to “2”. If not, it evaluates to “0”. Efficient, yes. Maintainable, no.

Instead, the Gadget class now includes a “GadgetBorderSize” struct, that contains individual sizes for each of the four borders. All of the hacky bitshifting code has been removed and replaced with calculations based on the new struct. This has allowed me to remove all custom “getClientRect()” methods from all gadgets and make the method in the base class non-virtual.

Next, I have moved the “bonus” folder, which contains the skinned gadgets, bitmap I/O classes, hashmap and linked list classes, out of the Woopsi section of the SVN repository and into a new section called “extras”. I’ve decided not to include these in any future distributions, at least until Woopsi 1.0 is out. Too much scope creep will prevent me from ever finishing this library.

To trim some of the flab from the API, I’ve been cutting unused functionality throughout the codebase. The following methods have been removed:

  • Gadget::clear()
  • Gadget::clear(clipRect)
  • Gadget::newInternalGraphicsPort(isForeground)
  • AmigaWindow::getBorderSize()
  • Screen::getBorderSize()
  • TextBox::getBorderSize()
  • Screen::getTitleHeight()
  • Window::getTitleHeight()
  • Gadget::getBackgroundRegions()
  • Woopsi::closeChild()
  • Woopsi::shelveChild()
  • Woopsi::release()
  • Woopsi::drag()
  • Woopsi::click()
  • Woopsi::shiftClick()
  • Woopsi::shelve()

Many more of the Gadget methods that used to be virtual cannot be overridden any more. This prevents essential pieces of functionalty being replaced.

The GraphicsPort has seen a few changes. It no longer tries to delete a null pointer if its clipRect has not been allocated. More interestingly, the gadgets use the GraphicsPort in a different way. Previously, a gadget would define the code to be called when it was to be drawn like this:

void draw(Rect clipRect) {
    GraphicsPort* port = newGraphicsPort(clipRect);
    port->drawFilledRect(0, 0, _width, _height, woopsiRGB(31, 31, 31));
    delete port;
}

Behind the scenes, this was very inefficient. A GraphicsPort object was created and deleted every time this method was called. As this method is called for every visible region of the gadget (and there could be dozens of visible regions), GraphicsPort objects were being created and deleted far too often.

The draw() method has been replaced with two new methods: drawBorder(), which can draw across the entire visible surface of a gadget (including its border space, hence the name) and drawContents(), which can only draw inside the border. The code above would look like this in the new model:

void drawBorder(GraphicsPort* port) {
    port->drawFilledRect(0, 0, _width, _height, woopsiRGB(31, 31, 31));
}

In the new design, two GraphicsPorts are created every time the gadget is redrawn. One can draw to the border area, whilst the other can only draw within that area. They are re-used for every region of the gadget drawn during that redraw operation.

It is still possible to get the clipping region by calling the GraphicsPort’s new “getClipRect()’ method.

The Gadget class’ GadgetStyle object used to be allocated using “new” and referenced with a pointer; it’s now allocated as a standard object. There’s no real reason for this change other than it’s a little tidier.

There were a number of fiddly problems with the Gadget::closeChild() and shelveChild() methods, which are now fixed. They were causing the focus to jump between objects even if the closed/shelved gadget did not have focus.

Lastly, I hit a problem with the shift-click code from yesterday. The clicks were allowed to propagate back up the hierarchy, which caused nasty problems. If the front-most screen did not contain a context menu, the screen below it would be sent the shift-click. If this screen had a context menu then it would be displayed despite the screen itself being totally obscured by the screen above it. I’ve added a new “checkCollisionWithForegroundRects()” method to the Gadget class that fixes this problem. It checks to see if the click falls within one of the regions of the gadget that is not obscured by its siblings or ancestors. If not, the shift-click is ignored.

2010-02-09

Simplifying the API Part 3

When I introduced the context-sensitive shift-click menu I ran into a bit of a problem. Clicks are automatically propagated down through the gadget hierarchy until the gadget that the stylus is touching is found. But what happens if we’re trying to capture a shift-click on a gadget that isn’t exactly the gadget that we clicked? Suppose we shift-click a button, but we want the window that contains the button to show a context-sensitive menu?

My solution was to introduce a new flag in the Gadget class, “shiftClickChildren”, that would prevent clicks from propagating down if it was set. This definitely worked, but it added more complexity to getting the context menu working. It also introduces some subtle problems - a gadget cannot have a context menu if its parent has a context menu.

I’ve changed the way that this works and removed the shiftClickChildren concept from Woopsi. Instead, shift-clicks propagate down through the hierarchy, but then propagate back up again until a gadget is located that defines a context menu. Thus, if a window contains a context menu definition it will be shown regardless of which sub-gadget is clicked. If one of those sub-gadgets contains a context menu definition, however, that will be shown instead.

Much simpler.

Along with this, I have fixed the Window gadget’s dragging routines so that they work correctly if the window’s parent is not a screen. In other words, windows can now contain other windows without it causing graphical glitches, should you feel like putting such an abomination together.

Lastly, I’ve added test projects for the Alert and Requester classes.

2010-02-08

Simplifying the API Part 2

In a continuation of yesterday’s refactoring session, I have added a bunch of new stub methods:

  • onKeyPress()
  • onKeyRepeat()
  • onKeyRelease()
  • onFocus()
  • onBlur()
  • onLidOpen()
  • onLidClose()
  • onEnable()
  • onDisable()
  • onResize()

As with the methods listed yesterday, these are now the methods that should be overridden in gadget subclasses instead of the old disable(), enable(), blur() (etc) methods. I have made the majority of the old methods non-virtual (should that be “concrete”?) to prevent them from being overridden.

This has several benefits. Firstly, it is now no longer necessary to know exactly how the methods should work before they are overridden. For example, overriding the resize() method was particularly complex, requiring at least two dozen lines of code to work correctly, or a dozen if you’re willing to live with a redundant call to redraw().

Overrides of the new methods need not contain any code at all, which is a huge improvement. Not only are the stub methods easier to override and implement, but they are faster and more resilient too. Good news all around.

Related to this, I have added onResize() methods to the Alert, Requester and FileRequester classes. These gadgets should now resize correctly.

Finally, I’ve increased the accuracy of the calculations in the scrollbars. They were previously using a fixed-point calculation with an 8-bit fractional part. I’ve changed this to a 16-bit fractional part and the remaining glitches have disappeared.

2010-02-07

Simplifying the API

Using Woopsi’s keyboard as an input device has been needlessly complicated. To output keyboard input, a programmer would need to:

  • Create a TextBox as the output gadget;
  • Create a WoopsiKeyboard as the input gadget;
  • Create a class that inherits from KeyboardEventHandler that will receive input from the keyboard and direct the output to the textbox;
  • Add the new class as an event handler to the keyboard.

This pattern would need to be repeated every time the keyboard needed to be used.

Clearly this is a bad idea, so instead I’ve made the TextBox and MultiLineTextBox gadgets into KeyboardEventHandlers. Using a keyboard now requires these steps:

  • Create a TextBox as the output gadget;
  • Create a WoopsiKeyboard as the input gadget;
  • Add the textbox as an event handler to the keyboard.

Missing out the middleman event handler reduces the amount of code necessary to achieve keyboard input from two dozen lines and an extra class down to around three lines.

Whilst on a simplifying drive, I was looking at the click(), release(), doubleClick() and similar functions in every gadget class. These were complicated to work with. Overriding them is essential if one is subclassing Gadget in order to make a new gadget, but it is very easy to break the methods by overriding them incorrectly.

Consider the click() method. This method contains around two dozen lines of code, all of which is absolutely crucial for the gadget to function properly when clicked. Subclasses had to call the base class’ click() method as the overridden method’s first step or the gadget wouldn’t work. The code worked like this:

click() method
    check that this is a valid click
        check that gadget is enabled
            run custom behaviour
            return true
    return false

Forgetting any of these steps resulted in a non- or semi-functional click() method.

Most of the time the click() method is overridden it is to add some trivial extra functionality, so having to write out a dozen lines of boilerplate code just to add a redraw() call or something similar is absurd.

The Gadget class now has a handful of stub methods that can be overridden in subclasses to avoid this problem. Instead of overriding click() - which is still possible, if really bizarre behaviour is required - developers should now override the new onClick() method. This function is called when the gadget determines that the click really is valid, so all that needs to go into the onClick() method is any code that is relevant to that method.

For example, if a gadget should redraw when clicked, this is the code needed:

void onClick(s16 x, s16 y) {
    redraw();
}

Conversely, an old-style click() method to do the same looks like this:

void click(s16 x, s16 y) {
    if (Gadget::click(x, y)) {
        if (isEnabled()) {
            redraw();
            return true;
        }
    }
    return false;
}

The new-style approach is considerably terser and has no vital steps that can be accidentally missed.

The new stub methods are as follows:

  • onClick() - called when the gadget is clicked.
  • onDoubleClick() - called when the gadget is double-clicked.
  • onShiftClick() - called when the gadget is shift-clicked (ie. when the shoulder button is held down; this triggers the context menu).
  • onDragStart() - called when a dragging starts on a gadget.
  • onDrag() - called when a gadget is dragged.
  • onDragStop() - called when a dragging stops.
  • onRelease() - called when a clicked gadget is released within its boundaries.
  • onReleaseOutside() - called when a gadget is released outside its boundaries.

I have switched to this new approach throughout the Woopsi library and it has made a lot of the gadget classes shorter and easier to understand.

Related to this, Gadget::click() no longer includes a call to setDragging(). If a gadget should respond to stylus drags, it should call this method in its onClick() override. I must get around to renaming that to “startDragging()” or something similar…

In other news, ScrollingPanel no longer includes a raiseScrollEvent() method. I’ve moved this into the GadgetEventHandlerList class. The delete key on the keyboard deletes characters in front of the cursor in the TextBox and MultiLineTextBox gadgets. Lastly, I’ve removed the parameters from the GadgetEventHandlerList::raiseActionEvent() method.

2009-12-04

Separating UI and Data Management - Lister Improvements

In my last post I discussed the possibility of ripping out the data management from the CycleButton and ContextMenu classes and replacing it with the ListData class. I was slightly dubious about this because of the potential extra overhead involved, but then realised that the current system did not offer any way to remove items from the CycleButton. In fact, the only possible operation that could be performed on CycleButton data was adding to it.

Worse, the ContextMenu duplicated the functionality of the ListBox but uses a gadget for each item in its list. This isn’t the fastest way of maintaining a list of options (I threw that model out in the early stages of ListBox development), and the option gadgets were being passed around as the arguments to ContextMenu events. Why pass around the entire gadget when only the data it contains is in any way worthwhile?

Based on these observations I’ve made a number of changes. The CycleButton now includes a ListData object to manage its options and exposes all relevant methods via a set of facade functions. The options can be sorted, added to, removed from, and so on.

The ContextMenu includes a ListBox gadget that does practically everything the ContextMenu tried to do; the ContextMenu is now little more than a wrapper that can raise relevant events and resize/reposition itself as necessary. It doesn’t expose any more methods, but it is more efficient now that each option isn’t an entire gadget. It also includes a getPreferredDimension() method that produces the correct values, and the resizing routine uses that instead of duplicating the functionality.

The ListData class has had several improvements and bugfixes. Its swapItems() method no longer raises a data changed event, as it is only called by one function - swap(). That method raises the event instead.

I’ve removed the ListDataItem struct from the listdata.h file and turned it into a separate class in its own file. That allows me to subclass it where necessary to change its behavioury. ListData’s sort() method uses a Java-esque “compareTo()” method in the ListDataIten class in order to sort its data, meaning subclasses of ListDataItem can override the default sort behaviour. This has proven useful in the FileRequester, which is now several times faster. The new sorting system means it doesn’t need to create two intermediate sorted arrays of files and directories in order to keep them separately ordered when displaying them.

Whilst on the subject of the FileRequester, I’ve split that into two classes. The FileRequester is still there, but most of the functionality has been moved out of it and into a new FileListBox class. This works just like the ListBox, but it automatically populates itself with a list of files when pointed at a directory. The FileRequester includes an instance of this new class in order to display its list of files.

Working with the ListBox so much made several bugs obvious. It now draws correctly - previously, a single rogue pixel was visible below the options in the list. It raises a double-click event when double-clicked, and ignores double-clicks if they occur on separate items within the list. It also redraws every time data in the list changes, does not overwrite items at the top of the list with those that have wrapped-around from the bottom (oops, clipping bug introduced in the last release), and adjusts its scroll position appropriately when items are removed.

Since the ListBox inherits from the ScrollingPanel and ScrollableBase classes, I inevitably found improvements to make there too. ScrollableBase and ScrollingPanel include functionality to disable scrolling either horizontally, vertically, or both. The ListBox uses this to prevent horizontal scrolling.

Aside from this, I’ve made some general improvements. Gadgets no longer respond to double-clicks if the first click actually falls on a different gadget. A new Gadget::isDoubleClick() method helps with this, and I’ve removed the unused Gadget::_doubleClickTime member. The Woopsi class does not attempt to retrieve a pointer to the system font before the font has been initialised. The WoopsiString class includes a new copyToCharArray() method for getting a copy of its internal string data. The TextWriter’s methods have been moved into the Graphics set of classes, making the class redundant so I have deleted it. Lastly, I’ve removed all of the individual colour variables from the Gadget class (_back, _highlight, _shine, etc) and merged them into a new GadgetColours struct.

I hope to get a new release out in the next few days.

2009-11-26

Multiple Inheritance and the Dreaded Diamond

Woopsi refactoring continues apace. Today’s latest changes were an attempt to consolidate the ListBox, ContextMenu and CycleButton gadgets.

Each of these gadgets is a different kind of view onto a list of items. The ContextMenu is the most primitive - it shows all of the items in its list as a vertical list. The ListBox is basically the same thing, except it introduces scrolling into the mix. The CycleButton shows just one option at a time; the other options can be paged through by clicking the button.

At present, the ListBox has the most advanced data mechanism. It leaves all of the data functionality to the ListData class, which wraps around a WoopsiArray (ie. vector) and provides such handy functions as sorting and item selection/deselection (enforcing single-only or multiple selection rules). It raises events to indicate changes to the data or selections within the data.

Meanwhile, the CycleButton and ContextMenu classes have very primitive data manipulation functionality. They both include an instance of the WoopsiArray and work with it directly. Wouldn’t it be better if they could benefit from the wealth of functionality in the ListData class?

Since the ContextMenu and ListBox work in very similar ways - the ContextMenu is basically a customised ListBox without scrolling - it makes sense to create a base class providing the most basic list view functionality that they can both inherit from. I split up the ListBox into “BasicListBox” (no scrolling) and “ListBox” (inherits from BasicListBox and ScrollingPanel, which adds the scrolling functionality). Here the problems began.

The BasicListBox inherits from the fundamental Gadget class, from which all Woopsi UI components inherit. The ScrollingPanel also inherits from the Gadget class. When the ListBox attempts to inherit from them both, it ends up with two copies of the Gadget class floating around in its inheritance hierarchy. Any attempts at working with the features of the Gadget class are now ambiguous and the compiler throws errors all over the place. This is called a “diamond” inheritance pattern:

           Gadget
       /             \
      /               \
     /                 \
BasicListBox    ScrollingPanel
     \                 /
      \               /
       \            /
           ListBox

The C++ FAQ Lite has a whole page about the diamond inheritance problem. The solution is to declare the inheritance from Gadget as virtual, which eliminates the ambiguities by eliminating the second copy of Gadget. However, attempting to do this has horrible problems in the Woopsi codebase. C-style casting cannot be used (uh oh) and the class at the top of the diamond (in this case, Gadget) should not include data members that need to be initialised (it can, but it shouldn’t).

The upshot of all this is that it is impossible to create a gadget that inherits from two other gadgets in Woopsi. If you think about it, this does make sense. It doesn’t help my goal of rationalising the list display gadgets, though. I could include an instance of the ScrollingPanel inside the ListBox to achieve the same effect, but that would be less efficient than the current solution. I could alternatively make the ListBox the basic list and have the ContextMenu inherit from that, and just not use the scrolling feature (though it would be handy if there are too many items to fit on-screen at once), but again that’s less efficient than the current solution.

I kept some of the changes I made before I reverted everything else back. ListData::swapItems() no longer raises a data changed event, as the only place it is called is in the sort() method. That method now raises the event instead. The ListDataItem struct has been replaced with a ListDataItem class, and it includes a C#/Java-style compareTo() method. Subclassing the ListDataItem and overriding this method allows the ListData class’ sort() method to sort the list differently. The ListBox’s scrolling canvas now has the correct height and the extra 1px at the bottom of every ListBox has now gone.

Finally, there are a couple of fixes in other places. The Woopsi class’ constructor no longer tries to retrieve the system font before it has been initialised, and the ContextMenu does not cast away the const-ness of the ContextMenuItem when raising a selection event to its listeners.