2008-03-09

Scrollbar Done?

The scrollbar now works. There were two hard-to-spot bugs conspiring to cause minor but annoying interface problems:

  • The Text::wrap() function was incorrectly increasing the line count by one in certain circumstances
  • The MultiLineTextBox was ignoring its padding value when telling the scrolling panel the size of its virtual canvas

These are both fixed.

I managed to come up with a solution to the rounding problem in the slider. It’s obvious, really - bitshift by different amounts to ensure that there’s always a fractional part in the equation, then round the fraction before removing the shift. To illustrate the solution:

// Calculate a ratio, ensuring that the result has a fractional byte
u32 ratio = (virtualHeight << 8) / gadgetHeight;

// Bitshift a value up by a byte and a half
u32 shiftedValue = someValue << 12;

// Dividing our values gives us a result with a half byte fraction
u32 result = shiftedValue / ratio;

// Round the value using a bitmask - if the fraction is equal to or
// greater than 8 (ie. a nibble halved, or 8/16, or 0.5 as a decimal)
// we increase the value by (1 << 4)
if (result & 0x8) {
    result += 0x10;
}

// Bitshift to remove the fraction before returning
return result >> 4;

In this code, we ensure that our ratio has a whole byte fraction (for accuracy). We then ensure that we shift any values working with that fraction up by a byte and a half, so when we perform the division we’ve still got a half byte fraction left to work with. I went with the 12-bit shift because the Woopsi code I was working with used signed values.

The bottom of the slider is still off by one in some situations, but it’s close enough.

I solved the infinite loop problem with a more generic, tidier solution. It’s now possible to tell a gadget to stop firing events by calling “Gadget::setRaisesEvents(false)“. If the textbox in the debug console fires an event, I disable event firing in the slider before calling any of its methods. Same applies the other way around.

Last updates today are to the SDL code. I’ve brought it up to date with the main codebase. I’ve also re-written the input routines in the VBL function so that they query the mouse/keyboard state directly without interfering with the event queue. The result of this is that events are now processed correctly. The mouse doesn’t get stuck down, and the program quits properly.

Whilst I’m on the topic of SDL, I got a version of Woopsi built for Windows the other day using the GP2X dev kit.

One last thing to do to the vertical slider. Should I integrate it into the MultiLineTextBox so that it’s always available (toggle on/off in the constructor)? Or should I integrate it into the ScrollingPanel? Or should I create a new MultiLineTextBoxWithScrollbars class? Or should I add a pointer to a new SliderBase class into the ScrollingPanel so that scrollbars can be enabled/disabled by setting the pointer? This would give me the option of having skinned sliders very easily. Or should I leave scrollbars out of the basic classes completely, and leave people to attach their own scrollbars? There’s a bit of work involved in getting it all working; not a great deal, but it’s the kind of thing I’d like the API to do for me.

Thoughts?

EDIT: Added a horizontal slider. There’s still a problem with the slider’s calculations somewhere. Back to the debugger…

Comments

Jeff on 2008-03-09 at 22:58 said:

I’m concerned that there is any rounding involved in the first place. Irrespective of how well intentioned it is, it nearly always introduces corner cases where someones numbers will be bigger than you anticipated. Why do you need to do it anyway? Its nearly always better to maintain ratios are pairs of integers (though obviously you may need long arithmetic if you are scrolling something that u32)

As to gluing code together, my preference (after looking at the linkmaps and the horrid interdependences) is to keep the classes as simple as possible, but provide simple mechanisms to glue them together.

The problem with “set the pointer to enable it” is that the base class will still need to wire in all the scroller logic, since it will know that that pointer is to a scroll bar, and will need access to its vtable to call it. This would be a case where it would be better if the scrollbar were communicating via high-level application events.

So, I’d be adding another subclass to introduce the scrollbars.

I don’t think you need to call it MultiLineTextBoxWithScrollbars - ScrollableTextBox would do - since by definition to be scrollable, it needs to do multi-line. The special case of a SingleLineScrollableTextBox doesn’t need a new class, just a height-limit.

ant on 2008-03-10 at 00:19 said:

