Bounty proposal: Paint

I’ve noticed one downside of making use of blend tables / inverse tables for brush opacity. Draw some transparent strokes on the image. Then choose a solid colour and, with Opacity turned down, paint bands of it across the transparent strokes. When the temporary layer is merged down, the intersecting areas are the wrong colour. It’s obviously blending with the RGB colour of the bottom sprite, ignoring the fact that those pixels are masked as transparent. I don’t think there’s any easy way around that.

Talking to myself, there’s one way around it, but it adds a few extra steps to the merge which may introduce a delay.

The flood fill tool, when filling from transparent to a solid colour, works by temporarily inverting the sprite’s mask so it can then plot the transparent parts of the mask onto the sprite in the solid colour (afterwards it inverts the mask back to its normal state). We could do the same thing here to make sure any transparent pixels that are being painted over will be in the currently selected RGB colour. It would introduce two CLG operations and an extra plot. Maybe I can make use of OS_ChangedBox to cut down the size of the sprite being updated.

I think even this fix will have a side effect. Suppose the sprite has a white background, and you paint a green translucent line across the white background and across a transparent region. When the blending happens, the above fix would make the region that was transparent come out in a more saturated green than the area that was white. Still, when we’re not using alpha masks or channels I don’t think there’s a single right answer for this.

I’m taking another look at the alpha mask / alpha channel stuff. I know I said I’d probably leave it to the end but I think it’s about time I at least enabled the UI to create these sprites. As it’s been years since we discussed this, I’m refreshing my memory and I’ll paste a few relevant comments from Jeffrey here.

I’ve got the minimal functionality working for editing the alpha channel. It lets you edit it as a separate greyscale image which is then applied to the main colour image.

Yeah, I guess that’s the only real option for alpha channels at the moment.

In-place editing of alpha masks should be pretty straightforward – you can redirect screen output to them and they’ll be treated as a 256 grey image. However for alpha channels we currently don’t have any support for that (essentially we’d have to teach the OS about a whole set of new pixel formats, and all the rendering operations for those pixel formats would have to read-modify-write for each pixel… ouch)

I don’t think we have a builtin way of directly rendering an alpha channel or alpha mask to the screen (there are the “plot sprite mask” ops, but they haven’t been extended to work sensibly with alpha). But if you clear the screen to black and then plot the sprite using a colour mapping function which just returns white then it should have the desired effect (the colourmap function isn’t supplied the alpha of the pixel, but it is called before the alpha is applied – so by returning white and plotting on a black background you’ll get the desired gradient from black-white). Or for <=256 colour sprites you could supply a palette which maps everything to white.

However, I have just remembered that OS_SpriteOp (and the VDU in general) doesn’t support proper alpha blending!

The VDU plot operations have no concept of blending at all, they only support logical operations (aka GCOL actions).

Meanwhile, OS_SpriteOp will do RGB blending (using the ARGB channels of the sprite and the RGB channel of the destination). But it completely ignores the alpha channel of the destination – when it blends it acts as if the destination has an alpha of 1, and when it writes pixels it always does so with an alpha of 1. (You can copy the sprite alpha channel to the destination by disabling the blending flag (bit 3), but that will obviously disable the RGB blending as well).

I think the only bit of the OS which can blend properly with an alpha destination is the font manager (Font_Paint flag bit 14).

Potentially it wouldn’t be too hard to add proper alpha blending to OS_SpriteOp. But supporting it in the VDU would be a lot more work. And we’d only be able to support it for alpha-channel formats; if the alpha is in a separate mask plane then there’s no way it can work (the screen alpha value is needed during the RGB blend stage, but the VDU has no concept of the framebuffer having a separate plane containing the alpha data)

So I think the best course of action would be to design the feature set of Paint around the limitations of the current system. You can still have an opacity slider for the brush tool, and that will affect the blending of the RGB, but (outside of the special alpha/mask editing mode) the user will have little or no control over what the alpha/mask is set to.

For most plot operations you could potentially allow the user to specify a custom alpha value to set, but that would only really work with non-blending operations, or with blending operations where the sprite being edited uses an alpha mask (since the mask update is a separate step, allowing you to use a blending op for the RGB update and non-blending op for the mask update)

The main point to take away from my earlier discussions with Jeffrey is there’s no in-built support for alpha blending that takes the destination pixel’s alpha value into account. This matters especially where I want to support soft brushes that have their own alpha masks (so the brushes have translucent edges).

At the moment, as I understand it, relying on the built-in routines would mean there’d be no good way to calculate appropriate alpha values where the translucent edges of those brushes touch the sprite. We certainly don’t want such a brush turning opaque destination pixels translucent (well, except when using the Transparent colour) and conversely, particularly at low opacity selections, we don’t really want transparent areas of the sprite turned completely opaque.

