Spriteextend 1.44 stops DPingScan showing scans in preview mode

I use DpingScan on my Iyonix and also Beagleboard and I have noticed that when I run the latest 5.17 softload ROM image dated 12th July on my Iyonix that DPlingScan v1.25 stops being able to show a scan in preview mode. In essence the scan area shows up as blank even though using the auto locate scan area does work and locks on to the seemingly none existant preview scan. Reverting to an earlier version of the SpriteExtend module, 1.22, by manually loading a saved module from the 5.16 restores the preview function. Is this something that should be reported to David Pilling or is it an issue with SpriteExtend 1.44?

There is more strict checking of pointers (for transformed sprites) and JPEGs (for JPEG_Info) which are likely to be causing the problem. David fixed an Ovation problem when JPEG_Info was tightened up, but maybe transformed sprites need some work too?

Perhaps the SpriteExtend checks can be relaxed a bit, since there are a few ops which have no reason to make use of the sprite area pointer as long as a sprite pointer was supplied.

Is this a separate issue from Re: DPScan + ARMini -> crash?

Is this a separate issue from Re: DPScan + ARMini → crash

I think so yes, since the range check on sprite areas was changed after the last ARMini ROM.

Could someone try this module and see if that helps? I just changed it to skip the range test on the sprite area if SpriteOp > 512.

Hi, I’ve tested your updated module and that indeed restores the preview scan.

I have also contact David Pilling and he says he will take a look at the issue.

So I guess the question now is will your change be reflect in future builds or is it something that David will need to change in DPlngScan and perhaps OvationPro as well?

I’ve tested your updated module and that indeed restores the preview scan.

It seems quite a few things rely on R1 not being valid for SpriteOps greater than 512, so perhaps they should be let through as before and only R2 checked for being sane. Any objections?

It seems quite a few things rely on R1 not being valid for SpriteOps greater than 512, so perhaps they should be let through as before and only R2 checked for being sane. Any objections?

I’m guessing that all you’ve done so far is modify getspriteaddr so that the CheckAlignedAndSensible R1 check is now just after the TEQ R14,#&200 check instead of just before it? That sounds fine to me (it’s what I was going to do to relax the checks, and probably should have done to start with). But if we’re doing that then we’d probably want to add checks to the operations that modify sprites (and therefore need a proper sprite area, e.g. CreateRemovePalette) since at the moment they’re relying on getspriteaddr in order to perform the checks for them.

This problem is not the one where DPScan crashes on ARMini. That problem is proving rather difficult to solve. Any offers? Story here:

http://www.freelists.org/post/davidpilling/DPScan-ARMini-crash

As to the above. I am doing transformed sprite plotting, my sprite is not in a sprite area so I set R1 to 0xFF. I do this because I saw it long ago in some Acorn code.

Implies to me that this problem should be appearing elsewhere in my stuff and maybe other stuff.

I think the discussion above has moved towards this POV since I started to look at it.

What am I supposed to do? Is this illegal, are people going to have problems, in other words is it going to change so my code works in future releases of RISC OS.

This problem is not the one where DPScan crashes on ARMini. That problem is proving rather difficult to solve. Any offers?

I’ll try and find the time to take a look at it tonight (unless someone else gets there before me!)

I do this because I saw it long ago in some Acorn code.

Probably Draw, since that’s the one that I managed to break when I made SpriteExtend start checking the pointers!

What am I supposed to do? Is this illegal, are people going to have problems, in other words is it going to change so my code works in future releases of RISC OS.

We’re going to fix it so it’ll work again. I’d expect new ROMs to be available by the end of the week, if not sooner.

Vantage shipped with a module called SprExtFix to work around the arbitrary, inconsistent and revisionist pointer checks when they were introduced with the RO4 version of SpriteExtend.

They should not be there. For example, the original specification of the sprite format had no restrictions on the next-sprite pointer (such as not being earlier in memory) and SpriteExtend had no business imposing them.

I’m guessing that all you’ve done so far is modify getspriteaddr so that the CheckAlignedAndSensible R1 check is now just after the TEQ R14,#&200 check instead of just before it?

Aye, because my other hand was feeding breakfast cereal into my mouth. You make a good point about those things relying on that name to address routine where R1 is required: I can take a look at those and check for sanity prior to calling the common code.

I’ll go dust off the PRM to see which things are defined as requiring pukka sprite areas too, per nemo’s comment.

Re: The DPScan crash. I’ve been able to reproduce it here. It looks like it must be a lazy task swapping issue, so it might take me a day or two to get to the bottom of it (I’ve run into a couple of similar issues in the past, and they’ve always been “fun” to fix).

Thanks for the comments on sprite plotting.

As to the DPScan crash – hope at last!

(I hope)

There are now new ROM images up on the downloads page with the SpriteExtend & DPScan crash fixes.

For the DPScan crash, it looks like it was something very similar to this issue, where the lazy abort handler was being re-entered due to the page that the aborting instruction came from not being mapped in. Except this time both aborts were for the same page, so by the time control returned to the first abort handler the page had been mapped in, causing the code to conclude that it was a genuine abort and not a lazy task swapping one.

For now I’ve gone with the simple fix of making the abort handler assume that cache maintenance instructions won’t be used from within application space. This means it won’t try loading the aborting instruction in order to examine it, and so (hopefully) avoid the possibility of any more recursive aborts in future. But at some point I will have to add a bunch of logging code to allow me to work out what’s going on, because somehow a page is being mapped out without the I-cache being flushed, leading the CPU to execute stale code (and then the abort handler to be re-entered if it tries looking at any instructions that aborted).