Ticket #375 (Fixed)Sat Jan 11 08:32:16 UTC 2014
I+S+T icons that are faded/inverted abort on redraw
| Reported by: | Sprow (202) | Severity: | Normal |
| Part: | RISC OS: Module | Release: | |
| Milestone: | Status | Fixed |
Details by Sprow (202):
Window Manager 5.30.
Icons with I+S+T set abort when being redrawn, simplest way to recreate is open a template editor, add a label icon, set I+S+T (such that the box behind it should be minimised to the size of the text) and bang!
If faded, seems to be in Go_CreateRemovePalette in SpriteExtend.
If inverted, seems to be in ValidateModeSelector in the Kernel.
In both cases the sprite pointer looks dodgy.
Spotted by Druck, whose DiscKnight main window had a faded box+label around some icons
Changelog:
Modified by Sprow (202) Sat, January 11 2014 - 22:47:05 GMT
..forgot the important bit! Another application on the iconbar is redrawing its iconbar sprite too. For example, !Alarm in analogue-with-seconds mode. The abort occurs when the iconbar icon is being redrawn.
If you instead use analogue-without-seconds then you can mess about in the template editor for most of the minute until the clock hand moves at which point there’s an abort when redrawing the I+S+T icon in the template editor.
Modified by Sprow (202) Tue, January 21 2014 - 20:18:00 GMT
Having turned off lazy task swapping, it still fails, so it’s not that.
But the dodgy sprite pointers look like the corresponding bit of memory from !Alarm, so it looks like !Alarm was left paged in somehow?
Modified by Jeffrey Lee (213) Mon, April 14 2014 - 20:38:01 GMT
- Status changed from Open to Fixed
99% certain this was fixed with the translation table caching code that was introduced in Wimp 5.36.
After deciphering a stack dump, it looks like the following was happening:
- !Alarm asks the wimp to redraw its icon (which is held in !Alarm’s application space)
- The wimp caches information for that sprite, including the parameters needed for calling ColourTrans_GenerateTable
- The wimp plots the sprite and everything is OK
- The user clicks the I+S+T icon to select it, and a redraw of that icon is triggered
- The icon doesn’t have a sprite, and so caching of the sprite information fails. However the icon redraw code tries to redraw the sprite regardless.
- The redraw code thinks “hmm, this icon is selected, I need to regenerate the translation table” and calls calcinverse
- Because the previous sprite which was cached successfully was <8bpp, calcinverse thinks “hmm, cachespritedata will have already been called, and so the colourtrans parameters will be up to date”, and so skips the calling cachespritedata (and exiting out if there was an error)
- It then issues ColourTrans_GenerateTable using the stale parameters, leading to a crash somewhere nasty due to the sprite pointer pointing at whatever random garbage the template editor has in its memory at the address !Alarm’s sprite was
Wimp 5.36 fixes this by moving the ColourTrans_GenerateTable call out of calcinverse and into the new cachespritepixtable routine, which will always make a call to cachespriteaddress and safely exit out if there was an error.