If you can think of a way to do this that doesn’t involve rounding I’d be very interested! Here’s the function to resize the scrollbar grip to the correct size. Some explanation as to the purpose of the variables:

  • rect is the client region inside the slider’s gutter that the grip fits inside.
  • newWidth is the new width of the grip that we’re calculating.
  • overspill is the width of the scrollable region that’s outside the viewport (so if you’ve got a gadget 10 pixels wide with a scrolling canvas 12 pixels wide, the overspill is 2 pixels).
  • _maximumValue is the largest value that can be represented by the slider (in the case of a scrollbar, it’s the width of the scrolling canvas).
  • _minimumValue is the smallest value that can be represented by the slider (always 0 for scrollbars).
  • _pageSize is the size of the viewport.
  • ratio is the size ratio of viewport to scrolling canvas.
  • _minimumGripWidth is a sensible minimum size for the grip, to prevent it being 1 pixel wide and unusable.

The problem is that, as far as I can tell, it is absolutely necessary to calculate the ratio in order to perform the rest of the calculations, and as we’re dealing with integers there’s always a rounding error here. Converting the overspill from a viewport quantity to a slider quantity by dividing by the ratio compounds the rounding error, and I keep getting incorrect values back.


void SliderHorizontal::resizeGrip() {

    // Get available size
    Rect rect;
    getClientRect(rect);

    // Calculate new width
    s32 newWidth = rect.width;

    // Calculate the width of the content that has overflowed the viewport
    s32 overspill = ((abs(_maximumValue - _minimumValue) - _pageSize) << 12);

    // Is there any overflow?
    if (overspill > 0) {

        // Calculate the ratio of content to gutter
        u32 ratio = (abs(_maximumValue - _minimumValue) << 8) / rect.width;

        // New width is equivalent to the width of the gutter minus
        // the ratio-converted overflow width
        newWidth = (rect.width << 4) - (overspill / ratio);

        // Handle rounding
        if (newWidth & 8) {
            newWidth += 0x10;
        }

        // Bitshift to remove fraction
        newWidth >>= 4;

        // Ensure width is within acceptable boundaries
        //if (newWidth < _minimumGripWidth) newWidth = _minimumGripWidth;
        //if (newWidth > rect.width) newWidth = rect.width;
    }

    // Perform resize
    _grip->resize(newWidth, rect.height);
}

ant on 2008-03-10 at 00:31 said:

On to something - switched the ratio upside down which results in a single divide instead of two. Also cast the results of the abs() call to a u32, which stops the values overflowing (duh).

EDIT: Yep, think that’s got it. Rounding removed and the errors are gone. Still off by one in the debug console, though…

Jeff on 2008-03-11 at 01:49 said:

Dividing by a ratio is never the right thing to do, as you’ve obviously found.

Does the grip actually get drawn if its not possible to drag it? I wonder if thats an optimisation, to not draw it at all if its irrelevant? Surely, if the grip is occupying the entire space, the scroll bar should hide it completely (and perhaps draw the arrows at both ends in different states?

Oh, and why do you need an abs() around max-min? Under what circumstance can max<min be true? Thats the sort of thing that should get rejected at construction time.

ant on 2008-03-11 at 09:46 said:

I included abs() in case _maximumValue and _minimumValue were both negative values. It seems that I thought that it would somehow make the result negative. The only explanation I can offer for this curious idea is that I must have been distracted at the time. I’ll get rid of abs().

The grip does get drawn if it’s the full size of the slider. However, due to the way the clipping system works this isn’t really a problem. Hiding it would be a very minor optimisation, but I like the look of the full-size grip.

Arrow buttons! I’d forgotten those. That’s why I’ve got the ScrollbarVertical class. It needs to contain arrow buttons. That’s the next thing to get sorted.

Regarding your earlier idea about the ScrollableTextBox class, that was my preference too - keep the basic classes basic, and build on top of them with more complex sub-classes. I’ll add that in once the scrollbars are done.

Jeff on 2008-03-11 at 22:56 said:

Yes, negative numbers get like that, don’t they? There was apparently a contest in the UK recently where you were supposed to compare two temperatures, and loads of people were claiming that “-6” was a lower temperature than “-8”.

On the issue of arrow buttons, think carefully - is the existing scroll bar code factored sufficiently such that the subclasser could implement OSX style scrollbars with both buttons at one end…

ant on 2008-03-11 at 23:04 said:

Yeah, I remember that - it was a national lottery scratchcard. The person in question decided, if I remember correctly, that she “wasn’t having it” and would sue. How I laughed! And now I’ve done the same thing. Fignuts.

I’m going for OSX-style buttons - the Amiga OS puts them in the same place. I think if I set the positions of the arrow buttons and scroller in the constructor, the subclasser should have no problem overridding those values to change their positions.

ant.simianzombie.com » Scrollbars and Greyscale Algorithms on 2009-11-23 at 23:10 said:

[…] have been one of the major banes of Woopsi’s existence for years. No matter what I’ve done, they’ve always been […]