I’m therefore convinced that the only way to support soft brushes in a meaningful way is for Paint to have its own pixel by pixel alpha blending routine.

Oh yes! :D

3. There’s a very strange bug, when Opacity < 255, where the first click of the brush results in the brush sprite being plotted 8 pixels (usually) offset to the right. Subsequent plots always work normally (e.g. drag the mouse 1 pixel over and it will plot correctly, regardless of zoom level). I’m working on a test program to try and isolate the cause of this bug.

I’ve finally figured out the cause of the above bug.

Some experimentation revealed that swapping output between my layer sprite and the screen multiple times gave slightly different values for the start of screen memory (reading the VDU variable). Specifically, the first time output was switched to my sprite, the address was off by a small amount. Every time after that, the address was correct.

I noticed the amount of the sprite’s misalignment seemed to vary according to the number of colours of the brush sprite. I wondered if a palette or translation table was overflowing its memory and corrupting my sprite pointer (I couldn’t find anything obvious). I even began to wonder about a bug in the VDU code.

It turned out in the end that I got bitten by the flex memory management. I figured it out when I finally noticed the address of my sprite itself was changing inside the brush plot function. The change seemed to happen when the brush translation tables were being recreated. I confirmed it for sure by adding a Watchpoint to my sprite file struct in DDT and it alerted me that the pointers changed the moment one of the translation tables was flex-freed.

I’ll bear this in mind when I’m looking at the rest of the code as it could possibly have been a source of other crashes in Paint in the past.

Phew, onto the next issue! =)

I’ve finally figured out the cause of the above bug.

Excellent!

Is this also likely to be the source of the longstanding crashes when using brush sprites in general?

Is this also likely to be the source of the longstanding crashes when using brush sprites in general?

This particular bug isn’t the source of those crashes because it was caused by code I wrote. It’s certainly possible that a similar set of circumstances could have made Paint crash in the past when painting with the brush, however so far I’ve not seen evidence of that in the code.

For flex to move the sprite far enough in memory for a plot to fall outside the sprite’s bounds and cause a crash, I suspect something large (like another sprite) would need to be deleted, unless you’re working with a small sprite when the crash happens or painting right at the edge of the sprite. In the latter case, it’s likely we’d have seen misaligned brush plots as well.

Are you still experiencing the crash?

Are you still experiencing the crash?

Yes, although (as ever) it’s not clear exactly what circumstances provoke it. It’s only with the brush tool. From memory, sometimes pressing OK in the toolbox (to select the new brush sprite) crashes it, sometimes attempting to paint with the new sprite brings everything down. I’ll try to look into this, and see if I can narrow down when it’s happening.

Are you still experiencing the crash?

Yes, although (as ever) it’s not clear exactly what circumstances provoke it. It’s only with the brush tool.

At the risk of sounding like a broken record, when you catch it happening, please could you get me the output of the following commands?:

*where
*showregs
*memoryi pc -40 +80
*modules

The first two are the most important. If the machine’s unresponsive of course, at least writing down the error message would give some clue what’s going on.

Using PNG export now included in Paint 2.39 a crash occurs after completing the PNG export of a 16M colour sprite. Reducing the input sprite to 256 colours with ChangeFSI avoids the crash. Exporting as JPEG works properly.

Test sprites are here.

As tested on a Titanium.

Edit Fixed in Paint 2.40.

The brush tool enhancements in Paint 2.42 (07-Jul-21) are welcome, making the brushes available much clearer.

By default there is one empty slot on the right of the brush selection toolbar where the most recently selected user brush appears. There is a scroll bar under the brush toolbar which does not seem to do anything. Choosing a second user brush only replaces the icon at the right. The presence of the scroll bar does suggest that it should be possible to have more than five entries in the brush toolbar.

By default there is one empty slot on the right of the brush selection toolbar where the most recently selected user brush appears. There is a scroll bar under the brush toolbar which does not seem to do anything. Choosing a second user brush only replaces the icon at the right. The presence of the scroll bar does suggest that it should be possible to have more than five entries in the brush toolbar.

That’s a good point about the scroll bar, David. I previously coded support for multiple user brushes on the brush pane at once (as posted in my test build in the link further up this thread). It was agreed I should reduce this to showing one user brush at a time to simplify the interface and I also widened the brush pane so all brushes are visible without scrolling.

It’s theoretically possible to add more built-in brushes if you add them to !Paint’s Sprites file and increase the BRUSHCOUNT value in !Paint.Messages accordingly. I’ve also been experimenting with making soft-edged brushes which may or may not make it into Paint soon (they still have a few problems at the moment). In both cases, the scroll bar would be needed. However, if we don’t end up supplying more built-in brushes, the scroll bar probably will need to go away.

It’s theoretically possible to add more built-in brushes if you add them to !Paint’s Sprites file and increase the BRUSHCOUNT value in !Paint.Messages accordingly

