Ticket #481 (Open)Mon Apr 06 18:18:36 UTC 2020
Using OS_SpriteOp 48/49 to update the mask of another sprite (with the same mode word) only works properly with numbered modes
Reported by: | Andy S (2979) | Severity: | Enhancement |
Part: | RISC OS: General | Release: | |
Milestone: | Status | Open |
Details by Andy S (2979):
I have tested OS_SpriteOp 49 (Plot mask at user coords) and OS_SpriteOp 50 (Plot Mask Scaled), and neither of them write the correct values to a target sprite’s mask using OS_SpriteOp 61 (Switch output to mask) when the sprite has a 1 bpp mask and more than 256 colours.
There is more discussion <a href=“https://www.riscosopen.org/forum/forums/4/topics/15159”>here</a>.
The quickest way to reproduce this is using !Paint with the Camera Export tool on (e.g.) a 16M colour sprite with some transparent pixels.
I’ve not tested OS_SpriteOp 48 (Plot sprite mask) or OS_SpriteOp 55 (Plot mask transformed).
Changelog:
Modified by Andy S (2979) Mon, April 06 2020 - 18:27:24 GMT
Some tests:
1. Make a 16M colour sprite with Transparency Mask ticked (should be the regular binary mask)
2. Do some painting in the transparent colour
3. Use the camera tool in export mode to make a copy of the region of the sprite you painted on
4. On the exported sprite all the pixels you tried to make transparent are instead showing in the foreground colour.
1. Make a 16M colour sprite with mask in Paint
2. Flood fill with a solid colour like green
3. Rotate the sprite 45 degrees
4. Transparent areas will appear around the rotated block, as expected
5. As before, use the camera tool to export part of the sprite, such that it includes transparent and solid areas.
6. This time, the exported sprite will look completely transparent, with no solid pixels visible.
1. I made a 3 × 3 16M colour sprite with a mask in Paint.
2. I coloured the top row red, middle row green, bottom row blue.
3. I set the middle pixel to transparent and saved the image.
4. When I open the image in a hex editor, the mask data for the image created in 3. is FFFFFFFF FFFFFFFD FFFFFFFF. 1 word has been written per row, but it is a 1 bpp mask as expected.
5. When I drag the camera tool over the whole image and use it to export the sprite, that sprite file’s mask data is then FFFFFFFF FFFFFFFF FFFFFFFF.
Modified by Andy S (2979) Tue, April 07 2020 - 18:09:07 GMT
- Severity changed from Normal to Enhancement
Correction: edwardx has demonstrated to me that OS_SpriteOp 50 does appear to work.
It’s OS_SpriteOp 49 (and consequently RISCOS_Lib’s sprite_put_mask_given() that Paint uses) that doesn’t support > 256 colour sprites.
I will fix the bug in Paint’s camera / scissor export with my next bounty submission.
I’ve downgraded this to “Enhancement” for now. It would be nice if the built-in SWIs like OS_SpriteOp 49 could support these 1 bpp masks with a 16M colour sprite or screen mode. Otherwise the PRM could just do with some clarification. It does say:
<blockquote>
This call does not attempt to translate between different colour depths. Unless you know the source and destination are in compatible modes, it’s recommended to use OS_SpriteOp 50 – Plot Mask Scaled instead
</blockquote>
but that’s not immediately clear that, when output is swapped to a sprite, it’s the colour depth of the sprite itself rather than the bpp of the mask that determines the behaviour.
Modified by Jeffrey Lee (213) Sun, May 03 2020 - 16:38:24 GMT
RISC OS 3.7 (with RISC OS 3.7 !Paint) is showing the same problem, so at least we’ve got the comfort that it wasn’t one of us who broke it.
It’ll affect at least OS_SpriteOp 48 and 49, since they both use the same code in the kernel. It looks like the problem is that the kernel expands the mask from 1bpp to the same bpp as the colour data. So it’ll work fine if you’re plotting the mask to a colour buffer (that’s the same BPP as the sprite), but fail if you’re plotting into another new-style mask.
OS_SpriteOp 50 & 55 are handled by SpriteExtend, and are designed to cope with BPP conversion (unlike all the kernel sprite ops), so should hopefully be fine.
Modified by Jeffrey Lee (213) Wed, May 06 2020 - 11:35:34 GMT
- Summary changed from Mask Plotting SWIs are Broken for > 256 Colour Sprites to Using OS_SpriteOp 48/49 to update the mask of another sprite (with the same mode word) only works properly with numbered modes
Modified by Jeffrey Lee (213) Tue, October 25 2022 - 14:01:38 GMT
After poking around in the code a bit, it looks like the main problem is that the GenSpritePlotParmBlk routine that PlotNewMask is using isn’t suitable for this purpose. It looks like it’s generating values based on the BPP of the source image (>1 bpp) instead of the dest (1 bpp). So our options are likely to be either:
1. Replace or modify GenSpritePlotParmBlk so that it calculates the values in the right way for this case. This would probably first require us to document exactly what all the different values are that it calculates (vdugrafdec is pretty light on comments)
or:
2. Avoid the scary kernel code by having SpriteExtend handle this case – e.g. by having the kernel issue OS_SpriteOp 50 if it detects that the dest BPP doesn’t match the sprite BPP. Or if we want a fix that’ll work with older OS versions, have SpriteExtend intercept OS_SpriteOp 48/49 and decide whether the kernel can handle it or not
(Or maybe it’s time we split the VDU/sprite code out of the kernel completely, which could then allow it to be merged with SpriteExtend, which would avoid the two having to constantly second-guess each other over what features they support and whether it’s safe to forward requests to each other or not)
Modified by Sprow (202) Fri, December 30 2022 - 16:48:11 GMT
Some extra notes from testing performed in May 2020 but trapped in my sprawling email.
The implementation of 48/49 also doesn’t account for ROL’s alpha level mask, however, neither does RISC OS 6:
- RISC OS 3.7 OS_SpriteOp 50 with new format mask seems fine
- RISC OS 6 OS_SpriteOp 48/49 with new format masks has the same issue as ours
- RISC OS 6 OS_SpriteOp 48/49 with 8bpp alpha masks fails with an abort on data transfer