Old-Style Mask / OS_SpriteOp Bug

There’s a bug (or a breaking change) in the way that RISC OS 5 renders 256 colour sprites with the old-style binary masks. On these, the mask data has one byte per pixel (same size as the image data), but conventionally any non-zero value is supposed to be rendered as a solid pixel.

If you make, for example, a Mode 13 sprite in Paint with a mask (tested whilst in a 16M colour mode), then use the camera tool with Export selected to save a portion of the sprite, under RISC OS 5 the exported sprite looks completely transparent in Paint, with only the striped transparency pattern visible. If you drop to Mode 13 and view the sprite at a 1:1 zoom level, I’ve had the colour pixels show up, but it seems to be trying to alpha blend them, as if it’s treating the old-style mask as an 8 bpp alpha mask! Zooming in makes it go 100% transparent again.

Sprites in Paint are rendered using OS_SpriteOp 52, so presumably that’s where this incompatibility arises.

I don’t think Paint’s code is helping because when the camera export tool attempts to copy the sprite’s mask, pixels that were FF in the original mask become FC in the exported sprite’s mask. It seems Paint has been doing that since at least RISC OS 3, but in RISC OS 3.5, the exported sprite renders correctly.

Here’s the code in Paint that tries to copy the mask:

          ftracef0 ("Creating mask\n");

          wimpt_noerr (sprite_create_mask (file.spritearea, &sid));
          ssid.tag = sprite_id_addr;
          ssid.s.addr = psprite_address (sprite);

          sprwindow_swap_output_to_mask (&tempsprite, 0);
          bbc_gcol (0, 0);
          bbc_gcol (3, 0);        /* set empty gcol */
          bbc_clg ();            /* clear it all out */
          bbc_gcol (0, 255);
          bbc_gcol (3, 3);        /* set solid gcol */
          error = sprite_put_mask_given (sprite->file->spritearea,
              &ssid, -tools_pixel_to_point_x(&window->data->sprite, sprite->toolspace [1]),
                     -tools_pixel_to_point_y(&window->data->sprite, sprite->toolspace [2]));
          sprwindow_swap_output_to_screen ();

sprite_put_mask_given() uses OS_SpriteOp 49.

Can anyone please explain what all these GCOL commands are trying to do?

The docs say OS_SpriteOp 49 uses the background colour to write to the mask for any non-zero pixels in the source mask.

Is bbc_gcol (3, 3) enabling an Exclusive OR operation?

I need this stuff to work really when I code the Undo / Redo function, as we obviously don’t want mask data changing at all on an Undo. I may well end up just copying the data manually rather than trusting OS_SpriteOp for this!

Ugh. GCOL.

bbc_gcol (0, 0);

Set graphics to plot colour zero.

bbc_gcol (3, 0);

Set graphics to EOR the colour already there with colour zero.
I don’t get why GCOL 3,0 follows GCOL 0,0.

bbc_clg ();

Clear graphics screen. To what? We haven’t set a background colour yet!

bbc_gcol (0, 255);

Now set the background to plot colour 127.

bbc_gcol (3, 3);

And finally, foreground to EOR with colour 3.

Either I’m really misunderstanding how RISC OS Lib’s gcol() works (assuming it’s the same as BASIC), or that code is bizarre.

bbc_gcol (3, 3) enabling an Exclusive OR operation?

Yes, if following BASIC, the first of two numbers is the plot action, and the plot action for 3 is EOR.
http://www.riscos.com/support/users/firstprog/app1.htm#GCOL

Either I’m really misunderstanding how RISC OS Lib’s gcol() works (assuming it’s the same as BASIC), or that code is bizarre.

Yes (glad I’m not the only one confused!) I assume the same as BASIC too, but RISCOS_Lib’s code for it is a bit odd:

/* Set graphics foreground/background colour and action. */
os_error *bbc_gcol(int a, int b)
{
   os_error *e = bbc_vdu(bbc_DefGraphColour);
   if (!e) e = bbc_vdu(a);
   if (!e) e = bbc_vdu(b);
   return(e);
}