Potentially a user configurable brushcount?
Or over complicating things?

Potentially a user configurable brushcount?
Or over complicating things?

The second one, unfortunately. Basically these brush tasks have far exceeded the allocated amount of work (yeah all the Paint tasks have to a degree but not like this) so it was agreed to cut the interface down to displaying only one user brush at a time.

The BRUSHCOUNT value in Messages is absolutely not intended for the end-user to alter; it’s just a kludge so that Paint knows how many of the sprites in Paint’s own Sprites file to use as brushes (currently four: “brush”, “circle”, “square” and “triangle”) because there are icons for the other tool buttons in there alongside them.

I quite possibly shouldn’t have even mentioned BRUSHCOUNT except it does give a way for developers and power-users to add their own built-in brushes. If you do do this, note also that BRUSH1ST tells Paint the name of the first sprite in its Sprite file to make into a brush.

Potentially a user configurable brushcount?

The initial design is possibly the issue here?
Looking at the sprites within Paint 2.37, I see “brush”, “circle”, “square”, and “triangle” as the basic brushes.
The problem is, if you bump the value up to six, say, then what are the additional two brushes called?

It probably would have been better for the sprites to have been called “01brush”, “02circle”, “03square”, “04triangle”, etc and in that way you have a number and a name. The name can be split off as the human readable part, and as for the number? Well, just enumerate the sprite file looking for ones with sequential numbers. So adding a new brush would be as simple as creating “05wavylines” and “06ellipse”…

(yeah all the Paint tasks have to a degree but not like this)

The joy of programming. There’s an estimate before the work is done, there’s an estimate whilst work is being done, and there’s how long it takes…three entirely different figures. ;-)

The joy of programming. There’s an estimate before the work is done, there’s an estimate whilst work is being done, and there’s how long it takes…three entirely different figures. ;-)

For sure. Although over the years I got pretty good at estimating a reasonable upper limit for the time required for a coding task. The trouble is, that was where I’d been spoilt in the pampered, luxurious world of modern IDEs and SDKs. When coding is like plugging together Tinkertoys for some boring bit of business logic it’s not so hard to estimate (at least until the customer changes their mind).

I’d never seriously attempted such a big RISC OS project as this. I dabbled with game coding in BASIC and a tiny bit of assembler in my youth but never got very far with the WIMP (I really could have done with a free template editor back then) so the learning curve with Paint has been like a mountain.

I quite possibly shouldn’t have even mentioned BRUSHCOUNT except it does give a way for developers and power-users to add their own built-in brushes. If you do do this, note also that BRUSH1ST tells Paint the name of the first sprite in its Sprite file to make into a brush.

Obviously I did have a fiddle around with this and got as far as discovering that the brush sprites need to be consecutive. This is just a comment and not a feature request, it’s fine as it is.

Oops, minor error, manage to loose square!

I did make one rather splendid discovery, which is that if a potential brush sprite file is dragged to Paint tools, brush window it loads into Paint and sets itself up as a brush in the toolbar. That is a one shot thing, to change the brush simply close the brush’s paint window and drag in whatever is next. Nice one, I thought.

That is a one shot thing, to change the brush simply close the brush’s paint window and drag in whatever is next. Nice one, I thought.

You can also drag a sprite from a sprite file window onto the brush pane to set the brush. Closing the file window isn’t essential by the way; you can just drag your new sprite on and the old brush is replaced anyway. If you close the file window while you’re using the sprite, it will switch back to the “circle” brush.

Closing the file window isn’t essential by the way; you can just drag your new sprite on and the old brush is replaced anyway.

Thanks, got that now.

I may have found a potential bug in Paint.

EDIT: Don’t panic just yet! This was with the 10th August 2020 version. I forgot that I’d dropped a softload (RO3.5 build) into Apps and it would replace the ROM version. I’ve moved that out of the way so the ROM (last nightly) can provide its version.
I’ll let you know if this happens again.

I’ll leave the description here in case it’s useful, or you’ve already spotted/fixed whatever this is:

When loading and renaming and saving sprites, all mostly identical, after a number of them (ten?) which are unloaded after being saved, I have noticed that after pressing Enter in the rename writeable, the Filer crashes.
So far, this has happened to me twice; though there’s no discernable pattern as yet other than “something to do with renaming?”.

I’m sorry I can’t be of any more use. The first time Filer crashed, it caused a cascade error until everything had fallen over. The second time, the subsequent error stiffed the machine. I wasn’t able to note down the error message as I have a tool that auto-cancels errors after a few seconds (because this machine is also a server, don’t want it stuck with a prompt on-screen!).

If it should ever crash in a more recoverable way, I’ll try and see if I can poke around to figure out how Paint crashes Filer, because there’s no obvious correlation that comes to mind.

I may have found a potential bug in Paint.

Yeah Rick the actual renaming is done through RISC_OSLib’s sprite functions and it just calls straight through to OS_SpriteOp 26. I struggle to see how Filer should even be involved at the point of renaming rather than saving because I’d have thought it should all take place in the sprite area, not on disk.

If it still happens it might be worth digging out a very old build of Paint (before the Paint and Clipboard bounties) to see if it still does it.

I struggle to see how Filer should even be involved

Stray pointer dropping data on top of Filer workspace? As we all know, RISC OS has super solid memory protection. 🤣

If it still happens

I’ll let you know if I spot it again.

I’ll let you know if I spot it again.

Happened again last night. This time I was using Alt-Click to rename.

It isn’t the rename itself that is failing. I noticed that the white writeable box was still on-screen, and I recall that the menu was still open when renaming the menu way. So it appears to be something between pressing Enter and the actual rename happening.

Sadly, still no further with a debug report. Filer (seems to always be Filer) throws an exception, and this is followed by a couple more before things appear to settle down. By this point, whatever is going wrong is in WindowManager and likely has little relation to the source of the problem.

I’ve set up to have a Debugger exception dump for “the next time”, but if there are multiple exceptions I rather expect to see the same result. :-/ ¹

I do not yet know what the exact pattern to trigger this is. Rebooting, loading Paint, renaming… that works. So it’s some sort of interaction elsewhere. I’m editing a set of 256 colour (mode 28, default VIDC palette, mask) sprites. The sprite file itself was created by using Paint for the individual sprites, and a BASIC program (a set of OS_SpriteOp calls) to put them into one big sprite file. Nothing weird, no direct access to data or anything like that.

¹ Perhaps if the Debugger exception file exists, then Debugger ought to try appending numbers (from 1) until the file is created?

I’ve set up to have a Debugger exception dump for “the next time”, but if there are multiple exceptions I rather expect to see the same result. :-/

Thanks Rick I really appreciate you digging into this.

When loading and renaming and saving sprites, all mostly identical, after a number of them (ten?) which are unloaded after being saved,

The sprite file itself was created by using Paint for the individual sprites, and a BASIC program (a set of OS_SpriteOp calls) to put them into one big sprite file

I’m a bit confused when you saying “loading and renaming and saving”, “unloaded after being saved”, it sounds like you’re opening a lot of sprite files one by one in Paint, renaming one sprite at a time and closing its sprite file before you open the next one. But then you say your program put them all into one big sprite file.

Do you mean you’re opening the big sprite file, renaming just one sprite, saving and closing the sprite file? Please can you clarify the exact steps?

Thanks Rick I really appreciate you digging into this.

I wish I had something useful to tell you.

Please can you clarify the exact steps?

Both. ;-)

The individual sprites are used to build up the big sprite file. I can’t draggy-droppy into Paint because of a possible bug (I’ll describe this in a separate post) so I use a little bit of BASIC to do it.

Most of the renaming happens with the individual sprites. I’d load up a few of them, five or ten or so (as I feel like). They would possibly have a few tweaks to fiddle individual pixels.
Once the changes were made, the sprite viewer/editor would be closed. In the sprite list window showing the sprite, the sprite would be renamed. Once that had been done, the sprite list window would be closed, which would pop up the save dialogue. Sprite saved, window then closed, unloaded and gone.

The reason of loading several at once is to speed things up. It’s quicker than finding the next file to load each time. Better to lassoo a bunch and open them up at once.

As for the composite file with all the sprites in, sometimes things get tweaked there because I’m lazy (and know full well it’ll all need to be done again on the relevant individual file). ;-)

That other bug.

Sprites get “selected” (drawn inverted) when clicked on in the sprite list window. I’m not sure when this was introduced, but it’s probably to mimic the behaviour of the filer.

However, there is one fundamental and important bug with this. Dragging a sprite into the sprite list window will replace the sprite that is highlighted. I’m not sure if this is supposed to happen or not. It feels wrong to silently delete things in that manner.

It gets even worse when you consider that a sprite file dragged to the list window is highlighted once it has been loaded.

Can you see where this is going? :-)

Yup. Drag fifty sprites into your list window and the process will be pretty much:

• Load sprite, replacing the currently highlighted one.
• This is a new sprite, so highlight it.
• GOTO 10.

<facepalm>

Now, that doesn’t just feel wrong, it is wrong!

[I just checked Paint on RISC OS 3.11 and it doesn’t highlight at all, and it happily loads multiple sprites into the list; the PC isn’t running else I’d see what 3.70 does]

I’m not sure when this was introduced, but it’s probably to mimic the behaviour of the filer.

Sprow, 2012, and yes it was to make it behave like the Filer…

https://gitlab.riscosopen.org/RiscOS/Sources/Apps/Paint/-/commit/938add00918b8db121bcb2d546e8e1618418d017