where bbc_DefGraphColour is 18. I’ve not found where RISCOS_Lib actually pulls in an implementation for bbc_vdu() though.

Clear graphics screen. To what? We haven’t set a background colour yet!

Going by the comments, it looks like they’re trying to initialize the whole mask to colour zero (fully transparent), but I agree, I’d expect to see bbc_gcol(0, 128) for that.

Now set the background to plot colour 127.

What colour’s that? I thought in a 256-colour mode there are only colours 0-63?

And finally, foreground to EOR with colour 3.

Well, FF EOR 3 = FC, which is what seems to get written for an originally opaque mask, but OS_SpriteOp 49 shouldn’t be using the foreground colours, so this really has me stumped! :(

I’ve not found where RISCOS_Lib actually pulls in an implementation for bbc_vdu() though.

It’s about halfway down s.swi. It’s a call to XOS_WriteC.

What colour’s that?

No idea. It says if b7 is set, then it is the background (not foreground) and the colour used is the value given with b7 masked out.

Maybe 127 is “the highest number that could be given that will be rounded down by the OS to what the mode actually supports”?

Maybe 127 is “the highest number that could be given that will be rounded down by the OS to what the mode actually supports”?

Hmm. The intention makes a bit of sense, but I think they’re missing a tint command for 256 colour modes. It still doesn’t explain the EORs. The mask has been CLGed to a single colour, so what could possibly be the thinking behind EORing that with anything? It’s gotta be a mistake. Oh, actually I bet bbc_gcol(3, 3) was supposed to be bbc_tint(3, 3). The bbc_gcol(3, 0) was probably an attempt at a dark tint as well.

The code in Paint that replots the mask after a rotation looks a lot closer to what I would have expected to see, although the old bbc_gcols() there are quite telling. Reading the comments it looks to have been fixed by Sean Connery! :-)

    if (sprwindow_swap_output_to_mask (sprite, TRUE))
    { ftracef0 ("Same for mask\n");
      os_swi2 (OS_SetColour, 1 << 4, 0);
    #if 0
      /*replaces the following, avoiding need to check full-palette bit*/
      bbc_gcol (0, 0); /*shurely this was shupposhed to set the background?*/
      bbc_tint (0, 0);
    #endif
      bbc_clg ();
      os_swi2 (OS_SetColour, 1 << 4, -1);
    #if 0
      /*replaces the following, avoiding need to check full-palette bit*/
      bbc_gcol (0, 0x80 | 63);
      bbc_tint (3, 3);
    #endif
      if ((error = sprite_put_mask_trans (sprite->file->spritearea, &sid2,
          NULL, &trans_mat)) != NULL)
      { ftracef0 ("ERROR\n");
        msg = error->errmess;
        goto finish;
      }
    }

sprite_put_mask_trans() uses OS_SpriteOp 55 (Plot Mask Transformed) but other than the transformation, the process looks to be essentially the same as the plot of the mask that the camera export tool is trying to use. It clears the mask using background colour zero, then replots it using background colour -1 which presumably ensure all bytes for opaque pixels will become FF. I think I’m going to put these OS_SetColour() commands in place of that export tool code.

The fact remains OS_SpriteOp is still broken for rendering these old masks though.

conventionally any non-zero value is supposed to be rendered as a solid pixel.

I don’t believe you.

It’s always been used as a mask (and a word mask, not featuring any individual pixel checking).

Certainly RO4 produces wobbly results if the mask pixels aren’t 0 or max.

In the Vantage renderer I thresholded the mask value at 50% by default, but it could optionally treat it as an alpha mask. I don’t remember the Kernel or SpriteExtend being happy with any intermediate values.

I don’t believe you.

Then you don’t believe the PRMs:

For old format sprites (those where the mode specifies a numbered mode), each mask pixel is at the same BPP as the image pixels. However the OS only treats the mask as a binary on/off value (0=invisible pixel, non-zero=visible).

Certainly RO4 produces wobbly results if the mask pixels aren’t 0 or max.

In the Vantage renderer I thresholded the mask value at 50% by default, but it could optionally treat it as an alpha mask. I don’t remember the Kernel or SpriteExtend being happy with any intermediate values.

Then that’s a change of behaviour from RISC OS 3.x when the values Paint wrote into the mask of exported sprites were rendered in a binary manner.

Either way, there’s some kind of bug in the rendering (to be fair I’ve only had a chance to test it in RPCEmu and ArcEm so far), because in the test I describe in the first post, the opacity of the image changes at different zoom levels under mode 13.

I’ve not really got a problem with RISC OS treating old-style masks as alpha channels, but the behaviour should be consistent and be documented.

Then you don’t believe the PRMs

For the avoidance of any doubt, I never believe anything but the code.

In fact, though the PRM was right about the Kernel SpriteOp code, it’s wrong about SpriteExtend. Viz:

That’s an old-style 256 colour sprite, all white, with a ramp of mask pixels &11,&22,&33 etc. This demonstrates that the Kernel and SpriteExtend can’t agree.

This is very much not the only thing those two implementations disagree about.

That’s interesting, but what versions of the Kernel and SpriteExtend?

Looks like I opened a big old can of worms here! We can probably close it again and I’ll just fix the bug in Paint’s camera tool and make sure Undo / Redo doesn’t break the masks as well (I’ll either leave them unaltered, which is my preferred option, or ensure the values are all 0 or FF).

Actually, I’ve misled you! That’s SpriteExtend disagreeing with itself of course. Here we go:

Again, white 256 colour sprite with sixteen solid mask pixels top left and 15 ramped mask pixels bottom right, being plotted into 256 colours.

You’ll note that none of these are “0 means transparent, anything else means solid”!

However, when plotting into 16 colours, &38 then does exhibit the >0 rule. So that’s the OS using three different masking rules depending on which SpriteOp you happened to use (and what kind of colour mapping is required). I am not surprised.

The old rule is be permissive on input and strict on output – anything Paint touches should end up more fixed, not less! Having non-maximal values in an old multibit mask is undefined behaviour. Paint should not permit such a thing (except when producing new-style masks, which are explicitly alpha).

RISC OS 4’s version of SpriteExtend also disagrees with the Kernel about what constitutes a valid Sprite area. This caused me some pain, back in the day.

The documentation here repeats the original original description of word 0 of a sprite:

Offset to next sprite in area

“Offset to next sprite”. Offset. That’s fine when all you’re doing with your SpriteOps is plotting or changing pixels (take a bow, Kernel SpriteOps). However, once you start adding or removing things from a sprite (palette, mask, rows, columns – hello there SpriteExtend) then the code needs to know the length of the sprite. This is not the same as “offset to next”. A length is not an offset. A length cannot be negative, or skip multiple sprites, or reach into another sprite area.

RO4’s SpriteExtend policed the tighter requirements for a valid length even when passing harmless plot functions to the Kernel (if the sprite name was given). This caused some memory arrangements to no longer work in RO4. It happened because SpriteExtend took over responsibility for finding sprites from the Kernel, even when the SpriteOp was going to be performed by the Kernel. And it applied the tighter checks even for those SpriteOps that didn’t need them.

I had to supply an RO4SprFix module that took over sprite finding from SpriteExtend for those Ops that don’t need a sprite length. Thanks RO4.

I think I’m going to put these OS_SetColour() commands in place of that export tool code.

The above fix is now in Paint 2.30, so any old-style masks of sprites exported with the camera or scissor tool should now correctly have all values 0×00 and 0xFF.

As described further up the page, the inconsistent rendering of intermediate mask values for old-style masks, between RISC OS versions, Sprite Extend versus the kernel, and even apparently between zoom levels, is something to remain aware of.